From patchwork Wed Mar 6 03:06:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 86847 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A6312385800A for ; Wed, 6 Mar 2024 03:07:21 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id 583DF38582BA for ; Wed, 6 Mar 2024 03:06:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 583DF38582BA Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 583DF38582BA Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709694410; cv=none; b=BYHms5OfOhsHxPewEtIlUjukqmm8tq63JL42Bbf1Nx5IVNMkWS+HqK+8rVDXSDwWxT6ODEXj8j41EvyUaZe9619L7vVile0EqUBy0hdjGMvgndIydtvRv7awDaNjLM7gogzcTdd667FTsPLG38+gXkQcbQq7uL20DlrAKEsyjVk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709694410; c=relaxed/simple; bh=dDQPpwjOH6PHkdPbRFcf6x4WC6TGRlrXR4riW/O8xn4=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=f+rbTQH8O5FgRwXoIYhg1/78Zk3gI+9bgDzLWkkC1Z84dvbynFHKVroNQJ4yZcWy2pL3RX6iRQsCXgc4HfnU0oZwdi9lxwHVqEIQoUBQJuzi9gRL0Pbxrm8xnk9w6bPuSTmabZzT1LzoRt8StW+i3vUtnYIcyZ9NR3NqB6Um0Gg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62f.google.com with SMTP id d9443c01a7336-1dc96f64c10so58556465ad.1 for ; Tue, 05 Mar 2024 19:06:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709694405; x=1710299205; darn=gcc.gnu.org; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=wQukrgDydydxrVwTd+sM+cUZw0+ddqEAlD/OxosM1ow=; b=jnTSjpjVTG7xZc6YIYRqMun84rtf57t136rGnxdZWyHBTFpU6DsoR9c/MPLMBY8I8H js7dfJsDdjM371OPQW/OvmS68oQ8b+livHO/d6CJE+PMij6CVVVFk/88BbtIEUHkKNsw zuV4c2lZTAfZjqKs6KATs8bFCsJEjlqOJfFyao4m9nTR9r1dmMd50lyK5v/m1ylW6Y4t tD37Y5AZi4U+UIWfRR3+qAip21sHXiEi4NkAGZ4A5yVhNZs15HSs9ZEd8mywUEg+ocpF QR2MTNnQWMqP2z93q9SfEyq2fUZdnXEUM1T9qmMSgsKuEI5wnsfa4OHhQ0gGAZXAHIY6 Qe3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709694405; x=1710299205; h=content-disposition:mime-version:subject:cc:to:from:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wQukrgDydydxrVwTd+sM+cUZw0+ddqEAlD/OxosM1ow=; b=FyJei9EX4ckP3n1ywOKTz+5AdHT7P7mejnWM/tMan2dq3UfABHwzyiy7J2I5WgtB4R kNlMCnsdPqvnc00ZvQ03kJrP/hsWiAvbesCGPrRUXg93SdAGr/Oz9+lco928UQJNraB7 bWcPmj9mHXuQL2LFxlN12FFHNVFph1JK9M132hYBTnzbEmil4cm2p071KvVGR7C40PMI NJlkQ6xnkkYtdYNmRB0s/Aw3OQ/8bb1fpllZnTHuZ2wq/yGgKCTcrfDFAxOZdgf+OrRz Eh8YNMs3e6KeS+nunCYhkJheZpVBcW0tJ1Is95CJS5km/oTAz33ZsrnlOX38MgYfkDeU ozMQ== X-Gm-Message-State: AOJu0YwBlEPE7Xiqd0wK4TxcwFPO8ls1txr3o8f9oS1l8uJqYYlUP6a4 4rF/iwB5feRkEuzcu1PAhXAZJOrFsPH2U4hGVzIXYlf11R8yZ3bpJzIIeQAu X-Google-Smtp-Source: AGHT+IHvMDdVMfZcgLR6GTLlbLFKPwTFDL4vgrEOXbxINun0hH2d2x4qepXHwwFvIbeVv480NSX90w== X-Received: by 2002:a17:902:d488:b0:1dc:e58:8ab4 with SMTP id c8-20020a170902d48800b001dc0e588ab4mr5201956plg.9.1709694405037; Tue, 05 Mar 2024 19:06:45 -0800 (PST) Received: from Thaum. (124-168-30-222.tpgi.com.au. [124.168.30.222]) by smtp.gmail.com with ESMTPSA id mm6-20020a1709030a0600b001db67377e8dsm11313320plb.248.2024.03.05.19.06.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Mar 2024 19:06:44 -0800 (PST) Message-ID: <65e7ddc4.170a0220.d647a.c749@mx.google.com> X-Google-Original-Message-ID: Date: Wed, 6 Mar 2024 14:06:39 +1100 From: Nathaniel Shead To: gcc-patches@gcc.gnu.org Cc: Jason Merrill , Nathan Sidwell , Patrick Palka Subject: [PATCH] c++/modules: Prevent emission of really-extern vtables in importers [PR114229] MIME-Version: 1.0 Content-Disposition: inline X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- Currently, reading a variable definition always marks that decl as DECL_NOT_REALLY_EXTERN, with anything else imported still being considered external. This is not sufficient for vtables, however; for an extern template, a vtable may be generated (and its definition emitted) but nonetheless the vtable should only be emitted in the TU where that template is actually instantiated. While playing around with various settings of DECL_EXTERNAL I also noticed that we mark variables not declared 'inline' as external, which makes sense (there's a definition in another TU, while for vague linkage variables we'll be importing the definition too), but we also do so for 'static constexpr' members that aren't directly marked 'inline'. This fixes that for consistency, if nothing else (I wasn't able to cause an actual issue exploiting this currently), but may become relevant with https://github.com/itanium-cxx-abi/cxx-abi/issues/170. PR c++/114229 gcc/cp/ChangeLog: * module.cc (trees_out::core_bools): Count 'static constexpr' vars also as inline. (trees_out::write_var_def): Stream DECL_NOT_REALLY_EXTERN for vtables. (trees_in::read_var_def): Read it. gcc/testsuite/ChangeLog: * g++.dg/modules/virt-2_c.C: * g++.dg/modules/virt-3_a.C: New test. * g++.dg/modules/virt-3_b.C: New test. * g++.dg/modules/virt-3_c.C: New test. * g++.dg/modules/virt-3_d.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/module.cc | 15 +++++++++++++-- gcc/testsuite/g++.dg/modules/virt-2_c.C | 14 +++++++++----- gcc/testsuite/g++.dg/modules/virt-3_a.C | 9 +++++++++ gcc/testsuite/g++.dg/modules/virt-3_b.C | 6 ++++++ gcc/testsuite/g++.dg/modules/virt-3_c.C | 3 +++ gcc/testsuite/g++.dg/modules/virt-3_d.C | 7 +++++++ 6 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_a.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_b.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_c.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-3_d.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 67f132d28d7..771e56245dc 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5418,7 +5418,7 @@ trees_out::core_bools (tree t) && !(TREE_STATIC (t) && DECL_FUNCTION_SCOPE_P (t) && DECL_DECLARED_INLINE_P (DECL_CONTEXT (t))) - && !DECL_VAR_DECLARED_INLINE_P (t)) + && !DECL_INLINE_VAR_P (t)) is_external = true; break; @@ -11799,6 +11799,12 @@ trees_out::write_var_def (tree decl) } tree_node (dyn_init); } + + /* For vtables we need to know if they're actually extern or not, + even if we get a definition; for other kinds of variables we + can assume that if we have a definition they can be emitted. */ + if (streaming_p () && VAR_P (decl) && DECL_VTABLE_OR_VTT_P (decl)) + u (DECL_NOT_REALLY_EXTERN (decl)); } void @@ -11816,6 +11822,11 @@ trees_in::read_var_def (tree decl, tree maybe_template) tree dyn_init = init ? NULL_TREE : tree_node (); unused -= vtable; + /* Assume for most vars that if we have a definition it's not extern. */ + bool not_really_extern = true; + if (vtable) + not_really_extern = u (); + if (get_overrun ()) return false; @@ -11826,7 +11837,7 @@ trees_in::read_var_def (tree decl, tree maybe_template) if (installing) { if (DECL_EXTERNAL (decl)) - DECL_NOT_REALLY_EXTERN (decl) = true; + DECL_NOT_REALLY_EXTERN (decl) = not_really_extern; if (VAR_P (decl)) { DECL_INITIALIZED_P (decl) = true; diff --git a/gcc/testsuite/g++.dg/modules/virt-2_c.C b/gcc/testsuite/g++.dg/modules/virt-2_c.C index 7b3eeebe508..8969cb04911 100644 --- a/gcc/testsuite/g++.dg/modules/virt-2_c.C +++ b/gcc/testsuite/g++.dg/modules/virt-2_c.C @@ -9,8 +9,12 @@ int Foo () return !(Visit (&v) == 0); } -// We do emit Visitor vtable -// andl also we do emit rtti here -// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } } -// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } } -// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } } +// Again, we do not emit Visitor vtable +// but we do emit rtti here + +// but see https://github.com/itanium-cxx-abi/cxx-abi/issues/170: +// we should only emit RTTI in virt-2_a.C, alongside the vtable + +// { dg-final { scan-assembler-not {_ZTVW3foo7Visitor:} } } +// { dg-final { scan-assembler-not {_ZTIW3foo7Visitor:} { xfail *-*-* } } } +// { dg-final { scan-assembler-not {_ZTSW3foo7Visitor:} { xfail *-*-* } } } diff --git a/gcc/testsuite/g++.dg/modules/virt-3_a.C b/gcc/testsuite/g++.dg/modules/virt-3_a.C new file mode 100644 index 00000000000..a7eae7f9d35 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_a.C @@ -0,0 +1,9 @@ +// PR c++/114229 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi modA } + +module; +template struct basic_streambuf { virtual void overflow() { } }; +extern template struct basic_streambuf; +export module modA; +export basic_streambuf *p; diff --git a/gcc/testsuite/g++.dg/modules/virt-3_b.C b/gcc/testsuite/g++.dg/modules/virt-3_b.C new file mode 100644 index 00000000000..4d87b965bbf --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_b.C @@ -0,0 +1,6 @@ +// PR c++/114229 +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } +// { dg-module-cmi modB } + +export module modB; +import modA; diff --git a/gcc/testsuite/g++.dg/modules/virt-3_c.C b/gcc/testsuite/g++.dg/modules/virt-3_c.C new file mode 100644 index 00000000000..224bb4aed49 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_c.C @@ -0,0 +1,3 @@ +// PR c++/114229 +template struct basic_streambuf { virtual void overflow() { } }; +template struct basic_streambuf; diff --git a/gcc/testsuite/g++.dg/modules/virt-3_d.C b/gcc/testsuite/g++.dg/modules/virt-3_d.C new file mode 100644 index 00000000000..b3a0aad7abe --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-3_d.C @@ -0,0 +1,7 @@ +// PR c++/114229 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } + +import modA; +import modB; +int main() { }