[Lldb-commits] [lldb] [lldb][NFCI] Unify DW_TAG -> string conversions (PR #90657)
Alex Langford via lldb-commits
lldb-commits at lists.llvm.org
Tue Apr 30 13:04:12 PDT 2024
https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/90657
The high level goal is to have 1 way of converting a DW_TAG value into a human-readable string.
There are 3 ways this change accomplishes that:
1.) Changing DW_TAG_value_to_name to not create custom error strings.
The way it was doing this is error-prone: Specifically, it was using a
function-local static char buffer and handing out a pointer to it.
Initialization of this is thread-safe, but mutating it is definitely
not. Multiple threads that want to call this function could step on
each others toes. The implementation in this patch sidesteps the issue
by just returning a StringRef with no mention of the tag value in it.
2.) Changing all uses of DW_TAG_value_to_name to log the value of the
tag since the function doesn't create a string with the value in it
anymore.
3.) Removing `DWARFBaseDIE::GetTagAsCString()`. Callers should call
DW_TAG_value_to_name on the tag directly.
>From bdcb733bcfd64e23edac44ae0a21897720084879 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Tue, 30 Apr 2024 11:30:22 -0700
Subject: [PATCH] [lldb][NFCI] Unify DW_TAG -> string conversions
The high level goal is to have 1 way of converting a DW_TAG value into a
human-readable string.
There are 3 ways this change accomplishes that:
1.) Changing DW_TAG_value_to_name to not create custom error strings.
The way it was doing this is error-prone: Specifically, it was using a
function-local static char buffer and handing out a pointer to it.
Initialization of this is thread-safe, but mutating it is definitely
not. Multiple threads that want to call this function could step on
each others toes. The implementation in this patch sidesteps the issue
by just returning a StringRef with no mention of the tag value in it.
2.) Changing all uses of DW_TAG_value_to_name to log the value of the
tag since the function doesn't create a string with the value in it
anymore.
3.) Removing `DWARFBaseDIE::GetTagAsCString()`. Callers should call
DW_TAG_value_to_name on the tag directly.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 62 +++++++++---------
.../Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp | 4 --
.../Plugins/SymbolFile/DWARF/DWARFBaseDIE.h | 2 -
.../Plugins/SymbolFile/DWARF/DWARFDefines.cpp | 16 ++---
.../Plugins/SymbolFile/DWARF/DWARFDefines.h | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 63 ++++++++++---------
6 files changed, 71 insertions(+), 78 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bea11e0e3840af..f8101aba5c6277 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -452,9 +452,9 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
log,
"DWARFASTParserClang::ParseTypeFromDWARF "
"(die = {0:x16}, decl_ctx = {1:p} (die "
- "{2:x16})) {3} name = '{4}')",
+ "{2:x16})) {3} ({4}) name = '{5}')",
die.GetOffset(), static_cast<void *>(context), context_die.GetOffset(),
- die.GetTagAsCString(), die.GetName());
+ DW_TAG_value_to_name(die.Tag()), die.Tag(), die.GetName());
}
Type *type_ptr = dwarf->GetDIEToType().lookup(die.GetDIE());
@@ -765,9 +765,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
if (log)
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' "
+ "SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
"is Objective-C 'id' built-in type.",
- die.GetOffset(), die.GetTagAsCString(), die.GetName());
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName());
clang_type = m_ast.GetBasicType(eBasicTypeObjCID);
encoding_data_type = Type::eEncodingIsUID;
attrs.type.Clear();
@@ -776,9 +777,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
if (log)
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' "
+ "SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
"is Objective-C 'Class' built-in type.",
- die.GetOffset(), die.GetTagAsCString(), die.GetName());
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName());
clang_type = m_ast.GetBasicType(eBasicTypeObjCClass);
encoding_data_type = Type::eEncodingIsUID;
attrs.type.Clear();
@@ -787,9 +789,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
if (log)
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF::ParseType (die = {0:x16}) {1} '{2}' "
+ "SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
"is Objective-C 'selector' built-in type.",
- die.GetOffset(), die.GetTagAsCString(), die.GetName());
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName());
clang_type = m_ast.GetBasicType(eBasicTypeObjCSel);
encoding_data_type = Type::eEncodingIsUID;
attrs.type.Clear();
@@ -808,10 +811,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
if (log)
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF::ParseType (die = {0:x16}) {1} "
- "'{2}' is 'objc_object*', which we overrode to "
- "'id'.",
- die.GetOffset(), die.GetTagAsCString(), die.GetName());
+ "SymbolFileDWARF::ParseType (die = {0:x16}) {1} ({2}) '{3}' "
+ "is 'objc_object*', which we overrode to 'id'.",
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName());
clang_type = m_ast.GetBasicType(eBasicTypeObjCID);
encoding_data_type = Type::eEncodingIsUID;
attrs.type.Clear();
@@ -870,10 +873,10 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext &sc,
if (log) {
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF({0:p}) - {1:x16}}: {2} type \"{3}\" is a "
- "forward declaration, complete type is {4:x8}",
+ "SymbolFileDWARF({0:p}) - {1:x16}}: {2} ({3}) type \"{4}\" is a "
+ "forward declaration, complete type is {5:x8}",
static_cast<void *>(this), die.GetOffset(),
- DW_TAG_value_to_name(tag), attrs.name.GetCString(),
+ DW_TAG_value_to_name(tag), tag, attrs.name.GetCString(),
type_sp->GetID());
}
@@ -1734,11 +1737,10 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (log) {
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF({0:p}) - {1:x16}: {2} type "
- "\"{3}\" is an "
- "incomplete objc type, complete type is {4:x8}",
+ "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is an "
+ "incomplete objc type, complete type is {5:x8}",
static_cast<void *>(this), die.GetOffset(),
- DW_TAG_value_to_name(tag), attrs.name.GetCString(),
+ DW_TAG_value_to_name(tag), tag, attrs.name.GetCString(),
type_sp->GetID());
}
@@ -1760,10 +1762,10 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (log) {
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a "
+ "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is a "
"forward declaration, trying to find complete type",
static_cast<void *>(this), die.GetOffset(), DW_TAG_value_to_name(tag),
- attrs.name.GetCString());
+ tag, attrs.name.GetCString());
}
// See if the type comes from a Clang module and if so, track down
@@ -1789,10 +1791,10 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (log) {
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a "
- "forward declaration, complete type is {4:x8}",
+ "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" is a "
+ "forward declaration, complete type is {5:x8}",
static_cast<void *>(this), die.GetOffset(),
- DW_TAG_value_to_name(tag), attrs.name.GetCString(),
+ DW_TAG_value_to_name(tag), tag, attrs.name.GetCString(),
type_sp->GetID());
}
@@ -1836,10 +1838,10 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (log) {
dwarf->GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" "
+ "SymbolFileDWARF({0:p}) - {1:x16}: {2} ({3}) type \"{4}\" "
"clang::ClassTemplateDecl failed to return a decl.",
static_cast<void *>(this), die.GetOffset(),
- DW_TAG_value_to_name(tag), attrs.name.GetCString());
+ DW_TAG_value_to_name(tag), tag, attrs.name.GetCString());
}
return TypeSP();
}
@@ -3018,11 +3020,11 @@ void DWARFASTParserClang::ParseSingleMember(
this_field_info.bit_offset)))) {
ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
objfile->GetModule()->ReportWarning(
- "{0:x16}: {1} bitfield named \"{2}\" has invalid "
- "bit offset ({3:x8}) member will be ignored. Please file a bug "
+ "{0:x16}: {1} ({2}) bitfield named \"{3}\" has invalid "
+ "bit offset ({4:x8}) member will be ignored. Please file a bug "
"against the "
- "compiler and include the preprocessed output for {4}\n",
- die.GetID(), DW_TAG_value_to_name(tag), attrs.name,
+ "compiler and include the preprocessed output for {5}\n",
+ die.GetID(), DW_TAG_value_to_name(tag), tag, attrs.name,
this_field_info.bit_offset, GetUnitName(parent_die).c_str());
return;
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
index 3a3b05acd26d6e..c2ebeed4c860d4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -35,10 +35,6 @@ dw_tag_t DWARFBaseDIE::Tag() const {
return llvm::dwarf::DW_TAG_null;
}
-const char *DWARFBaseDIE::GetTagAsCString() const {
- return DW_TAG_value_to_name(Tag());
-}
-
const char *DWARFBaseDIE::GetAttributeValueAsString(const dw_attr_t attr,
const char *fail_value) const {
if (IsValid())
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
index 75c822703cd80e..235343d2271223 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -85,8 +85,6 @@ class DWARFBaseDIE {
// Accessing information about a DIE
dw_tag_t Tag() const;
- const char *GetTagAsCString() const;
-
dw_offset_t GetOffset() const;
// Get the LLDB user ID for this DIE. This is often just the DIE offset,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
index 9a88aed85e979c..1f0e10ef27018b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
@@ -15,18 +15,12 @@
namespace lldb_private::plugin {
namespace dwarf {
-const char *DW_TAG_value_to_name(uint32_t val) {
- static char invalid[100];
-
- if (val == 0)
- return "NULL";
+llvm::StringRef DW_TAG_value_to_name(dw_tag_t tag) {
+ static constexpr llvm::StringLiteral s_unknown_tag_name("<unknown DW_TAG>");
+ if (llvm::StringRef tag_name = llvm::dwarf::TagString(tag); !tag_name.empty())
+ return tag_name;
- llvm::StringRef llvmstr = llvm::dwarf::TagString(val);
- if (llvmstr.empty()) {
- snprintf(invalid, sizeof(invalid), "Unknown DW_TAG constant: 0x%x", val);
- return invalid;
- }
- return llvmstr.data();
+ return s_unknown_tag_name;
}
const char *DW_AT_value_to_name(uint32_t val) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
index 3ed92cc203bf8f..e87ac458fe962c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.h
@@ -17,7 +17,7 @@ namespace dwarf {
typedef uint32_t DRC_class; // Holds DRC_* class bitfields
-const char *DW_TAG_value_to_name(uint32_t val);
+llvm::StringRef DW_TAG_value_to_name(dw_tag_t tag);
const char *DW_AT_value_to_name(uint32_t val);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 49f13d2c89e380..f6f152726bf74e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1555,8 +1555,10 @@ Type *SymbolFileDWARF::ResolveTypeUID(const DWARFDIE &die,
Log *log = GetLog(DWARFLog::DebugInfo);
if (log)
GetObjectFile()->GetModule()->LogMessage(
- log, "SymbolFileDWARF::ResolveTypeUID (die = {0:x16}) {1} '{2}'",
- die.GetOffset(), die.GetTagAsCString(), die.GetName());
+ log,
+ "SymbolFileDWARF::ResolveTypeUID (die = {0:x16}) {1} ({2}) '{3}'",
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName());
// We might be coming in in the middle of a type tree (a class within a
// class, an enum within a class), so parse any needed parent DIEs before
@@ -1572,11 +1574,10 @@ Type *SymbolFileDWARF::ResolveTypeUID(const DWARFDIE &die,
if (log)
GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF::ResolveTypeUID (die = {0:x16}) "
- "{1} '{2}' "
- "resolve parent forward type for {3:x16})",
- die.GetOffset(), die.GetTagAsCString(), die.GetName(),
- decl_ctx_die.GetOffset());
+ "SymbolFileDWARF::ResolveTypeUID (die = {0:x16}) {1} ({2}) "
+ "'{3}' resolve parent forward type for {4:x16})",
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName(), decl_ctx_die.GetOffset());
} break;
default:
@@ -1644,9 +1645,9 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) {
Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
if (log)
GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
- log, "{0:x8}: {1} '{2}' resolving forward declaration...",
- dwarf_die.GetID(), dwarf_die.GetTagAsCString(),
- type->GetName().AsCString());
+ log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
+ dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()),
+ dwarf_die.Tag(), type->GetName().AsCString());
assert(compiler_type);
if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
@@ -1665,8 +1666,9 @@ Type *SymbolFileDWARF::ResolveType(const DWARFDIE &die,
return type;
GetObjectFile()->GetModule()->ReportError(
- "Parsing a die that is being parsed die: {0:x16}: {1} {2}",
- die.GetOffset(), die.GetTagAsCString(), die.GetName());
+ "Parsing a die that is being parsed die: {0:x16}: {1} ({2}) {3}",
+ die.GetOffset(), DW_TAG_value_to_name(die.Tag()), die.Tag(),
+ die.GetName());
} else
return type;
@@ -3134,9 +3136,9 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
if (log) {
GetObjectFile()->GetModule()->LogMessage(
log,
- "SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(tag={0}, "
- "name='{1}')",
- DW_TAG_value_to_name(tag), die.GetName());
+ "SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(tag={0} "
+ "({1}), name='{2}')",
+ DW_TAG_value_to_name(tag), tag, die.GetName());
}
// Get the type system that we are looking to find a type for. We will
@@ -3184,10 +3186,10 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::"
- "FindDefinitionTypeForDWARFDeclContext(tag={0}, "
- "name='{1}') ignoring die={2:x16} ({3})",
- DW_TAG_value_to_name(tag), die.GetName(), type_die.GetOffset(),
- type_die.GetName());
+ "FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), "
+ "name='{2}') ignoring die={3:x16} ({4})",
+ DW_TAG_value_to_name(tag), tag, die.GetName(),
+ type_die.GetOffset(), type_die.GetName());
}
return true;
}
@@ -3197,9 +3199,9 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
GetObjectFile()->GetModule()->LogMessage(
log,
"SymbolFileDWARF::"
- "FindDefinitionTypeForDWARFDeclContext(tag={0}, "
- "name='{1}') trying die={2:x16} ({3})",
- DW_TAG_value_to_name(tag), die.GetName(), type_die.GetOffset(),
+ "FindDefinitionTypeForDWARFDeclContext(tag={0} ({1}), name='{2}') "
+ "trying die={3:x16} ({4})",
+ DW_TAG_value_to_name(tag), tag, die.GetName(), type_die.GetOffset(),
type_dwarf_decl_ctx.GetQualifiedName());
}
@@ -3650,8 +3652,8 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
StreamString strm;
location->DumpLocation(&strm, eDescriptionLevelFull, nullptr);
GetObjectFile()->GetModule()->ReportError(
- "{0:x16}: {1} has an invalid location: {2}", die.GetOffset(),
- die.GetTagAsCString(), strm.GetData());
+ "{0:x16}: {1} ({2}) has an invalid location: {3}", die.GetOffset(),
+ DW_TAG_value_to_name(die.Tag()), die.Tag(), strm.GetData());
}
if (location_DW_OP_addr != LLDB_INVALID_ADDRESS)
is_static_lifetime = true;
@@ -3839,10 +3841,11 @@ void SymbolFileDWARF::ParseAndAppendGlobalVariable(
variable_list_sp = sc.comp_unit->GetVariableList(false);
} else {
GetObjectFile()->GetModule()->ReportError(
- "parent {0:x8} {1} with no valid compile unit in "
- "symbol context for {2:x8} {3}.\n",
- sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), die.GetID(),
- die.GetTagAsCString());
+ "parent {0:x8} {1} ({2}) with no valid compile unit in "
+ "symbol context for {3:x8} {4} ({5}).\n",
+ sc_parent_die.GetID(), DW_TAG_value_to_name(sc_parent_die.Tag()),
+ sc_parent_die.Tag(), die.GetID(), DW_TAG_value_to_name(die.Tag()),
+ die.Tag());
return;
}
break;
@@ -3850,8 +3853,8 @@ void SymbolFileDWARF::ParseAndAppendGlobalVariable(
default:
GetObjectFile()->GetModule()->ReportError(
"didn't find appropriate parent DIE for variable list for {0:x8} "
- "{1}.\n",
- die.GetID(), die.GetTagAsCString());
+ "{1} ({2}).\n",
+ die.GetID(), DW_TAG_value_to_name(die.Tag()), die.Tag());
return;
}
More information about the lldb-commits
mailing list