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"))); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
