[pushed] c++: [[no_unique_address]] and cv-qualified type

Message ID 20230906031931.622583-1-jason@redhat.com
State Unresolved
Headers
Series [pushed] c++: [[no_unique_address]] and cv-qualified type |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jason Merrill Sept. 6, 2023, 3:19 a.m. UTC
  Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

We were checking for overlap using same_type_p and therefore allocating two
Empty subobjects at the same offset because one was cv-qualified.

This gives the warning at the location of the class name rather than the
member declaration, but this should be a rare enough issue that it doesn't
seem worth trying to be more precise.

gcc/ChangeLog:

	* common.opt: Update -fabi-version=19.

gcc/cp/ChangeLog:

	* class.cc (check_subobject_offset): Check
	same_type_ignoring_top_level_qualifiers_p.

gcc/testsuite/ChangeLog:

	* g++.dg/abi/no_unique_address8.C: New test.
	* g++.dg/abi/no_unique_address8a.C: New test.
---
 gcc/common.opt                                |  1 +
 gcc/cp/class.cc                               | 28 +++++++++++++++--
 gcc/testsuite/g++.dg/abi/no_unique_address8.C | 30 ++++++++++++++++++
 .../g++.dg/abi/no_unique_address8a.C          | 31 +++++++++++++++++++
 4 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/abi/no_unique_address8.C
 create mode 100644 gcc/testsuite/g++.dg/abi/no_unique_address8a.C


base-commit: 244d1321340116b7780e78096356f69662fd0e18
  

Comments

Jason Merrill Sept. 18, 2023, 3:36 a.m. UTC | #1
On 9/5/23 23:19, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
> -- 8< --
> 
> We were checking for overlap using same_type_p and therefore allocating two
> Empty subobjects at the same offset because one was cv-qualified.
> 
> This gives the warning at the location of the class name rather than the
> member declaration, but this should be a rare enough issue that it doesn't
> seem worth trying to be more precise.

The ABI and language seem to be settling on referring to "similar types" 
here rather than same ignoring top-level cv-qualifiers.

Tested x86_64-pc-linux-gnu, applying to trunk.
  

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 3e1939293e8..f137a1f81ac 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1011,6 +1011,7 @@  Driver Undocumented
 ;     Default in G++ 13.
 ;
 ; 19: Emits ABI tags if needed in structured binding mangled names.
+;     Ignores cv-quals on [[no_unique_object]] members.
 ;     Default in G++ 14.
 ;
 ; Additional positive integers will be assigned as new versions of
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 778759237dc..9139a0075ab 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -4053,9 +4053,33 @@  check_subobject_offset (tree type, tree offset, splay_tree offsets)
   if (!n)
     return 0;
 
+  enum { ignore, fast, slow, warn }
+  cv_check = (abi_version_crosses (19) ? slow
+	      : abi_version_at_least (19) ? fast
+	      : ignore);
   for (t = (tree) n->value; t; t = TREE_CHAIN (t))
-    if (same_type_p (TREE_VALUE (t), type))
-      return 1;
+    {
+      tree elt = TREE_VALUE (t);
+
+      if (same_type_p (elt, type))
+	return 1;
+
+      if (cv_check != ignore
+	  && same_type_ignoring_top_level_qualifiers_p (elt, type))
+	{
+	  if (cv_check == fast)
+	    return 1;
+	  cv_check = warn;
+	}
+    }
+
+  if (cv_check == warn)
+    {
+      warning (OPT_Wabi, "layout of %qs member of type %qT changes in %qs",
+	       "[[no_unique_address]]", type, "-fabi-version=19");
+      if (abi_version_at_least (19))
+	return 1;
+    }
 
   return 0;
 }
diff --git a/gcc/testsuite/g++.dg/abi/no_unique_address8.C b/gcc/testsuite/g++.dg/abi/no_unique_address8.C
new file mode 100644
index 00000000000..6aa2bba7810
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/no_unique_address8.C
@@ -0,0 +1,30 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=19 -Wabi=18" }
+
+#include <cstddef>
+
+#define NOUNIQUE [[no_unique_address]]
+
+struct Empty { };
+#define CHECK_DISTINCT(type, field1, field2) static_assert(offsetof(type, field1) != offsetof(type, field2))
+
+struct A1 {
+    NOUNIQUE Empty a;
+    Empty b;
+};
+CHECK_DISTINCT(A1, a, b);
+struct A2 {
+    NOUNIQUE const Empty a;
+    const Empty b;
+};
+CHECK_DISTINCT(A2, a, b);
+struct A3 {			// { dg-warning "layout" }
+    NOUNIQUE const Empty a;
+    Empty b;
+};
+CHECK_DISTINCT(A3, a, b);
+struct A4 {			// { dg-warning "layout" }
+    NOUNIQUE Empty a;
+    const Empty b;
+};
+CHECK_DISTINCT(A4, a, b);
diff --git a/gcc/testsuite/g++.dg/abi/no_unique_address8a.C b/gcc/testsuite/g++.dg/abi/no_unique_address8a.C
new file mode 100644
index 00000000000..c5d48088529
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/no_unique_address8a.C
@@ -0,0 +1,31 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=18 -Wabi=19" }
+
+#include <cstddef>
+
+#define NOUNIQUE [[no_unique_address]]
+
+struct Empty { };
+#define CHECK_DISTINCT(type, field1, field2) static_assert(offsetof(type, field1) != offsetof(type, field2))
+#define CHECK_SAME(type, field1, field2) static_assert(offsetof(type, field1) == offsetof(type, field2))
+
+struct A1 {
+    NOUNIQUE Empty a;
+    Empty b;
+};
+CHECK_DISTINCT(A1, a, b);
+struct A2 {
+    NOUNIQUE const Empty a;
+    const Empty b;
+};
+CHECK_DISTINCT(A2, a, b);
+struct A3 {			// { dg-warning "layout" }
+    NOUNIQUE const Empty a;
+    Empty b;
+};
+CHECK_SAME(A3, a, b);
+struct A4 {			// { dg-warning "layout" }
+    NOUNIQUE Empty a;
+    const Empty b;
+};
+CHECK_SAME(A4, a, b);