From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id DC6A83858C36 for ; Tue, 26 Mar 2024 14:24:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DC6A83858C36 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DC6A83858C36 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711463105; cv=none; b=PJqpIBhTRh8vukN9ujMhaOFn9Ip4vpxs4CT7/ML7qm7KhOt4WwY+qY4r9Ys2fNxrrZVX79EcpBlZEALY5brNxRS/wiNDp/f4NHVRdBJS/N88Nd+kaejPuXDYComGF5+TrhSe42I8Is5XtbqyNFE0FvAug+74rhRpZHaMmxkPFho= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711463105; c=relaxed/simple; bh=oHkJM61D8JOfAj2LYL6L31byNVXnQYfTcS5jkYYbY70=; h=DKIM-Signature:From:Date:To:Subject:Message-ID:MIME-Version; b=Sqxn02dRAtzMISMYNTjFvaSEUT0zI7znkoEUX0H5RMdsWc4f649xffxREDvjH4z16w+m20k5y92G5lVDsKRMlNX8P2UT1BHNJOb2LvBo60U4vGnmK2ZpeluAnhYM7X7ylPCAZ1lRAqVOuEF9PyZxglnrg2/zdpcew6b2AOJzZaw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711463099; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8bjmPENQEVo3s3pOOliIW7Qv2tpUAKe2j9TFkP9haTA=; b=bosRpHQEgcVBpA2BhyErx86+VoR4R5s9b4tmm72AzNps5EqKhfihGZSm4NoIamuwpnowO4 SXZEYcNzPEvLHVruPkE7d8FvAY88KUxxbNKDobYSWsvx7khqNL4T4WlBFK+QI96/563ifW GB14NsQ4vJWsdB1ABto3EahWCPSyNCw= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-662-M-IIuHDiP0CBnXsSWwdmYQ-1; Tue, 26 Mar 2024 10:24:56 -0400 X-MC-Unique: M-IIuHDiP0CBnXsSWwdmYQ-1 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-6958736b921so64180856d6.1 for ; Tue, 26 Mar 2024 07:24:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711463095; x=1712067895; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=8bjmPENQEVo3s3pOOliIW7Qv2tpUAKe2j9TFkP9haTA=; b=n5DS0VcuiuT8mSDjppxUtM9t9mU0/MamUieSnhVS+kBPbSHdn3o0qnT/6RLdyo6uW/ BgFN70Af/Vx8lIsAsmNct83NbLKN26mDQ9dHUgcLhPNAwTjPbryT7uT6wVhj10UalJWQ BFD6gcLXWoHFmFwHeSfvX9DZZEJBtbJ8tc7W9hyqd3rgkUuhkjgmYAtYlio60oIxyBvT AzWOBn+CsZjqFXdlNTEInOwrKHdp/4hkAkjxAw3sH+RQvsZ73awEhPk2xc0X8OhkRmfC ecCSVZ6kA6I8FJwsB7bvQkhKsoZL1I5zz+xBidAvhsLoSvAWvRCrBOe9e8P/KV5qUKHN bBww== X-Gm-Message-State: AOJu0Yz27jiRC3icRBLy9SVEqYtHb10408VNmLaa2sumABU0ZKMZGctN WwJfFLU9coRlrSEIpXFPHhp7gOO4Od9CEzWPMAl1J4IHu1i/FptMFZoaSOkGeMWGs8LfXPOQ1zZ IvDiDTxrdvvUtcyO9mSzdU0YSwS01ggn7wFMlGs2+nxQ34zUAqx2mcvM= X-Received: by 2002:ad4:5c62:0:b0:696:4084:d6f6 with SMTP id i2-20020ad45c62000000b006964084d6f6mr13780357qvh.8.1711463095248; Tue, 26 Mar 2024 07:24:55 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEqf3TVkF0eRiQylAWV8Vic3UkV44UFOd0kGgmZfmAmykLkcnw+B2ki/MZ1c+EHqqZc/X9tfg== X-Received: by 2002:ad4:5c62:0:b0:696:4084:d6f6 with SMTP id i2-20020ad45c62000000b006964084d6f6mr13780331qvh.8.1711463094717; Tue, 26 Mar 2024 07:24:54 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id p21-20020a05620a057500b00789ee9e15b2sm3041367qkp.123.2024.03.26.07.24.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Mar 2024 07:24:54 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 26 Mar 2024 10:24:53 -0400 (EDT) To: Patrick Palka cc: gcc-patches@gcc.gnu.org, jason@redhat.com, nathan@acm.org Subject: Re: [PATCH] c++/modules: local class merging [PR99426] In-Reply-To: Message-ID: References: <20240227023734.2742095-1-ppalka@redhat.com> <9d4f1a3e-99ea-5146-639d-22add3e22301@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,TXREP,URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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) : ... here. Stream > > > BLOCK_VARS manually. > > > (trees_in::core_vals) : 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) : Return > > > MK_local_class for a local class. > > > (trees_out::key_mergeable) : Use > > > key_local_class. > > > (trees_in::key_mergeable) : 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 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 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. > > -- >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) : ... here. Stream > BLOCK_VARS manually. > (trees_in::core_vals) : 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) : Return > MK_local_type for a local type. > (trees_out::key_mergeable) : Use > key_local_type. > (trees_in::key_mergeable) : 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 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 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 > +inline auto ft() { > + decltype(+T()) x; > + return [&x] { }; > +} > + > +inline auto g() { > + enum E { e }; > + return e; > +} > + > +template > +inline auto gt() { > + enum E : T { e }; > + return e; > +} > + > +using ty1 = decltype(f()); > +using ty2 = decltype(ft()); > +using ty3 = decltype(g()); > +using ty4 = decltype(gt()); > 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 > >