Skip to content

Commit a118f5f

Browse files
authored
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.
1 parent cce1feb commit a118f5f

File tree

5 files changed

+202
-0
lines changed

5 files changed

+202
-0
lines changed

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,18 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
477477
case DW_TAG_base_type:
478478
push_ctx(CompilerContextKind::Builtin, name);
479479
break;
480+
// If any of the tags below appear in the parent chain, stop the decl
481+
// context and return. Prior to these being in here, if a type existed in a
482+
// namespace "a" like "a::my_struct", but we also have a function in that
483+
// same namespace "a" which contained a type named "my_struct", both would
484+
// return "a::my_struct" as the declaration context since the
485+
// DW_TAG_subprogram would be skipped and its parent would be found.
486+
case DW_TAG_compile_unit:
487+
case DW_TAG_type_unit:
488+
case DW_TAG_subprogram:
489+
case DW_TAG_lexical_block:
490+
case DW_TAG_inlined_subroutine:
491+
return;
480492
default:
481493
break;
482494
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
CXX_SOURCES := main.cpp
2+
include Makefile.rules
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""
2+
Test the SBModule and SBTarget type lookup APIs to find multiple types.
3+
"""
4+
5+
import lldb
6+
from lldbsuite.test.lldbtest import *
7+
from lldbsuite.test import lldbutil
8+
9+
10+
class TypeFindFirstTestCase(TestBase):
11+
def test_find_first_type(self):
12+
"""
13+
Test SBTarget::FindTypes() and SBModule::FindTypes() APIs.
14+
15+
We had issues where our declaration context when finding types was
16+
incorrectly calculated where a type in a namepace, and a type in a
17+
function that was also in the same namespace would match a lookup. For
18+
example:
19+
20+
namespace a {
21+
struct Foo {
22+
int foo;
23+
};
24+
25+
unsigned foo() {
26+
typedef unsigned Foo;
27+
Foo foo = 12;
28+
return foo;
29+
}
30+
} // namespace a
31+
32+
33+
Previously LLDB would calculate the declaration context of "a::Foo"
34+
correctly, but incorrectly calculate the declaration context of "Foo"
35+
from within the foo() function as "a::Foo". Adding tests to ensure this
36+
works correctly.
37+
"""
38+
self.build()
39+
target = self.createTestTarget()
40+
exe_module = target.GetModuleAtIndex(0)
41+
self.assertTrue(exe_module.IsValid())
42+
# Test the SBTarget and SBModule APIs for FindFirstType
43+
for api in [target, exe_module]:
44+
# We should find the "a::Foo" but not the "Foo" type in the function
45+
types = api.FindTypes("a::Foo")
46+
self.assertEqual(types.GetSize(), 1)
47+
type_str0 = str(types.GetTypeAtIndex(0))
48+
self.assertIn('struct Foo {', type_str0)
49+
50+
# When we search by type basename, we should find any type whose
51+
# basename matches "Foo", so "a::Foo" and the "Foo" type in the
52+
# function.
53+
types = api.FindTypes("Foo")
54+
self.assertEqual(types.GetSize(), 2)
55+
type_str0 = str(types.GetTypeAtIndex(0))
56+
type_str1 = str(types.GetTypeAtIndex(1))
57+
# We don't know which order the types will come back as, so
58+
self.assertEqual(set([str(t).split('\n')[0] for t in types]), set(["typedef Foo", "struct Foo {"]))
59+
60+
# When we search by type basename with "::" prepended, we should
61+
# only types in the root namespace which means only "Foo" type in
62+
# the function.
63+
types = api.FindTypes("::Foo")
64+
self.assertEqual(types.GetSize(), 1)
65+
type_str0 = str(types.GetTypeAtIndex(0))
66+
self.assertIn('typedef Foo', type_str0)
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
namespace a {
2+
struct Foo {};
3+
4+
unsigned foo() {
5+
typedef unsigned Foo;
6+
Foo foo = 12;
7+
return foo;
8+
}
9+
} // namespace a
10+
11+
int main() {
12+
a::Foo f = {};
13+
a::foo();
14+
return 0;
15+
}

lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,3 +258,110 @@ TEST(DWARFDIETest, GetContext) {
258258
struct_die.GetTypeLookupContext(),
259259
testing::ElementsAre(make_namespace("NAMESPACE"), make_struct("STRUCT")));
260260
}
261+
262+
TEST(DWARFDIETest, GetContextInFunction) {
263+
// Make sure we get the right context fo each "struct_t" type. The first
264+
// should be "a::struct_t" and the one defined in the "foo" function should be
265+
// "struct_t". Previous DWARFDIE::GetTypeLookupContext() function calls would
266+
// have the "struct_t" in "foo" be "a::struct_t" because it would traverse the
267+
// entire die parent tree and ignore DW_TAG_subprogram and keep traversing the
268+
// parents.
269+
//
270+
// 0x0000000b: DW_TAG_compile_unit
271+
// 0x0000000c: DW_TAG_namespace
272+
// DW_AT_name("a")
273+
// 0x0000000f: DW_TAG_structure_type
274+
// DW_AT_name("struct_t")
275+
// 0x00000019: DW_TAG_subprogram
276+
// DW_AT_name("foo")
277+
// 0x0000001e: DW_TAG_structure_type
278+
// DW_AT_name("struct_t")
279+
// 0x00000028: NULL
280+
// 0x00000029: NULL
281+
// 0x0000002a: NULL
282+
const char *yamldata = R"(
283+
--- !ELF
284+
FileHeader:
285+
Class: ELFCLASS64
286+
Data: ELFDATA2LSB
287+
Type: ET_EXEC
288+
Machine: EM_386
289+
DWARF:
290+
debug_str:
291+
- ''
292+
debug_abbrev:
293+
- ID: 0
294+
Table:
295+
- Code: 0x1
296+
Tag: DW_TAG_compile_unit
297+
Children: DW_CHILDREN_yes
298+
- Code: 0x2
299+
Tag: DW_TAG_namespace
300+
Children: DW_CHILDREN_yes
301+
Attributes:
302+
- Attribute: DW_AT_name
303+
Form: DW_FORM_string
304+
- Code: 0x3
305+
Tag: DW_TAG_structure_type
306+
Children: DW_CHILDREN_no
307+
Attributes:
308+
- Attribute: DW_AT_name
309+
Form: DW_FORM_string
310+
- Code: 0x4
311+
Tag: DW_TAG_subprogram
312+
Children: DW_CHILDREN_yes
313+
Attributes:
314+
- Attribute: DW_AT_name
315+
Form: DW_FORM_string
316+
debug_info:
317+
- Length: 0x27
318+
Version: 4
319+
AbbrevTableID: 0
320+
AbbrOffset: 0x0
321+
AddrSize: 8
322+
Entries:
323+
- AbbrCode: 0x1
324+
- AbbrCode: 0x2
325+
Values:
326+
- Value: 0xDEADBEEFDEADBEEF
327+
CStr: a
328+
- AbbrCode: 0x3
329+
Values:
330+
- Value: 0xDEADBEEFDEADBEEF
331+
CStr: struct_t
332+
- AbbrCode: 0x4
333+
Values:
334+
- Value: 0xDEADBEEFDEADBEEF
335+
CStr: foo
336+
- AbbrCode: 0x3
337+
Values:
338+
- Value: 0xDEADBEEFDEADBEEF
339+
CStr: struct_t
340+
- AbbrCode: 0x0
341+
- AbbrCode: 0x0
342+
- AbbrCode: 0x0)";
343+
344+
YAMLModuleTester t(yamldata);
345+
auto *symbol_file =
346+
llvm::cast<SymbolFileDWARF>(t.GetModule()->GetSymbolFile());
347+
DWARFUnit *unit = symbol_file->DebugInfo().GetUnitAtIndex(0);
348+
ASSERT_TRUE(unit);
349+
350+
auto make_namespace = [](llvm::StringRef name) {
351+
return CompilerContext(CompilerContextKind::Namespace, ConstString(name));
352+
};
353+
auto make_struct = [](llvm::StringRef name) {
354+
return CompilerContext(CompilerContextKind::Struct, ConstString(name));
355+
};
356+
// Grab the "a::struct_t" type from the "a" namespace
357+
DWARFDIE a_struct_die = unit->DIE().GetFirstChild().GetFirstChild();
358+
ASSERT_TRUE(a_struct_die);
359+
EXPECT_THAT(
360+
a_struct_die.GetDeclContext(),
361+
testing::ElementsAre(make_namespace("a"), make_struct("struct_t")));
362+
// Grab the "struct_t" defined in the "foo" function.
363+
DWARFDIE foo_struct_die =
364+
unit->DIE().GetFirstChild().GetFirstChild().GetSibling().GetFirstChild();
365+
EXPECT_THAT(foo_struct_die.GetTypeLookupContext(),
366+
testing::ElementsAre(make_struct("struct_t")));
367+
}

0 commit comments

Comments
 (0)