public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: gcc-patches@gcc.gnu.org, jason@redhat.com, nathan@acm.org
Subject: Re: [PATCH] c++/modules: local class merging [PR99426]
Date: Tue, 5 Mar 2024 10:31:12 -0500 (EST)	[thread overview]
Message-ID: <befba40c-6e93-3cda-c37b-c7ab297a2264@idea> (raw)
In-Reply-To: <9d4f1a3e-99ea-5146-639d-22add3e22301@idea>

On Tue, 27 Feb 2024, Patrick Palka wrote:

> On Mon, 26 Feb 2024, Patrick Palka wrote:
> 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this approach
> > look reasonable?
> > 
> > -- >8 --
> > 
> > One known missing piece in the modules implementation is merging of a
> > streamed-in local class with the corresponding in-TU version of the
> > local class.  This missing piece turns out to cause a hard-to-reduce
> > use-after-free GC issue due to the entity_ary not being marked as a GC
> > root (deliberately), and manifests as a serialization error on stream-in
> > as in PR99426 (see comment #6 for a reduction).  It's also reproducible
> > on trunk when running the xtreme-header tests without -fno-module-lazy.
> > 
> > This patch makes us merge such local classes according to their position
> > within the containing function's definition, similar to how we merge
> > FIELD_DECLs of a class according to their index in the TYPE_FIELDS
> > list.
> > 
> > 	PR c++/99426
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* module.cc (merge_kind::MK_local_class): New enumerator.
> > 	(merge_kind_name): Update.
> > 	(trees_out::chained_decls): Move BLOCK-specific handling
> > 	of DECL_LOCAL_DECL_P decls to ...
> > 	(trees_out::core_vals) <case BLOCK>: ... here.  Stream
> > 	BLOCK_VARS manually.
> > 	(trees_in::core_vals) <case BLOCK>: Stream BLOCK_VARS
> > 	manually.  Handle deduplicated local classes.
> > 	(trees_out::key_local_class): Define.
> > 	(trees_in::key_local_class): Define.
> > 	(trees_out::get_merge_kind) <case FUNCTION_DECL>: Return
> > 	MK_local_class for a local class.
> > 	(trees_out::key_mergeable) <case FUNCTION_DECL>: Use
> > 	key_local_class.
> > 	(trees_in::key_mergeable) <case FUNCTION_DECL>: Likewise.
> > 	(trees_in::is_matching_decl): Be flexible with type mismatches
> > 	for local entities.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/xtreme-header-7_a.H: New test.
> > 	* g++.dg/modules/xtreme-header-7_b.C: New test.
> 
> > ---
> >  gcc/cp/module.cc                              | 167 +++++++++++++++---
> >  .../g++.dg/modules/xtreme-header-7_a.H        |   4 +
> >  .../g++.dg/modules/xtreme-header-7_b.C        |   6 +
> >  3 files changed, 149 insertions(+), 28 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H
> >  create mode 100644 gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index fa91c6ff9cb..f77f73a59ed 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -2771,6 +2771,7 @@ enum merge_kind
> >  
> >    MK_enum,	/* Found by CTX, & 1stMemberNAME.  */
> >    MK_keyed,     /* Found by key & index.  */
> > +  MK_local_class, /* Found by CTX, index.  */
> >  
> >    MK_friend_spec,  /* Like named, but has a tmpl & args too.  */
> >    MK_local_friend, /* Found by CTX, index.  */
> > @@ -2799,7 +2800,7 @@ static char const *const merge_kind_name[MK_hwm] =
> >      "unique", "named", "field", "vtable",	/* 0...3  */
> >      "asbase", "partial", "enum", "attached",	/* 4...7  */
> >  
> > -    "friend spec", "local friend", NULL, NULL,  /* 8...11 */
> > +    "local class", "friend spec", "local friend", NULL,  /* 8...11 */
> >      NULL, NULL, NULL, NULL,
> >  
> >      "type spec", "type tmpl spec",	/* 16,17 type (template).  */
> > @@ -2928,6 +2929,7 @@ public:
> >    unsigned binfo_mergeable (tree *);
> >  
> >  private:
> > +  tree key_local_class (const merge_key&, tree);
> >    uintptr_t *find_duplicate (tree existing);
> >    void register_duplicate (tree decl, tree existing);
> >    /* Mark as an already diagnosed bad duplicate.  */
> > @@ -3086,6 +3088,7 @@ public:
> >    void binfo_mergeable (tree binfo);
> >  
> >  private:
> > +  void key_local_class (merge_key&, tree, tree);
> >    bool decl_node (tree, walk_kind ref);
> >    void type_node (tree);
> >    void tree_value (tree);
> > @@ -4952,18 +4955,7 @@ void
> >  trees_out::chained_decls (tree decls)
> >  {
> >    for (; decls; decls = DECL_CHAIN (decls))
> > -    {
> > -      if (VAR_OR_FUNCTION_DECL_P (decls)
> > -	  && DECL_LOCAL_DECL_P (decls))
> > -	{
> > -	  /* Make sure this is the first encounter, and mark for
> > -	     walk-by-value.  */
> > -	  gcc_checking_assert (!TREE_VISITED (decls)
> > -			       && !DECL_TEMPLATE_INFO (decls));
> > -	  mark_by_value (decls);
> > -	}
> > -      tree_node (decls);
> > -    }
> > +    tree_node (decls);
> >    tree_node (NULL_TREE);
> >  }
> >  
> > @@ -6204,7 +6196,21 @@ trees_out::core_vals (tree t)
> >  
> >        /* DECL_LOCAL_DECL_P decls are first encountered here and
> >           streamed by value.  */
> > -      chained_decls (t->block.vars);
> > +      for (tree decls = t->block.vars; decls; decls = DECL_CHAIN (decls))
> > +	{
> > +	  if (VAR_OR_FUNCTION_DECL_P (decls)
> > +	      && DECL_LOCAL_DECL_P (decls))
> > +	    {
> > +	      /* Make sure this is the first encounter, and mark for
> > +		 walk-by-value.  */
> > +	      gcc_checking_assert (!TREE_VISITED (decls)
> > +				   && !DECL_TEMPLATE_INFO (decls));
> > +	      mark_by_value (decls);
> > +	    }
> > +	  tree_node (decls);
> > +	}
> > +      tree_node (NULL_TREE);
> > +
> >        /* nonlocalized_vars is a middle-end thing.  */
> >        WT (t->block.subblocks);
> >        WT (t->block.supercontext);
> > @@ -6717,7 +6723,34 @@ trees_in::core_vals (tree t)
> >      case BLOCK:
> >        t->block.locus = state->read_location (*this);
> >        t->block.end_locus = state->read_location (*this);
> > -      t->block.vars = chained_decls ();
> > +
> > +      for (tree *chain = &t->block.vars;;)
> > +	if (tree decl = tree_node ())
> > +	  {
> > +	    /* For a deduplicated local class, chain the to-be-discarded
> > +	       decl not the in-TU decl (which is already chained to in-TU
> > +	       entities).  */
> > +	    if (is_duplicate (decl))
> > +	      decl = maybe_duplicate (decl);
> > +	    else if (DECL_IMPLICIT_TYPEDEF_P (decl)
> > +		     && TYPE_TEMPLATE_INFO (TREE_TYPE (decl)))
> > +	      {
> > +		tree tmpl = TYPE_TI_TEMPLATE (TREE_TYPE (decl));
> > +		if (DECL_TEMPLATE_RESULT (tmpl) == decl && is_duplicate (tmpl))
> > +		  decl = DECL_TEMPLATE_RESULT (maybe_duplicate (tmpl));
> > +	      }
> > +
> > +	    if (!DECL_P (decl) || DECL_CHAIN (decl))
> > +	      {
> > +		set_overrun ();
> > +		break;
> > +	      }
> > +	    *chain = decl;
> > +	    chain = &DECL_CHAIN (decl);
> > +	  }
> > +	else
> > +	  break;
> > +
> >        /* nonlocalized_vars is middle-end.  */
> >        RT (t->block.subblocks);
> >        RT (t->block.supercontext);
> > @@ -10335,6 +10368,83 @@ trees_in::fn_parms_fini (int tag, tree fn, tree existing, bool is_defn)
> >      }
> >  }
> >  
> > +/* Encode into KEY the position of the local class declaration DECL
> > +   within FN.  The position is encoded as the index of the innermost
> > +   BLOCK (numbered in BFS order) along with the index within its
> > +   BLOCK_VARS list.  */
> > +
> > +void
> > +trees_out::key_local_class (merge_key& key, tree decl, tree fn)
> > +{
> > +  auto_vec<tree, 4> blocks;
> > +  blocks.quick_push (DECL_INITIAL (fn));
> > +  unsigned block_ix = 0;
> > +  while (block_ix != blocks.length ())
> > +    {
> > +      tree block = blocks[block_ix];
> > +      unsigned decl_ix = 0;
> > +      for (tree var = BLOCK_VARS (block); var; var = DECL_CHAIN (var))
> > +	{
> > +	  if (TREE_CODE (var) != TYPE_DECL)
> > +	    continue;
> > +	  if (var == decl)
> > +	    {
> > +	      key.index = (block_ix << 10) | decl_ix;
> > +	      return;
> > +	    }
> > +	  ++decl_ix;
> > +	}
> > +      for (tree sub = BLOCK_SUBBLOCKS (block); sub; sub = BLOCK_CHAIN (sub))
> > +	blocks.safe_push (sub);
> > +      ++block_ix;
> > +    }
> > +
> > +  /* Not-found value.  */
> > +  key.index = 1023;
> > +}
> > +
> > +/* Look up the local class corresponding at the position encoded by
> > +   KEY within FN.  */
> > +
> > +tree
> > +trees_in::key_local_class (const merge_key& key, tree fn)
> > +{
> > +  if (!DECL_INITIAL (fn))
> > +    return NULL_TREE;
> > +
> > +  const unsigned block_pos = key.index >> 10;
> > +  const unsigned decl_pos = key.index & 1023;
> > +
> > +  if (decl_pos == 1023)
> > +    return NULL_TREE;
> > +
> > +  auto_vec<tree, 4> blocks;
> > +  blocks.quick_push (DECL_INITIAL (fn));
> > +  unsigned block_ix = 0;
> > +  while (block_ix != blocks.length ())
> > +    {
> > +      tree block = blocks[block_ix];
> > +      if (block_ix == block_pos)
> > +	{
> > +	  unsigned decl_ix = 0;
> > +	  for (tree var = BLOCK_VARS (block); var; var = DECL_CHAIN (var))
> > +	    {
> > +	      if (TREE_CODE (var) != TYPE_DECL)
> > +		continue;
> > +	      if (decl_ix == decl_pos)
> > +		return var;
> > +	      ++decl_ix;
> > +	    }
> > +	  return NULL_TREE;
> > +	}
> > +      for (tree sub = BLOCK_SUBBLOCKS (block); sub; sub = BLOCK_CHAIN (sub))
> > +	blocks.safe_push (sub);
> > +      ++block_ix;
> > +    }
> > +
> > +  return NULL_TREE;
> > +}
> > +
> >  /* DEP is the depset of some decl we're streaming by value.  Determine
> >     the merging behaviour.  */
> >  
> > @@ -10454,17 +10564,10 @@ trees_out::get_merge_kind (tree decl, depset *dep)
> >  	    gcc_unreachable ();
> >  
> >  	  case FUNCTION_DECL:
> > -	    // FIXME: This can occur for (a) voldemorty TYPE_DECLS
> > -	    // (which are returned from a function), or (b)
> > -	    // block-scope class definitions in template functions.
> > -	    // These are as unique as the containing function.  While
> > -	    // on read-back we can discover if the CTX was a
> > -	    // duplicate, we don't have a mechanism to get from the
> > -	    // existing CTX to the existing version of this decl.
> >  	    gcc_checking_assert
> >  	      (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl)));
> >  
> > -	    mk = MK_unique;
> > +	    mk = MK_local_class;
> >  	    break;
> >  
> >  	  case RECORD_TYPE:
> > @@ -10768,6 +10871,10 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> >  	  }
> >  	  break;
> >  
> > +	case MK_local_class:
> > +	  key_local_class (key, STRIP_TEMPLATE (decl), container);
> > +	  break;
> > +
> >  	case MK_enum:
> >  	  {
> >  	    /* Anonymous enums are located by their first identifier,
> > @@ -11117,11 +11224,10 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> >  	    break;
> >  
> >  	  case FUNCTION_DECL:
> > -	    // FIXME: What about a voldemort? how do we find what it
> > -	    // duplicates? Do we have to number vmorts relative to
> > -	    // their containing function?  But how would that work
> > -	    // when matching an in-TU declaration?
> > -	    kind = "unique";
> > +	    gcc_checking_assert (mk == MK_local_class);
> > +	    existing = key_local_class (key, container);
> > +	    if (existing && inner != decl)
> > +	      existing = TYPE_TI_TEMPLATE (TREE_TYPE (existing));
> >  	    break;
> >  
> >  	  case TYPE_DECL:
> > @@ -11374,6 +11480,11 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
> >  	/* Just like duplicate_decls, presum the user knows what
> >  	   they're doing in overriding a builtin.   */
> >  	TREE_TYPE (existing) = TREE_TYPE (decl);
> > +      else if (decl_function_context (decl))
> > +	/* The type of a mergeable local entity (such as a function scope
> > +	   capturing lambda's closure type fields) can depend on an
> > +	   unmergeable local entity (such as a local variable), so type
> > +	   equality isn't feasible in general for local entities.  */;
> >        else
> >  	{
> >  	  // FIXME:QOI Might be template specialization from a module,
> > diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H b/gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H
> > new file mode 100644
> > index 00000000000..bf7859fba99
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H
> > @@ -0,0 +1,4 @@
> > +// { dg-additional-options -fmodule-header }
> > +
> > +// { dg-module-cmi {} }
> > +#include "xtreme-header.h"
> > diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C b/gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C
> > new file mode 100644
> > index 00000000000..03f3dc1bae6
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C
> > @@ -0,0 +1,6 @@
> > +// A version of xtreme-header_{a.H,b.C} that doesn't pass
> > +// -fno-module-lazy.
> > +// { dg-additional-options -fmodules-ts }
> > +
> > +#include "xtreme-header.h"
> > +import "xtreme-header-7_a.H";
> > -- 
> 
> Consider the following minimal testcase added that verifies we now
> properly merge local classes (and local enums):

Ping.  I opted to rename "local class" to "local type" throughout to
capture that enums are included too.

-- >8 --

Subject: [PATCH] c++/modules: local type merging [PR99426]

One known missing piece in the modules implementation is merging of a
streamed-in local type (class or enum) with the corresponding in-TU
version of the local type.  This missing piece turns out to cause a
hard-to-reduce use-after-free GC issue due to the entity_ary not being
marked as a GC root (deliberately), and manifests as a serialization
error on stream-in as in PR99426 (see comment #6 for a reduction).  It's
also reproducible on trunk when running the xtreme-header tests without
-fno-module-lazy.

This patch makes us merge such local types according to their position
within the containing function's definition, analogous to how we merge
FIELD_DECLs of a class according to their index in the TYPE_FIELDS
list.

	PR c++/99426

gcc/cp/ChangeLog:

	* module.cc (merge_kind::MK_local_type): New enumerator.
	(merge_kind_name): Update.
	(trees_out::chained_decls): Move BLOCK-specific handling
	of DECL_LOCAL_DECL_P decls to ...
	(trees_out::core_vals) <case BLOCK>: ... here.  Stream
	BLOCK_VARS manually.
	(trees_in::core_vals) <case BLOCK>: Stream BLOCK_VARS
	manually.  Handle deduplicated local types..
	(trees_out::key_local_type): Define.
	(trees_in::key_local_type): Define.
	(trees_out::get_merge_kind) <case FUNCTION_DECL>: Return
	MK_local_type for a local type.
	(trees_out::key_mergeable) <case FUNCTION_DECL>: Use
	key_local_type.
	(trees_in::key_mergeable) <case FUNCTION_DECL>: Likewise.
	(trees_in::is_matching_decl): Be flexible with type mismatches
	for local entities.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/merge-17.h: New test.
	* g++.dg/modules/merge-17_a.H: New test.
	* g++.dg/modules/merge-17_b.C: New test.
	* g++.dg/modules/xtreme-header-7_a.H: New test.
	* g++.dg/modules/xtreme-header-7_b.C: New test.
---
 gcc/cp/module.cc                              | 170 +++++++++++++++---
 gcc/testsuite/g++.dg/modules/merge-17.h       |  28 +++
 gcc/testsuite/g++.dg/modules/merge-17_a.H     |   3 +
 gcc/testsuite/g++.dg/modules/merge-17_b.C     |   3 +
 .../g++.dg/modules/xtreme-header-7_a.H        |   4 +
 .../g++.dg/modules/xtreme-header-7_b.C        |   6 +
 6 files changed, 186 insertions(+), 28 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/merge-17.h
 create mode 100644 gcc/testsuite/g++.dg/modules/merge-17_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/merge-17_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 80b63a70a62..d9e34e9a4b9 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2770,6 +2770,7 @@ enum merge_kind
 
   MK_enum,	/* Found by CTX, & 1stMemberNAME.  */
   MK_keyed,     /* Found by key & index.  */
+  MK_local_type, /* Found by CTX, index.  */
 
   MK_friend_spec,  /* Like named, but has a tmpl & args too.  */
   MK_local_friend, /* Found by CTX, index.  */
@@ -2798,7 +2799,7 @@ static char const *const merge_kind_name[MK_hwm] =
     "unique", "named", "field", "vtable",	/* 0...3  */
     "asbase", "partial", "enum", "attached",	/* 4...7  */
 
-    "friend spec", "local friend", NULL, NULL,  /* 8...11 */
+    "local type", "friend spec", "local friend", NULL,  /* 8...11 */
     NULL, NULL, NULL, NULL,
 
     "type spec", "type tmpl spec",	/* 16,17 type (template).  */
@@ -2927,6 +2928,7 @@ public:
   unsigned binfo_mergeable (tree *);
 
 private:
+  tree key_local_type (const merge_key&, tree);
   uintptr_t *find_duplicate (tree existing);
   void register_duplicate (tree decl, tree existing);
   /* Mark as an already diagnosed bad duplicate.  */
@@ -3085,6 +3087,7 @@ public:
   void binfo_mergeable (tree binfo);
 
 private:
+  void key_local_type (merge_key&, tree, tree);
   bool decl_node (tree, walk_kind ref);
   void type_node (tree);
   void tree_value (tree);
@@ -4951,18 +4954,7 @@ void
 trees_out::chained_decls (tree decls)
 {
   for (; decls; decls = DECL_CHAIN (decls))
-    {
-      if (VAR_OR_FUNCTION_DECL_P (decls)
-	  && DECL_LOCAL_DECL_P (decls))
-	{
-	  /* Make sure this is the first encounter, and mark for
-	     walk-by-value.  */
-	  gcc_checking_assert (!TREE_VISITED (decls)
-			       && !DECL_TEMPLATE_INFO (decls));
-	  mark_by_value (decls);
-	}
-      tree_node (decls);
-    }
+    tree_node (decls);
   tree_node (NULL_TREE);
 }
 
@@ -6201,7 +6193,21 @@ trees_out::core_vals (tree t)
 
       /* DECL_LOCAL_DECL_P decls are first encountered here and
          streamed by value.  */
-      chained_decls (t->block.vars);
+      for (tree decls = t->block.vars; decls; decls = DECL_CHAIN (decls))
+	{
+	  if (VAR_OR_FUNCTION_DECL_P (decls)
+	      && DECL_LOCAL_DECL_P (decls))
+	    {
+	      /* Make sure this is the first encounter, and mark for
+		 walk-by-value.  */
+	      gcc_checking_assert (!TREE_VISITED (decls)
+				   && !DECL_TEMPLATE_INFO (decls));
+	      mark_by_value (decls);
+	    }
+	  tree_node (decls);
+	}
+      tree_node (NULL_TREE);
+
       /* nonlocalized_vars is a middle-end thing.  */
       WT (t->block.subblocks);
       WT (t->block.supercontext);
@@ -6714,7 +6720,37 @@ trees_in::core_vals (tree t)
     case BLOCK:
       t->block.locus = state->read_location (*this);
       t->block.end_locus = state->read_location (*this);
-      t->block.vars = chained_decls ();
+
+      for (tree *chain = &t->block.vars;;)
+	if (tree decl = tree_node ())
+	  {
+	    /* For a deduplicated local type or enumerator, chain the
+	       duplicate decl instead of the canonical in-TU decl.  Seeing
+	       a duplicate here means the containing function whose body
+	       we're streaming in is a duplicate too, so we'll end up
+	       discarding this BLOCK (and the rest of the duplicate function
+	       body) anyway.  */
+	    if (is_duplicate (decl))
+	      decl = maybe_duplicate (decl);
+	    else if (DECL_IMPLICIT_TYPEDEF_P (decl)
+		     && TYPE_TEMPLATE_INFO (TREE_TYPE (decl)))
+	      {
+		tree tmpl = TYPE_TI_TEMPLATE (TREE_TYPE (decl));
+		if (DECL_TEMPLATE_RESULT (tmpl) == decl && is_duplicate (tmpl))
+		  decl = DECL_TEMPLATE_RESULT (maybe_duplicate (tmpl));
+	      }
+
+	    if (!DECL_P (decl) || DECL_CHAIN (decl))
+	      {
+		set_overrun ();
+		break;
+	      }
+	    *chain = decl;
+	    chain = &DECL_CHAIN (decl);
+	  }
+	else
+	  break;
+
       /* nonlocalized_vars is middle-end.  */
       RT (t->block.subblocks);
       RT (t->block.supercontext);
@@ -10337,6 +10373,83 @@ trees_in::fn_parms_fini (int tag, tree fn, tree existing, bool is_defn)
     }
 }
 
+/* Encode into KEY the position of the local type (class or enum)
+   declaration DECL within FN.  The position is encoded as the
+   index of the innermost BLOCK (numbered in BFS order) along with
+   the index within its BLOCK_VARS list.  */
+
+void
+trees_out::key_local_type (merge_key& key, tree decl, tree fn)
+{
+  auto_vec<tree, 4> blocks;
+  blocks.quick_push (DECL_INITIAL (fn));
+  unsigned block_ix = 0;
+  while (block_ix != blocks.length ())
+    {
+      tree block = blocks[block_ix];
+      unsigned decl_ix = 0;
+      for (tree var = BLOCK_VARS (block); var; var = DECL_CHAIN (var))
+	{
+	  if (TREE_CODE (var) != TYPE_DECL)
+	    continue;
+	  if (var == decl)
+	    {
+	      key.index = (block_ix << 10) | decl_ix;
+	      return;
+	    }
+	  ++decl_ix;
+	}
+      for (tree sub = BLOCK_SUBBLOCKS (block); sub; sub = BLOCK_CHAIN (sub))
+	blocks.safe_push (sub);
+      ++block_ix;
+    }
+
+  /* Not-found value.  */
+  key.index = 1023;
+}
+
+/* Look up the local type corresponding at the position encoded by
+   KEY within FN.  */
+
+tree
+trees_in::key_local_type (const merge_key& key, tree fn)
+{
+  if (!DECL_INITIAL (fn))
+    return NULL_TREE;
+
+  const unsigned block_pos = key.index >> 10;
+  const unsigned decl_pos = key.index & 1023;
+
+  if (decl_pos == 1023)
+    return NULL_TREE;
+
+  auto_vec<tree, 4> blocks;
+  blocks.quick_push (DECL_INITIAL (fn));
+  unsigned block_ix = 0;
+  while (block_ix != blocks.length ())
+    {
+      tree block = blocks[block_ix];
+      if (block_ix == block_pos)
+	{
+	  unsigned decl_ix = 0;
+	  for (tree var = BLOCK_VARS (block); var; var = DECL_CHAIN (var))
+	    {
+	      if (TREE_CODE (var) != TYPE_DECL)
+		continue;
+	      if (decl_ix == decl_pos)
+		return var;
+	      ++decl_ix;
+	    }
+	  return NULL_TREE;
+	}
+      for (tree sub = BLOCK_SUBBLOCKS (block); sub; sub = BLOCK_CHAIN (sub))
+	blocks.safe_push (sub);
+      ++block_ix;
+    }
+
+  return NULL_TREE;
+}
+
 /* DEP is the depset of some decl we're streaming by value.  Determine
    the merging behaviour.  */
 
@@ -10456,17 +10569,10 @@ trees_out::get_merge_kind (tree decl, depset *dep)
 	    gcc_unreachable ();
 
 	  case FUNCTION_DECL:
-	    // FIXME: This can occur for (a) voldemorty TYPE_DECLS
-	    // (which are returned from a function), or (b)
-	    // block-scope class definitions in template functions.
-	    // These are as unique as the containing function.  While
-	    // on read-back we can discover if the CTX was a
-	    // duplicate, we don't have a mechanism to get from the
-	    // existing CTX to the existing version of this decl.
 	    gcc_checking_assert
 	      (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl)));
 
