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

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.

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?

-- >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