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

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.

> 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.  */
>   
>     static hashval_t
> -  hash (tree context, tree fullname)
> +  hash (tree context, tree name)
>     {
>       hashval_t hash = 0;
> -    hash = iterative_hash_object (context, hash);
> -    hash = iterative_hash_object (fullname, hash);
> -    return hash;
> +    if (context)
> +      hash = iterative_hash_hashval_t (TYPE_P (context) ? TYPE_UID (context)
> +				       : DECL_P (context) ? DECL_UID (context)
> +				       : 0, hash);
> +    return iterative_hash_hashval_t (IDENTIFIER_HASH_VALUE (name), hash);
>     }
>   
>     static hashval_t
>     hash (const typename_info *ti)
>     {
> -    return typename_hasher::hash (ti->scope, ti->template_id);
> +    return typename_hasher::hash (ti->scope, ti->name);
>     }
>   
>     static hashval_t
>     hash (tree t)
>     {
> -    return typename_hasher::hash (TYPE_CONTEXT (t), TYPENAME_TYPE_FULLNAME (t));
> +    return typename_hasher::hash (TYPE_CONTEXT (t), TYPE_IDENTIFIER (t));
>     }
>   
>     /* Compare two TYPENAME_TYPEs.  */