Skip to content

Commit e011990

Browse files
labathlabath
committed
SymbolVendor: Move compile unit handling into the SymbolFile class
Summary: SymbolFile classes are responsible for creating CompileUnit instances and they already need to have a notion of the id<->CompileUnit mapping (because of APIs like ParseCompileUnitAtIndex). However, the SymbolVendor has remained as the thing responsible for caching created units (which the SymbolFiles were calling via convoluted constructs like "m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(...)"). This patch moves the responsibility of caching the units into the SymbolFile class. It does this by moving the implementation of SymbolVendor::{GetNumCompileUnits,GetCompileUnitAtIndex} into the equivalent SymbolFile functions. The SymbolVendor functions become just a passthrough much like the rest of SymbolVendor. The original implementations of SymbolFile::GetNumCompileUnits is moved to "CalculateNumCompileUnits", and are made protected, as the "Get" function is the external api of the class. SymbolFile::ParseCompileUnitAtIndex is made protected for the same reason. This is the first step in removing the SymbolVendor indirection, as proposed in <http://lists.llvm.org/pipermail/lldb-dev/2019-June/015071.html>. After removing all interesting logic from the SymbolVendor class, I'll proceed with removing the indirection itself. Reviewers: clayborg, jingham, JDevlieghere Subscribers: jdoerfert, lldb-commits Differential Revision: https://reviews.llvm.org/D65089 llvm-svn: 366791
1 parent fdedf24 commit e011990

17 files changed

+127
-143
lines changed

‎lldb/include/lldb/Symbol/SymbolFile.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ class SymbolFile : public PluginInterface {
110110

111111
// Compile Unit function calls
112112
// Approach 1 - iterator
113-
virtual uint32_t GetNumCompileUnits() = 0;
114-
virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) = 0;
113+
uint32_t GetNumCompileUnits();
114+
lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx);
115115

116116
virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
117117
virtual size_t ParseFunctions(CompileUnit &comp_unit) = 0;
@@ -235,12 +235,17 @@ class SymbolFile : public PluginInterface {
235235
return nullptr;
236236
}
237237

238-
virtual void Dump(Stream &s) {}
238+
virtual void Dump(Stream &s);
239239

240240
protected:
241241
void AssertModuleLock();
242+
virtual uint32_t CalculateNumCompileUnits() = 0;
243+
virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t idx) = 0;
244+
245+
void SetCompileUnitAtIndex(uint32_t idx, const lldb::CompUnitSP &cu_sp);
242246

243247
ObjectFile *m_obj_file; // The object file that symbols can be extracted from.
248+
llvm::Optional<std::vector<lldb::CompUnitSP>> m_compile_units;
244249
uint32_t m_abilities;
245250
bool m_calculated_abilities;
246251

