Author: Michael Buch Date: 2024-12-23T11:51:51Z New Revision: c660b281b60085cbe40d73d692badd43d7708d20
URL: https://github.com/llvm/llvm-project/commit/c660b281b60085cbe40d73d692badd43d7708d20 DIFF: https://github.com/llvm/llvm-project/commit/c660b281b60085cbe40d73d692badd43d7708d20.diff LOG: [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (#120809) This addresses an issue encountered when investigating https://github.com/llvm/llvm-project/pull/120569. In `ParseTypeFromDWARF`, we insert the parsed type into `UniqueDWARFASTTypeMap` using the typename and `Declaration` obtained with `GetUniqueTypeNameAndDeclaration`. For C++ `GetUniqueTypeNameAndDeclaration` will zero the `Declaration` info (presumably because forward declaration may not have it, and for C++, ODR means we can rely on fully qualified typenames for uniqueing). When we then called `CompleteType`, we would first `MapDeclDIEToDefDIE`, updating the `UniqueDWARFASTType` we inserted previously with the `Declaration` of the definition DIE (without zeroing it). We would then call into `ParseTypeFromDWARF` for the same type again, and search the `UniqueDWARFASTTypeMap`. But we do the search using a zeroed declaration we get from `GetUniqueTypeNameAndDeclaration`. This particular example was fixed by https://github.com/llvm/llvm-project/pull/120569 but this still feels a little inconsistent. I'm not entirely sure we can get into that situation after that patch anymore. So the only test-case I could come up with was the unit-test which calls `MapDeclDIEToDefDIE` directly. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp index 3d201e96f92c3c..b598768b6e49f9 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp @@ -7,8 +7,10 @@ //===----------------------------------------------------------------------===// #include "UniqueDWARFASTType.h" +#include "SymbolFileDWARF.h" #include "lldb/Core/Declaration.h" +#include "lldb/Target/Language.h" using namespace lldb_private::dwarf; using namespace lldb_private::plugin::dwarf; @@ -18,6 +20,33 @@ static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) { Tag == llvm::dwarf::Tag::DW_TAG_structure_type; } +static bool IsSizeAndDeclarationMatching(UniqueDWARFASTType const &udt, + DWARFDIE const &die, + const lldb_private::Declaration &decl, + const int32_t byte_size, + bool is_forward_declaration) { + + // If they are not both definition DIEs or both declaration DIEs, then + // don't check for byte size and declaration location, because declaration + // DIEs usually don't have those info. + if (udt.m_is_forward_declaration != is_forward_declaration) + return true; + + if (udt.m_byte_size > 0 && byte_size > 0 && udt.m_byte_size != byte_size) + return false; + + // For C++, we match the behaviour of + // DWARFASTParserClang::GetUniqueTypeNameAndDeclaration. We rely on the + // one-definition-rule: for a given fully qualified name there exists only one + // definition, and there should only be one entry for such name, so ignore + // location of where it was declared vs. defined. + if (lldb_private::Language::LanguageIsCPlusPlus( + SymbolFileDWARF::GetLanguage(*die.GetCU()))) + return true; + + return udt.m_declaration == decl; +} + UniqueDWARFASTType *UniqueDWARFASTTypeList::Find( const DWARFDIE &die, const lldb_private::Declaration &decl, const int32_t byte_size, bool is_forward_declaration) { @@ -25,17 +54,11 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find( // Make sure the tags match if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) && IsStructOrClassTag(die.Tag()))) { - // If they are not both definition DIEs or both declaration DIEs, then - // don't check for byte size and declaration location, because declaration - // DIEs usually don't have those info. - bool matching_size_declaration = - udt.m_is_forward_declaration != is_forward_declaration - ? true - : (udt.m_byte_size < 0 || byte_size < 0 || - udt.m_byte_size == byte_size) && - udt.m_declaration == decl; - if (!matching_size_declaration) + + if (!IsSizeAndDeclarationMatching(udt, die, decl, byte_size, + is_forward_declaration)) continue; + // The type has the same name, and was defined on the same file and // line. Now verify all of the parent DIEs match. DWARFDIE parent_arg_die = die.GetParent(); diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp index ee90855437a71e..f22d76b3973e5f 100644 --- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp +++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp @@ -598,3 +598,146 @@ TEST_F(DWARFASTParserClangTests, TestDefaultTemplateParamParsing) { } } } + +TEST_F(DWARFASTParserClangTests, TestUniqueDWARFASTTypeMap_CppInsertMapFind) { + // This tests the behaviour of UniqueDWARFASTTypeMap under + // following scenario: + // 1. DWARFASTParserClang parses a forward declaration and + // inserts it into the UniqueDWARFASTTypeMap. + // 2. We then MapDeclDIEToDefDIE which updates the map + // entry with the line number/file information of the definition. + // 3. Parse the definition DIE, which should return the previously + // parsed type from the UniqueDWARFASTTypeMap. + + const char *yamldata = R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_EXEC + Machine: EM_AARCH64 +DWARF: + debug_str: + - Foo + + debug_line: + - Version: 4 + MinInstLength: 1 + MaxOpsPerInst: 1 + DefaultIsStmt: 1 + LineBase: 0 + LineRange: 0 + Files: + - Name: main.cpp + DirIdx: 0 + ModTime: 0 + Length: 0 + + debug_abbrev: + - ID: 0 + Table: + - Code: 0x01 + Tag: DW_TAG_compile_unit + Children: DW_CHILDREN_yes + Attributes: + - Attribute: DW_AT_language + Form: DW_FORM_data2 + - Attribute: DW_AT_stmt_list + Form: DW_FORM_sec_offset + - Code: 0x02 + Tag: DW_TAG_structure_type + Children: DW_CHILDREN_no + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Attribute: DW_AT_declaration + Form: DW_FORM_flag_present + - Code: 0x03 + Tag: DW_TAG_structure_type + Children: DW_CHILDREN_no + Attributes: + - Attribute: DW_AT_name + Form: DW_FORM_strp + - Attribute: DW_AT_decl_file + Form: DW_FORM_data1 + - Attribute: DW_AT_decl_line + Form: DW_FORM_data1 + + debug_info: + - Version: 5 + UnitType: DW_UT_compile + AddrSize: 8 + Entries: +# 0x0c: DW_TAG_compile_unit +# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus) +# DW_AT_stmt_list [DW_FORM_sec_offset] + - AbbrCode: 0x01 + Values: + - Value: 0x04 + - Value: 0x0000000000000000 + +# 0x0d: DW_TAG_structure_type +# DW_AT_name [DW_FORM_strp] (\"Foo\") +# DW_AT_declaration [DW_FORM_flag_present] (true) + - AbbrCode: 0x02 + Values: + - Value: 0x00 + +# 0x0f: DW_TAG_structure_type +# DW_AT_name [DW_FORM_strp] (\"Foo\") +# DW_AT_decl_file [DW_FORM_data1] (main.cpp) +# DW_AT_decl_line [DW_FORM_data1] (3) + - AbbrCode: 0x03 + Values: + - Value: 0x00 + - Value: 0x01 + - Value: 0x03 + + - AbbrCode: 0x00 # end of child tags of 0x0c +... +)"; + YAMLModuleTester t(yamldata); + + DWARFUnit *unit = t.GetDwarfUnit(); + ASSERT_NE(unit, nullptr); + const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE(); + ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit); + ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus); + DWARFDIE cu_die(unit, cu_entry); + + auto holder = std::make_unique<clang_utils::TypeSystemClangHolder>("ast"); + auto &ast_ctx = *holder->GetAST(); + DWARFASTParserClangStub ast_parser(ast_ctx); + + DWARFDIE decl_die; + DWARFDIE def_die; + for (auto const &die : cu_die.children()) { + if (die.Tag() != DW_TAG_structure_type) + continue; + + if (die.GetAttributeValueAsOptionalUnsigned(llvm::dwarf::DW_AT_declaration)) + decl_die = die; + else + def_die = die; + } + + ASSERT_TRUE(decl_die.IsValid()); + ASSERT_TRUE(def_die.IsValid()); + ASSERT_NE(decl_die, def_die); + + ParsedDWARFTypeAttributes attrs(def_die); + ASSERT_TRUE(attrs.decl.IsValid()); + + SymbolContext sc; + bool new_type = false; + lldb::TypeSP type_sp = ast_parser.ParseTypeFromDWARF(sc, decl_die, &new_type); + ASSERT_NE(type_sp, nullptr); + + ast_parser.MapDeclDIEToDefDIE(decl_die, def_die); + + lldb::TypeSP reparsed_type_sp = + ast_parser.ParseTypeFromDWARF(sc, def_die, &new_type); + ASSERT_NE(reparsed_type_sp, nullptr); + + ASSERT_EQ(type_sp, reparsed_type_sp); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
