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