[llvm][dsymutil] Use the DW_AT_name of the uniqued DIE for insertion into .debug_names #168513
Conversation
This file contains hidden or 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. |
🐧 Linux x64 Test Results
|
Michael137
force-pushed
the
dsymutil/canonical-debug-names-insertion
branch
from
1b4e191 to
0a60a60
Compare
Michael137
force-pushed
the
dsymutil/canonical-debug-names-insertion
branch
from
0a60a60 to
917ae94
Compare
Member
Author
|
FYI, @dwblaikie, this is the issue that originally motivated us to look into #159592 dsymutil uses the linkage name for uniquing, so we end up with a DW_AT_name mismatch due to anonymous lambdas having the file/column location in their names. If we continue only uniquing by linkage name, then we'll have to make sure dsymutil updates debug_names appropriately (which I try to do in this patch). Though would still be good to remove the file/column info from names anyway. But from what I can tell, the real issue lies in dsymutil. Keeping in draft for now until I manage to write a smaller test-case. |
Member
Author
|
Actually easy to repro if we force ODR using |
Collaborator
|
Yeah, mixed feelings - as I've said elsewhere, there are lots of ways one could end up with equivalent-but-not-identical names in DWARF, and so supporting those correctly in dsymutil seems nice (especially if you expect to/likely will support inputs from different compilers, or at least different compiler versions - we wouldn't want an improvement to clang pretty name printing to break dsymutil). But, similarly, clang should generate names that agree with itself and are exactly as unique as the linkage and isn't in this case. |
Member
Author
Yea agreed. I still want to strip the file/column info from DW_AT_names. But we probably need the dsymutil side fix too, for the reasons you enumerated. Long-term I'd love for |
Michael137
force-pushed
the
dsymutil/canonical-debug-names-insertion
branch
2 times, most recently
from
610f013 to
24941c5
Compare
Member
|
@llvm/pr-subscribers-debuginfo Author: Michael Buch (Michael137) ChangesWe've been seeing dsymutil verification failures like: Not how the name of the DIE has a different lambda path than the one that was used to insert the DIE into debug_names. The root cause of the issue is that we have a DW_AT_subprogram definition whose DW_AT_specification DIE got deduplicated. But the DW_AT_name of the original specification is different than the one it got uniqued to. That’s technically fine because dsymutil uniques by linkage name, which uniquely identifies any function with non-internal linkage. But we insert the definition DIE into the debug-names table using the DW_AT_name of the original specification (we call We have to account for the possibility of multiple levels of TODO:
Full diff: https://github.com/llvm/llvm-project/pull/168513.diff 12 Files Affected:
diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h
index 5b9535380aebf..d6f5d926f022c 100644
--- a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h
@@ -708,7 +708,11 @@ class LLVM_ABI DWARFLinker : public DWARFLinkerBase {
/// already there.
/// \returns is a name was found.
bool getDIENames(const DWARFDie &Die, AttributesInfo &Info,
- OffsetsStringPool &StringPool, bool StripTemplate = false);
+ OffsetsStringPool &StringPool, const DWARFFile &File,
+ CompileUnit &Unit, bool StripTemplate = false);
+
+ llvm::StringRef getCanonicalDIEName(DWARFDie Die, const DWARFFile &File,
+ CompileUnit *Unit);
uint32_t hashFullyQualifiedName(DWARFDie DIE, CompileUnit &U,
const DWARFFile &File,
diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h
index 9fb1b3f80e2ff..5ced6d05cc231 100644
--- a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h
+++ b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h
@@ -84,11 +84,13 @@ class DeclContext {
DeclContext() : DefinedInClangModule(0), Parent(*this) {}
DeclContext(unsigned Hash, uint32_t Line, uint32_t ByteSize, uint16_t Tag,
- StringRef Name, StringRef File, const DeclContext &Parent,
- DWARFDie LastSeenDIE = DWARFDie(), unsigned CUId = 0)
+ StringRef Name, StringRef NameForUniquing, StringRef File,
+ const DeclContext &Parent, DWARFDie LastSeenDIE = DWARFDie(),
+ unsigned CUId = 0)
: QualifiedNameHash(Hash), Line(Line), ByteSize(ByteSize), Tag(Tag),
- DefinedInClangModule(0), Name(Name), File(File), Parent(Parent),
- LastSeenDIE(LastSeenDIE), LastSeenCompileUnitID(CUId) {}
+ DefinedInClangModule(0), Name(Name), NameForUniquing(NameForUniquing),
+ File(File), Parent(Parent), LastSeenDIE(LastSeenDIE),
+ LastSeenCompileUnitID(CUId) {}
uint32_t getQualifiedNameHash() const { return QualifiedNameHash; }
@@ -100,6 +102,7 @@ class DeclContext {
uint32_t getCanonicalDIEOffset() const { return CanonicalDIEOffset; }
void setCanonicalDIEOffset(uint32_t Offset) { CanonicalDIEOffset = Offset; }
+ llvm::StringRef getCanonicalName() const { return Name; }
bool isDefinedInClangModule() const { return DefinedInClangModule; }
void setDefinedInClangModule(bool Val) { DefinedInClangModule = Val; }
@@ -115,6 +118,7 @@ class DeclContext {
uint16_t Tag = dwarf::DW_TAG_compile_unit;
unsigned DefinedInClangModule : 1;
StringRef Name;
+ StringRef NameForUniquing;
StringRef File;
const DeclContext &Parent;
DWARFDie LastSeenDIE;
@@ -180,7 +184,7 @@ struct DeclMapInfo : private DenseMapInfo<DeclContext *> {
return RHS == LHS;
return LHS->QualifiedNameHash == RHS->QualifiedNameHash &&
LHS->Line == RHS->Line && LHS->ByteSize == RHS->ByteSize &&
- LHS->Name.data() == RHS->Name.data() &&
+ LHS->NameForUniquing.data() == RHS->NameForUniquing.data() &&
LHS->File.data() == RHS->File.data() &&
LHS->Parent.QualifiedNameHash == RHS->Parent.QualifiedNameHash;
}
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
index 8637b55c78f9c..daf3788639451 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -151,22 +151,84 @@ static bool isTypeTag(uint16_t Tag) {
return false;
}
-bool DWARFLinker::DIECloner::getDIENames(const DWARFDie &Die,
- AttributesInfo &Info,
- OffsetsStringPool &StringPool,
- bool StripTemplate) {
+/// Recurse through the input DIE's canonical references until we find a
+/// DW_AT_name.
+llvm::StringRef
+DWARFLinker::DIECloner::getCanonicalDIEName(DWARFDie Die, const DWARFFile &File,
+ CompileUnit *Unit) {
+ if (!Die)
+ return {};
+
+ std::optional<DWARFFormValue> Ref;
+
+ auto GetDieName = [](const DWARFDie &D) -> llvm::StringRef {
+ auto NameForm = D.find(llvm::dwarf::DW_AT_name);
+ if (!NameForm)
+ return {};
+
+ auto NameOrErr = NameForm->getAsCString();
+ if (!NameOrErr) {
+ llvm::consumeError(NameOrErr.takeError());
+ return {};
+ }
+
+ return *NameOrErr;
+ };
+
+ llvm::StringRef Name = GetDieName(Die);
+ if (!Name.empty())
+ return Name;
+
+ while (true) {
+ if (!(Ref = Die.find(llvm::dwarf::DW_AT_specification)) &&
+ !(Ref = Die.find(llvm::dwarf::DW_AT_abstract_origin)))
+ break;
+
+ Die = Linker.resolveDIEReference(File, CompileUnits, *Ref, Die, Unit);
+ if (!Die)
+ break;
+
+ assert(Unit);
+
+ unsigned SpecIdx = Unit->getOrigUnit().getDIEIndex(Die);
+ CompileUnit::DIEInfo &SpecInfo = Unit->getInfo(SpecIdx);
+ if (SpecInfo.Ctxt && SpecInfo.Ctxt->hasCanonicalDIE()) {
+ if (!SpecInfo.Ctxt->getCanonicalName().empty()) {
+ Name = SpecInfo.Ctxt->getCanonicalName();
+ break;
+ }
+ }
+
+ Name = GetDieName(Die);
+ if (!Name.empty())
+ break;
+ }
+
+ return Name;
+}
+
+bool DWARFLinker::DIECloner::getDIENames(
+ const DWARFDie &Die, AttributesInfo &Info, OffsetsStringPool &StringPool,
+ const DWARFFile &File, CompileUnit &Unit, bool StripTemplate) {
// This function will be called on DIEs having low_pcs and
// ranges. As getting the name might be more expansive, filter out
// blocks directly.
if (Die.getTag() == dwarf::DW_TAG_lexical_block)
return false;
+ // The mangled name of an specification DIE will by virtue of the
+ // uniquing algorithm be the same as the one it got uniqued into.
+ // So just use the input DIE's linkage name.
if (!Info.MangledName)
if (const char *MangledName = Die.getLinkageName())
Info.MangledName = StringPool.getEntry(MangledName);
+ // For subprograms with linkage names, we unique on the linkage name,
+ // so DW_AT_name's may differ between the input and canonical DIEs.
+ // Use the name of the canonical DIE.
if (!Info.Name)
- if (const char *Name = Die.getShortName())
+ if (llvm::StringRef Name = getCanonicalDIEName(Die, File, &Unit);
+ !Name.empty())
Info.Name = StringPool.getEntry(Name);
if (!Info.MangledName)
@@ -1939,7 +2001,7 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
// accelerator tables too. For now stick with dsymutil's behavior.
if ((Info.InDebugMap || AttrInfo.HasLowPc || AttrInfo.HasRanges) &&
Tag != dwarf::DW_TAG_compile_unit &&
- getDIENames(InputDIE, AttrInfo, DebugStrPool,
+ getDIENames(InputDIE, AttrInfo, DebugStrPool, File, Unit,
Tag != dwarf::DW_TAG_inlined_subroutine)) {
if (AttrInfo.MangledName && AttrInfo.MangledName != AttrInfo.Name)
Unit.addNameAccelerator(Die, AttrInfo.MangledName,
@@ -1962,7 +2024,7 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
} else if (Tag == dwarf::DW_TAG_imported_declaration && AttrInfo.Name) {
Unit.addNamespaceAccelerator(Die, AttrInfo.Name);
} else if (isTypeTag(Tag) && !AttrInfo.IsDeclaration) {
- bool Success = getDIENames(InputDIE, AttrInfo, DebugStrPool);
+ bool Success = getDIENames(InputDIE, AttrInfo, DebugStrPool, File, Unit);
uint64_t RuntimeLang =
dwarf::toUnsigned(InputDIE.find(dwarf::DW_AT_APPLE_runtime_class))
.value_or(0);
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp
index c9c8dddce9c44..66a1ba9c6711f 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp
@@ -84,24 +84,26 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
break;
}
- StringRef NameRef;
+ StringRef Name = DIE.getShortName();
+ StringRef NameForUniquing;
StringRef FileRef;
if (const char *LinkageName = DIE.getLinkageName())
- NameRef = StringPool.internString(LinkageName);
- else if (const char *ShortName = DIE.getShortName())
- NameRef = StringPool.internString(ShortName);
+ NameForUniquing = StringPool.internString(LinkageName);
+ else if (!Name.empty())
+ NameForUniquing = StringPool.internString(Name);
- bool IsAnonymousNamespace = NameRef.empty() && Tag == dwarf::DW_TAG_namespace;
+ bool IsAnonymousNamespace =
+ NameForUniquing.empty() && Tag == dwarf::DW_TAG_namespace;
if (IsAnonymousNamespace) {
// FIXME: For dsymutil-classic compatibility. I think uniquing within
// anonymous namespaces is wrong. There is no ODR guarantee there.
- NameRef = "(anonymous namespace)";
+ NameForUniquing = "(anonymous namespace)";
}
if (Tag != dwarf::DW_TAG_class_type && Tag != dwarf::DW_TAG_structure_type &&
Tag != dwarf::DW_TAG_union_type &&
- Tag != dwarf::DW_TAG_enumeration_type && NameRef.empty())
+ Tag != dwarf::DW_TAG_enumeration_type && NameForUniquing.empty())
return PointerIntPair<DeclContext *, 1>(nullptr);
unsigned Line = 0;
@@ -140,10 +142,10 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
}
}
- if (!Line && NameRef.empty())
+ if (!Line && NameForUniquing.empty())
return PointerIntPair<DeclContext *, 1>(nullptr);
- // We hash NameRef, which is the mangled name, in order to get most
+ // We hash NameForUniquing, which is the mangled name, in order to get most
// overloaded functions resolve correctly.
//
// Strictly speaking, hashing the Tag is only necessary for a
@@ -153,7 +155,8 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
// FIXME: dsymutil-classic won't unique the same type presented
// once as a struct and once as a class. Using the Tag in the fully
// qualified name hash to get the same effect.
- unsigned Hash = hash_combine(Context.getQualifiedNameHash(), Tag, NameRef);
+ unsigned Hash =
+ hash_combine(Context.getQualifiedNameHash(), Tag, NameForUniquing);
// FIXME: dsymutil-classic compatibility: when we don't have a name,
// use the filename.
@@ -161,15 +164,16 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
Hash = hash_combine(Hash, FileRef);
// Now look if this context already exists.
- DeclContext Key(Hash, Line, ByteSize, Tag, NameRef, FileRef, Context);
+ DeclContext Key(Hash, Line, ByteSize, Tag, Name, NameForUniquing, FileRef,
+ Context);
auto ContextIter = Contexts.find(&Key);
if (ContextIter == Contexts.end()) {
// The context wasn't found.
bool Inserted;
- DeclContext *NewContext =
- new (Allocator) DeclContext(Hash, Line, ByteSize, Tag, NameRef, FileRef,
- Context, DIE, U.getUniqueID());
+ DeclContext *NewContext = new (Allocator)
+ DeclContext(Hash, Line, ByteSize, Tag, Name, NameForUniquing, FileRef,
+ Context, DIE, U.getUniqueID());
std::tie(ContextIter, Inserted) = Contexts.insert(NewContext);
assert(Inserted && "Failed to insert DeclContext");
(void)Inserted;
diff --git a/llvm/test/tools/dsymutil/ARM/dummy-debug-map-amr64.map b/llvm/test/tools/dsymutil/ARM/dummy-debug-map-amr64.map
index 50d860290422c..bd2b2014ee22c 100644
--- a/llvm/test/tools/dsymutil/ARM/dummy-debug-map-amr64.map
+++ b/llvm/test/tools/dsymutil/ARM/dummy-debug-map-amr64.map
@@ -11,9 +11,13 @@ objects:
- filename: 1.o
symbols:
- { sym: _bar, objAddr: 0x0, binAddr: 0x10000, size: 0x10 }
+ - { sym: __Z13lib1_internalv, objAddr: 0x0, binAddr: 0x10020, size: 0x20 }
+ - { sym: __ZN3Foo4funcIZ13lib1_internalvE3$_0EEvv, objAddr: 0x0, binAddr: 0x10040, size: 0x20 }
- filename: 2.o
symbols:
- { sym: __Z3foov, objAddr: 0x0, binAddr: 0x20000, size: 0x10 }
+ - { sym: __Z13lib1_internalv, objAddr: 0x0, binAddr: 0x20020, size: 0x20 }
+ - { sym: __ZN3Foo4funcIZ13lib1_internalvE3$_0EEvv, objAddr: 0x0, binAddr: 0x20040, size: 0x20 }
- filename: 3.o
symbols:
- { sym: __Z3foov, objAddr: 0x0, binAddr: 0x30000, size: 0x10 }
diff --git a/llvm/test/tools/dsymutil/ARM/odr-uniquing-DW_AT_name-conflict.test b/llvm/test/tools/dsymutil/ARM/odr-uniquing-DW_AT_name-conflict.test
new file mode 100644
index 0000000000000..c5956afe0da75
--- /dev/null
+++ b/llvm/test/tools/dsymutil/ARM/odr-uniquing-DW_AT_name-conflict.test
@@ -0,0 +1,28 @@
+# Tests the case where a DW_TAG_subprogram for a method declaration
+# got uniqued into a DW_TAG_subprogram with the same linkage name (but
+# different DW_AT_name). Make sure the DW_TAG_subprogram DIE for the
+# definition, which previously pointed to the now de-deduplicated declaration,
+# gets inserted into the .debug_names table using the DW_AT_name of the canonical
+# declaration DW_TAG_subprogram.
+#
+# Object files compiled as follows:
+# clang -g -c -o 1.o Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp
+# clang -g -c -o 2.o Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp
+
+# RUN: dsymutil -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-amr64.map -o - \
+# RUN: | llvm-dwarfdump --verify - | FileCheck %s
+
+# RUN: dsymutil --linker parallel -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-amr64.map -o - \
+# RUN: | not llvm-dwarfdump --verify - | FileCheck %s --check-prefix=PARALLEL-ODR
+
+# RUN: dsymutil -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-amr64.map -no-odr -o - \
+# RUN: | llvm-dwarfdump --verify - | FileCheck %s
+
+# RUN: dsymutil --linker parallel -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-amr64.map -no-odr -o - \
+# RUN: | llvm-dwarfdump --verify - | FileCheck %s
+
+# CHECK: No errors.
+
+# FIXME: parallel DWARFLinker uses wrong DW_AT_name when inserting uniqued subprogram into .debug_names
+# PARALLEL-ODR: Verifying .debug_names...
+# PARALLEL-ODR-NEXT: error: Name Index {{.*}} mismatched Name of DIE
diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/1.o b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/1.o
new file mode 100644
index 0000000000000..5932a3c89aaeb
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/1.o differ
diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/2.o b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/2.o
new file mode 100644
index 0000000000000..607b47059e982
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/2.o differ
diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp
new file mode 100644
index 0000000000000..4cf90f071ee8c
--- /dev/null
+++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp
@@ -0,0 +1,5 @@
+#include "lib1.h"
+
+[[gnu::weak]] void lib1_internal() {
+ Foo{}.func<decltype([]{})>();
+}
diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h
new file mode 100644
index 0000000000000..3b3cefbaeac17
--- /dev/null
+++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h
@@ -0,0 +1,3 @@
+struct Foo {
+ template<typename T> void func() {}
+};
diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp
new file mode 100644
index 0000000000000..4cf90f071ee8c
--- /dev/null
+++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp
@@ -0,0 +1,5 @@
+#include "lib1.h"
+
+[[gnu::weak]] void lib1_internal() {
+ Foo{}.func<decltype([]{})>();
+}
diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/main.cpp b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/main.cpp
new file mode 100644
index 0000000000000..77f2cc4c8affe
--- /dev/null
+++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/main.cpp
@@ -0,0 +1,6 @@
+[[gnu::weak]] void lib1_internal();
+
+int main() {
+ lib1_internal();
+ __builtin_debugtrap();
+}
|
Michael137
force-pushed
the
dsymutil/canonical-debug-names-insertion
branch
2 times, most recently
from
f3a8655 to
294b03e
Compare
Better describes what the `Name` field is. I'm also planning on adding a new `Name` field to the `DeclContext`, so I wanted to differentiate the two.
…into .debug_names The root cause of the issue is that we have a DW_AT_subprogram definition whose DW_AT_specification DIE got deduplicated. But the DW_AT_name of the original specification is different than the one it got uniqued to. That’s technically fine because dsymutil uniques by linkage name, which uniquely identifies any function with non-internal linkage. But we insert the definition DIE into the debug-names table using the DW_AT_name of the original specification (we call getDIENames(InputDIE…)). But what we really want to do is use the name of the adjusted DW_AT_specifcation (i.e., the DW_AT_specification of the output DIE). That’s not as simple as it sounds because we can’t just get ahold of the DIE in the output CU. We have to grab the ODR DeclContext of the input DIE’s specification. That is the only link back to the canonical specification DIE. For that to be of any use, we have to stash the DW_AT_name into DeclContext so we can use it in getDIENames. We have to account for the possibility of multiple levels of DW_AT_specification/DW_AT_abstract_origin. So my proposed solution is to recursively scan the referenced DIE’s, grab the canonical DIE for those and get the name from the DeclContext (if none exists then use the DW_AT_name of the DIE itself).
Michael137
force-pushed
the
dsymutil/canonical-debug-names-insertion
branch
from
294b03e to
f6e39be
Compare
JDevlieghere
approved these changes
Nov 20, 2025
Member
JDevlieghere
left a comment
JDevlieghere
left a comment
There was a problem hiding this comment.
Michael and I have been discussing this issue offline. I agree with everyone else that it would be great if we could avoid this in the input, but barring that, this is the only way to make sure we generate DWARF that doesn't trip up the verifier. LGTM.
We changed the debug-map to add more symbols. So adjust the tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Depends on:
Note, the last commit is the one with the actual fix. The others are drive-by/test changes
We've been seeing dsymutil verification failures like:
Not how the name of the DIE has a different lambda path than the one that was used to insert the DIE into debug_names.
The root cause of the issue is that we have a DW_AT_subprogram definition whose DW_AT_specification DIE got deduplicated. But the DW_AT_name of the original specification is different than the one it got uniqued to. That’s technically fine because dsymutil uniques by linkage name, which uniquely identifies any function with non-internal linkage.
But we insert the definition DIE into the debug-names table using the DW_AT_name of the original specification (we call
getDIENames(InputDIE…)). But what we really want to do is use the name of the adjustedDW_AT_specifcation(i.e., theDW_AT_specificationof the output DIE). That’s not as simple as it sounds because we can’t just get ahold of the DIE in the output CU. We have to grab the ODRDeclContextof the input DIE’s specification. That is the only link back to the canonical specification DIE. For that to be of any use, we have to stash theDW_AT_nameintoDeclContextso we can use it ingetDIENames.We have to account for the possibility of multiple levels of
DW_AT_specification/DW_AT_abstract_origin. So my proposed solution is to recursively scan the referenced DIE’s, grab the canonical DIE for those and get the name from theDeclContext(if none exists then use theDW_AT_nameof the DIE itself).One remaining question is whether we need to handle the case where a DIE has a
DW_AT_specificationand aDW_AT_abstract_origin? That complicates the way we locateDW_AT_name. We'd have to adjustgetCanonicalDIENameto handle this. But it's not clear what aDW_AT_namewould be for such cases. Worst case at the moment we take the wrong path up the specifications and don't find anyDW_AT_name, and don't end up indexing that DIE. Something to keep an eye out for.rdar://149239553