JDevlieghere created this revision.
JDevlieghere added reviewers: avl, friss, aprantl.
Herald added a subscriber: hiraditya.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
D125469 <https://reviews.llvm.org/D125469> changed the assumptions regarding
forward references. Before the patch, a reference to an uncloned DIE was always
a forward reference. After the change, that assumption no longer holds. It was
enforced by the assertion in
`DWARFLinker::DIECloner::cloneDieReferenceAttribute` (DWARFLinker.cpp:937):
assert(Ref > InputDIE.getOffset());
This patch is relatively straightforward: it adds a field to the `DIEInfo` to
remember the DIE is a forward or backward reference. We then treat backward
references like a forward references by fixing them up after the fact, when all
DIEs have been cloned.
We tripped this assertion for an internal project built with LTO. Unfortunately
that means I'm not able to share a test case. I've spent a few hours trying to
craft a test case that would hit this case but didn't succeed. Getting a
backward reference is easy, but a backward reference to a DIE that hasn't been
cloned yet is not something I managed to reproduce. I'd love to have a test
case to avoid regressing this in the future, so please let me know if you have
an idea on how to trigger this behavior from an artificial test case.
https://reviews.llvm.org/D138176
Files:
llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
llvm/lib/DWARFLinker/DWARFLinker.cpp
Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===================================================================
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
}
if (!RefInfo.Clone) {
- assert(Ref > InputDIE.getOffset());
// We haven't cloned this DIE yet. Just create an empty one and
// store it. It'll get really cloned when we process it.
+ RefInfo.Reference = true;
RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
}
NewRefDie = RefInfo.Clone;
@@ -948,17 +948,16 @@
// to find the unit offset. (We don't have a DwarfDebug)
// FIXME: we should be able to design DIEEntry reliance on
// DwarfDebug away.
- uint64_t Attr;
- if (Ref < InputDIE.getOffset()) {
+ if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
// We must have already cloned that DIE.
uint32_t NewRefOffset =
RefUnit->getStartOffset() + NewRefDie->getOffset();
- Attr = NewRefOffset;
+ uint64_t Attr = NewRefOffset;
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
} else {
// A forward reference. Note and fixup later.
- Attr = 0xBADDEF;
+ uint64_t Attr = 0xBADDEF;
Unit.noteForwardReference(
NewRefDie, RefUnit, RefInfo.Ctxt,
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===================================================================
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
/// Is ODR marking done?
bool ODRMarkingDone : 1;
+
+ /// Is DIE a forward or backward reference?
+ bool Reference : 1;
};
CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR,
Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===================================================================
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
}
if (!RefInfo.Clone) {
- assert(Ref > InputDIE.getOffset());
// We haven't cloned this DIE yet. Just create an empty one and
// store it. It'll get really cloned when we process it.
+ RefInfo.Reference = true;
RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
}
NewRefDie = RefInfo.Clone;
@@ -948,17 +948,16 @@
// to find the unit offset. (We don't have a DwarfDebug)
// FIXME: we should be able to design DIEEntry reliance on
// DwarfDebug away.
- uint64_t Attr;
- if (Ref < InputDIE.getOffset()) {
+ if (Ref < InputDIE.getOffset() && !RefInfo.Reference) {
// We must have already cloned that DIE.
uint32_t NewRefOffset =
RefUnit->getStartOffset() + NewRefDie->getOffset();
- Attr = NewRefOffset;
+ uint64_t Attr = NewRefOffset;
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
} else {
// A forward reference. Note and fixup later.
- Attr = 0xBADDEF;
+ uint64_t Attr = 0xBADDEF;
Unit.noteForwardReference(
NewRefDie, RefUnit, RefInfo.Ctxt,
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===================================================================
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
/// Is ODR marking done?
bool ODRMarkingDone : 1;
+
+ /// Is DIE a forward or backward reference?
+ bool Reference : 1;
};
CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
