Skip to content

Commit 8e12904

Browse files
labathlabath
authored
[lldb/DWARF] Refactor DWARFDIE::Get{Decl,TypeLookup}Context (#93291)
After a bug (the bug is that the functions don't handle DW_AT_signature, aka type units) led me to one of these similar-but-different functions, I started to realize that most of the differences between these two functions are actually bugs. As a first step towards merging them, this patch rewrites both of them to follow the same pattern, while preserving all of their differences. The main change is that GetTypeLookupContext now also uses a `seen` list to avoid reference loops (currently that's not necessary because the function strictly follows parent links, but that will change with DW_AT_signatures). I've also optimized both functions to avoid recursion by starting contruction with the deepest scope first (and then reversing it).
1 parent 35f2caf commit 8e12904

File tree

4 files changed

+184
-93
lines changed

4 files changed

+184
-93
lines changed

‎lldb/include/lldb/Symbol/Type.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ struct CompilerContext {
6262
CompilerContextKind kind;
6363
ConstString name;
6464
};
65+
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
66+
const CompilerContext &rhs);
6567

6668
/// Match \p context_chain against \p pattern, which may contain "Any"
6769
/// kinds. The \p context_chain should *not* contain any "Any" kinds.

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

Lines changed: 104 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "DWARFDebugInfoEntry.h"
1414
#include "DWARFDeclContext.h"
1515
#include "DWARFUnit.h"
16+
#include "lldb/Symbol/Type.h"
1617

1718
#include "llvm/ADT/iterator.h"
1819

@@ -379,108 +380,118 @@ std::vector<DWARFDIE> DWARFDIE::GetDeclContextDIEs() const {
379380
return result;
380381
}
381382

382-
static std::vector<lldb_private::CompilerContext>
383-
GetDeclContextImpl(llvm::SmallSet<lldb::user_id_t, 4> &seen, DWARFDIE die) {
384-
std::vector<lldb_private::CompilerContext> context;
383+
static void GetDeclContextImpl(DWARFDIE die,
384+
llvm::SmallSet<lldb::user_id_t, 4> &seen,
385+
std::vector<CompilerContext> &context) {
385386
// Stop if we hit a cycle.
386-
if (!die || !seen.insert(die.GetID()).second)
387-
return context;
388-
389-
// Handle outline member function DIEs by following the specification.
390-
if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification))
391-
return GetDeclContextImpl(seen, spec);
392-
393-
// Get the parent context chain.
394-
context = GetDeclContextImpl(seen, die.GetParent());
387+
while (die && seen.insert(die.GetID()).second) {
388+
// Handle outline member function DIEs by following the specification.
389+
if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification)) {
390+
die = spec;
391+
continue;
392+
}
395393

396-
// Add this DIE's contribution at the end of the chain.
397-
auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
398-
context.push_back({kind, ConstString(name)});
399-
};
400-
switch (die.Tag()) {
401-
case DW_TAG_module:
402-
push_ctx(CompilerContextKind::Module, die.GetName());
403-
break;
404-
case DW_TAG_namespace:
405-
push_ctx(CompilerContextKind::Namespace, die.GetName());
406-
break;
407-
case DW_TAG_structure_type:
408-
push_ctx(CompilerContextKind::Struct, die.GetName());
409-
break;
410-
case DW_TAG_union_type:
411-
push_ctx(CompilerContextKind::Union, die.GetName());
412-
break;
413-
case DW_TAG_class_type:
414-
push_ctx(CompilerContextKind::Class, die.GetName());
415-
break;
416-
case DW_TAG_enumeration_type:
417-
push_ctx(CompilerContextKind::Enum, die.GetName());
418-
break;
419-
case DW_TAG_subprogram:
420-
push_ctx(CompilerContextKind::Function, die.GetName());
421-
break;
422-
case DW_TAG_variable:
423-
push_ctx(CompilerContextKind::Variable, die.GetPubname());
424-
break;
425-
case DW_TAG_typedef:
426-
push_ctx(CompilerContextKind::Typedef, die.GetName());
427-
break;
428-
default:
429-
break;
394+
// Add this DIE's contribution at the end of the chain.
395+
auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
396+
context.push_back({kind, ConstString(name)});
397+
};
398+
switch (die.Tag()) {
399+
case DW_TAG_module:
400+
push_ctx(CompilerContextKind::Module, die.GetName());
401+
break;
402+
case DW_TAG_namespace:
403+
push_ctx(CompilerContextKind::Namespace, die.GetName());
404+
break;
405+
case DW_TAG_structure_type:
406+
push_ctx(CompilerContextKind::Struct, die.GetName());
407+
break;
408+
case DW_TAG_union_type:
409+
push_ctx(CompilerContextKind::Union, die.GetName());
410+
break;
411+
case DW_TAG_class_type:
412+
push_ctx(CompilerContextKind::Class, die.GetName());
413+
break;
414+
case DW_TAG_enumeration_type:
415+
push_ctx(CompilerContextKind::Enum, die.GetName());
416+
break;
417+
case DW_TAG_subprogram:
418+
push_ctx(CompilerContextKind::Function, die.GetName());
419+
break;
420+
case DW_TAG_variable:
421+
push_ctx(CompilerContextKind::Variable, die.GetPubname());
422+
break;
423+
case DW_TAG_typedef:
424+
push_ctx(CompilerContextKind::Typedef, die.GetName());
425+
break;
426+
default:
427+
break;
428+
}
429+
// Now process the parent.
430+
die = die.GetParent();
430431
}
431-
return context;
432432
}
433433

