[lldb][dwarf] Compute fully qualified names on simplified template names with DWARFTypePrinter #112811
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ZequanWu
force-pushed
the
lldb-dwarf-printer
branch
2 times, most recently
from
cee4a4f to
db1e81b
Compare
…mes with DWARFTypePrinter
ZequanWu
force-pushed
the
lldb-dwarf-printer
branch
from
db1e81b to
3fc0675
Compare
ZequanWu
requested review from
labath,
clayborg,
felipepiovezan,
dwblaikie and
bulbazord
|
@llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) ChangesThis is the second half of #90008. Essentially, it replaces the work of resolving template types when we just need the qualified names with walking the DIE tree using ResultFor an internal target, the time spent on Patch is 20.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112811.diff 13 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a30d898a93cc4d..41cb11f94a45e8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -44,6 +44,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Type.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h"
#include "llvm/Demangle/Demangle.h"
#include <map>
@@ -825,11 +826,11 @@ std::string DWARFASTParserClang::GetDIEClassTemplateParams(DWARFDIE die) {
if (llvm::StringRef(die.GetName()).contains("<"))
return {};
- TypeSystemClang::TemplateParameterInfos template_param_infos;
- if (ParseTemplateParameterInfos(die, template_param_infos))
- return m_ast.PrintTemplateParams(template_param_infos);
-
- return {};
+ std::string name;
+ llvm::raw_string_ostream os(name);
+ llvm::DWARFTypePrinter<DWARFDIE> type_printer(os);
+ type_printer.appendAndTerminateTemplateParameters(die);
+ return name;
}
void DWARFASTParserClang::MapDeclDIEToDefDIE(
@@ -1617,9 +1618,9 @@ void DWARFASTParserClang::GetUniqueTypeNameAndDeclaration(
case DW_TAG_structure_type:
case DW_TAG_union_type: {
if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) {
- qualified_name.insert(
- 0, GetDIEClassTemplateParams(parent_decl_ctx_die));
qualified_name.insert(0, "::");
+ qualified_name.insert(0,
+ GetDIEClassTemplateParams(parent_decl_ctx_die));
qualified_name.insert(0, class_union_struct_name);
}
parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
@@ -1672,6 +1673,12 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (attrs.name) {
GetUniqueTypeNameAndDeclaration(die, cu_language, unique_typename,
unique_decl);
+ if (log) {
+ dwarf->GetObjectFile()->GetModule()->LogMessage(
+ log, "SymbolFileDWARF({0:p}) - {1:x16}: {2} has unique name: {3} ",
+ static_cast<void *>(this), die.GetID(), DW_TAG_value_to_name(tag),
+ unique_typename.AsCString());
+ }
if (UniqueDWARFASTType *unique_ast_entry_type =
dwarf->GetUniqueDWARFASTTypeMap().Find(
unique_typename, die, unique_decl, byte_size,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
index 235343d2271223..ca25801137be38 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -24,9 +24,11 @@ class DWARFUnit;
class DWARFDebugInfoEntry;
class DWARFDeclContext;
class SymbolFileDWARF;
+class DWARFFormValue;
class DWARFBaseDIE {
public:
+ using DWARFFormValue = dwarf::DWARFFormValue;
DWARFBaseDIE() = default;
DWARFBaseDIE(DWARFUnit *cu, DWARFDebugInfoEntry *die)
@@ -46,6 +48,7 @@ class DWARFBaseDIE {
explicit operator bool() const { return IsValid(); }
bool IsValid() const { return m_cu && m_die; }
+ bool isValid() const { return IsValid(); }
bool HasChildren() const;
@@ -85,6 +88,8 @@ class DWARFBaseDIE {
// Accessing information about a DIE
dw_tag_t Tag() const;
+ dw_tag_t getTag() const { return Tag(); }
+
dw_offset_t GetOffset() const;
// Get the LLDB user ID for this DIE. This is often just the DIE offset,
@@ -95,6 +100,8 @@ class DWARFBaseDIE {
const char *GetName() const;
+ const char *getShortName() const { return GetName(); }
+
lldb::ModuleSP GetModule() const;
// Getting attribute values from the DIE.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index d83740f8e2113b..3b4330c6de31d3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -86,6 +86,12 @@ elaborating_dies(const DWARFDIE &die) {
}
} // namespace
+std::optional<uint64_t> DWARFDIE::getLanguage() const {
+ if (IsValid())
+ return m_cu->GetDWARFLanguageType();
+ return std::nullopt;
+}
+
DWARFDIE
DWARFDIE::GetParent() const {
if (IsValid())
@@ -118,6 +124,22 @@ DWARFDIE::GetReferencedDIE(const dw_attr_t attr) const {
return {};
}
+DWARFDIE DWARFDIE::resolveReferencedType(dw_attr_t attr) const {
+ return GetReferencedDIE(attr);
+}
+
+DWARFDIE DWARFDIE::resolveReferencedType(DWARFFormValue v) const {
+ if (IsValid())
+ return v.Reference();
+ return {};
+}
+
+DWARFDIE DWARFDIE::resolveTypeUnitReference() const {
+ if (DWARFDIE reference = GetReferencedDIE(DW_AT_signature))
+ return reference;
+ return *this;
+}
+
DWARFDIE
DWARFDIE::GetDIE(dw_offset_t die_offset) const {
if (IsValid())
@@ -575,3 +597,16 @@ bool DWARFDIE::GetDIENamesAndRanges(
llvm::iterator_range<DWARFDIE::child_iterator> DWARFDIE::children() const {
return llvm::make_range(child_iterator(*this), child_iterator());
}
+
+DWARFDIE::child_iterator DWARFDIE::begin() const {
+ return child_iterator(*this);
+}
+
+DWARFDIE::child_iterator DWARFDIE::end() const { return child_iterator(); }
+
+std::optional<DWARFFormValue> DWARFDIE::find(const dw_attr_t attr) const {
+ DWARFFormValue form_value;
+ if (m_die->GetAttributeValue(m_cu, attr, form_value, nullptr, false))
+ return form_value;
+ return std::nullopt;
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index e1318953a384cd..273ac79e453703 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -44,9 +44,13 @@ class DWARFDIE : public DWARFBaseDIE {
// Functions for obtaining DIE relations and references
+ std::optional<uint64_t> getLanguage() const;
+
DWARFDIE
GetParent() const;
+ DWARFDIE getParent() const { return GetParent(); }
+
DWARFDIE
GetFirstChild() const;
@@ -56,6 +60,12 @@ class DWARFDIE : public DWARFBaseDIE {
DWARFDIE
GetReferencedDIE(const dw_attr_t attr) const;
+ DWARFDIE resolveReferencedType(dw_attr_t attr) const;
+
+ DWARFDIE resolveReferencedType(DWARFFormValue v) const;
+
+ DWARFDIE resolveTypeUnitReference() const;
+
// Get a another DIE from the same DWARF file as this DIE. This will
// check the current DIE's compile unit first to see if "die_offset" is
// in the same compile unit, and fall back to checking the DWARF file.
@@ -96,6 +106,8 @@ class DWARFDIE : public DWARFBaseDIE {
DWARFDIE
GetAttributeValueAsReferenceDIE(const dw_attr_t attr) const;
+ std::optional<DWARFFormValue> find(const dw_attr_t attr) const;
+
bool GetDIENamesAndRanges(
const char *&name, const char *&mangled, DWARFRangeList &ranges,
std::optional<int> &decl_file, std::optional<int> &decl_line,
@@ -105,6 +117,9 @@ class DWARFDIE : public DWARFBaseDIE {
/// The range of all the children of this DIE.
llvm::iterator_range<child_iterator> children() const;
+
+ child_iterator begin() const;
+ child_iterator end() const;
};
class DWARFDIE::child_iterator
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index f58c6262349c6f..bc5d0a9555ebd4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -569,6 +569,31 @@ uint64_t DWARFFormValue::Reference(dw_offset_t base_offset) const {
}
}
+std::optional<uint64_t> DWARFFormValue::getAsUnsignedConstant() const {
+ if ((!IsDataForm(m_form)) || m_form == lldb_private::dwarf::DW_FORM_sdata)
+ return std::nullopt;
+ return m_value.uval;
+}
+
+std::optional<int64_t> DWARFFormValue::getAsSignedConstant() const {
+ if ((!IsDataForm(m_form)) ||
+ (m_form == lldb_private::dwarf::DW_FORM_udata &&
+ uint64_t(std::numeric_limits<int64_t>::max()) < m_value.uval))
+ return std::nullopt;
+ switch (m_form) {
+ case lldb_private::dwarf::DW_FORM_data4:
+ return int32_t(m_value.uval);
+ case lldb_private::dwarf::DW_FORM_data2:
+ return int16_t(m_value.uval);
+ case lldb_private::dwarf::DW_FORM_data1:
+ return int8_t(m_value.uval);
+ case lldb_private::dwarf::DW_FORM_sdata:
+ case lldb_private::dwarf::DW_FORM_data8:
+ default:
+ return m_value.sval;
+ }
+}
+
const uint8_t *DWARFFormValue::BlockData() const { return m_value.data; }
bool DWARFFormValue::IsBlockForm(const dw_form_t form) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
index 8ab9163e645fea..66fb6e855f29f8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -58,11 +58,14 @@ class DWARFFormValue {
uint64_t Reference(dw_offset_t offset) const;
bool Boolean() const { return m_value.uval != 0; }
+ std::optional<uint64_t> getAsUnsignedConstant() const;
+ std::optional<int64_t> getAsSignedConstant() const;
uint64_t Unsigned() const { return m_value.uval; }
void SetUnsigned(uint64_t uval) { m_value.uval = uval; }
int64_t Signed() const { return m_value.sval; }
void SetSigned(int64_t sval) { m_value.sval = sval; }
const char *AsCString() const;
+ const char *getAsCString() const { return AsCString(); }
dw_addr_t Address() const;
bool IsValid() const { return m_form != 0; }
bool SkipValue(const DWARFDataExtractor &debug_info_data,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 9287d4baf19e9c..40c145c8ff1550 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -9,6 +9,7 @@
#include "SymbolFileDWARF.h"
#include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
+#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/FileUtilities.h"
#include "llvm/Support/Format.h"
@@ -2821,33 +2822,14 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
return true; // Keep iterating over index types, language mismatch.
}
- // Check the context matches
- std::vector<lldb_private::CompilerContext> die_context;
- if (query.GetModuleSearch())
- die_context = die.GetDeclContext();
- else
- die_context = die.GetTypeLookupContext();
- assert(!die_context.empty());
- if (!query_simple.ContextMatches(die_context))
- return true; // Keep iterating over index types, context mismatch.
-
- // Try to resolve the type.
- if (Type *matching_type = ResolveType(die, true, true)) {
- ConstString name = matching_type->GetQualifiedName();
- // We have found a type that still might not match due to template
- // parameters. If we create a new TypeQuery that uses the new type's
- // fully qualified name, we can find out if this type matches at all
- // context levels. We can't use just the "match_simple" context
- // because all template parameters were stripped off. The fully
- // qualified name of the type will have the template parameters and
- // will allow us to make sure it matches correctly.
- TypeQuery die_query(name.GetStringRef(),
- TypeQueryOptions::e_exact_match);
- if (!query.ContextMatches(die_query.GetContextRef()))
- return true; // Keep iterating over index types, context mismatch.
-
- results.InsertUnique(matching_type->shared_from_this());
- }
+ std::string qualified_name;
+ llvm::raw_string_ostream os(qualified_name);
+ llvm::DWARFTypePrinter<DWARFDIE> type_printer(os);
+ type_printer.appendQualifiedName(die);
+ TypeQuery die_query(qualified_name, e_exact_match);
+ if (query.ContextMatches(die_query.GetContextRef()))
+ if (Type *matching_type = ResolveType(die, true, true))
+ results.InsertUnique(matching_type->shared_from_this());
return !results.Done(query); // Keep iterating if we aren't done.
});
if (results.Done(query)) {
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 50115a638b9589..798535a4c390d6 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1403,26 +1403,6 @@ static TemplateParameterList *CreateTemplateParameterList(
return template_param_list;
}
-std::string TypeSystemClang::PrintTemplateParams(
- const TemplateParameterInfos &template_param_infos) {
- llvm::SmallVector<NamedDecl *, 8> ignore;
- clang::TemplateParameterList *template_param_list =
- CreateTemplateParameterList(getASTContext(), template_param_infos,
- ignore);
- llvm::SmallVector<clang::TemplateArgument, 2> args(
- template_param_infos.GetArgs());
- if (template_param_infos.hasParameterPack()) {
- llvm::ArrayRef<TemplateArgument> pack_args =
- template_param_infos.GetParameterPackArgs();
- args.append(pack_args.begin(), pack_args.end());
- }
- std::string str;
- llvm::raw_string_ostream os(str);
- clang::printTemplateArgumentList(os, args, GetTypePrintingPolicy(),
- template_param_list);
- return str;
-}
-
clang::FunctionTemplateDecl *TypeSystemClang::CreateFunctionTemplateDecl(
clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
clang::FunctionDecl *func_decl,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index e39aedec7e3902..678eaed381fd49 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1148,10 +1148,6 @@ class TypeSystemClang : public TypeSystem {
bool SetDeclIsForcefullyCompleted(const clang::TagDecl *td);
- /// Return the template parameters (including surrounding <>) in string form.
- std::string
- PrintTemplateParams(const TemplateParameterInfos &template_param_infos);
-
private:
/// Returns the PrintingPolicy used when generating the internal type names.
/// These type names are mostly used for the formatter selection.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp
new file mode 100644
index 00000000000000..b1563c5b2ba443
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp
@@ -0,0 +1,31 @@
+// Test lldb is able to compute the fully qualified names on templates with
+// -gsimple-template-names and -fdebug-types-section.
+
+// REQUIRES: lld
+
+// Test against logging to see if we print the fully qualified names correctly.
+// RUN: %clangxx --target=x86_64-pc-linux -g -gsimple-template-names %s -o %t
+// RUN: %lldb %t -o "log enable dwarf comp" -o "target variable v1 v2" -o exit | FileCheck %s --check-prefix=LOG
+
+// Test that we following DW_AT_signature correctly. If not, lldb might confuse the types of v1 and v2.
+// RUN: %clangxx --target=x86_64-pc-linux -g -gsimple-template-names -fdebug-types-section %s -o %t
+// RUN: %lldb %t -o "target variable v1 v2" -o exit | FileCheck %s --check-prefix=TYPE
+
+// LOG: unique name: ::t2<outer_struct1::t1<int> >
+// LOG: unique name: ::t2<outer_struct2::t1<int> >
+
+// TYPE: (t2<outer_struct1::t1<int> >) v1 = {}
+// TYPE-NEXT: (t2<outer_struct2::t1<int> >) v2 = {}
+
+struct outer_struct1 {
+ template <typename> struct t1 {};
+};
+
+struct outer_struct2 {
+ template <typename> struct t1 {};
+};
+
+template <typename> struct t2 {};
+t2<outer_struct1::t1<int>> v1;
+t2<outer_struct2::t1<int>> v2;
+int main() {}
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
index 69c91835a4d9a1..2e98a4a397147b 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
@@ -226,6 +226,8 @@ class DWARFDie {
bool addressRangeContainsAddress(const uint64_t Address) const;
+ std::optional<uint64_t> getLanguage() const;
+
Expected<DWARFLocationExpressionsVector>
getLocations(dwarf::Attribute Attr) const;
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index 87e876273c4b97..b6d42f686cd266 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -11,6 +11,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/Support/Error.h"
#include <string>
@@ -107,13 +108,11 @@ void DWARFTypePrinter<DieType>::appendArrayType(const DieType &D) {
if (std::optional<typename DieType::DWARFFormValue> UpperV =
C.find(dwarf::DW_AT_upper_bound))
UB = UpperV->getAsUnsignedConstant();
- if (std::optional<typename DieType::DWARFFormValue> LV =
- D.getDwarfUnit()->getUnitDIE().find(dwarf::DW_AT_language))
- if (std::optional<uint64_t> LC = LV->getAsUnsignedConstant())
- if ((DefaultLB =
- LanguageLowerBound(static_cast<dwarf::SourceLanguage>(*LC))))
- if (LB && *LB == *DefaultLB)
- LB = std::nullopt;
+ if (std::optional<uint64_t> LV = D.getLanguage())
+ if ((DefaultLB =
+ LanguageLowerBound(static_cast<dwarf::SourceLanguage>(*LV))))
+ if (LB && *LB == *DefaultLB)
+ LB = std::nullopt;
if (!LB && !Count && !UB)
OS << "[]";
else if (!LB && (Count || UB) && DefaultLB)
@@ -150,6 +149,16 @@ template <typename DieType>
DieType resolveReferencedType(DieType D, typename DieType::DWARFFormValue F) {
return D.resolveReferencedType(F);
}
+template <typename DWARFFormValueType>
+const char *toString(std::optional<DWARFFormValueType> F) {
+ if (F) {
+ llvm::Expected<const char *> E = F->getAsCString();
+ if (E)
+ return *E;
+ llvm::consumeError(E.takeError());
+ }
+ return nullptr;
+}
} // namespace detail
template <typename DieType>
@@ -239,7 +248,7 @@ DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
appendConstVolatileQualifierBefore(D);
break;
case dwarf::DW_TAG_namespace: {
- if (const char *Name = dwarf::toString(D.find(dwarf::DW_AT_name), nullptr))
+ if (const char *Name = detail::toString(D.find(dwarf::DW_AT_name)))
OS << Name;
else
OS << "(anonymous namespace)";
@@ -261,7 +270,7 @@ DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
case DW_TAG_base_type:
*/
default: {
- const char *NamePtr = dwarf::toString(D.find(dwarf::DW_AT_name), nullptr);
+ const char *NamePtr = detail::toString(D.find(dwarf::DW_AT_name));
if (!NamePtr) {
appendTypeTagName(D.getTag());
return DieType();
@@ -440,7 +449,7 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
if (T.getTag() == dwarf::DW_TAG_pointer_type ||
T.getTag() == dwarf::DW_TAG_reference_type)
continue;
- const char *RawName = dwarf::toString(T.find(dwarf::DW_AT_name), nullptr);
+ const char *RawName = detail::toString(T.find(dwarf::DW_AT_name));
assert(RawName);
StringRef Name = RawName;
auto V = C.find(dwarf::DW_AT_const_value);
@@ -533,7 +542,7 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
}
if (C.getTag() == dwarf::DW_TAG_GNU_template_template_param) {
const char *RawName =
- dwarf::toString(C.find(dwarf::DW_AT_GNU_template_name), nullptr);
+ detail::toString(C.find(dwarf::DW_AT_GNU_template_name));
assert(RawName);
StringRef Name = RawName;
Sep();
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index 6e7b5870dfa29f..f62526d08e54e8 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -414,6 +414,15 @@ bool DWARFDie::addressRangeContainsAddress(const uint64_t Address) const {
return false;
...
[truncated]
|
|
@llvm/pr-subscribers-debuginfo Author: Zequan Wu (ZequanWu) ChangesThis is the second half of #90008. Essentially, it replaces the work of resolving template types when we just need the qualified names with walking the DIE tree using ResultFor an internal target, the time spent on Patch is 20.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112811.diff 13 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a30d898a93cc4d..41cb11f94a45e8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -44,6 +44,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Type.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h"
#include "llvm/Demangle/Demangle.h"
#include <map>
@@ -825,11 +826,11 @@ std::string DWARFASTParserClang::GetDIEClassTemplateParams(DWARFDIE die) {
if (llvm::StringRef(die.GetName()).contains("<"))
return {};
- TypeSystemClang::TemplateParameterInfos template_param_infos;
- if (ParseTemplateParameterInfos(die, template_param_infos))
- return m_ast.PrintTemplateParams(template_param_infos);
-
- return {};
+ std::string name;
+ llvm::raw_string_ostream os(name);
+ llvm::DWARFTypePrinter<DWARFDIE> type_printer(os);
+ type_printer.appendAndTerminateTemplateParameters(die);
+ return name;
}
void DWARFASTParserClang::MapDeclDIEToDefDIE(
@@ -1617,9 +1618,9 @@ void DWARFASTParserClang::GetUniqueTypeNameAndDeclaration(
case DW_TAG_structure_type:
case DW_TAG_union_type: {
if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) {
- qualified_name.insert(
- 0, GetDIEClassTemplateParams(parent_decl_ctx_die));
qualified_name.insert(0, "::");
+ qualified_name.insert(0,
+ GetDIEClassTemplateParams(parent_decl_ctx_die));
qualified_name.insert(0, class_union_struct_name);
}
parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
@@ -1672,6 +1673,12 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (attrs.name) {
GetUniqueTypeNameAndDeclaration(die, cu_language, unique_typename,
unique_decl);
+ if (log) {
+ dwarf->GetObjectFile()->GetModule()->LogMessage(
+ log, "SymbolFileDWARF({0:p}) - {1:x16}: {2} has unique name: {3} ",
+ static_cast<void *>(this), die.GetID(), DW_TAG_value_to_name(tag),
+ unique_typename.AsCString());
+ }
if (UniqueDWARFASTType *unique_ast_entry_type =
dwarf->GetUniqueDWARFASTTypeMap().Find(
unique_typename, die, unique_decl, byte_size,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
index 235343d2271223..ca25801137be38 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -24,9 +24,11 @@ class DWARFUnit;
class DWARFDebugInfoEntry;
class DWARFDeclContext;
class SymbolFileDWARF;
+class DWARFFormValue;
class DWARFBaseDIE {
public:
+ using DWARFFormValue = dwarf::DWARFFormValue;
DWARFBaseDIE() = default;
DWARFBaseDIE(DWARFUnit *cu, DWARFDebugInfoEntry *die)
@@ -46,6 +48,7 @@ class DWARFBaseDIE {
explicit operator bool() const { return IsValid(); }
bool IsValid() const { return m_cu && m_die; }
+ bool isValid() const { return IsValid(); }
bool HasChildren() const;
@@ -85,6 +88,8 @@ class DWARFBaseDIE {
// Accessing information about a DIE
dw_tag_t Tag() const;
+ dw_tag_t getTag() const { return Tag(); }
+
dw_offset_t GetOffset() const;
// Get the LLDB user ID for this DIE. This is often just the DIE offset,
@@ -95,6 +100,8 @@ class DWARFBaseDIE {
const char *GetName() const;
+ const char *getShortName() const { return GetName(); }
+
lldb::ModuleSP GetModule() const;
// Getting attribute values from the DIE.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index d83740f8e2113b..3b4330c6de31d3 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -86,6 +86,12 @@ elaborating_dies(const DWARFDIE &die) {
}
} // namespace
+std::optional<uint64_t> DWARFDIE::getLanguage() const {
+ if (IsValid())
+ return m_cu->GetDWARFLanguageType();
+ return std::nullopt;
+}
+
DWARFDIE
DWARFDIE::GetParent() const {
if (IsValid())
@@ -118,6 +124,22 @@ DWARFDIE::GetReferencedDIE(const dw_attr_t attr) const {
return {};
}
+DWARFDIE DWARFDIE::resolveReferencedType(dw_attr_t attr) const {
+ return GetReferencedDIE(attr);
+}
+
+DWARFDIE DWARFDIE::resolveReferencedType(DWARFFormValue v) const {
+ if (IsValid())
+ return v.Reference();
+ return {};
+}
+
+DWARFDIE DWARFDIE::resolveTypeUnitReference() const {
+ if (DWARFDIE reference = GetReferencedDIE(DW_AT_signature))
+ return reference;
+ return *this;
+}
+
DWARFDIE
DWARFDIE::GetDIE(dw_offset_t die_offset) const {
if (IsValid())
@@ -575,3 +597,16 @@ bool DWARFDIE::GetDIENamesAndRanges(
llvm::iterator_range<DWARFDIE::child_iterator> DWARFDIE::children() const {
return llvm::make_range(child_iterator(*this), child_iterator());
}
+
+DWARFDIE::child_iterator DWARFDIE::begin() const {
+ return child_iterator(*this);
+}
+
+DWARFDIE::child_iterator DWARFDIE::end() const { return child_iterator(); }
+
+std::optional<DWARFFormValue> DWARFDIE::find(const dw_attr_t attr) const {
+ DWARFFormValue form_value;
+ if (m_die->GetAttributeValue(m_cu, attr, form_value, nullptr, false))
+ return form_value;
+ return std::nullopt;
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index e1318953a384cd..273ac79e453703 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -44,9 +44,13 @@ class DWARFDIE : public DWARFBaseDIE {
// Functions for obtaining DIE relations and references
+ std::optional<uint64_t> getLanguage() const;
+
DWARFDIE
GetParent() const;
+ DWARFDIE getParent() const { return GetParent(); }
+
DWARFDIE
GetFirstChild() const;
@@ -56,6 +60,12 @@ class DWARFDIE : public DWARFBaseDIE {
DWARFDIE
GetReferencedDIE(const dw_attr_t attr) const;
+ DWARFDIE resolveReferencedType(dw_attr_t attr) const;
+
+ DWARFDIE resolveReferencedType(DWARFFormValue v) const;
+
+ DWARFDIE resolveTypeUnitReference() const;
+
// Get a another DIE from the same DWARF file as this DIE. This will
// check the current DIE's compile unit first to see if "die_offset" is
// in the same compile unit, and fall back to checking the DWARF file.
@@ -96,6 +106,8 @@ class DWARFDIE : public DWARFBaseDIE {
DWARFDIE
GetAttributeValueAsReferenceDIE(const dw_attr_t attr) const;
+ std::optional<DWARFFormValue> find(const dw_attr_t attr) const;
+
bool GetDIENamesAndRanges(
const char *&name, const char *&mangled, DWARFRangeList &ranges,
std::optional<int> &decl_file, std::optional<int> &decl_line,
@@ -105,6 +117,9 @@ class DWARFDIE : public DWARFBaseDIE {
/// The range of all the children of this DIE.
llvm::iterator_range<child_iterator> children() const;
+
+ child_iterator begin() const;
+ child_iterator end() const;
};
class DWARFDIE::child_iterator
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index f58c6262349c6f..bc5d0a9555ebd4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -569,6 +569,31 @@ uint64_t DWARFFormValue::Reference(dw_offset_t base_offset) const {
}
}
+std::optional<uint64_t> DWARFFormValue::getAsUnsignedConstant() const {
+ if ((!IsDataForm(m_form)) || m_form == lldb_private::dwarf::DW_FORM_sdata)
+ return std::nullopt;
+ return m_value.uval;
+}
+
+std::optional<int64_t> DWARFFormValue::getAsSignedConstant() const {
+ if ((!IsDataForm(m_form)) ||
+ (m_form == lldb_private::dwarf::DW_FORM_udata &&
+ uint64_t(std::numeric_limits<int64_t>::max()) < m_value.uval))
+ return std::nullopt;
+ switch (m_form) {
+ case lldb_private::dwarf::DW_FORM_data4:
+ return int32_t(m_value.uval);
+ case lldb_private::dwarf::DW_FORM_data2:
+ return int16_t(m_value.uval);
+ case lldb_private::dwarf::DW_FORM_data1:
+ return int8_t(m_value.uval);
+ case lldb_private::dwarf::DW_FORM_sdata:
+ case lldb_private::dwarf::DW_FORM_data8:
+ default:
+ return m_value.sval;
+ }
+}
+
const uint8_t *DWARFFormValue::BlockData() const { return m_value.data; }
bool DWARFFormValue::IsBlockForm(const dw_form_t form) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
index 8ab9163e645fea..66fb6e855f29f8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -58,11 +58,14 @@ class DWARFFormValue {
uint64_t Reference(dw_offset_t offset) const;
bool Boolean() const { return m_value.uval != 0; }
+ std::optional<uint64_t> getAsUnsignedConstant() const;
+ std::optional<int64_t> getAsSignedConstant() const;
uint64_t Unsigned() const { return m_value.uval; }
void SetUnsigned(uint64_t uval) { m_value.uval = uval; }
int64_t Signed() const { return m_value.sval; }
void SetSigned(int64_t sval) { m_value.sval = sval; }
const char *AsCString() const;
+ const char *getAsCString() const { return AsCString(); }
dw_addr_t Address() const;
bool IsValid() const { return m_form != 0; }
bool SkipValue(const DWARFDataExtractor &debug_info_data,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 9287d4baf19e9c..40c145c8ff1550 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -9,6 +9,7 @@
#include "SymbolFileDWARF.h"
#include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
+#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/FileUtilities.h"
#include "llvm/Support/Format.h"
@@ -2821,33 +2822,14 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) {
return true; // Keep iterating over index types, language mismatch.
}
- // Check the context matches
- std::vector<lldb_private::CompilerContext> die_context;
- if (query.GetModuleSearch())
- die_context = die.GetDeclContext();
- else
- die_context = die.GetTypeLookupContext();
- assert(!die_context.empty());
- if (!query_simple.ContextMatches(die_context))
- return true; // Keep iterating over index types, context mismatch.
-
- // Try to resolve the type.
- if (Type *matching_type = ResolveType(die, true, true)) {
- ConstString name = matching_type->GetQualifiedName();
- // We have found a type that still might not match due to template
- // parameters. If we create a new TypeQuery that uses the new type's
- // fully qualified name, we can find out if this type matches at all
- // context levels. We can't use just the "match_simple" context
- // because all template parameters were stripped off. The fully
- // qualified name of the type will have the template parameters and
- // will allow us to make sure it matches correctly.
- TypeQuery die_query(name.GetStringRef(),
- TypeQueryOptions::e_exact_match);
- if (!query.ContextMatches(die_query.GetContextRef()))
- return true; // Keep iterating over index types, context mismatch.
-
- results.InsertUnique(matching_type->shared_from_this());
- }
+ std::string qualified_name;
+ llvm::raw_string_ostream os(qualified_name);
+ llvm::DWARFTypePrinter<DWARFDIE> type_printer(os);
+ type_printer.appendQualifiedName(die);
+ TypeQuery die_query(qualified_name, e_exact_match);
+ if (query.ContextMatches(die_query.GetContextRef()))
+ if (Type *matching_type = ResolveType(die, true, true))
+ results.InsertUnique(matching_type->shared_from_this());
return !results.Done(query); // Keep iterating if we aren't done.
});
if (results.Done(query)) {
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 50115a638b9589..798535a4c390d6 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1403,26 +1403,6 @@ static TemplateParameterList *CreateTemplateParameterList(
return template_param_list;
}
-std::string TypeSystemClang::PrintTemplateParams(
- const TemplateParameterInfos &template_param_infos) {
- llvm::SmallVector<NamedDecl *, 8> ignore;
- clang::TemplateParameterList *template_param_list =
- CreateTemplateParameterList(getASTContext(), template_param_infos,
- ignore);
- llvm::SmallVector<clang::TemplateArgument, 2> args(
- template_param_infos.GetArgs());
- if (template_param_infos.hasParameterPack()) {
- llvm::ArrayRef<TemplateArgument> pack_args =
- template_param_infos.GetParameterPackArgs();
- args.append(pack_args.begin(), pack_args.end());
- }
- std::string str;
- llvm::raw_string_ostream os(str);
- clang::printTemplateArgumentList(os, args, GetTypePrintingPolicy(),
- template_param_list);
- return str;
-}
-
clang::FunctionTemplateDecl *TypeSystemClang::CreateFunctionTemplateDecl(
clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
clang::FunctionDecl *func_decl,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index e39aedec7e3902..678eaed381fd49 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1148,10 +1148,6 @@ class TypeSystemClang : public TypeSystem {
bool SetDeclIsForcefullyCompleted(const clang::TagDecl *td);
- /// Return the template parameters (including surrounding <>) in string form.
- std::string
- PrintTemplateParams(const TemplateParameterInfos &template_param_infos);
-
private:
/// Returns the PrintingPolicy used when generating the internal type names.
/// These type names are mostly used for the formatter selection.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp
new file mode 100644
index 00000000000000..b1563c5b2ba443
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp
@@ -0,0 +1,31 @@
+// Test lldb is able to compute the fully qualified names on templates with
+// -gsimple-template-names and -fdebug-types-section.
+
+// REQUIRES: lld
+
+// Test against logging to see if we print the fully qualified names correctly.
+// RUN: %clangxx --target=x86_64-pc-linux -g -gsimple-template-names %s -o %t
+// RUN: %lldb %t -o "log enable dwarf comp" -o "target variable v1 v2" -o exit | FileCheck %s --check-prefix=LOG
+
+// Test that we following DW_AT_signature correctly. If not, lldb might confuse the types of v1 and v2.
+// RUN: %clangxx --target=x86_64-pc-linux -g -gsimple-template-names -fdebug-types-section %s -o %t
+// RUN: %lldb %t -o "target variable v1 v2" -o exit | FileCheck %s --check-prefix=TYPE
+
+// LOG: unique name: ::t2<outer_struct1::t1<int> >
+// LOG: unique name: ::t2<outer_struct2::t1<int> >
+
+// TYPE: (t2<outer_struct1::t1<int> >) v1 = {}
+// TYPE-NEXT: (t2<outer_struct2::t1<int> >) v2 = {}
+
+struct outer_struct1 {
+ template <typename> struct t1 {};
+};
+
+struct outer_struct2 {
+ template <typename> struct t1 {};
+};
+
+template <typename> struct t2 {};
+t2<outer_struct1::t1<int>> v1;
+t2<outer_struct2::t1<int>> v2;
+int main() {}
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
index 69c91835a4d9a1..2e98a4a397147b 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
@@ -226,6 +226,8 @@ class DWARFDie {
bool addressRangeContainsAddress(const uint64_t Address) const;
+ std::optional<uint64_t> getLanguage() const;
+
Expected<DWARFLocationExpressionsVector>
getLocations(dwarf::Attribute Attr) const;
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
index 87e876273c4b97..b6d42f686cd266 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFTypePrinter.h
@@ -11,6 +11,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/Support/Error.h"
#include <string>
@@ -107,13 +108,11 @@ void DWARFTypePrinter<DieType>::appendArrayType(const DieType &D) {
if (std::optional<typename DieType::DWARFFormValue> UpperV =
C.find(dwarf::DW_AT_upper_bound))
UB = UpperV->getAsUnsignedConstant();
- if (std::optional<typename DieType::DWARFFormValue> LV =
- D.getDwarfUnit()->getUnitDIE().find(dwarf::DW_AT_language))
- if (std::optional<uint64_t> LC = LV->getAsUnsignedConstant())
- if ((DefaultLB =
- LanguageLowerBound(static_cast<dwarf::SourceLanguage>(*LC))))
- if (LB && *LB == *DefaultLB)
- LB = std::nullopt;
+ if (std::optional<uint64_t> LV = D.getLanguage())
+ if ((DefaultLB =
+ LanguageLowerBound(static_cast<dwarf::SourceLanguage>(*LV))))
+ if (LB && *LB == *DefaultLB)
+ LB = std::nullopt;
if (!LB && !Count && !UB)
OS << "[]";
else if (!LB && (Count || UB) && DefaultLB)
@@ -150,6 +149,16 @@ template <typename DieType>
DieType resolveReferencedType(DieType D, typename DieType::DWARFFormValue F) {
return D.resolveReferencedType(F);
}
+template <typename DWARFFormValueType>
+const char *toString(std::optional<DWARFFormValueType> F) {
+ if (F) {
+ llvm::Expected<const char *> E = F->getAsCString();
+ if (E)
+ return *E;
+ llvm::consumeError(E.takeError());
+ }
+ return nullptr;
+}
} // namespace detail
template <typename DieType>
@@ -239,7 +248,7 @@ DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
appendConstVolatileQualifierBefore(D);
break;
case dwarf::DW_TAG_namespace: {
- if (const char *Name = dwarf::toString(D.find(dwarf::DW_AT_name), nullptr))
+ if (const char *Name = detail::toString(D.find(dwarf::DW_AT_name)))
OS << Name;
else
OS << "(anonymous namespace)";
@@ -261,7 +270,7 @@ DieType DWARFTypePrinter<DieType>::appendUnqualifiedNameBefore(
case DW_TAG_base_type:
*/
default: {
- const char *NamePtr = dwarf::toString(D.find(dwarf::DW_AT_name), nullptr);
+ const char *NamePtr = detail::toString(D.find(dwarf::DW_AT_name));
if (!NamePtr) {
appendTypeTagName(D.getTag());
return DieType();
@@ -440,7 +449,7 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
if (T.getTag() == dwarf::DW_TAG_pointer_type ||
T.getTag() == dwarf::DW_TAG_reference_type)
continue;
- const char *RawName = dwarf::toString(T.find(dwarf::DW_AT_name), nullptr);
+ const char *RawName = detail::toString(T.find(dwarf::DW_AT_name));
assert(RawName);
StringRef Name = RawName;
auto V = C.find(dwarf::DW_AT_const_value);
@@ -533,7 +542,7 @@ bool DWARFTypePrinter<DieType>::appendTemplateParameters(DieType D,
}
if (C.getTag() == dwarf::DW_TAG_GNU_template_template_param) {
const char *RawName =
- dwarf::toString(C.find(dwarf::DW_AT_GNU_template_name), nullptr);
+ detail::toString(C.find(dwarf::DW_AT_GNU_template_name));
assert(RawName);
StringRef Name = RawName;
Sep();
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index 6e7b5870dfa29f..f62526d08e54e8 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -414,6 +414,15 @@ bool DWARFDie::addressRangeContainsAddress(const uint64_t Address) const {
return false;
...
[truncated]
|
labath
reviewed
Nov 5, 2024
| @@ -46,6 +48,7 @@ class DWARFBaseDIE { | |||
| explicit operator bool() const { return IsValid(); } | |||
|
|
|||
| bool IsValid() const { return m_cu && m_die; } | |||
| bool isValid() const { return IsValid(); } | |||
There was a problem hiding this comment.
How about using operator bool as the common api ?
There was a problem hiding this comment.
Removed both IsValid() and IsValid() and switched to use operator bool ().
There was a problem hiding this comment.
I'm sorry, I guess I wasn't clear about this. What I was suggesting is to just revert the addition isValid -- as there already exists a comon API. I didn't want to remove things that are already there. (I mean, I'd be fine with that doing that under the flag of moving the LLDB API closer to LLVM, but I don't think we ought to do that as a part of this PR)
Michael137
reviewed
Nov 6, 2024
| @@ -95,6 +100,8 @@ class DWARFBaseDIE { | |||
|
|
|||
| const char *GetName() const; | |||
|
|
|||
| const char *getShortName() const { return GetName(); } | |||
There was a problem hiding this comment.
Nit: How do people feel about grouping all the new APIs that are required for LLVM compatbility together and adding a comment stating why we have them?
There was a problem hiding this comment.
Grouped those new APIs for each lldb class and added a comment for it.
There was a problem hiding this comment.
(I was considering the same thing, but I don't particularly care either way)
| // LOG: unique name: ::t2<outer_struct2::t1<int> > | ||
|
|
||
| // TYPE: (t2<outer_struct1::t1<int> >) v1 = {} | ||
| // TYPE-NEXT: (t2<outer_struct2::t1<int> >) v2 = {} |
There was a problem hiding this comment.
Do we need to check the typename that gets displayed here? Is that actually affected by this patch?
There was a problem hiding this comment.
No, they are not affected by this patch. When I working on this change, I had an oversight on not calling DWARFDIE::resolveTypeUnitReference in https://github.com/llvm/llvm-project/pull/112811/files/3fc0675398547617731d0501e3d4b98e2ffc480e#diff-5e94a4eb2c54c062b27821639b3c2732886de49229c99b11b6b2602f35b0ccdcR788 and existing lldb tests didn't catch it. So, I added this for test coverage.
| // RUN: %lldb %t -o "target variable v1 v2" -o exit | FileCheck %s --check-prefix=TYPE | ||
|
|
||
| // LOG: unique name: ::t2<outer_struct1::t1<int> > | ||
| // LOG: unique name: ::t2<outer_struct2::t1<int> > |
There was a problem hiding this comment.
Do we strictly need this test? Technically the existing -gstimple-template-names tests should already exercise this codepath right?
A more interesting test could be a unit-test that uses the DWARFTypePrinter with LLDB DWARFDIEs. Not sure how difficult that is to set up though.
There was a problem hiding this comment.
I added extra logging in https://github.com/llvm/llvm-project/pull/112811/files/3fc0675398547617731d0501e3d4b98e2ffc480e#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1676-R1681 so we can test the names are generated correctly here. Maybe it's not necessary.
There was a problem hiding this comment.
I'm not exactly thrilled by the idea of using logs for testing, and even less about adding log statements with the sole purpose of testing a piece of code.
A unit test for the DWARFTypePrinter would definitely be better, and I think it should be possible to reuse what we have in unittests/SymbolFile/DWARF/DWARFDIETest.cpp. For testing its integration into the rest of lldb, an end-to-end might be better (maybe the existing ones are fine, but we can definitely add new ones if you think something is missing).
There was a problem hiding this comment.
I recalled why I added this extra logging. It's to test this change in DWARFASTParserClang::GetUniqueTypeNameAndDeclaration, which is outside the DWARFTypePrinter. Though the change shouldn't change the lldb's behaviour at all (as long as the names generated by this function is consistent and unambiguous, the unique dwarf ast type map will work properly), it will be good the internally used names are correctly generated.
For example, it was generating names like the following before:
unique name: t3::<t2<int> >t4
After that change:
unique name: t3<t2<int> >::t4
I updated this shell test to reflect this and added an unit test for DWARFTypePrinter.
| clang::printTemplateArgumentList(os, args, GetTypePrintingPolicy(), | ||
| template_param_list); | ||
| return str; | ||
| } |
labath
approved these changes
Nov 8, 2024
|
This patch broke the lldb incremental bot on greendragon https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/15486/ ******************** TEST 'lldb-shell :: SymbolFile/DWARF/x86/simplified-template-names.cpp' FAILED ******************** Command Output (stderr):RUN: at line 7: /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang --driver-mode=g++ --target=specify-a-target-or-use-a-_host-substitution --target=x86_64-pc-linux -g -gsimple-template-names /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp -o /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/simplified-template-names.cpp.tmp
|
rastogishubham
added a commit
that referenced
this pull request
Nov 19, 2024
…plate names with DWARFTypePrinter (#112811)" This reverts commit 94d100f. Reverted because of greendragon failure on the incremental arm64 bot ******************** TEST 'lldb-shell :: SymbolFile/DWARF/x86/simplified-template-names.cpp' FAILED ******************** Exit Code: 1 RUN: at line 7: /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang --driver-mode=g++ --target=specify-a-target-or-use-a-_host-substitution --target=x86_64-pc-linux -g -gsimple-template-names /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp -o /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/simplified-template-names.cpp.tmp /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang --driver-mode=g++ --target=specify-a-target-or-use-a-_host-substitution --target=x86_64-pc-linux -g -gsimple-template-names /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/simplified-template-names.cpp -o /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/simplified-template-names.cpp.tmp ld: warning: -m is obsolete ld: unknown option: --hash-style=gnu clang: error: linker command failed with exit code 1 (use -v to see invocation)
|
I reverted this change with c51786b to keep the bot green |
I should explicitly call ld.lld. Relanded here |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the second half of #90008.
Essentially, it replaces the work of resolving template types when we just need the qualified names with walking the DIE tree using
DWARFTypePrinter.Result
For an internal target, the time spent on
expr *thisfor the first time reduced from 28 secs to 17 secs.