[Lldb-commits] [lldb] c660b28 - [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (#120809)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 23 03:51:54 PST 2024
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);
+}
More information about the lldb-commits
mailing list