kpdev42 updated this revision to Diff 512945.
kpdev42 added a comment.

Thanks for pointing out the bug @Michael137 . It seems that clang assigns 
arbitrary offsets for non_unique_address so analyzing them brings me nowhere. 
In this patch I tried assigning no_unique_address to every empty structure and 
this fixed issue you found, making the code changes much smaller and simpler. 
The lldb test suite pass for me as well


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143347/new/

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/no_unique_address/Makefile
  lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
  lldb/test/API/lang/cpp/no_unique_address/main.cpp

Index: lldb/test/API/lang/cpp/no_unique_address/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/main.cpp
@@ -0,0 +1,77 @@
+struct C
+{
+ long c,d;
+};
+
+struct Q
+{
+ long h;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct E
+{
+  [[no_unique_address]] D x;
+};
+
+struct Foo1 : B,E,C
+{
+ long a = 42,b = 52;
+} _f1;
+
+struct Foo2 : B,E
+{
+ long v = 42;
+} _f2;
+
+struct Foo3 : C,B,E
+{
+ long v = 42;
+} _f3;
+
+struct Foo4 : B,C,E,Q
+{
+ long v = 42;
+} _f4;
+
+struct Foo5 : B,C,E
+{
+ [[no_unique_address]] D x1;
+ [[no_unique_address]] D x2;
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+ [[no_unique_address]] D z1;
+ [[no_unique_address]] D z2;
+} _f5;
+
+struct Foo6 : B,E
+{
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+} _f6;
+
+namespace NS {
+template<class T> struct Wrap {};
+}
+
+struct Foo7 : NS::Wrap<Foo6>,B,E {
+  NS::Wrap<Foo5> w1;
+  B b1;
+  long v = 42;
+} _f7;
+
+int main() {
+  return 0; // Set breakpoint here.
+}
Index: lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -0,0 +1,33 @@
+"""
+Test that we correctly handle [[no_unique_address]] attribute.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestInlineNamespace(TestBase):
+
+    def test(self):
+        self.build()
+
+        lldbutil.run_to_source_breakpoint(self,
+            "// Set breakpoint here.", lldb.SBFileSpec("main.cpp"))
+
+
+        self.expect_expr("_f3", result_type="Foo3")
+        self.expect_expr("_f7", result_type="Foo7")
+
+        self.expect_expr("_f1.a", result_type="long", result_value="42")
+        self.expect_expr("_f1.b", result_type="long", result_value="52")
+        self.expect_expr("_f2.v", result_type="long", result_value="42")
+        self.expect_expr("_f3.v", result_type="long", result_value="42")
+        self.expect_expr("_f4.v", result_type="long", result_value="42")
+        self.expect_expr("_f5.v1", result_type="long", result_value="42")
+        self.expect_expr("_f5.v2", result_type="long", result_value="52")
+        self.expect_expr("_f6.v1", result_type="long", result_value="42")
+        self.expect_expr("_f6.v2", result_type="long", result_value="52")
+        self.expect_expr("_f7.v", result_type="long", result_value="42")
Index: lldb/test/API/lang/cpp/no_unique_address/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1566,6 +1566,16 @@
   return qualified_name;
 }
 
+static bool IsEmptyStruct(const DWARFDIE &die) {
+  if (!die.HasChildren())
+    return true;
+
+  // Empty templates are actually empty structures.
+  return llvm::all_of(die.children(), [](const DWARFDIE &die) {
+    return die.Tag() == DW_TAG_template_type_parameter;
+  });
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
                                            const DWARFDIE &die,
@@ -1829,7 +1839,7 @@
     // has child classes or types that require the class to be created
     // for use as their decl contexts the class will be ready to accept
     // these child definitions.
-    if (!die.HasChildren()) {
+    if (IsEmptyStruct(die)) {
       // No children for this struct/union/class, lets finish it
       if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
         TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
@@ -1852,6 +1862,9 @@
         clang::RecordDecl *record_decl =
             TypeSystemClang::GetAsRecordDecl(clang_type);
         if (record_decl) {
+          if (auto *cxx_record_decl =
+                  llvm::dyn_cast<clang::CXXRecordDecl>(record_decl))
+            cxx_record_decl->markEmpty();
           ClangASTImporter::LayoutInfo layout;
           layout.bit_size = *attrs.byte_size * 8;
           GetClangASTImporter().SetRecordLayout(record_decl, layout);
@@ -2196,8 +2209,29 @@
 
     clang::CXXRecordDecl *record_decl =
         m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-    if (record_decl)
+    if (record_decl) {
+      bool is_empty = true;
+      if (layout_info.vbase_offsets.empty()) {
+        for (auto *field : record_decl->fields()) {
+          if (auto *cxx_record_decl = field->getType()->getAsCXXRecordDecl()) {
+            bool is_empty_field = cxx_record_decl->isEmpty();
+            if (is_empty_field)
+              field->addAttr(clang::NoUniqueAddressAttr::Create(
+                  m_ast.getASTContext(), clang::SourceRange()));
+            is_empty &= is_empty_field;
+
+          } else {
+            is_empty = false;
+          }
+        }
+        if (is_empty)
+          for (auto &parent : layout_info.base_offsets)
+            is_empty &= parent.first->isEmpty();
+        if (is_empty)
+          record_decl->markEmpty();
+      }
       GetClangASTImporter().SetRecordLayout(record_decl, layout_info);
+    }
   }
 
   return (bool)clang_type;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to