https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/93809

>From 91276f5b2dc05032a285b465c0c8a69541bb25c4 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Thu, 2 May 2024 09:05:01 +0100
Subject: [PATCH 1/3] [clang][lldb] Don't assert structure layout correctness
 for layouts provided by LLDB

This is the outcome of the discussions we had in
https://discourse.llvm.org/t/rfc-lldb-handling-no-unique-address-in-lldb/77483

To summarize, LLDB creates AST nodes by parsing debug-info and hands
those off to Clang for codegen. There are certain Clang attributes
that affect structure layout which are not encoded in DWARF, one of
which is `[[no_unique_address]]`. But the affects of such attributes
*is* encoded in DWARF in terms of member offsets and sizes. So some
of the correctness checks that the `RecordLayoutBuilder` performs
aren't really necessary for layouts provided by LLDB since we know that
the offsets we get from DWARF are correct (modulo corrupt DWARF).
Hence this patch guards those asserts behind the `DebuggerSupport` flag.
---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp   | 18 ++++---
 .../API/lang/cpp/no_unique_address/Makefile   |  3 ++
 .../no_unique_address/TestNoUniqueAddress.py  | 50 +++++++++++++++++++
 .../API/lang/cpp/no_unique_address/main.cpp   | 35 +++++++++++++
 4 files changed, 100 insertions(+), 6 deletions(-)
 create mode 100644 lldb/test/API/lang/cpp/no_unique_address/Makefile
 create mode 100644 
lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
 create mode 100644 lldb/test/API/lang/cpp/no_unique_address/main.cpp

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 5169be204c14d..32fb032214e28 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -950,6 +950,9 @@ void CGRecordLowering::calculateZeroInit() {
 // Verify accumulateBitfields computed the correct storage representations.
 void CGRecordLowering::checkBitfieldClipping(bool IsNonVirtualBaseType) const {
 #ifndef NDEBUG
+  if (Context.getLangOpts().DebuggerSupport)
+    return;
+
   auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
   auto Tail = CharUnits::Zero();
   for (const auto &M : Members) {
@@ -1008,7 +1011,8 @@ void CGRecordLowering::insertPadding() {
     if (!Member->Data)
       continue;
     CharUnits Offset = Member->Offset;
-    assert(Offset >= Size);
+    if (!Context.getLangOpts().DebuggerSupport)
+      assert(Offset >= Size);
     // Insert padding if we need to.
     if (Offset !=
         Size.alignTo(Packed ? CharUnits::One() : getAlignment(Member->Data)))
@@ -1138,8 +1142,9 @@ CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, 
llvm::StructType *Ty) {
   const ASTRecordLayout &Layout = getContext().getASTRecordLayout(D);
 
   uint64_t TypeSizeInBits = getContext().toBits(Layout.getSize());
-  assert(TypeSizeInBits == getDataLayout().getTypeAllocSizeInBits(Ty) &&
-         "Type size mismatch!");
+  if (!Context.getLangOpts().DebuggerSupport)
+    assert(TypeSizeInBits == getDataLayout().getTypeAllocSizeInBits(Ty) &&
+           "Type size mismatch!");
 
   if (BaseTy) {
     CharUnits NonVirtualSize  = Layout.getNonVirtualSize();
@@ -1147,9 +1152,10 @@ CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, 
llvm::StructType *Ty) {
     uint64_t AlignedNonVirtualTypeSizeInBits =
       getContext().toBits(NonVirtualSize);
 
-    assert(AlignedNonVirtualTypeSizeInBits ==
-           getDataLayout().getTypeAllocSizeInBits(BaseTy) &&
-           "Type size mismatch!");
+    if (!Context.getLangOpts().DebuggerSupport)
+      assert(AlignedNonVirtualTypeSizeInBits ==
+             getDataLayout().getTypeAllocSizeInBits(BaseTy) &&
+             "Type size mismatch!");
   }
 
   // Verify that the LLVM and AST field offsets agree.
diff --git a/lldb/test/API/lang/cpp/no_unique_address/Makefile 
b/lldb/test/API/lang/cpp/no_unique_address/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/no_unique_address/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py 
b/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
new file mode 100644
index 0000000000000..373bcb9062cde
--- /dev/null
+++ b/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly handles fields
+marked with [[no_unique_address]].
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class NoUniqueAddressTestCase(TestBase):
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "return 0", lldb.SBFileSpec("main.cpp", False)
+        )
+
+        # Qualified/unqualified lookup to templates in namespace
+        self.expect_expr("b1", result_type="basic::Foo", 
result_children=[ValueCheck(
+            name="a", type="Empty"
+        )])
+
+        self.expect_expr("b2", result_type="bases::Foo", result_children=[
+            ValueCheck(type="bases::B", children=[
+                ValueCheck(name="x", type="Empty")
+            ]),
+            ValueCheck(type="bases::A", children=[
+                ValueCheck(name="c", type="long", value="1"),
+                ValueCheck(name="d", type="long", value="2")
+            ]),
+            ValueCheck(type="bases::C", children=[
+                ValueCheck(name="x", type="Empty")
+            ]),
+        ])
+        self.expect_expr("b3", result_type="bases::Bar", result_children=[
+            ValueCheck(type="bases::B", children=[
+                ValueCheck(name="x", type="Empty")
+            ]),
+            ValueCheck(type="bases::C", children=[
+                ValueCheck(name="x", type="Empty")
+            ]),
+            ValueCheck(type="bases::A", children=[
+                ValueCheck(name="c", type="long", value="5"),
+                ValueCheck(name="d", type="long", value="6")
+            ]),
+        ])
+
+        self.expect("frame var b1")
+        self.expect("frame var b2")
+        self.expect("frame var b3")
diff --git a/lldb/test/API/lang/cpp/no_unique_address/main.cpp 
b/lldb/test/API/lang/cpp/no_unique_address/main.cpp
new file mode 100644
index 0000000000000..424fa90859cea
--- /dev/null
+++ b/lldb/test/API/lang/cpp/no_unique_address/main.cpp
@@ -0,0 +1,35 @@
+struct Empty {};
+
+namespace basic {
+struct Foo {
+  [[no_unique_address]] Empty a;
+};
+} // namespace basic
+
+namespace bases {
+struct A {
+  long c, d;
+};
+
+struct B {
+  [[no_unique_address]] Empty x;
+};
+
+struct C {
+  [[no_unique_address]] Empty x;
+};
+
+struct Foo : B, A, C {};
+struct Bar : B, C, A {};
+} // namespace bases
+
+int main() {
+  basic::Foo b1;
+  bases::Foo b2;
+  bases::Bar b3;
+  b2.c = 1;
+  b2.d = 2;
+  b3.c = 5;
+  b3.d = 6;
+  return 0;
+}

>From 882d231ac16ba635e9914900822b58f6ae9ced6d Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Thu, 30 May 2024 12:56:19 +0100
Subject: [PATCH 2/3] fixup! clang-format

---
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp 
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 32fb032214e28..b7aab51660b5e 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -1154,7 +1154,7 @@ CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, 
llvm::StructType *Ty) {
 
     if (!Context.getLangOpts().DebuggerSupport)
       assert(AlignedNonVirtualTypeSizeInBits ==
-             getDataLayout().getTypeAllocSizeInBits(BaseTy) &&
+                 getDataLayout().getTypeAllocSizeInBits(BaseTy) &&
              "Type size mismatch!");
   }
 

>From d5e8bd0eac21268d05358a81bdc04612539148ef Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Thu, 30 May 2024 12:58:08 +0100
Subject: [PATCH 3/3] fixup! python format

---
 .../no_unique_address/TestNoUniqueAddress.py  | 71 ++++++++++++-------
 1 file changed, 44 insertions(+), 27 deletions(-)

diff --git a/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py 
b/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
index 373bcb9062cde..d16aaa14153fd 100644
--- a/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
+++ b/lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -8,6 +8,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+
 class NoUniqueAddressTestCase(TestBase):
     def test(self):
         self.build()
@@ -16,34 +17,50 @@ def test(self):
         )
 
         # Qualified/unqualified lookup to templates in namespace
-        self.expect_expr("b1", result_type="basic::Foo", 
result_children=[ValueCheck(
-            name="a", type="Empty"
-        )])
+        self.expect_expr(
+            "b1",
+            result_type="basic::Foo",
+            result_children=[ValueCheck(name="a", type="Empty")],
+        )
 
-        self.expect_expr("b2", result_type="bases::Foo", result_children=[
-            ValueCheck(type="bases::B", children=[
-                ValueCheck(name="x", type="Empty")
-            ]),
-            ValueCheck(type="bases::A", children=[
-                ValueCheck(name="c", type="long", value="1"),
-                ValueCheck(name="d", type="long", value="2")
-            ]),
-            ValueCheck(type="bases::C", children=[
-                ValueCheck(name="x", type="Empty")
-            ]),
-        ])
-        self.expect_expr("b3", result_type="bases::Bar", result_children=[
-            ValueCheck(type="bases::B", children=[
-                ValueCheck(name="x", type="Empty")
-            ]),
-            ValueCheck(type="bases::C", children=[
-                ValueCheck(name="x", type="Empty")
-            ]),
-            ValueCheck(type="bases::A", children=[
-                ValueCheck(name="c", type="long", value="5"),
-                ValueCheck(name="d", type="long", value="6")
-            ]),
-        ])
+        self.expect_expr(
+            "b2",
+            result_type="bases::Foo",
+            result_children=[
+                ValueCheck(
+                    type="bases::B", children=[ValueCheck(name="x", 
type="Empty")]
+                ),
+                ValueCheck(
+                    type="bases::A",
+                    children=[
+                        ValueCheck(name="c", type="long", value="1"),
+                        ValueCheck(name="d", type="long", value="2"),
+                    ],
+                ),
+                ValueCheck(
+                    type="bases::C", children=[ValueCheck(name="x", 
type="Empty")]
+                ),
+            ],
+        )
+        self.expect_expr(
+            "b3",
+            result_type="bases::Bar",
+            result_children=[
+                ValueCheck(
+                    type="bases::B", children=[ValueCheck(name="x", 
type="Empty")]
+                ),
+                ValueCheck(
+                    type="bases::C", children=[ValueCheck(name="x", 
type="Empty")]
+                ),
+                ValueCheck(
+                    type="bases::A",
+                    children=[
+                        ValueCheck(name="c", type="long", value="5"),
+                        ValueCheck(name="d", type="long", value="6"),
+                    ],
+                ),
+            ],
+        )
 
         self.expect("frame var b1")
         self.expect("frame var b2")

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to