[llvm] 4546015 - [DWARFv5][DWARFLinker] Remove dsymutil-classic compatibility feature as it leads to an error.
Alexey Lapshin via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 06:47:54 PDT 2023
Author: Alexey Lapshin
Date: 2023-06-29T15:40:04+02:00
New Revision: 4546015f867ac19823116374bb1fb21f27bd2447
URL: https://github.com/llvm/llvm-project/commit/4546015f867ac19823116374bb1fb21f27bd2447
DIFF: https://github.com/llvm/llvm-project/commit/4546015f867ac19823116374bb1fb21f27bd2447.diff
LOG: [DWARFv5][DWARFLinker] Remove dsymutil-classic compatibility feature as it leads to an error.
DWARFLinker has a compatibility feature with dsymutil-classic.
It may keep location expression attribute even if does not
reference live address. Current llvm-dwarfdump --verify
reports a error if variable references an address but is not
added into the .debug_names table.
error: Name Index @ 0x0: Entry for DIE @ 0xf35 (DW_TAG_variable) with name seed missing.
DW_TAG_variable
DW_AT_name ("seed")
DW_AT_type (0x00000000000047b7 "uint64_t")
DW_AT_location (DW_OP_addr 0x9ff8) <<<< dead address
DWARFLinker does not add the variable into .debug_names table
because it references dead address. To have a valid variable and
consistent accelerator table it is necessary to remove location expression
referencing dead address. This patch removes dsymutil-classic
compatibilty feature.
Differential Revision: https://reviews.llvm.org/D153988
Added:
Modified:
llvm/include/llvm/DWARFLinker/DWARFLinker.h
llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
llvm/lib/DWARFLinker/DWARFLinker.cpp
llvm/test/tools/dsymutil/X86/dead-stripped.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index 8f1d2d079c16bc..e24fa2ef704b5e 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -592,9 +592,12 @@ class DWARFLinker {
/// This function checks whether variable has DWARF expression containing
/// operation referencing live address(f.e. DW_OP_addr, DW_OP_addrx...).
- /// \returns relocation adjustment value if live address is referenced.
- std::optional<int64_t> getVariableRelocAdjustment(AddressesMap &RelocMgr,
- const DWARFDie &DIE);
+ /// \returns first is true if the expression has an operation referencing an
+ /// address.
+ /// second is the relocation adjustment value if the live address is
+ /// referenced.
+ std::pair<bool, std::optional<int64_t>>
+ getVariableRelocAdjustment(AddressesMap &RelocMgr, const DWARFDie &DIE);
/// Check if a variable describing DIE should be kept.
/// \returns updated TraversalFlags.
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
index 3736033f3a0460..08ebd4bc70bc93 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -95,6 +95,9 @@ class CompileUnit {
/// Is this a reference to a DIE that hasn't been cloned yet?
bool UnclonedReference : 1;
+ /// Is this a variable with a location attribute referencing address?
+ bool HasLocationExpressionAddr : 1;
+
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
LLVM_DUMP_METHOD void dump();
#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index 206ba9d4099047..a954a1adb45be7 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -427,7 +427,7 @@ static bool isTlsAddressCode(uint8_t DW_OP_Code) {
DW_OP_Code == dwarf::DW_OP_GNU_push_tls_address;
}
-std::optional<int64_t>
+std::pair<bool, std::optional<int64_t>>
DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
const DWARFDie &DIE) {
assert((DIE.getTag() == dwarf::DW_TAG_variable ||
@@ -441,7 +441,7 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
std::optional<uint32_t> LocationIdx =
Abbrev->findAttributeIndex(dwarf::DW_AT_location);
if (!LocationIdx)
- return std::nullopt;
+ return std::make_pair(false, std::nullopt);
// Get offset to the DW_AT_location attribute.
uint64_t AttrOffset =
@@ -451,14 +451,14 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
std::optional<DWARFFormValue> LocationValue =
Abbrev->getAttributeValueFromOffset(*LocationIdx, AttrOffset, *U);
if (!LocationValue)
- return std::nullopt;
+ return std::make_pair(false, std::nullopt);
// Check that DW_AT_location attribute is of 'exprloc' class.
// Handling value of location expressions for attributes of 'loclist'
// class is not implemented yet.
std::optional<ArrayRef<uint8_t>> Expr = LocationValue->getAsBlock();
if (!Expr)
- return std::nullopt;
+ return std::make_pair(false, std::nullopt);
// Parse 'exprloc' expression.
DataExtractor Data(toStringRef(*Expr), U->getContext().isLittleEndian(),
@@ -466,6 +466,7 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
DWARFExpression Expression(Data, U->getAddressByteSize(),
U->getFormParams().Format);
+ bool HasLocationAddress = false;
uint64_t CurExprOffset = 0;
for (DWARFExpression::iterator It = Expression.begin();
It != Expression.end(); ++It) {
@@ -482,15 +483,17 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
break;
[[fallthrough]];
case dwarf::DW_OP_addr: {
+ HasLocationAddress = true;
// Check relocation for the address.
if (std::optional<int64_t> RelocAdjustment =
RelocMgr.getExprOpAddressRelocAdjustment(
*U, Op, AttrOffset + CurExprOffset,
AttrOffset + Op.getEndOffset()))
- return *RelocAdjustment;
+ return std::make_pair(HasLocationAddress, *RelocAdjustment);
} break;
case dwarf::DW_OP_constx:
case dwarf::DW_OP_addrx: {
+ HasLocationAddress = true;
if (std::optional<uint64_t> AddressOffset =
DIE.getDwarfUnit()->getIndexedAddressOffset(
Op.getRawOperand(0))) {
@@ -499,7 +502,7 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
RelocMgr.getExprOpAddressRelocAdjustment(
*U, Op, *AddressOffset,
*AddressOffset + DIE.getDwarfUnit()->getAddressByteSize()))
- return *RelocAdjustment;
+ return std::make_pair(HasLocationAddress, *RelocAdjustment);
}
} break;
default: {
@@ -509,7 +512,7 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
CurExprOffset = Op.getEndOffset();
}
- return std::nullopt;
+ return std::make_pair(HasLocationAddress, std::nullopt);
}
/// Check if a variable describing DIE should be kept.
@@ -532,16 +535,20 @@ unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr,
// if the variable has a valid relocation, so that the DIEInfo is filled.
// However, we don't want a static variable in a function to force us to keep
// the enclosing function, unless requested explicitly.
- std::optional<int64_t> RelocAdjustment =
+ std::pair<bool, std::optional<int64_t>> LocExprAddrAndRelocAdjustment =
getVariableRelocAdjustment(RelocMgr, DIE);
- if (RelocAdjustment) {
- MyInfo.AddrAdjust = *RelocAdjustment;
- MyInfo.InDebugMap = true;
- }
+ if (LocExprAddrAndRelocAdjustment.first)
+ MyInfo.HasLocationExpressionAddr = true;
- if (!RelocAdjustment || ((Flags & TF_InFunctionScope) &&
- !LLVM_UNLIKELY(Options.KeepFunctionForStatic)))
+ if (!LocExprAddrAndRelocAdjustment.second)
+ return Flags;
+
+ MyInfo.AddrAdjust = *LocExprAddrAndRelocAdjustment.second;
+ MyInfo.InDebugMap = true;
+
+ if (((Flags & TF_InFunctionScope) &&
+ !LLVM_UNLIKELY(Options.KeepFunctionForStatic)))
return Flags;
if (Options.Verbose) {
@@ -1652,9 +1659,10 @@ void DWARFLinker::DIECloner::addObjCAccelerator(CompileUnit &Unit,
}
}
-static bool shouldSkipAttribute(
- bool Update, DWARFAbbreviationDeclaration::AttributeSpec AttrSpec,
- uint16_t Tag, bool InDebugMap, bool SkipPC, bool InFunctionScope) {
+static bool
+shouldSkipAttribute(bool Update,
+ DWARFAbbreviationDeclaration::AttributeSpec AttrSpec,
+ bool SkipPC) {
switch (AttrSpec.Attr) {
default:
return false;
@@ -1682,15 +1690,7 @@ static bool shouldSkipAttribute(
return !Update;
case dwarf::DW_AT_location:
case dwarf::DW_AT_frame_base:
- // FIXME: for some reason dsymutil-classic keeps the location attributes
- // when they are of block type (i.e. not location lists). This is totally
- // wrong for globals where we will keep a wrong address. It is mostly
- // harmless for locals, but there is no point in keeping these anyway when
- // the function wasn't linked.
- return !Update &&
- (SkipPC || (!InFunctionScope && Tag == dwarf::DW_TAG_variable &&
- !InDebugMap)) &&
- !DWARFFormValue(AttrSpec.Form).isFormClass(DWARFFormValue::FC_Block);
+ return !Update && SkipPC;
}
}
@@ -1770,11 +1770,15 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
// is not, e.g., inlined functions.
if ((Flags & TF_InFunctionScope) && Info.InDebugMap)
Flags &= ~TF_SkipPC;
+ // Location expressions referencing an address which is not in debug map
+ // should be deleted.
+ else if (!Info.InDebugMap && Info.HasLocationExpressionAddr &&
+ LLVM_LIKELY(!Update))
+ Flags |= TF_SkipPC;
}
for (const auto &AttrSpec : Abbrev->attributes()) {
- if (shouldSkipAttribute(Update, AttrSpec, Die->getTag(), Info.InDebugMap,
- Flags & TF_SkipPC, Flags & TF_InFunctionScope)) {
+ if (shouldSkipAttribute(Update, AttrSpec, Flags & TF_SkipPC)) {
DWARFFormValue::skipValue(AttrSpec.Form, Data, &Offset,
U.getFormParams());
continue;
diff --git a/llvm/test/tools/dsymutil/X86/dead-stripped.cpp b/llvm/test/tools/dsymutil/X86/dead-stripped.cpp
index 16bbbd672d6d4d..6ea5ef22a65acb 100644
--- a/llvm/test/tools/dsymutil/X86/dead-stripped.cpp
+++ b/llvm/test/tools/dsymutil/X86/dead-stripped.cpp
@@ -18,13 +18,12 @@
// CHECK: DW_TAG_namespace
namespace N {
int blah = 42;
-// This is actually a dsymutil-classic bug that we reproduced
// CHECK: DW_TAG_variable
-// CHECK: DW_AT_location
+// CHECK-NOT: DW_AT_location
__attribute__((always_inline)) int foo() { return blah; }
// CHECK: DW_TAG_subprogram
-// CHECK: DW_AT_frame_base
+// CHECK-NOT: DW_AT_frame_base
// CHECK: DW_TAG_subprogram
@@ -35,7 +34,7 @@ int bar(unsigned i) {
return foo();
}
// CHECK: DW_TAG_subprogram
-// CHECK: DW_AT_frame_base
+// CHECK-NOT: DW_AT_frame_base
// CHECK: DW_TAG_formal_parameter
// CHECK: DW_TAG_variable
// CHECK: DW_TAG_inlined_subroutine
More information about the llvm-commits
mailing list