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, 9 Apr 2024 16:27:45 -0400 (EDT) [thread overview]
Message-ID: <4e3e4a5d-e4d3-1da1-512e-2f7ea06a232d@idea> (raw)
In-Reply-To: <a0e7e4f9-0ac6-d0ca-c639-0304596a3aed@idea>
On Tue, 26 Mar 2024, Patrick Palka wrote:
> On Tue, 5 Mar 2024, Patrick Palka wrote:
>
> > 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.
>
> Ping.
Ping.
>
> >
> > -- >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
> >
> >
>
next prev parent reply other threads:[~2024-04-09 20:27 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
2024-03-26 14:24 ` Patrick Palka
2024-04-09 20:27 ` Patrick Palka [this message]
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=4e3e4a5d-e4d3-1da1-512e-2f7ea06a232d@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).