[llvm] 4c273cd - [DWARFLinker] Refactor cloneAddressAttribute().
Alexey Lapshin via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 13 03:26:15 PST 2023
Author: Alexey Lapshin
Date: 2023-02-13T12:25:22+01:00
New Revision: 4c273cd071150912fd6f1e4aee12148cf78a6410
URL: https://github.com/llvm/llvm-project/commit/4c273cd071150912fd6f1e4aee12148cf78a6410
DIFF: https://github.com/llvm/llvm-project/commit/4c273cd071150912fd6f1e4aee12148cf78a6410.diff
LOG: [DWARFLinker] Refactor cloneAddressAttribute().
As a preparation for implementing DWARFv5 address ranges generation,
this patch refactors cloneAddressAttribute() method. It has special
handling for addresses which can be relocated in some unrelated value,
for applying relocations twice, for indexed addresses. Instead of
all these special handlings this patch uses general handling:
Read attribute value from InputDIE and apply PCOffset.
Another thing is that current handling of DW_FORM_addrx misses the
fact that relocations might be applied twice in some cases. This
patch fixes this problem also.
Differential Revision: https://reviews.llvm.org/D143269
Added:
llvm/test/tools/dsymutil/Inputs/call-dwarf5.o
llvm/test/tools/dsymutil/X86/dwarf5-call-site-entry-reloc.test
Modified:
llvm/include/llvm/DWARFLinker/DWARFLinker.h
llvm/lib/DWARFLinker/DWARFLinker.cpp
llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
llvm/tools/dsymutil/DwarfLinkerForBinary.h
llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index d12b6d1f5f2f9..30f0c1c825602 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -68,10 +68,6 @@ class AddressesMap {
virtual bool applyValidRelocs(MutableArrayRef<char> Data, uint64_t BaseOffset,
bool IsLittleEndian) = 0;
- /// Relocate the given address offset if a valid relocation exists.
- virtual llvm::Expected<uint64_t> relocateIndexedAddr(uint64_t StartOffset,
- uint64_t EndOffset) = 0;
-
/// Returns all valid functions address ranges(i.e., those ranges
/// which points to sections with code).
virtual RangesTy &getValidAddressRanges() = 0;
@@ -635,18 +631,6 @@ class DWARFLinker {
uint32_t NameOffset = 0;
uint32_t MangledNameOffset = 0;
- /// Value of AT_low_pc in the input DIE
- uint64_t OrigLowPc = std::numeric_limits<uint64_t>::max();
-
- /// Value of AT_high_pc in the input DIE
- uint64_t OrigHighPc = 0;
-
- /// Value of DW_AT_call_return_pc in the input DIE
- uint64_t OrigCallReturnPc = 0;
-
- /// Value of DW_AT_call_pc in the input DIE
- uint64_t OrigCallPc = 0;
-
/// Offset to apply to PC addresses inside a function.
int64_t PCOffset = 0;
@@ -704,8 +688,9 @@ class DWARFLinker {
/// Clone an attribute referencing another DIE and add
/// it to \p Die.
/// \returns the size of the new attribute.
- unsigned cloneAddressAttribute(DIE &Die, AttributeSpec AttrSpec,
- unsigned AttrSize, const DWARFFormValue &Val,
+ unsigned cloneAddressAttribute(DIE &Die, const DWARFDie &InputDIE,
+ AttributeSpec AttrSpec, unsigned AttrSize,
+ const DWARFFormValue &Val,
const CompileUnit &Unit,
AttributesInfo &Info);
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index 6d6bd3385dfc2..abe77ea04cee9 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -1151,83 +1151,65 @@ unsigned DWARFLinker::DIECloner::cloneBlockAttribute(
}
unsigned DWARFLinker::DIECloner::cloneAddressAttribute(
- DIE &Die, AttributeSpec AttrSpec, unsigned AttrSize,
- const DWARFFormValue &Val, const CompileUnit &Unit, AttributesInfo &Info) {
+ DIE &Die, const DWARFDie &InputDIE, AttributeSpec AttrSpec,
+ unsigned AttrSize, const DWARFFormValue &Val, const CompileUnit &Unit,
+ AttributesInfo &Info) {
+ if (AttrSpec.Attr == dwarf::DW_AT_low_pc)
+ Info.HasLowPc = true;
+
if (LLVM_UNLIKELY(Linker.Options.Update)) {
- if (AttrSpec.Attr == dwarf::DW_AT_low_pc)
- Info.HasLowPc = true;
Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::Form(AttrSpec.Form), DIEInteger(Val.getRawUValue()));
return AttrSize;
}
+ // Cloned Die may have address attributes relocated to a
+ // totally unrelated value. This can happen:
+ // - If high_pc is an address (Dwarf version == 2), then it might have been
+ // relocated to a totally unrelated value (because the end address in the
+ // object file might be start address of another function which got moved
+ // independently by the linker).
+ // - If address relocated in an inline_subprogram that happens at the
+ // beginning of its inlining function.
+ // To avoid above cases and to not apply relocation twice (in applyValidRelocs
+ // and here), read address attribute from InputDIE and apply Info.PCOffset
+ // here.
+
+ std::optional<DWARFFormValue> AddrAttribute = InputDIE.find(AttrSpec.Attr);
+ if (!AddrAttribute)
+ llvm_unreachable("Cann't find attribute.");
+
+ std::optional<uint64_t> Addr = AddrAttribute->getAsAddress();
+ if (!Addr) {
+ Linker.reportWarning("Cann't read address attribute value.", ObjFile);
+ Addr = 0;
+ }
+
+ if (InputDIE.getTag() == dwarf::DW_TAG_compile_unit &&
+ AttrSpec.Attr == dwarf::DW_AT_low_pc) {
+ if (std::optional<uint64_t> LowPC = Unit.getLowPc())
+ Addr = *LowPC;
+ else
+ return 0;
+ } else if (InputDIE.getTag() == dwarf::DW_TAG_compile_unit &&
+ AttrSpec.Attr == dwarf::DW_AT_high_pc) {
+ if (uint64_t HighPc = Unit.getHighPc())
+ Addr = HighPc;
+ else
+ return 0;
+ } else {
+ *Addr += Info.PCOffset;
+ }
+
dwarf::Form Form = AttrSpec.Form;
- uint64_t Addr = 0;
- if (Form == dwarf::DW_FORM_addrx) {
- if (std::optional<uint64_t> AddrOffsetSectionBase =
- Unit.getOrigUnit().getAddrOffsetSectionBase()) {
- uint64_t StartOffset =
- *AddrOffsetSectionBase +
- Val.getRawUValue() * Unit.getOrigUnit().getAddressByteSize();
- uint64_t EndOffset =
- StartOffset + Unit.getOrigUnit().getAddressByteSize();
- if (llvm::Expected<uint64_t> RelocAddr =
- ObjFile.Addresses->relocateIndexedAddr(StartOffset, EndOffset))
- Addr = *RelocAddr;
- else
- Linker.reportWarning(toString(RelocAddr.takeError()), ObjFile);
- } else
- Linker.reportWarning("no base offset for address table", ObjFile);
- // Generation of DWARFv5 .debug_addr table is not supported yet.
- // Convert attribute into the dwarf::DW_FORM_addr.
+ // FIXME: Generation of DWARFv5 .debug_addr table is not supported yet.
+ // Convert attribute into the dwarf::DW_FORM_addr.
+ if (Form == dwarf::DW_FORM_addrx)
Form = dwarf::DW_FORM_addr;
- } else
- Addr = *Val.getAsAddress();
-
- if (AttrSpec.Attr == dwarf::DW_AT_low_pc) {
- if (Die.getTag() == dwarf::DW_TAG_inlined_subroutine ||
- Die.getTag() == dwarf::DW_TAG_lexical_block ||
- Die.getTag() == dwarf::DW_TAG_label) {
- // The low_pc of a block or inline subroutine might get
- // relocated because it happens to match the low_pc of the
- // enclosing subprogram. To prevent issues with that, always use
- // the low_pc from the input DIE if relocations have been applied.
- Addr = (Info.OrigLowPc != std::numeric_limits<uint64_t>::max()
- ? Info.OrigLowPc
- : Addr) +
- Info.PCOffset;
- } else if (Die.getTag() == dwarf::DW_TAG_compile_unit) {
- if (std::optional<uint64_t> LowPC = Unit.getLowPc())
- Addr = *LowPC;
- else
- return 0;
- }
- Info.HasLowPc = true;
- } else if (AttrSpec.Attr == dwarf::DW_AT_high_pc) {
- if (Die.getTag() == dwarf::DW_TAG_compile_unit) {
- if (uint64_t HighPc = Unit.getHighPc())
- Addr = HighPc;
- else
- return 0;
- } else
- // If we have a high_pc recorded for the input DIE, use
- // it. Otherwise (when no relocations where applied) just use the
- // one we just decoded.
- Addr = (Info.OrigHighPc ? Info.OrigHighPc : Addr) + Info.PCOffset;
- } else if (AttrSpec.Attr == dwarf::DW_AT_call_return_pc) {
- // Relocate a return PC address within a call site entry.
- if (Die.getTag() == dwarf::DW_TAG_call_site)
- Addr = (Info.OrigCallReturnPc ? Info.OrigCallReturnPc : Addr) +
- Info.PCOffset;
- } else if (AttrSpec.Attr == dwarf::DW_AT_call_pc) {
- // Relocate the address of a branch instruction within a call site entry.
- if (Die.getTag() == dwarf::DW_TAG_call_site)
- Addr = (Info.OrigCallPc ? Info.OrigCallPc : Addr) + Info.PCOffset;
- }
Die.addValue(DIEAlloc, static_cast<dwarf::Attribute>(AttrSpec.Attr),
- static_cast<dwarf::Form>(Form), DIEInteger(Addr));
+ static_cast<dwarf::Form>(Form), DIEInteger(*Addr));
return Unit.getOrigUnit().getAddressByteSize();
}
@@ -1350,7 +1332,8 @@ unsigned DWARFLinker::DIECloner::cloneAttribute(
IsLittleEndian);
case dwarf::DW_FORM_addr:
case dwarf::DW_FORM_addrx:
- return cloneAddressAttribute(Die, AttrSpec, AttrSize, Val, Unit, Info);
+ return cloneAddressAttribute(Die, InputDIE, AttrSpec, AttrSize, Val, Unit,
+ Info);
case dwarf::DW_FORM_data1:
case dwarf::DW_FORM_data2:
case dwarf::DW_FORM_data4:
@@ -1500,27 +1483,7 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
DWARFDataExtractor(DIECopy, Data.isLittleEndian(), Data.getAddressSize());
// Modify the copy with relocated addresses.
- if (ObjFile.Addresses->applyValidRelocs(DIECopy, Offset,
- Data.isLittleEndian())) {
- // If we applied relocations, we store the value of high_pc that was
- // potentially stored in the input DIE. If high_pc is an address
- // (Dwarf version == 2), then it might have been relocated to a
- // totally unrelated value (because the end address in the object
- // file might be start address of another function which got moved
- // independently by the linker). The computation of the actual
- // high_pc value is done in cloneAddressAttribute().
- AttrInfo.OrigHighPc =
- dwarf::toAddress(InputDIE.find(dwarf::DW_AT_high_pc), 0);
- // Also store the low_pc. It might get relocated in an
- // inline_subprogram that happens at the beginning of its
- // inlining function.
- AttrInfo.OrigLowPc = dwarf::toAddress(InputDIE.find(dwarf::DW_AT_low_pc),
- std::numeric_limits<uint64_t>::max());
- AttrInfo.OrigCallReturnPc =
- dwarf::toAddress(InputDIE.find(dwarf::DW_AT_call_return_pc), 0);
- AttrInfo.OrigCallPc =
- dwarf::toAddress(InputDIE.find(dwarf::DW_AT_call_pc), 0);
- }
+ ObjFile.Addresses->applyValidRelocs(DIECopy, Offset, Data.isLittleEndian());
// Reset the Offset to 0 as we will be working on the local copy of
// the data.
diff --git a/llvm/test/tools/dsymutil/Inputs/call-dwarf5.o b/llvm/test/tools/dsymutil/Inputs/call-dwarf5.o
new file mode 100644
index 0000000000000..70417f5a2fc6f
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/call-dwarf5.o
diff er
diff --git a/llvm/test/tools/dsymutil/X86/dwarf5-call-site-entry-reloc.test b/llvm/test/tools/dsymutil/X86/dwarf5-call-site-entry-reloc.test
new file mode 100644
index 0000000000000..b960d4496ce7d
--- /dev/null
+++ b/llvm/test/tools/dsymutil/X86/dwarf5-call-site-entry-reloc.test
@@ -0,0 +1,37 @@
+## Test binaries created with the following commands:
+
+## $ cat call-dwarf5.c
+## __attribute__((noinline, noreturn)) void foo() {
+## asm volatile("" ::: "memory");
+## __builtin_unreachable();
+## }
+## __attribute__((noinline)) void bar() {
+## asm volatile("nop" :::);
+## foo();
+## }
+
+## int main() { bar(); }
+
+## $ clang -gdwarf-5 call-dwarf5.c -fomit-frame-pointer -c -Os -o call-dwarf5.o
+## $ clang -gdwarf-5 call-dwarf5.o -o call-dwarf5
+
+## The test requires the return PC to match a relocation (in this case the
+## DW_AT_high_pc of main). Without this change the value would get relocated
+## twice.
+
+#RUN: dsymutil -oso-prepend-path %p/../Inputs -y %s -o %t.dSYM
+#RUN: llvm-dwarfdump %t.dSYM | FileCheck %s -implicit-check-not=DW_AT_call_return_pc
+
+#CHECK: DW_AT_call_return_pc (0x0000000100000f72)
+#CHECK: DW_AT_call_return_pc (0x0000000100000f78)
+
+---
+triple: 'x86_64-apple-darwin'
+objects:
+ - filename: 'call-dwarf5.o'
+ timestamp: 1675373912
+ symbols:
+ - { sym: _foo, objAddr: 0x0, binAddr: 0x100000F69, size: 0x2 }
+ - { sym: _bar, objAddr: 0x2, binAddr: 0x100000F6B, size: 0x7 }
+ - { sym: _main, objAddr: 0x9, binAddr: 0x100000F72, size: 0x6 }
+...
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index 6f23eb02e699b..413cbec5c51db 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -1063,19 +1063,6 @@ bool DwarfLinkerForBinary::AddressManager::applyValidRelocs(
return Relocs.size() > 0;
}
-llvm::Expected<uint64_t>
-DwarfLinkerForBinary::AddressManager::relocateIndexedAddr(uint64_t StartOffset,
- uint64_t EndOffset) {
- std::vector<ValidReloc> Relocs =
- getRelocations(ValidDebugAddrRelocs, StartOffset, EndOffset);
- if (Relocs.size() == 0)
- return createStringError(
- std::make_error_code(std::errc::invalid_argument),
- "no relocation for offset %llu in debug_addr section", StartOffset);
-
- return relocate(Relocs[0]);
-}
-
bool linkDwarf(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
const DebugMap &DM, LinkOptions Options) {
DwarfLinkerForBinary Linker(OutFile, BinHolder, std::move(Options));
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.h b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
index 623441c749a5d..1f7422f43401e 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.h
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
@@ -177,9 +177,6 @@ class DwarfLinkerForBinary {
bool applyValidRelocs(MutableArrayRef<char> Data, uint64_t BaseOffset,
bool IsLittleEndian) override;
- llvm::Expected<uint64_t> relocateIndexedAddr(uint64_t StartOffset,
- uint64_t EndOffset) override;
-
RangesTy &getValidAddressRanges() override { return AddressRanges; }
void clear() override {
diff --git a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
index ef222f8cc1a4f..5d2352e23b634 100644
--- a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
+++ b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
@@ -41,7 +41,7 @@ class ObjFileAddressMap : public AddressesMap {
public:
ObjFileAddressMap(DWARFContext &Context, const Options &Options,
object::ObjectFile &ObjFile)
- : Opts(Options), Context(Context) {
+ : Opts(Options) {
// Remember addresses of existing text sections.
for (const object::SectionRef &Sect : ObjFile.sections()) {
if (!Sect.isText())
@@ -138,30 +138,6 @@ class ObjFileAddressMap : public AddressesMap {
void clear() override { DWARFAddressRanges.clear(); }
- llvm::Expected<uint64_t> relocateIndexedAddr(uint64_t StartOffset,
- uint64_t EndOffset) override {
- // No relocations in linked binary. Return just address value.
-
- const char *AddrPtr =
- Context.getDWARFObj().getAddrSection().Data.data() + StartOffset;
- support::endianness Endianess =
- Context.getDWARFObj().isLittleEndian() ? support::little : support::big;
-
- assert(EndOffset > StartOffset);
- switch (EndOffset - StartOffset) {
- case 1:
- return *AddrPtr;
- case 2:
- return support::endian::read16(AddrPtr, Endianess);
- case 4:
- return support::endian::read32(AddrPtr, Endianess);
- case 8:
- return support::endian::read64(AddrPtr, Endianess);
- }
-
- llvm_unreachable("relocateIndexedAddr unhandled case!");
- }
-
protected:
// returns true if specified address range is inside address ranges
// of executable sections.
@@ -231,7 +207,6 @@ class ObjFileAddressMap : public AddressesMap {
RangesTy DWARFAddressRanges;
AddressRanges TextAddressRanges;
const Options &Opts;
- DWARFContext &Context;
};
static bool knownByDWARFUtil(StringRef SecName) {
More information about the llvm-commits
mailing list