-	    mk = MK_unique;
+	    mk = MK_local_type;
 	    break;
 
 	  case RECORD_TYPE:
@@ -10775,6 +10881,10 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 	  }
 	  break;
 
+	case MK_local_type:
+	  key_local_type (key, STRIP_TEMPLATE (decl), container);
+	  break;
+
 	case MK_enum:
 	  {
 	    /* Anonymous enums are located by their first identifier,
@@ -11131,11 +11241,10 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 	    break;
 
 	  case FUNCTION_DECL:
-	    // FIXME: What about a voldemort? how do we find what it
-	    // duplicates? Do we have to number vmorts relative to
-	    // their containing function?  But how would that work
-	    // when matching an in-TU declaration?
-	    kind = "unique";
+	    gcc_checking_assert (mk == MK_local_type);
+	    existing = key_local_type (key, container);
+	    if (existing && inner != decl)
+	      existing = TYPE_TI_TEMPLATE (TREE_TYPE (existing));
 	    break;
 
 	  case TYPE_DECL:
@@ -11388,6 +11497,11 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
 	/* Just like duplicate_decls, presum the user knows what
 	   they're doing in overriding a builtin.   */
 	TREE_TYPE (existing) = TREE_TYPE (decl);
+      else if (decl_function_context (decl))
+	/* The type of a mergeable local entity (such as a function scope
+	   capturing lambda's closure type fields) can depend on an
+	   unmergeable local entity (such as a local variable), so type
+	   equality isn't feasible in general for local entities.  */;
       else
 	{
 	  // FIXME:QOI Might be template specialization from a module,
diff --git a/gcc/testsuite/g++.dg/modules/merge-17.h b/gcc/testsuite/g++.dg/modules/merge-17.h
new file mode 100644
index 00000000000..246ccd8011d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/merge-17.h
@@ -0,0 +1,28 @@
+// PR c++/99426
+
+inline auto f() {
+  struct A { int m = 42; };
+  return A{};
+}
+
+template<class T>
+inline auto ft() {
+  decltype(+T()) x;
+  return [&x] { };
+}
+
+inline auto g() {
+  enum E { e };
+  return e;
+}
+
+template<class T>
+inline auto gt() {
+  enum E : T { e };
+  return e;
+}
+
+using ty1 = decltype(f());
+using ty2 = decltype(ft<int>());
+using ty3 = decltype(g());
+using ty4 = decltype(gt<int>());
diff --git a/gcc/testsuite/g++.dg/modules/merge-17_a.H b/gcc/testsuite/g++.dg/modules/merge-17_a.H
new file mode 100644
index 00000000000..0440cd765e9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/merge-17_a.H
@@ -0,0 +1,3 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+#include "merge-17.h"
diff --git a/gcc/testsuite/g++.dg/modules/merge-17_b.C b/gcc/testsuite/g++.dg/modules/merge-17_b.C
new file mode 100644
index 00000000000..4315b99f172
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/merge-17_b.C
@@ -0,0 +1,3 @@
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+#include "merge-17.h"
+import "merge-17_a.H";
diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H b/gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H
new file mode 100644
index 00000000000..bf7859fba99
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/xtreme-header-7_a.H
@@ -0,0 +1,4 @@
+// { dg-additional-options -fmodule-header }
+
+// { dg-module-cmi {} }
+#include "xtreme-header.h"
diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C b/gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C
new file mode 100644
index 00000000000..201d092e883
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/xtreme-header-7_b.C
@@ -0,0 +1,6 @@
+// A version of xtreme-header_{a.H,b.C} that doesn't use
+// -fno-module-lazy.
+// { dg-additional-options -fmodules-ts }
+
+#include "xtreme-header.h"
+import "xtreme-header-7_a.H";
-- 
2.44.0.84.gb387623c12


  reply	other threads:[~2024-03-05 15:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-27  2:37 Patrick Palka
2024-02-27 18:10 ` Patrick Palka
2024-03-05 15:31   ` Patrick Palka [this message]
2024-03-26 14:24     ` Patrick Palka
2024-04-09 20:27       ` Patrick Palka
2024-04-09 21:57     ` Jason Merrill
2024-04-10 18:48       ` Patrick Palka
2024-04-10 22:55         ` Jason Merrill
2024-04-12 14:35           ` Patrick Palka
2024-04-12 17:18             ` Jason Merrill
2024-04-12 17:48               ` Patrick Palka
2024-04-12 18:07                 ` Jason Merrill
2024-04-12 18:39                   ` Patrick Palka
2024-04-12 19:08                     ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=befba40c-6e93-3cda-c37b-c7ab297a2264@idea \
    --to=ppalka@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=nathan@acm.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).