Re: [PATCH] c++: do not hash TYPENAME_TYPEs on pointers [PR124811]

> Am 12.08.2026 um 18:30 schrieb Patrick Palka <[email protected]>:
> 
> On Tue, 11 Aug 2026, Richard Biener wrote:
> 
>>> On Mon, 10 Aug 2026, Patrick Palka wrote:
>>> 
>>> 
>>>> On Mon, 10 Aug 2026, Jason Merrill wrote:
>>> 
>>>> On 8/7/26 9:55 AM, Richard Biener wrote:
>>>>> From: Bernhard M. Wiedemann <[email protected]>
>>>>> 
>>>>> typename_htab is written to a precompiled header.  A hash table is
>>>>> streamed out slot array and all: gt_pch_nx() relocates the pointers
>>>>> inside the entries but leaves every entry in the slot it happened to
>>>>> occupy.  typename_hasher hashed on the addresses of the scope and the
>>>>> fullname, so the slots were chosen from addresses that ASLR randomises
>>>>> in the process writing the header, and they no longer correspond to the
>>>>> hash of anything once the header has been read back at a different
>>>>> address.
>>>>> 
>>>>> Lookups then find a restored TYPENAME_TYPE only when it happens to lie
>>>>> on the probe sequence of the slot the new hash points at, so most miss
>>>>> and build a duplicate, and which ones miss depends on the layout the
>>>>> writing process had.  That makes a compile using a PCH differ from the
>>>>> same compile without one, and differ from itself between runs: the
>>>>> duplicates consume DECL_UIDs, every later DECL_UID shifts, and
>>>>> var-tracking hashes on DECL_UID, so .debug_loclists comes out different.
>>>>> 
>>>>> Hash on TYPE_UID/DECL_UID and IDENTIFIER_HASH_VALUE instead, which the
>>>>> header preserves.  The name replaces the fullname in the hash because a
>>>>> TEMPLATE_ID_EXPR fullname has no address-independent hash of its own;
>>>>> both are compared by equal() either way.
>>>> 
>>>> We could handle TEMPLATE_ID_EXPR by using iterative_hash_template_arg instead
>>>> of iterative_hash_object?
>>>> 
>>>> But I suppose it's unlikely that we'd need a bunch of typenames that differ
>>>> only in template args, so OK either way.
>>> 
>>> IIRC the PR65328 compile-time-hog testcase had a lot of TEMPLATE_ID_EXPR
>>> typenames, and indeed this patch as-is significantly regresses compile
>>> time of the PR65328#c6 testcase since it effectively undoes part of
>>> r13-1047-g343d83c7a89d0c.  So I think we need to go with using
>>> iterative_hash_template_arg instead (with comparing_specializations
>>> set so that we produce distinct hashes for nested TYPENAME_TYPE).
>>> 
>>>> 
>>>>> Compiling qgstopologicalmesh.cpp of qgis 4.2.1, 4757 lookups in the
>>>>> table:
>>>>> 
>>>>>   no PCH                 3046 misses, 3046 entries
>>>>>   PCH, before            483-484 misses, 3135-3136 entries, varies per run
>>>>>   PCH, after             394 misses, 3046 entries
>>>>> 
>>>>> and the object file now matches the one built without a PCH, byte for
>>>>> byte, from any of eight independently generated headers.
>>>>> 
>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>>>>> 
>>>>> OK?
>>>>> 
>>>>> Thanks,
>>>>> Richard.
>>>>> 
>>>>> gcc/cp/ChangeLog:
>>>>> 
>>>>>    PR pch/124811
>>>>>    * decl.cc (typename_hasher::hash): Hash the UIDs of the context
>>>>>    and the name rather than their addresses.
>>>>> 
>>>>> Assisted-by: Claude
>>>>> ---
>>>>>  gcc/cp/decl.cc | 20 +++++++++++++-------
>>>>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>>>> 
>>>>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>>>>> index b5d9ed05874..c66f59fef79 100644
>>>>> --- a/gcc/cp/decl.cc
>>>>> +++ b/gcc/cp/decl.cc
>>>>> @@ -4920,27 +4920,33 @@ struct typename_hasher : ggc_ptr_hash<tree_node>
>>>>>  {
>>>>>    typedef typename_info *compare_type;
>>>>>  -  /* Hash a TYPENAME_TYPE.  */
>>>>> +  /* Hash a TYPENAME_TYPE.  This table goes into a precompiled header,
>>>>> which
>>>>> +     moves everything it contains to a different address; entries keep the
>>>>> +     slot they were put in, so hashing on the addresses of CONTEXT and NAME
>>>>> +     would leave them unfindable once the header has been read back.  Hash
>>>>> on
>>>>> +     the UIDs instead, which the header preserves.  */
>>> 
>>> Since this is a common problem with GC'd hash tables and PCH and not
>>> specific to this hash table, I don't think this comment is necessary
>>> here FWIW.  It'd be better suited in a more central piece of
>>> documentation such as in the gccint manual, if it's not already
>>> documented there.
>> 
>> ISTR seeing such comment in other places, so yes it's redundant.
>> 
>> Can you take the patch from here?  I'd have to second-guess
>> which tree is the TEMPLATE_ID_EXPR while it's probably obvious to
>> you.
>> 
>> The PR identifies two other GC'd and PCH streamed hash tables that
>> hash pointers (but any incremental improvement is good!), which
>> are cp/tree.cc:list_hash_table and cp/constraint.cc:atom_cache
> 
> FWIW atom_cache is marked 'deletable' so it's not written to PCH and
> shouldn't affect TYPE/DECL_UID generation, I think...
> 
> list_hash_table seems to be only used sometimes for sharing identical
> TYPE_ARG_TYPES nodes inside a FUNCTION_TYPE.  At first glance this
> seems to be purely a memory-use optimization and so shouldn't result in
> extra TYPE/DECL_UID nodes being created with PCH vs without.

Ah, that means it should possibly deletable al well (at least not PCH streamed)

> For the typename table, this is what I had in mind, bootstrapped and
> regtested on x86_64-pc-linux-gnu, does this look OK for trunk?

Thanks for taking care of this.

Richard 

> -- >8 --
> 
> Subject: [PATCH] c++: do not hash TYPENAME_TYPEs on pointers [PR124811]
> 
> typename_htab is written to a precompiled header.  A hash table is
> streamed out slot array and all: gt_pch_nx() relocates the pointers
> inside the entries but leaves every entry in the slot it happened to
> occupy.  typename_hasher hashed on the addresses of the scope and the
> fullname, so the slots were chosen from addresses that ASLR randomises
> in the process writing the header, and they no longer correspond to the
> hash of anything once the header has been read back at a different
> address.
> 
> Lookups then find a restored TYPENAME_TYPE only when it happens to lie
> on the probe sequence of the slot the new hash points at, so most miss
> and build a duplicate, and which ones miss depends on the layout the
> writing process had.  That makes a compile using a PCH differ from the
> same compile without one, and differ from itself between runs: the
> duplicates consume DECL_UIDs, every later DECL_UID shifts, and
> var-tracking hashes on DECL_UID, so .debug_loclists comes out different.
> 
> Hash on TYPE_UID/DECL_UID instead, which the header preserves, and use
> iterative_hash_template_arg to safely hash fullname which can be an
> arbitrary TEMPLATE_ID_EXPR.  It's important to hash the fullname instead
> of just the name for sake of the PR c++/65328 compile-time-hog testcase.
> 
>    PR c++/124811
> 
> gcc/cp/ChangeLog:
> 
>    * decl.cc (typename_hasher::hash): Hash the TYPE_HASH of the
>    context and use iterative_hash_template_arg for hashing the
>    fullname instead of their addresses.
> 
> Co-authored-by: Bernhard M. Wiedemann <[email protected]>
> ---
> gcc/cp/decl.cc | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index b5d9ed058748..9699517ee2d4 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -4926,8 +4926,12 @@ struct typename_hasher : ggc_ptr_hash<tree_node>
>   hash (tree context, tree fullname)
>   {
>     hashval_t hash = 0;
> -    hash = iterative_hash_object (context, hash);
> -    hash = iterative_hash_object (fullname, hash);
> +    hash = iterative_hash_hashval_t (TYPE_HASH (context), hash);
> +    /* FULLNAME could be a template-id, so use iterative_hash_template_arg here.
> +       And might as well set comparing_specializations for stronger hashing.  */
> +    ++comparing_specializations;
> +    hash = iterative_hash_template_arg (fullname, hash);
> +    --comparing_specializations;
>     return hash;
>   }
> 
> --
> 2.55.0.559.g11c6700f10
>