‎lldb/include/lldb/Symbol/SymbolVendor.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,6 @@ class SymbolVendor : public ModuleChild, public PluginInterface {
110110

111111
virtual size_t GetNumCompileUnits();
112112

113-
virtual bool SetCompileUnitAtIndex(size_t cu_idx,
114-
const lldb::CompUnitSP &cu_sp);
115-
116113
virtual lldb::CompUnitSP GetCompileUnitAtIndex(size_t idx);
117114

118115
TypeList &GetTypeList() { return m_type_list; }
@@ -142,13 +139,7 @@ class SymbolVendor : public ModuleChild, public PluginInterface {
142139
uint32_t GetPluginVersion() override;
143140

144141
protected:
145-
// Classes that inherit from SymbolVendor can see and modify these
146-
typedef std::vector<lldb::CompUnitSP> CompileUnits;
147-
typedef CompileUnits::iterator CompileUnitIter;
148-
typedef CompileUnits::const_iterator CompileUnitConstIter;
149-
150142
TypeList m_type_list; // Uniqued types for all parsers owned by this module
151-
CompileUnits m_compile_units; // The current compile units
152143
lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
153144
// case it isn't the same as the module
154145
// object file (debug symbols in a separate

‎lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ uint32_t SymbolFileBreakpad::CalculateAbilities() {
187187
return CompileUnits | Functions | LineTables;
188188
}
189189

190-
uint32_t SymbolFileBreakpad::GetNumCompileUnits() {
190+
uint32_t SymbolFileBreakpad::CalculateNumCompileUnits() {
191191
ParseCUData();
192192
return m_cu_data->GetSize();
193193
}
@@ -218,7 +218,7 @@ CompUnitSP SymbolFileBreakpad::ParseCompileUnitAtIndex(uint32_t index) {
218218
eLanguageTypeUnknown,
219219
/*is_optimized*/ eLazyBoolNo);
220220

221-
GetSymbolVendor().SetCompileUnitAtIndex(index, cu_sp);
221+
SetCompileUnitAtIndex(index, cu_sp);
222222
return cu_sp;
223223
}
224224

@@ -260,7 +260,7 @@ SymbolFileBreakpad::ResolveSymbolContext(const Address &so_addr,
260260
if (idx == UINT32_MAX)
261261
return 0;
262262

263-
sc.comp_unit = GetSymbolVendor().GetCompileUnitAtIndex(idx).get();
263+
sc.comp_unit = GetCompileUnitAtIndex(idx).get();
264264
SymbolContextItem result = eSymbolContextCompUnit;
265265
if (resolve_scope & eSymbolContextLineEntry) {
266266
if (sc.comp_unit->GetLineTable()->FindLineEntryByAddress(so_addr,
@@ -280,7 +280,7 @@ uint32_t SymbolFileBreakpad::ResolveSymbolContext(
280280

281281
uint32_t old_size = sc_list.GetSize();
282282
for (size_t i = 0, size = GetNumCompileUnits(); i < size; ++i) {
283-
CompileUnit &cu = *GetSymbolVendor().GetCompileUnitAtIndex(i);
283+
CompileUnit &cu = *GetCompileUnitAtIndex(i);
284284
cu.ResolveSymbolContext(file_spec, line, check_inlines,
285285
/*exact*/ false, resolve_scope, sc_list);
286286
}
@@ -522,10 +522,6 @@ SymbolFileBreakpad::GetUnwindPlan(const Address &address,
522522
return plan_sp;
523523
}
524524

525-
SymbolVendor &SymbolFileBreakpad::GetSymbolVendor() {
526-
return *m_obj_file->GetModule()->GetSymbolVendor();
527-
}
528-
529525
addr_t SymbolFileBreakpad::GetBaseFileAddress() {
530526
return m_obj_file->GetModule()
531527
->GetObjectFile()

‎lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,6 @@ class SymbolFileBreakpad : public SymbolFile {
4646

4747
// Compile Unit function calls
4848

49-
uint32_t GetNumCompileUnits() override;
50-
51-
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
52-
5349
lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) override {
5450
return lldb::eLanguageTypeUnknown;
5551
}
@@ -196,7 +192,9 @@ class SymbolFileBreakpad : public SymbolFile {
196192

197193
};
198194

199-
SymbolVendor &GetSymbolVendor();
195+
uint32_t CalculateNumCompileUnits() override;
196+
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
197+
200198
lldb::addr_t GetBaseFileAddress();
201199
void ParseFileRecords();
202200
void ParseCUData();

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,7 @@ lldb::CompUnitSP SymbolFileDWARF::ParseCompileUnit(DWARFCompileUnit &dwarf_cu) {
677677

678678
dwarf_cu.SetUserData(cu_sp.get());
679679

680-
m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
681-
dwarf_cu.GetID(), cu_sp);
680+
SetCompileUnitAtIndex(dwarf_cu.GetID(), cu_sp);
682681
}
683682
}
684683
}
@@ -715,7 +714,7 @@ llvm::Optional<uint32_t> SymbolFileDWARF::GetDWARFUnitIndex(uint32_t cu_idx) {
715714
return m_lldb_cu_to_dwarf_unit[cu_idx];
716715
}
717716

718-
uint32_t SymbolFileDWARF::GetNumCompileUnits() {
717+
uint32_t SymbolFileDWARF::CalculateNumCompileUnits() {
719718
DWARFDebugInfo *info = DebugInfo();
720719
if (!info)
721720
return 0;
@@ -3713,7 +3712,10 @@ ConstString SymbolFileDWARF::GetPluginName() { return GetPluginNameStatic(); }
37133712

37143713
uint32_t SymbolFileDWARF::GetPluginVersion() { return 1; }
37153714

3716-
void SymbolFileDWARF::Dump(lldb_private::Stream &s) { m_index->Dump(s); }
3715+
void SymbolFileDWARF::Dump(lldb_private::Stream &s) {
3716+
SymbolFile::Dump(s);
3717+
m_index->Dump(s);
3718+
}
37173719

37183720
void SymbolFileDWARF::DumpClangAST(Stream &s) {
37193721
TypeSystem *ts = GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,6 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
9595

9696
// Compile Unit function calls
9797

98-
uint32_t GetNumCompileUnits() override;
99-
100-
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
101-
10298
lldb::LanguageType
10399
ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
104100

@@ -331,6 +327,10 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
331327
bool DeclContextMatchesThisSymbolFile(
332328
const lldb_private::CompilerDeclContext *decl_ctx);
333329

330+
uint32_t CalculateNumCompileUnits() override;
331+
332+
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
333+
334334
virtual DWARFUnit *
335335
GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit);
336336

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ uint32_t SymbolFileDWARFDebugMap::CalculateAbilities() {
562562
return 0;
563563
}
564564

565-
uint32_t SymbolFileDWARFDebugMap::GetNumCompileUnits() {
565+
uint32_t SymbolFileDWARFDebugMap::CalculateNumCompileUnits() {
566566
InitOSO();
567567
return m_compile_unit_infos.size();
568568
}
@@ -585,9 +585,8 @@ CompUnitSP SymbolFileDWARFDebugMap::ParseCompileUnitAtIndex(uint32_t cu_idx) {
585585
eLanguageTypeUnknown, eLazyBoolCalculate);
586586

587587
if (m_compile_unit_infos[cu_idx].compile_unit_sp) {
588-
// Let our symbol vendor know about this compile unit
589-
m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
590-
cu_idx, m_compile_unit_infos[cu_idx].compile_unit_sp);
588+
SetCompileUnitAtIndex(cu_idx,
589+
m_compile_unit_infos[cu_idx].compile_unit_sp);
591590
}
592591
}
593592
}
@@ -1284,8 +1283,7 @@ void SymbolFileDWARFDebugMap::SetCompileUnit(SymbolFileDWARF *oso_dwarf,
12841283
cu_sp.get());
12851284
} else {
12861285
m_compile_unit_infos[cu_idx].compile_unit_sp = cu_sp;
1287-
m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
1288-
cu_idx, cu_sp);
1286+
SetCompileUnitAtIndex(cu_idx, cu_sp);
12891287
}
12901288
}
12911289
}

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFile {
4444
void InitializeObject() override;
4545

4646
// Compile Unit function calls
47-
uint32_t GetNumCompileUnits() override;
48-
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
49-
5047
lldb::LanguageType
5148
ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
5249

@@ -174,6 +171,9 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFile {
174171
// Protected Member Functions
175172
void InitOSO();
176173

174+
uint32_t CalculateNumCompileUnits() override;
175+
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
176+
177177
static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
178178
return (uint32_t)((uid >> 32ull) - 1ull);
179179
}

‎lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ void SymbolFileNativePDB::InitializeObject() {
329329
m_ast = llvm::make_unique<PdbAstBuilder>(*m_obj_file, *m_index);
330330
}
331331

332-
uint32_t SymbolFileNativePDB::GetNumCompileUnits() {
332+
uint32_t SymbolFileNativePDB::CalculateNumCompileUnits() {
333333
const DbiModuleList &modules = m_index->dbi().modules();
334334
uint32_t count = modules.getModuleCount();
335335
if (count == 0)
@@ -433,8 +433,7 @@ SymbolFileNativePDB::CreateCompileUnit(const CompilandIndexItem &cci) {
433433
std::make_shared<CompileUnit>(m_obj_file->GetModule(), nullptr, fs,
434434
toOpaqueUid(cci.m_id), lang, optimized);
435435

436-
m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
437-
cci.m_id.modi, cu_sp);
436+
SetCompileUnitAtIndex(cci.m_id.modi, cu_sp);
438437
return cu_sp;
439438
}
440439

‎lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,9 @@ class SymbolFileNativePDB : public SymbolFile {
6868

6969
// Compile Unit function calls
7070

71-
uint32_t GetNumCompileUnits() override;
72-
7371
void
7472
ParseDeclsForContext(lldb_private::CompilerDeclContext decl_ctx) override;
7573

76-
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
77-
7874
lldb::LanguageType
7975
ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
8076

@@ -157,6 +153,9 @@ class SymbolFileNativePDB : public SymbolFile {
157153
void DumpClangAST(Stream &s) override;
158154

159155
private:
156+
uint32_t CalculateNumCompileUnits() override;
157+
158+
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
160159

161160
size_t FindTypesByName(llvm::StringRef name, uint32_t max_matches,
162161
TypeMap &types);

‎lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,7 @@ SymbolFilePDB::CreateInstance(lldb_private::ObjectFile *obj_file) {
123123
}
124124

125125
SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file)
126-
: SymbolFile(object_file), m_session_up(), m_global_scope_up(),
127-
m_cached_compile_unit_count(0) {}
126+
: SymbolFile(object_file), m_session_up(), m_global_scope_up() {}
128127

129128
SymbolFilePDB::~SymbolFilePDB() {}
130129

@@ -191,33 +190,30 @@ void SymbolFilePDB::InitializeObject() {
191190
lldbassert(m_global_scope_up.get());
192191
}
193192

194-
uint32_t SymbolFilePDB::GetNumCompileUnits() {
195-
if (m_cached_compile_unit_count == 0) {
196-
auto compilands = m_global_scope_up->findAllChildren<PDBSymbolCompiland>();
197-
if (!compilands)
198-
return 0;
193+
uint32_t SymbolFilePDB::CalculateNumCompileUnits() {
194+
auto compilands = m_global_scope_up->findAllChildren<PDBSymbolCompiland>();
195+
if (!compilands)
196+
return 0;
199197

200-
// The linker could link *.dll (compiland language = LINK), or import
201-
// *.dll. For example, a compiland with name `Import:KERNEL32.dll` could be
202-
// found as a child of the global scope (PDB executable). Usually, such
203-
// compilands contain `thunk` symbols in which we are not interested for
204-
// now. However we still count them in the compiland list. If we perform
205-
// any compiland related activity, like finding symbols through
206-
// llvm::pdb::IPDBSession methods, such compilands will all be searched
207-
// automatically no matter whether we include them or not.
208-
m_cached_compile_unit_count = compilands->getChildCount();
209-
210-
// The linker can inject an additional "dummy" compilation unit into the
211-
// PDB. Ignore this special compile unit for our purposes, if it is there.
212-
// It is always the last one.
213-
auto last_compiland_up =
214-
compilands->getChildAtIndex(m_cached_compile_unit_count - 1);
215-
lldbassert(last_compiland_up.get());
216-
std::string name = last_compiland_up->getName();
217-
if (name == "* Linker *")
218-
--m_cached_compile_unit_count;
219-
}
220-
return m_cached_compile_unit_count;
198+
// The linker could link *.dll (compiland language = LINK), or import
199+
// *.dll. For example, a compiland with name `Import:KERNEL32.dll` could be
200+
// found as a child of the global scope (PDB executable). Usually, such
201+
// compilands contain `thunk` symbols in which we are not interested for
202+
// now. However we still count them in the compiland list. If we perform
203+
// any compiland related activity, like finding symbols through
204+
// llvm::pdb::IPDBSession methods, such compilands will all be searched
205+
// automatically no matter whether we include them or not.
206+
uint32_t compile_unit_count = compilands->getChildCount();
207+
208+
// The linker can inject an additional "dummy" compilation unit into the
209+
// PDB. Ignore this special compile unit for our purposes, if it is there.
210+
// It is always the last one.
211+
auto last_compiland_up = compilands->getChildAtIndex(compile_unit_count - 1);
212+
lldbassert(last_compiland_up.get());
213+
std::string name = last_compiland_up->getName();
214+
if (name == "* Linker *")
215+
--compile_unit_count;
216+
return compile_unit_count;
221217
}
222218

223219
void SymbolFilePDB::GetCompileUnitIndex(
@@ -1698,8 +1694,7 @@ lldb::CompUnitSP SymbolFilePDB::ParseCompileUnitForUID(uint32_t id,
16981694
if (index == UINT32_MAX)
16991695
GetCompileUnitIndex(*compiland_up, index);
17001696
lldbassert(index != UINT32_MAX);
1701-
m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(index,
1702-
cu_sp);
1697+
SetCompileUnitAtIndex(index, cu_sp);
17031698
return cu_sp;
17041699
}
17051700

‎lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,6 @@ class SymbolFilePDB : public lldb_private::SymbolFile {
4848

4949
// Compile Unit function calls
5050

51-
uint32_t GetNumCompileUnits() override;
52-
53-
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
54-
5551
lldb::LanguageType
5652
ParseLanguage(lldb_private::CompileUnit &comp_unit) override;
5753

@@ -173,6 +169,10 @@ class SymbolFilePDB : public lldb_private::SymbolFile {
173169
};
174170
using SecContribsMap = std::map<uint32_t, std::vector<SecContribInfo>>;
175171

172+
uint32_t CalculateNumCompileUnits() override;
173+
174+
lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
175+
176176
lldb::CompUnitSP ParseCompileUnitForUID(uint32_t id,
177177
uint32_t index = UINT32_MAX);
178178

@@ -245,7 +245,6 @@ class SymbolFilePDB : public lldb_private::SymbolFile {
245245
std::vector<lldb::TypeSP> m_builtin_types;
246246
std::unique_ptr<llvm::pdb::IPDBSession> m_session_up;
247247
std::unique_ptr<llvm::pdb::PDBSymbolExe> m_global_scope_up;
248-
uint32_t m_cached_compile_unit_count;
249248

250249
lldb_private::UniqueCStringMap<uint32_t> m_func_full_names;
251250
lldb_private::UniqueCStringMap<uint32_t> m_func_base_names;

‎lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ uint32_t SymbolFileSymtab::CalculateAbilities() {
104104
return abilities;
105105
}
106106

107-
uint32_t SymbolFileSymtab::GetNumCompileUnits() {
107+
uint32_t SymbolFileSymtab::CalculateNumCompileUnits() {
108108
// If we don't have any source file symbols we will just have one compile
109109
// unit for the entire object file
110110
if (m_source_indexes.empty())

0 commit comments

Comments
 (0)