434-
std::vector<lldb_private::CompilerContext> DWARFDIE::GetDeclContext() const {
434+
std::vector<CompilerContext> DWARFDIE::GetDeclContext() const {
435435
llvm::SmallSet<lldb::user_id_t, 4> seen;
436-
return GetDeclContextImpl(seen, *this);
436+
std::vector<CompilerContext> context;
437+
GetDeclContextImpl(*this, seen, context);
438+
std::reverse(context.begin(), context.end());
439+
return context;
437440
}
438441

439-
std::vector<lldb_private::CompilerContext>
440-
DWARFDIE::GetTypeLookupContext() const {
441-
std::vector<lldb_private::CompilerContext> context;
442-
// If there is no name, then there is no need to look anything up for this
443-
// DIE.
444-
const char *name = GetName();
445-
if (!name || !name[0])
446-
return context;
447-
const dw_tag_t tag = Tag();
448-
if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
449-
return context;
450-
DWARFDIE parent = GetParent();
451-
if (parent)
452-
context = parent.GetTypeLookupContext();
453-
auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
454-
context.push_back({kind, ConstString(name)});
455-
};
456-
switch (tag) {
457-
case DW_TAG_namespace:
458-
push_ctx(CompilerContextKind::Namespace, name);
459-
break;
460-
case DW_TAG_structure_type:
461-
push_ctx(CompilerContextKind::Struct, name);
462-
break;
463-
case DW_TAG_union_type:
464-
push_ctx(CompilerContextKind::Union, name);
465-
break;
466-
case DW_TAG_class_type:
467-
push_ctx(CompilerContextKind::Class, name);
468-
break;
469-
case DW_TAG_enumeration_type:
470-
push_ctx(CompilerContextKind::Enum, name);
471-
break;
472-
case DW_TAG_variable:
473-
push_ctx(CompilerContextKind::Variable, GetPubname());
474-
break;
475-
case DW_TAG_typedef:
476-
push_ctx(CompilerContextKind::Typedef, name);
477-
break;
478-
case DW_TAG_base_type:
479-
push_ctx(CompilerContextKind::Builtin, name);
480-
break;
481-
default:
482-
break;
442+
static void GetTypeLookupContextImpl(DWARFDIE die,
443+
llvm::SmallSet<lldb::user_id_t, 4> &seen,
444+
std::vector<CompilerContext> &context) {
445+
// Stop if we hit a cycle.
446+
while (die && seen.insert(die.GetID()).second) {
447+
// If there is no name, then there is no need to look anything up for this
448+
// DIE.
449+
const char *name = die.GetName();
450+
if (!name || !name[0])
451+
return;
452+
453+
// Add this DIE's contribution at the end of the chain.
454+
auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
455+
context.push_back({kind, ConstString(name)});
456+
};
457+
switch (die.Tag()) {
458+
case DW_TAG_namespace:
459+
push_ctx(CompilerContextKind::Namespace, die.GetName());
460+
break;
461+
case DW_TAG_structure_type:
462+
push_ctx(CompilerContextKind::Struct, die.GetName());
463+
break;
464+
case DW_TAG_union_type:
465+
push_ctx(CompilerContextKind::Union, die.GetName());
466+
break;
467+
case DW_TAG_class_type:
468+
push_ctx(CompilerContextKind::Class, die.GetName());
469+
break;
470+
case DW_TAG_enumeration_type:
471+
push_ctx(CompilerContextKind::Enum, die.GetName());
472+
break;
473+
case DW_TAG_variable:
474+
push_ctx(CompilerContextKind::Variable, die.GetPubname());
475+
break;
476+
case DW_TAG_typedef:
477+
push_ctx(CompilerContextKind::Typedef, die.GetName());
478+
break;
479+
case DW_TAG_base_type:
480+
push_ctx(CompilerContextKind::Builtin, name);
481+
break;
482+
default:
483+
break;
484+
}
485+
// Now process the parent.
486+
die = die.GetParent();
483487
}
488+
}
489+
490+
std::vector<CompilerContext> DWARFDIE::GetTypeLookupContext() const {
491+
llvm::SmallSet<lldb::user_id_t, 4> seen;
492+
std::vector<CompilerContext> context;
493+
GetTypeLookupContextImpl(*this, seen, context);
494+
std::reverse(context.begin(), context.end());
484495
return context;
485496
}
486497

‎lldb/source/Symbol/Type.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@
3636
using namespace lldb;
3737
using namespace lldb_private;
3838

39+
llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &os,
40+
const CompilerContext &rhs) {
41+
StreamString lldb_stream;
42+
rhs.Dump(lldb_stream);
43+
return os << lldb_stream.GetString();
44+
}
45+
3946
bool lldb_private::contextMatches(llvm::ArrayRef<CompilerContext> context_chain,
4047
llvm::ArrayRef<CompilerContext> pattern) {
4148
auto ctx = context_chain.begin();

‎lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
#include "Plugins/SymbolFile/DWARF/DWARFDebugInfo.h"
1111
#include "TestingSupport/Symbol/YAMLModuleTester.h"
1212
#include "lldb/Core/dwarf.h"
13+
#include "lldb/Symbol/Type.h"
14+
#include "lldb/lldb-private-enumerations.h"
1315
#include "llvm/ADT/STLExtras.h"
1416
#include "gmock/gmock.h"
1517
#include "gtest/gtest.h"
@@ -187,3 +189,72 @@ TEST(DWARFDIETest, PeekName) {
187189
dw_offset_t fifth_die_offset = 26;
188190
EXPECT_EQ(unit->PeekDIEName(fifth_die_offset), "NameType2");
189191
}
192+
193+
TEST(DWARFDIETest, GetContext) {
194+
const char *yamldata = R"(
195+
--- !ELF
196+
FileHeader:
197+
Class: ELFCLASS64
198+
Data: ELFDATA2LSB
199+
Type: ET_EXEC
200+
Machine: EM_386
201+
DWARF:
202+
debug_abbrev:
203+
- ID: 0
204+
Table:
205+
- Code: 0x1
206+
Tag: DW_TAG_compile_unit
207+
Children: DW_CHILDREN_yes
208+
Attributes:
209+
- Attribute: DW_AT_language
210+
Form: DW_FORM_data2
211+
- Code: 0x2
212+
Tag: DW_TAG_namespace
213+
Children: DW_CHILDREN_yes
214+
Attributes:
215+
- Attribute: DW_AT_name
216+
Form: DW_FORM_string
217+
- Code: 0x3
218+
Tag: DW_TAG_structure_type
219+
Children: DW_CHILDREN_no
220+
Attributes:
221+
- Attribute: DW_AT_name
222+
Form: DW_FORM_string
223+
debug_info:
224+
- Version: 4
225+
AddrSize: 8
226+
Entries:
227+
- AbbrCode: 0x1
228+
Values:
229+
- Value: 0x000000000000000C
230+
- AbbrCode: 0x2
231+
Values:
232+
- CStr: NAMESPACE
233+
- AbbrCode: 0x3
234+
Values:
235+
- CStr: STRUCT
236+
- AbbrCode: 0x0
237+
- AbbrCode: 0x0
238+
)";
239+
240+
YAMLModuleTester t(yamldata);
241+
auto *symbol_file =
242+
llvm::cast<SymbolFileDWARF>(t.GetModule()->GetSymbolFile());
243+
DWARFUnit *unit = symbol_file->DebugInfo().GetUnitAtIndex(0);
244+
ASSERT_TRUE(unit);
245+
246+
auto make_namespace = [](llvm::StringRef name) {
247+
return CompilerContext(CompilerContextKind::Namespace, ConstString(name));
248+
};
249+
auto make_struct = [](llvm::StringRef name) {
250+
return CompilerContext(CompilerContextKind::Struct, ConstString(name));
251+
};
252+
DWARFDIE struct_die = unit->DIE().GetFirstChild().GetFirstChild();
253+
ASSERT_TRUE(struct_die);
254+
EXPECT_THAT(
255+
struct_die.GetDeclContext(),
256+
testing::ElementsAre(make_namespace("NAMESPACE"), make_struct("STRUCT")));
257+
EXPECT_THAT(
258+
struct_die.GetTypeLookupContext(),
259+
testing::ElementsAre(make_namespace("NAMESPACE"), make_struct("STRUCT")));
260+
}

0 commit comments

Comments
 (0)