c++/modules: Handle conflicting ABI tags [PR118920]

The ICE in the linked PR is caused because out_ptr_t inherits an ABI tag
in a module that it does not in the importing module.  When we try to
build a qualified 'const out_ptr_t' during stream-in, we find the
existing 'const out_ptr_t' variant type that has been built, but discard
it due to having a mismatching attribute list.  This causes us to build
a new copy of this variant, and ultimately fail a checking assertion due
to this being an identical type with different TYPE_CANONICAL.

This patch adds checking that ABI tags between an imported and existing
declaration match, and errors if they are incompatible.  We make use of
'equal_abi_tags' from mangle.cc to determine if we should error; in the
case in the PR, because the ABI tag was an implicit tag that doesn't
affect name mangling, we don't need to error.  To fix the ICE we ensure
that (regardless of whether we errored or not) later processing
considers the ABI tags as equivalent.

	PR c++/118920

gcc/cp/ChangeLog:

	* cp-tree.h (equal_abi_tags): Declare.
	* mangle.cc (equal_abi_tags): Make external, fix comparison.
	(tree_string_cmp): Make internal.
	* module.cc (trees_in::check_abi_tags): New function.
	(trees_in::decl_value): Use it.
	(trees_in::is_matching_decl): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/attrib-3_a.H: New test.
	* g++.dg/modules/attrib-3_b.C: New test.
	* g++.dg/modules/pr118920.h: New test.
	* g++.dg/modules/pr118920_a.H: New test.
	* g++.dg/modules/pr118920_b.H: New test.
	* g++.dg/modules/pr118920_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 07500fa2b21ab087fde1a3a545416da70257dcd4..6f0f5cc14a25672f3aa8983be52770de86a5f7d7 100644 (file)
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8623,6 +8623,7 @@ extern void mangle_module_substitution            (int);
 extern int mangle_module_component             (tree id, bool partition);
 extern tree mangle_module_global_init          (int);
 extern unsigned HOST_WIDE_INT range_expr_nelts (tree);
+extern bool equal_abi_tags                     (tree, tree);
 
 /* in dump.cc */
 extern bool cp_dump_tree                       (void *, tree);

diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 9ca5cf6eecdcbcc10ed23bba3f43fb8c30a66d26..02129c6b89a156d61bcb3aed10710b6609d73ad1 100644 (file)
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -238,7 +238,6 @@ static void write_local_name (tree, const tree, const tree);
 static void dump_substitution_candidates (void);
 static tree mangle_decl_string (const tree);
 static void maybe_check_abi_tags (tree, tree = NULL_TREE, int = 10);
-static bool equal_abi_tags (tree, tree);
 
 /* Control functions.  */
 
@@ -1673,7 +1672,7 @@ write_source_name (tree identifier)
 
 /* Compare two TREE_STRINGs like strcmp.  */
 
