https://gcc.gnu.org/g:4cd99e41ef687fd62d6908f4807de277c7dc9803
commit r15-8012-g4cd99e41ef687fd62d6908f4807de277c7dc9803
Author: Nathaniel Shead <nathanielosh...@gmail.com>
Date: Mon Mar 10 23:35:40 2025 +1100
c++/modules: Handle gnu_inline attribute, cleanup linkage determination
[PR119154]
Currently, note_vague_linkage_fn is called on all definitions imported
from modules. This is not correct, however; among other things, a
gnu_inline function does not have vague linkage, and causes an ICE if
its treated as such.
There are other things that we seem to potentially miss (e.g. dllexport
handling), so it seems sensible to stop trying to manage linkage in such
an ad-hoc manner but to use the normal interfaces more often. While
looking at this I also found that we seem to miss marking vague linkage
variables as COMDAT, so this patch fixes that as well.
The change to use expand_or_defer_fn exposes a checking-only ICE in
trees_in::assert_definition, where we forget that we already installed a
definition for a function. We work around this by instead of clearing
DECL_SAVED_TREE entirely in expand_or_defer_fn_1, we instead set it to a
dummy value. This way we can also avoid the check for !TREE_ASM_WRITTEN
beforehand.
PR c++/119154
gcc/cp/ChangeLog:
* decl2.cc (vague_linkage_p): Don't treat gnu_inline functions
as having vague linkage.
* module.cc (trees_out::core_bools): Clear DECL_INTERFACE_KNOWN
for vague-linkage entities.
(read_var_def): Maybe set comdat linkage for imported var
definitions.
(module_state::read_cluster): Use expand_or_defer_fn instead of
ad-hoc linkage management.
(post_load_processing): Likewise.
* semantics.cc (expand_or_defer_fn_1): Don't forget that we had
a definition at all.
gcc/testsuite/ChangeLog:
* g++.dg/modules/linkage-3_a.C: New test.
* g++.dg/modules/linkage-3_b.C: New test.
* g++.dg/modules/pr119154_a.C: New test.
* g++.dg/modules/pr119154_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
Diff:
---
gcc/cp/decl2.cc | 4 +++-
gcc/cp/module.cc | 33 +++++++-----------------------
gcc/cp/semantics.cc | 2 +-
gcc/testsuite/g++.dg/modules/linkage-3_a.C | 5 +++++
gcc/testsuite/g++.dg/modules/linkage-3_b.C | 9 ++++++++
gcc/testsuite/g++.dg/modules/pr119154_a.C | 6 ++++++
gcc/testsuite/g++.dg/modules/pr119154_b.C | 10 +++++++++
7 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index fe9c56b66373..4a9fb1c3c00c 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2482,7 +2482,9 @@ vague_linkage_p (tree decl)
DECL_COMDAT. */
if (DECL_COMDAT (decl)
|| (TREE_CODE (decl) == FUNCTION_DECL
- && DECL_DECLARED_INLINE_P (decl))
+ && DECL_DECLARED_INLINE_P (decl)
+ /* But gnu_inline functions are always external. */
+ && !lookup_attribute ("gnu_inline", DECL_ATTRIBUTES (decl)))
|| (DECL_LANG_SPECIFIC (decl)
&& DECL_TEMPLATE_INSTANTIATION (decl))
|| (VAR_P (decl) && DECL_INLINE_VAR_P (decl)))
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 9a1cd5244c2c..a17d8433e34c 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -5657,10 +5657,10 @@ trees_out::core_bools (tree t, bits_out& bits)
{
/* This is DECL_INTERFACE_KNOWN: We should redetermine whether
- we need to import or export any vtables or typeinfo objects
- on stream-in. */
+ we need to import or export any vague-linkage entities on
+ stream-in. */
bool interface_known = t->decl_common.lang_flag_5;
- if (VAR_P (t) && (DECL_VTABLE_OR_VTT_P (t) || DECL_TINFO_P (t)))
+ if (interface_known && vague_linkage_p (t))
interface_known = false;
WB (interface_known);
}
@@ -12616,6 +12616,7 @@ trees_in::read_var_def (tree decl, tree maybe_template)
bool installing = maybe_dup && !initialized;
if (installing)
{
+ DECL_INITIAL (decl) = init;
if (DECL_EXTERNAL (decl))
DECL_NOT_REALLY_EXTERN (decl) = true;
if (VAR_P (decl))
@@ -12623,13 +12624,13 @@ trees_in::read_var_def (tree decl, tree
maybe_template)
DECL_INITIALIZED_P (decl) = true;
if (maybe_dup && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P
(maybe_dup))
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = true;
+ tentative_decl_linkage (decl);
if (DECL_IMPLICIT_INSTANTIATION (decl)
|| (DECL_CLASS_SCOPE_P (decl)
&& !DECL_VTABLE_OR_VTT_P (decl)
&& !DECL_TEMPLATE_INFO (decl)))
note_vague_linkage_variable (decl);
}
- DECL_INITIAL (decl) = init;
if (!dyn_init)
;
else if (CP_DECL_THREAD_LOCAL_P (decl))
@@ -16534,12 +16535,7 @@ module_state::read_cluster (unsigned snum)
cfun->returns_pcc_struct = aggr;
#endif
cfun->returns_struct = aggr;
-
- if (DECL_COMDAT (decl))
- // FIXME: Comdat grouping?
- comdat_linkage (decl);
- note_vague_linkage_fn (decl);
- cgraph_node::finalize_function (decl, true);
+ expand_or_defer_fn (decl);
}
}
@@ -19018,22 +19014,7 @@ post_load_processing ()
dump () && dump ("Post-load processing of %N", decl);
gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CDTOR_P (decl));
-
- if (DECL_COMDAT (decl))
- comdat_linkage (decl);
- if (!TREE_ASM_WRITTEN (decl))
- {
- /* Cloning can cause loading -- specifically operator delete for
- the deleting dtor. */
- if (maybe_clone_body (decl))
- TREE_ASM_WRITTEN (decl) = 1;
- else
- {
- /* We didn't clone the cdtor, make sure we emit it. */
- note_vague_linkage_fn (decl);
- cgraph_node::finalize_function (decl, true);
- }
- }
+ expand_or_defer_fn (decl);
}
cfun = old_cfun;
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 6e10893262dd..b0a5f9ad6607 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -5502,7 +5502,7 @@ expand_or_defer_fn_1 (tree fn)
need it anymore. */
if (!DECL_DECLARED_CONSTEXPR_P (fn)
&& !(module_maybe_has_cmi_p () && vague_linkage_p (fn)))
- DECL_SAVED_TREE (fn) = NULL_TREE;
+ DECL_SAVED_TREE (fn) = void_node;
return false;
}
diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_a.C
b/gcc/testsuite/g++.dg/modules/linkage-3_a.C
new file mode 100644
index 000000000000..2bee9c87957b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/linkage-3_a.C
@@ -0,0 +1,5 @@
+// { dg-do compile { target *-*-*gnu* } }
+// { dg-additional-options "-fmodules" }
+
+export module M;
+export inline int x = 0;
diff --git a/gcc/testsuite/g++.dg/modules/linkage-3_b.C
b/gcc/testsuite/g++.dg/modules/linkage-3_b.C
new file mode 100644
index 000000000000..ae145217cacb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/linkage-3_b.C
@@ -0,0 +1,9 @@
+// { dg-do compile { target *-*-*gnu* } }
+// { dg-additional-options "-fmodules" }
+// { dg-final { scan-assembler "_ZW1M1x,comdat" } }
+
+import M;
+
+int main() {
+ return x;
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr119154_a.C
b/gcc/testsuite/g++.dg/modules/pr119154_a.C
new file mode 100644
index 000000000000..23bd186fe191
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr119154_a.C
@@ -0,0 +1,6 @@
+// PR c++/119154
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi foo }
+
+export module foo;
+extern "C++" inline __attribute__((__gnu_inline__)) void bar() {}
diff --git a/gcc/testsuite/g++.dg/modules/pr119154_b.C
b/gcc/testsuite/g++.dg/modules/pr119154_b.C
new file mode 100644
index 000000000000..1558e717761d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr119154_b.C
@@ -0,0 +1,10 @@
+// PR c++/119154
+// { dg-module-do link }
+// { dg-additional-options "-fmodules" }
+
+void bar();
+import foo;
+
+int main() {
+ bar();
+}
