labath created this revision.
labath added reviewers: clayborg, JDevlieghere, aprantl, jingham.
Herald added a subscriber: jdoerfert.
This patch isn't meant to be committed, but it is here to demostrate
what it would take to implement the idea proposed by Greg in
D62073#1507063 <https://reviews.llvm.org/D62073#1507063>.
The background here is that a type unit build will have substantially
more "units" than a regular one (e.g., a type unit build of lldb will
have about 1k compile units, but almost 400k type units). Vending them
all as lldb_private::CompileUnits is a bit wasteful. It can be made less
wasteful by making sure the CompileUnits share line tables and stuff
(just like DWARF units do), but it's not clear to me whether this is
really useful/needed. For instance, lldb CompileUnits expect to have a
"name" (a file path), but type units don't have that as they're supposed
to be independent/shared between real compile units.
The majority of changes here just deal with the remapping of dwarf vs.
lldb unit indexes, as now we can now no longer use a 1:1 mapping.
However, there are a couple of things that are worth calling out
explicitly:
- DWARFASTParserClang goes through the lldb CompileUnits in order to fetch file
names. Right now I've just deleted that code. In a proper solution, we'd need
to find a way to get this information strictly through DWARF structures.
- The main visible side effect I can see from this is that we will stop filling
out the SymbolContextScope field
https://lldb.llvm.org/cpp_reference/classlldb__private_1_1Type.html#aea1b2a6336880f67455036803bf42ea1
of the Type objects that we hand about. More precisely, we can still fill out
the "module" part, but not the "compile unit". I'm not sure what are the
implications of that, but maybe that is even more correct as the type unit does
not really belong to any single compilation. This patch breaks just three
tests, and all of them seem related to me stopping to parse the file names, and
not this...
So, does anyone see why (a cleaned up version of) this would be a bad
idea?
https://reviews.llvm.org/D62395
Files:
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -17,8 +17,7 @@
~SymbolFileDWARFDwo() override = default;
- lldb::CompUnitSP ParseCompileUnit(DWARFUnit *dwarf_cu,
- uint32_t cu_idx) override;
+ lldb::CompUnitSP ParseCompileUnit(DWARFCompileUnit *dwarf_cu) override;
DWARFUnit *GetCompileUnit();
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -12,9 +12,11 @@
#include "lldb/Expression/DWARFExpression.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Utility/LLDBAssert.h"
+#include "llvm/Support/Casting.h"
-#include "DWARFUnit.h"
+#include "DWARFCompileUnit.h"
#include "DWARFDebugInfo.h"
+#include "DWARFUnit.h"
using namespace lldb;
using namespace lldb_private;
@@ -46,12 +48,12 @@
}
lldb::CompUnitSP
-SymbolFileDWARFDwo::ParseCompileUnit(DWARFUnit *dwarf_cu,
- uint32_t cu_idx) {
+SymbolFileDWARFDwo::ParseCompileUnit(DWARFCompileUnit *dwarf_cu) {
assert(GetCompileUnit() == dwarf_cu && "SymbolFileDWARFDwo::ParseCompileUnit "
"called with incompatible compile "
"unit");
- return GetBaseSymbolFile()->ParseCompileUnit(m_base_dwarf_cu, UINT32_MAX);
+ return GetBaseSymbolFile()->ParseCompileUnit(
+ llvm::cast<DWARFCompileUnit>(m_base_dwarf_cu));
}
DWARFUnit *SymbolFileDWARFDwo::GetCompileUnit() {
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -49,6 +49,7 @@
class DWARFDebugRangesBase;
class DWARFDeclContext;
class DWARFFormValue;
+class DWARFCompileUnit;
class SymbolFileDWARFDebugMap;
class SymbolFileDWARFDwo;
class SymbolFileDWARFDwp;
@@ -252,8 +253,7 @@
static DWARFDIE GetParentSymbolContextDIE(const DWARFDIE &die);
- virtual lldb::CompUnitSP ParseCompileUnit(DWARFUnit *dwarf_cu,
- uint32_t cu_idx);
+ virtual lldb::CompUnitSP ParseCompileUnit(DWARFCompileUnit *dwarf_cu);
virtual lldb_private::DWARFExpression::LocationListFormat
GetLocationListFormat() const;
@@ -434,6 +434,8 @@
return m_forward_decl_clang_type_to_die;
}
+ void BuildLldbCuTranslationTable();
+
struct DecodedUID {
SymbolFileDWARF *dwarf;
DIERef ref;
@@ -481,6 +483,7 @@
DIEToVariableSP m_die_to_variable_sp;
DIEToClangType m_forward_decl_die_to_clang_type;
ClangTypeToDIE m_forward_decl_clang_type_to_die;
+ std::vector<uint32_t> m_lldb_cu_to_dwarf_unit;
};
#endif // SymbolFileDWARF_SymbolFileDWARF_h_
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -54,6 +54,7 @@
#include "AppleDWARFIndex.h"
#include "DWARFASTParser.h"
#include "DWARFASTParserClang.h"
+#include "DWARFCompileUnit.h"
#include "DWARFDebugAbbrev.h"
#include "DWARFDebugAranges.h"
#include "DWARFDebugInfo.h"
@@ -640,8 +641,7 @@
return m_ranges.get();
}
-lldb::CompUnitSP SymbolFileDWARF::ParseCompileUnit(DWARFUnit *dwarf_cu,
- uint32_t cu_idx) {
+lldb::CompUnitSP SymbolFileDWARF::ParseCompileUnit(DWARFCompileUnit *dwarf_cu) {
CompUnitSP cu_sp;
if (dwarf_cu) {
CompileUnit *comp_unit = (CompileUnit *)dwarf_cu->GetUserData();
@@ -650,8 +650,7 @@
cu_sp = comp_unit->shared_from_this();
} else {
if (dwarf_cu->GetSymbolFileDWARF() != this) {
- return dwarf_cu->GetSymbolFileDWARF()->ParseCompileUnit(dwarf_cu,
- cu_idx);
+ return dwarf_cu->GetSymbolFileDWARF()->ParseCompileUnit(dwarf_cu);
} else if (dwarf_cu->GetOffset() == 0 && GetDebugMapSymfile()) {
// Let the debug map create the compile unit
cu_sp = m_debug_map_symfile->GetCompileUnit(this);
@@ -678,8 +677,13 @@
cu_die.GetAttributeValueAsUnsigned(DW_AT_language, 0));
bool is_optimized = dwarf_cu->GetIsOptimized();
+ uint32_t dwarf_idx;
+ if (m_lldb_cu_to_dwarf_unit.empty())
+ dwarf_idx = dwarf_cu->GetID();
+ else
+ dwarf_idx = m_lldb_cu_to_dwarf_unit[dwarf_cu->GetID()];
cu_sp = std::make_shared<CompileUnit>(
- module_sp, dwarf_cu, cu_file_spec, dwarf_cu->GetID(),
+ module_sp, dwarf_cu, cu_file_spec, dwarf_idx,
cu_language, is_optimized ? eLazyBoolYes : eLazyBoolNo);
if (cu_sp) {
// If we just created a compile unit with an invalid file spec,
@@ -697,12 +701,8 @@
dwarf_cu->SetUserData(cu_sp.get());
- // Figure out the compile unit index if we weren't given one
- if (cu_idx == UINT32_MAX)
- cu_idx = dwarf_cu->GetID();
-
m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
- cu_idx, cu_sp);
+ dwarf_cu->GetID(), cu_sp);
}
}
}
@@ -712,11 +712,31 @@
return cu_sp;
}
+void SymbolFileDWARF::BuildLldbCuTranslationTable() {
+ if (!m_lldb_cu_to_dwarf_unit.empty())
+ return;
+
+ DWARFDebugInfo *info = DebugInfo();
+ if (!info)
+ return;
+ if (!info->ContainsTypeUnits()) {
+ // We can use a 1-to-1 mapping. No need to build a translation table.
+ return;
+ }
+ for (uint32_t i = 0, num = info->GetNumUnits(); i < num; ++i) {
+ if (auto *cu = llvm::dyn_cast<DWARFCompileUnit>(info->GetUnitAtIndex(i))) {
+ cu->SetID(m_lldb_cu_to_dwarf_unit.size());
+ m_lldb_cu_to_dwarf_unit.push_back(i);
+ }
+ }
+}
+
uint32_t SymbolFileDWARF::GetNumCompileUnits() {
DWARFDebugInfo *info = DebugInfo();
- if (info)
- return info->GetNumUnits();
- return 0;
+ if (!info)
+ return 0;
+ BuildLldbCuTranslationTable();
+ return m_lldb_cu_to_dwarf_unit.empty() ? info->GetNumUnits() : m_lldb_cu_to_dwarf_unit.size();
}
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
@@ -724,9 +744,18 @@
CompUnitSP cu_sp;
DWARFDebugInfo *info = DebugInfo();
if (info) {
- DWARFUnit *dwarf_cu = info->GetUnitAtIndex(cu_idx);
+ BuildLldbCuTranslationTable();
+ uint32_t dwarf_idx;
+ if (m_lldb_cu_to_dwarf_unit.empty())
+ dwarf_idx = cu_idx;
+ else {
+ if (cu_idx >= m_lldb_cu_to_dwarf_unit.size())
+ return nullptr;
+ dwarf_idx = m_lldb_cu_to_dwarf_unit[cu_idx];
+ }
+ DWARFUnit *dwarf_cu = info->GetUnitAtIndex(dwarf_idx);
if (dwarf_cu)
- cu_sp = ParseCompileUnit(dwarf_cu, cu_idx);
+ cu_sp = ParseCompileUnit(llvm::cast<DWARFCompileUnit>(dwarf_cu));
}
return cu_sp;
}
@@ -1406,15 +1435,18 @@
}
CompileUnit *
-SymbolFileDWARF::GetCompUnitForDWARFCompUnit(DWARFUnit *dwarf_cu,
+SymbolFileDWARF::GetCompUnitForDWARFCompUnit(DWARFUnit *unit,
uint32_t cu_idx) {
+ BuildLldbCuTranslationTable();
// Check if the symbol vendor already knows about this compile unit?
- if (dwarf_cu->GetUserData() == nullptr) {
- // The symbol vendor doesn't know about this compile unit, we need to parse
- // and add it to the symbol vendor object.
- return ParseCompileUnit(dwarf_cu, cu_idx).get();
+ if (unit->GetUserData() == nullptr) {
+ if (auto *cu = llvm::dyn_cast<DWARFCompileUnit>(unit)) {
+ // The symbol vendor doesn't know about this compile unit, we need to
+ // parse and add it to the symbol vendor object.
+ return ParseCompileUnit(cu).get();
+ }
}
- return (CompileUnit *)dwarf_cu->GetUserData();
+ return (CompileUnit *)unit->GetUserData();
}
size_t SymbolFileDWARF::GetObjCMethodDIEOffsets(ConstString class_name,
@@ -1522,7 +1554,7 @@
DWARFDebugInfo *debug_info = DebugInfo();
- const uint32_t num_compile_units = GetNumCompileUnits();
+ const uint32_t num_compile_units = debug_info->GetNumUnits();
for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) {
DWARFUnit *dwarf_cu = debug_info->GetUnitAtIndex(cu_idx);
@@ -2515,9 +2547,10 @@
if (die) {
Type *type_ptr = GetDIEToType().lookup(die.GetDIE());
if (type_ptr == nullptr) {
- CompileUnit *lldb_cu = GetCompUnitForDWARFCompUnit(die.GetCU());
- assert(lldb_cu);
- SymbolContext sc(lldb_cu);
+ SymbolContextScope *scope = GetCompUnitForDWARFCompUnit(die.GetCU());
+ if (!scope)
+ scope = GetObjectFile()->GetModule().get();
+ SymbolContext sc(scope);
const DWARFDebugInfoEntry *parent_die = die.GetParent().GetDIE();
while (parent_die != nullptr) {
if (parent_die->Tag() == DW_TAG_subprogram)
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -48,6 +48,7 @@
dw_offset_t die_offset);
DWARFUnit *GetUnit(const DIERef &die_ref);
DWARFTypeUnit *GetTypeUnitForHash(uint64_t hash);
+ bool ContainsTypeUnits();
DWARFDIE GetDIEForDIEOffset(DIERef::Section section,
dw_offset_t die_offset);
DWARFDIE GetDIE(const DIERef &die_ref);
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -182,6 +182,11 @@
return llvm::cast<DWARFTypeUnit>(GetUnitAtIndex(pos->second));
}
+bool DWARFDebugInfo::ContainsTypeUnits() {
+ ParseUnitHeadersIfNeeded();
+ return !m_type_hash_to_unit_index.empty();
+}
+
DWARFDIE
DWARFDebugInfo::GetDIEForDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
Index: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -18,6 +18,10 @@
void Dump(lldb_private::Stream *s) const override;
+ static bool classof(const DWARFUnit *unit) {
+ return unit->GetUnitType() != DW_UT_type;
+ }
+
private:
DWARFCompileUnit(SymbolFileDWARF *dwarf, lldb::user_id_t uid,
const DWARFUnitHeader &header,
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -315,8 +315,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
@@ -469,8 +467,8 @@
}
bool translation_unit_is_objc =
- (sc.comp_unit->GetLanguage() == eLanguageTypeObjC ||
- sc.comp_unit->GetLanguage() == eLanguageTypeObjC_plus_plus);
+ (die.GetLanguage() == eLanguageTypeObjC ||
+ die.GetLanguage() == eLanguageTypeObjC_plus_plus);
if (translation_unit_is_objc) {
if (type_name_cstr != nullptr) {
@@ -564,8 +562,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
@@ -668,7 +664,7 @@
}
if (byte_size && *byte_size == 0 && type_name_cstr && !die.HasChildren() &&
- sc.comp_unit->GetLanguage() == eLanguageTypeObjC) {
+ die.GetLanguage() == eLanguageTypeObjC) {
// Work around an issue with clang at the moment where forward
// declarations for objective C classes are emitted as:
// DW_TAG_structure_type [2]
@@ -995,8 +991,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
@@ -1122,7 +1116,7 @@
if (ClangASTContext::StartTagDeclarationDefinition(clang_type)) {
if (die.HasChildren()) {
- SymbolContext cu_sc(die.GetLLDBCompileUnit());
+ SymbolContext cu_sc;
bool is_signed = false;
enumerator_clang_type.IsIntegerType(is_signed);
ParseChildEnumerators(cu_sc, clang_type, is_signed,
@@ -1166,8 +1160,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
@@ -1674,8 +1666,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
@@ -2176,7 +2166,7 @@
default_accessibility = eAccessPrivate;
}
- SymbolContext sc(die.GetLLDBCompileUnit());
+ SymbolContext sc;
std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> bases;
std::vector<int> member_accessibilities;
bool is_a_class = false;
@@ -2375,7 +2365,7 @@
case DW_TAG_enumeration_type:
if (ClangASTContext::StartTagDeclarationDefinition(clang_type)) {
if (die.HasChildren()) {
- SymbolContext sc(die.GetLLDBCompileUnit());
+ SymbolContext sc;
bool is_signed = false;
clang_type.IsIntegerType(is_signed);
ParseChildEnumerators(sc, clang_type, is_signed,
@@ -2468,8 +2458,6 @@
case DW_AT_description:
default:
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
@@ -2717,8 +2705,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
@@ -3185,8 +3171,6 @@
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
switch (attr) {
case DW_AT_decl_file:
- decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
- form_value.Unsigned()));
break;
case DW_AT_decl_line:
decl.SetLine(form_value.Unsigned());
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