-int
+static int
 tree_string_cmp (const void *p1, const void *p2)
 {
   if (p1 == p2)
@@ -1728,7 +1727,7 @@ write_abi_tags (tree tags)
 
 /* True iff the TREE_LISTS T1 and T2 of ABI tags are equivalent.  */
 
-static bool
+bool
 equal_abi_tags (tree t1, tree t2)
 {
   releasing_vec v1 = sorted_abi_tags (t1);
@@ -1738,7 +1737,8 @@ equal_abi_tags (tree t1, tree t2)
   if (len1 != v2->length())
     return false;
   for (unsigned i = 0; i < len1; ++i)
-    if (tree_string_cmp (v1[i], v2[i]) != 0)
+    if (strcmp (TREE_STRING_POINTER (v1[i]),
+               TREE_STRING_POINTER (v2[i])) != 0)
       return false;
   return true;
 }

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f2d64bf3b5f75633a0d20067ee696d09911c4292..214fb918e8d8bfedd54a5319b3fa7ec3abf09c2d 100644 (file)
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3012,6 +3012,7 @@ public:
   bool read_definition (tree decl);
 
 private:
+  void check_abi_tags (tree existing, tree decl, tree &eattr, tree &dattr);
   bool is_matching_decl (tree existing, tree decl, bool is_typedef);
   static bool install_implicit_member (tree decl);
   bool read_function_def (tree decl, tree maybe_template);
@@ -8832,8 +8833,11 @@ trees_in::decl_value ()
          tree etype = TREE_TYPE (existing);
 
          /* Handle separate declarations with different attributes.  */
+         tree &dattr = TYPE_ATTRIBUTES (type);
          tree &eattr = TYPE_ATTRIBUTES (etype);
-         eattr = merge_attributes (eattr, TYPE_ATTRIBUTES (type));
+         check_abi_tags (existing, decl, eattr, dattr);
+         // TODO: handle other conflicting type attributes
+         eattr = merge_attributes (eattr, dattr);
 
          /* When merging a partial specialisation, the existing decl may have
             had its TYPE_CANONICAL adjusted.  If so we should use structural
@@ -12011,6 +12015,51 @@ trees_in::binfo_mergeable (tree *type)
   return u ();
 }
 
+/* DECL is a just streamed declaration with attributes DATTR that should
+   have matching ABI tags as EXISTING's attributes EATTR.  Check that the
+   ABI tags match, and report an error if not.  */
+
+void
+trees_in::check_abi_tags (tree existing, tree decl, tree &eattr, tree &dattr)
+{
+  tree etags = lookup_attribute ("abi_tag", eattr);
+  tree dtags = lookup_attribute ("abi_tag", dattr);
+  if ((etags == nullptr) != (dtags == nullptr)
+      || (etags && !attribute_value_equal (etags, dtags)))
+    {
+      if (etags)
+       etags = TREE_VALUE (etags);
+      if (dtags)
+       dtags = TREE_VALUE (dtags);
+
+      /* We only error if mangling wouldn't consider the tags equivalent.  */
+      if (!equal_abi_tags (etags, dtags))
+       {
+         auto_diagnostic_group d;
+         if (dtags)
+           error_at (DECL_SOURCE_LOCATION (decl),
+                     "mismatching abi tags for %qD with tags %qE",
+                     decl, dtags);
+         else
+           error_at (DECL_SOURCE_LOCATION (decl),
+                     "mismatching abi tags for %qD with no tags", decl);
+         if (etags)
+           inform (DECL_SOURCE_LOCATION (existing),
+                   "existing declaration here with tags %qE", etags);
+         else
+           inform (DECL_SOURCE_LOCATION (existing),
+                   "existing declaration here with no tags");
+       }
+
+      /* Always use the existing abi_tags as the canonical set so that
+        later processing doesn't get confused.  */
+      if (dtags)
+       dattr = remove_attribute ("abi_tag", dattr);
+      if (etags)
+       duplicate_one_attribute (&dattr, eattr, "abi_tag");
+    }
+}
+
 /* DECL is a just streamed mergeable decl that should match EXISTING.  Check
    it does and issue an appropriate diagnostic if not.  Merge any
    bits from DECL to EXISTING.  This is stricter matching than
@@ -12222,6 +12271,10 @@ trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
   if (!DECL_EXTERNAL (d_inner))
     DECL_EXTERNAL (e_inner) = false;
 
+  if (VAR_OR_FUNCTION_DECL_P (d_inner))
+    check_abi_tags (existing, decl,
+                   DECL_ATTRIBUTES (e_inner), DECL_ATTRIBUTES (d_inner));
+
   if (TREE_CODE (decl) == TEMPLATE_DECL)
     {
       /* Merge default template arguments.  */

diff --git a/gcc/testsuite/g++.dg/modules/attrib-3_a.H b/gcc/testsuite/g++.dg/modules/attrib-3_a.H
new file mode 100644 (file)
index 0000000..d1f672a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/attrib-3_a.H
@@ -0,0 +1,16 @@
+// PR c++/118920
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+struct [[gnu::abi_tag("a", "b")]] A;
+struct [[gnu::abi_tag("a", "b")]] B;
+struct [[gnu::abi_tag("c")]] C;
+struct D;
+struct E;
+
+[[gnu::abi_tag("f")]] void f();
+void g();
+[[gnu::abi_tag("x", "y")]] void h();
+
+[[gnu::abi_tag("test", "some", "more")]] extern int i;
+[[gnu::abi_tag("test", "some", "more")]] extern int j;

diff --git a/gcc/testsuite/g++.dg/modules/attrib-3_b.C b/gcc/testsuite/g++.dg/modules/attrib-3_b.C
new file mode 100644 (file)
index 0000000..557c18b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/attrib-3_b.C
@@ -0,0 +1,30 @@
+// PR c++/118920
+// { dg-additional-options "-fmodules -fno-module-lazy" }
+
+struct A {};  // { dg-message "existing declaration here" }
+struct [[gnu::abi_tag("b", "a")]] B;  // OK
+struct [[gnu::abi_tag("x")]] C;  // { dg-message "existing declaration here" }
+struct [[gnu::abi_tag("d")]] D;  // { dg-message "existing declaration here" }
+
+void f();  // { dg-message "existing declaration here" }
+[[gnu::abi_tag("g")]] void g();  // { dg-message "existing declaration here" }
+[[gnu::abi_tag("y", "x")]] void h();  // OK
+
+[[gnu::abi_tag("more", "test")]] extern int i;  // { dg-message "existing declaration here" }
+[[gnu::abi_tag("more", "test", "really", "some")]] extern int j;  // { dg-message "existing declaration here" }
+
+import "attrib-3_a.H";
+
+struct [[gnu::abi_tag("e")]] E;  // { dg-error "adds abi tag" }
+
+// { dg-error "mismatching abi tags for .struct A." "" { target *-*-* } 0 }
+// B is OK
+// { dg-error "mismatching abi tags for .struct C." "" { target *-*-* } 0 }
+// { dg-error "mismatching abi tags for .struct D." "" { target *-*-* } 0 }
+
+// { dg-error "mismatching abi tags for .void f()." "" { target *-*-* } 0 }
+// { dg-error "mismatching abi tags for .void g()." "" { target *-*-* } 0 }
+// h is OK
+
+// { dg-error "mismatching abi tags for .i." "" { target *-*-* } 0 }
+// { dg-error "mismatching abi tags for .j." "" { target *-*-* } 0 }

diff --git a/gcc/testsuite/g++.dg/modules/pr118920.h b/gcc/testsuite/g++.dg/modules/pr118920.h
new file mode 100644 (file)
index 0000000..73d38b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118920.h
@@ -0,0 +1,3 @@
+template <typename T> struct out_ptr_t {
+  operator int() const;
+};

diff --git a/gcc/testsuite/g++.dg/modules/pr118920_a.H b/gcc/testsuite/g++.dg/modules/pr118920_a.H
new file mode 100644 (file)
index 0000000..75023b3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118920_a.H
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+#include "pr118920.h"

diff --git a/gcc/testsuite/g++.dg/modules/pr118920_b.H b/gcc/testsuite/g++.dg/modules/pr118920_b.H
new file mode 100644 (file)
index 0000000..23e37a6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118920_b.H
@@ -0,0 +1,12 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <typename T> struct __shared_ptr {
+  template <typename> friend struct out_ptr_t;
+};
+inline namespace __cxx11 __attribute__((__abi_tag__)) {
+  struct _Impl;
+}
+struct path {
+  __shared_ptr<_Impl> _M_impl;
+};

diff --git a/gcc/testsuite/g++.dg/modules/pr118920_c.C b/gcc/testsuite/g++.dg/modules/pr118920_c.C
new file mode 100644 (file)
index 0000000..b1b6c71
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118920_c.C
@@ -0,0 +1,5 @@
+// { dg-additional-options "-fmodules -fno-module-lazy" }
+
+#include "pr118920.h"
+import "pr118920_b.H";
+import "pr118920_a.H";