On 8/12/26 12:29 PM, Patrick Palka wrote:
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?
OK.
-- >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;
}
