On 8/20/24 7:40 PM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
OK.
-- >8 -- The ICE in the linked PR is caused because maybe_clone_decl is not prepared to be called on a declaration that has already had clones created; what happens otherwise is that start_preparsed_function early exits and never sets up cfun, causing a segfault later on. To fix this we ensure that post_load_processing only calls maybe_clone_decl if TREE_ASM_WRITTEN has not been marked on the declaration yet, and (if maybe_clone_decls succeeds) marks this flag on the decl so that it doesn't get called again later when finalising deferred vague linkage declarations in c_parse_final_cleanups. As a bonus this now allows us to only keep the DECL_SAVED_TREE around in expand_or_defer_fn_1 for modules which have CMIs, which will have benefits for LTO performance in non-interface TUs. For clarity we also update the streaming code to do post_load_decls for maybe in-charge cdtors rather than any DECL_ABSTRACT_P declaration, as this is more accurate to the decls affected by maybe_clone_body. PR c++/115007 gcc/cp/ChangeLog: * module.cc (module_state::read_cluster): Replace DECL_ABSTRACT_P with DECL_MAYBE_IN_CHARGE_CDTOR_P. (post_load_processing): Check and mark TREE_ASM_WRITTEN. * semantics.cc (expand_or_defer_fn_1): Use the more specific module_maybe_has_cmi_p instead of modules_p. gcc/testsuite/ChangeLog: * g++.dg/modules/virt-6_a.C: New test. * g++.dg/modules/virt-6_b.C: New test. Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com> --- gcc/cp/module.cc | 7 ++++--- gcc/cp/semantics.cc | 2 +- gcc/testsuite/g++.dg/modules/virt-6_a.C | 13 +++++++++++++ gcc/testsuite/g++.dg/modules/virt-6_b.C | 6 ++++++ 4 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/virt-6_a.C create mode 100644 gcc/testsuite/g++.dg/modules/virt-6_b.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 7c42aea05ee..5cd4f313933 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -15525,7 +15525,7 @@ module_state::read_cluster (unsigned snum)if (abstract); - else if (DECL_ABSTRACT_P (decl)) + else if (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)) vec_safe_push (post_load_decls, decl); else { @@ -17947,10 +17947,11 @@ post_load_processing ()dump () && dump ("Post-load processing of %N", decl); - gcc_checking_assert (DECL_ABSTRACT_P (decl));+ gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl)); /* Cloning can cause loading -- specifically operator delete for the deleting dtor. */ - maybe_clone_body (decl); + if (!TREE_ASM_WRITTEN (decl) && maybe_clone_body (decl)) + TREE_ASM_WRITTEN (decl) = 1; }cfun = old_cfun;diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 5ab2076b673..f7ae8e68dcf 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -5122,7 +5122,7 @@ expand_or_defer_fn_1 (tree fn) demand, so we also need to keep the body. Otherwise we don't need it anymore. */ if (!DECL_DECLARED_CONSTEXPR_P (fn) - && !(modules_p () && vague_linkage_p (fn))) + && !(module_maybe_has_cmi_p () && vague_linkage_p (fn))) DECL_SAVED_TREE (fn) = NULL_TREE; return false; } diff --git a/gcc/testsuite/g++.dg/modules/virt-6_a.C b/gcc/testsuite/g++.dg/modules/virt-6_a.C new file mode 100644 index 00000000000..68e466ace3f --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-6_a.C @@ -0,0 +1,13 @@ +// PR c++/115007 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi M:a } + +module; +struct S { + virtual ~S() = default; + virtual void f() = 0; +}; +module M:a; +extern S* p; +template <typename T> void format(T) { p->~S(); } +template void format(int); diff --git a/gcc/testsuite/g++.dg/modules/virt-6_b.C b/gcc/testsuite/g++.dg/modules/virt-6_b.C new file mode 100644 index 00000000000..c53f5fac742 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/virt-6_b.C @@ -0,0 +1,6 @@ +// PR c++/115007 +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M } + +export module M; +import :a;
