[Lldb-commits] [lldb] a118f5f - Fix type lookup bug where wrong decl context was being used for a DIE. (#94846)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 11 13:58:29 PDT 2024
Author: Greg Clayton
Date: 2024-06-11T13:58:26-07:00
New Revision: a118f5f398bf099ec76ebf889234ebbc58b28f0c
URL: https://github.com/llvm/llvm-project/commit/a118f5f398bf099ec76ebf889234ebbc58b28f0c
DIFF: https://github.com/llvm/llvm-project/commit/a118f5f398bf099ec76ebf889234ebbc58b28f0c.diff
LOG: Fix type lookup bug where wrong decl context was being used for a DIE. (#94846)
The function that calculated the declaration context for a DIE was incorrectly transparently traversing acrosss DW_TAG_subprogram dies when climbing the parent DIE chain. This meant that types defined in functions would appear to have the declaration context of anything above the function. I fixed the GetTypeLookupContextImpl(...) function in DWARFDIE.cpp to not transparently skip over functions, lexical blocks and inlined functions and compile and type units. Added a test to verify things are working.
Added:
lldb/test/API/functionalities/type_types/Makefile
lldb/test/API/functionalities/type_types/TestFindTypes.py
lldb/test/API/functionalities/type_types/main.cpp
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 0ef94ed9f17c3..992d814793f9d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -477,6 +477,18 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
case DW_TAG_base_type:
push_ctx(CompilerContextKind::Builtin, name);
break;
+ // If any of the tags below appear in the parent chain, stop the decl
+ // context and return. Prior to these being in here, if a type existed in a
+ // namespace "a" like "a::my_struct", but we also have a function in that
+ // same namespace "a" which contained a type named "my_struct", both would
+ // return "a::my_struct" as the declaration context since the
+ // DW_TAG_subprogram would be skipped and its parent would be found.
+ case DW_TAG_compile_unit:
+ case DW_TAG_type_unit:
+ case DW_TAG_subprogram:
+ case DW_TAG_lexical_block:
+ case DW_TAG_inlined_subroutine:
+ return;
default:
break;
}
diff --git a/lldb/test/API/functionalities/type_types/Makefile b/lldb/test/API/functionalities/type_types/Makefile
new file mode 100644
index 0000000000000..3d0b98f13f3d7
--- /dev/null
+++ b/lldb/test/API/functionalities/type_types/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/type_types/TestFindTypes.py b/lldb/test/API/functionalities/type_types/TestFindTypes.py
new file mode 100644
index 0000000000000..42b5c4cfaaf77
--- /dev/null
+++ b/lldb/test/API/functionalities/type_types/TestFindTypes.py
@@ -0,0 +1,66 @@
+"""
+Test the SBModule and SBTarget type lookup APIs to find multiple types.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TypeFindFirstTestCase(TestBase):
+ def test_find_first_type(self):
+ """
+ Test SBTarget::FindTypes() and SBModule::FindTypes() APIs.
+
+ We had issues where our declaration context when finding types was
+ incorrectly calculated where a type in a namepace, and a type in a
+ function that was also in the same namespace would match a lookup. For
+ example:
+
+ namespace a {
+ struct Foo {
+ int foo;
+ };
+
+ unsigned foo() {
+ typedef unsigned Foo;
+ Foo foo = 12;
+ return foo;
+ }
+ } // namespace a
+
+
+ Previously LLDB would calculate the declaration context of "a::Foo"
+ correctly, but incorrectly calculate the declaration context of "Foo"
+ from within the foo() function as "a::Foo". Adding tests to ensure this
+ works correctly.
+ """
+ self.build()
+ target = self.createTestTarget()
+ exe_module = target.GetModuleAtIndex(0)
+ self.assertTrue(exe_module.IsValid())
+ # Test the SBTarget and SBModule APIs for FindFirstType
+ for api in [target, exe_module]:
+ # We should find the "a::Foo" but not the "Foo" type in the function
+ types = api.FindTypes("a::Foo")
+ self.assertEqual(types.GetSize(), 1)
+ type_str0 = str(types.GetTypeAtIndex(0))
+ self.assertIn('struct Foo {', type_str0)
+
+ # When we search by type basename, we should find any type whose
+ # basename matches "Foo", so "a::Foo" and the "Foo" type in the
+ # function.
+ types = api.FindTypes("Foo")
+ self.assertEqual(types.GetSize(), 2)
+ type_str0 = str(types.GetTypeAtIndex(0))
+ type_str1 = str(types.GetTypeAtIndex(1))
+ # We don't know which order the types will come back as, so
+ self.assertEqual(set([str(t).split('\n')[0] for t in types]), set(["typedef Foo", "struct Foo {"]))
+
+ # When we search by type basename with "::" prepended, we should
+ # only types in the root namespace which means only "Foo" type in
+ # the function.
+ types = api.FindTypes("::Foo")
+ self.assertEqual(types.GetSize(), 1)
+ type_str0 = str(types.GetTypeAtIndex(0))
+ self.assertIn('typedef Foo', type_str0)
diff --git a/lldb/test/API/functionalities/type_types/main.cpp b/lldb/test/API/functionalities/type_types/main.cpp
new file mode 100644
index 0000000000000..095328932cdc4
--- /dev/null
+++ b/lldb/test/API/functionalities/type_types/main.cpp
@@ -0,0 +1,15 @@
+namespace a {
+struct Foo {};
+
+unsigned foo() {
+ typedef unsigned Foo;
+ Foo foo = 12;
+ return foo;
+}
+} // namespace a
+
+int main() {
+ a::Foo f = {};
+ a::foo();
+ return 0;
+}
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index bea07dfa27cc6..65da7de1ba2d8 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -258,3 +258,110 @@ TEST(DWARFDIETest, GetContext) {
struct_die.GetTypeLookupContext(),
testing::ElementsAre(make_namespace("NAMESPACE"), make_struct("STRUCT")));
}
+
+TEST(DWARFDIETest, GetContextInFunction) {
+ // Make sure we get the right context fo each "struct_t" type. The first
+ // should be "a::struct_t" and the one defined in the "foo" function should be
+ // "struct_t". Previous DWARFDIE::GetTypeLookupContext() function calls would
+ // have the "struct_t" in "foo" be "a::struct_t" because it would traverse the
+ // entire die parent tree and ignore DW_TAG_subprogram and keep traversing the
+ // parents.
+ //
+ // 0x0000000b: DW_TAG_compile_unit
+ // 0x0000000c: DW_TAG_namespace
+ // DW_AT_name("a")
+ // 0x0000000f: DW_TAG_structure_type
+ // DW_AT_name("struct_t")
+ // 0x00000019: DW_TAG_subprogram
+ // DW_AT_name("foo")
+ // 0x0000001e: DW_TAG_structure_type
+ // DW_AT_name("struct_t")
+ // 0x00000028: NULL
+ // 0x00000029: NULL
+ // 0x0000002a: NULL
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_386
+DWARF:
+ debug_str:
+ - ''
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ - Code: 0x2
+ Tag: DW_TAG_namespace
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Code: 0x3
+ Tag: DW_TAG_structure_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Code: 0x4
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ debug_info:
+ - Length: 0x27
+ Version: 4
+ AbbrevTableID: 0
+ AbbrOffset: 0x0
+ AddrSize: 8
+ Entries:
+ - AbbrCode: 0x1
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: a
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: struct_t
+ - AbbrCode: 0x4
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: foo
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0xDEADBEEFDEADBEEF
+ CStr: struct_t
+ - AbbrCode: 0x0
+ - AbbrCode: 0x0
+ - AbbrCode: 0x0)";
+
+ YAMLModuleTester t(yamldata);
+ auto *symbol_file =
+ llvm::cast<SymbolFileDWARF>(t.GetModule()->GetSymbolFile());
+ DWARFUnit *unit = symbol_file->DebugInfo().GetUnitAtIndex(0);
+ ASSERT_TRUE(unit);
+
+ auto make_namespace = [](llvm::StringRef name) {
+ return CompilerContext(CompilerContextKind::Namespace, ConstString(name));
+ };
+ auto make_struct = [](llvm::StringRef name) {
+ return CompilerContext(CompilerContextKind::Struct, ConstString(name));
+ };
+ // Grab the "a::struct_t" type from the "a" namespace
+ DWARFDIE a_struct_die = unit->DIE().GetFirstChild().GetFirstChild();
+ ASSERT_TRUE(a_struct_die);
+ EXPECT_THAT(
+ a_struct_die.GetDeclContext(),
+ testing::ElementsAre(make_namespace("a"), make_struct("struct_t")));
+ // Grab the "struct_t" defined in the "foo" function.
+ DWARFDIE foo_struct_die =
+ unit->DIE().GetFirstChild().GetFirstChild().GetSibling().GetFirstChild();
+ EXPECT_THAT(foo_struct_die.GetTypeLookupContext(),
+ testing::ElementsAre(make_struct("struct_t")));
+}
More information about the lldb-commits
mailing list