[llvm] 8f5a68a - [DWARFLinker][NFC] Remove RangesTy &getValidAddressRanges().
Alexey Lapshin via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 4 04:06:00 PDT 2023
Author: Alexey Lapshin
Date: 2023-07-04T13:00:10+02:00
New Revision: 8f5a68ab03280dd3d9099f767d8d8ccbc4ae595a
URL: https://github.com/llvm/llvm-project/commit/8f5a68ab03280dd3d9099f767d8d8ccbc4ae595a
DIFF: https://github.com/llvm/llvm-project/commit/8f5a68ab03280dd3d9099f767d8d8ccbc4ae595a.diff
LOG: [DWARFLinker][NFC] Remove RangesTy &getValidAddressRanges().
This patch simplifies line table generation. It removes global
array of all units ranges(RangesTy &getValidAddressRanges()).
The comment says that global array of all units ranges is necessary
to handle corner cases inside line table rows. Removing that
special handling shows that its current usage is handling of
"end of range case" which is already handled correctly
(without special handling). .debug_line tables for clang binary
built with and without this patch are equal.
Differential Revision: https://reviews.llvm.org/D154288
Added:
Modified:
llvm/include/llvm/DWARFLinker/DWARFLinker.h
llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
llvm/lib/DWARFLinker/DWARFLinker.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 e24fa2ef704b5e..90834b6e3f9ba3 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -69,10 +69,6 @@ class AddressesMap {
virtual bool applyValidRelocs(MutableArrayRef<char> Data, uint64_t BaseOffset,
bool IsLittleEndian) = 0;
- /// Returns all valid functions address ranges(i.e., those ranges
- /// which points to sections with code).
- virtual RangesTy &getValidAddressRanges() = 0;
-
/// Erases all data.
virtual void clear() = 0;
};
@@ -546,10 +542,9 @@ class DWARFLinker {
/// keep. Store that information in \p CU's DIEInfo.
///
/// The return value indicates whether the DIE is incomplete.
- void lookForDIEsToKeep(AddressesMap &RelocMgr, RangesTy &Ranges,
- const UnitListTy &Units, const DWARFDie &DIE,
- const DWARFFile &File, CompileUnit &CU,
- unsigned Flags);
+ void lookForDIEsToKeep(AddressesMap &RelocMgr, const UnitListTy &Units,
+ const DWARFDie &DIE, const DWARFFile &File,
+ CompileUnit &CU, unsigned Flags);
/// Check whether specified \p CUDie is a Clang module reference.
/// if \p Quiet is false then display error messages.
@@ -585,10 +580,9 @@ class DWARFLinker {
OffsetsStringPool &DebugLineStrPool,
unsigned Indent = 0);
- unsigned shouldKeepDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
- const DWARFDie &DIE, const DWARFFile &File,
- CompileUnit &Unit, CompileUnit::DIEInfo &MyInfo,
- unsigned Flags);
+ unsigned shouldKeepDIE(AddressesMap &RelocMgr, const DWARFDie &DIE,
+ const DWARFFile &File, CompileUnit &Unit,
+ CompileUnit::DIEInfo &MyInfo, unsigned Flags);
/// This function checks whether variable has DWARF expression containing
/// operation referencing live address(f.e. DW_OP_addr, DW_OP_addrx...).
@@ -604,9 +598,8 @@ class DWARFLinker {
unsigned shouldKeepVariableDIE(AddressesMap &RelocMgr, const DWARFDie &DIE,
CompileUnit::DIEInfo &MyInfo, unsigned Flags);
- unsigned shouldKeepSubprogramDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
- const DWARFDie &DIE, const DWARFFile &File,
- CompileUnit &Unit,
+ unsigned shouldKeepSubprogramDIE(AddressesMap &RelocMgr, const DWARFDie &DIE,
+ const DWARFFile &File, CompileUnit &Unit,
CompileUnit::DIEInfo &MyInfo,
unsigned Flags);
@@ -803,8 +796,7 @@ class DWARFLinker {
void emitAcceleratorEntriesForUnit(CompileUnit &Unit);
/// Patch the frame info for an object file and emit it.
- void patchFrameInfoForObject(const DWARFFile &, RangesTy &Ranges,
- DWARFContext &, unsigned AddressSize);
+ void patchFrameInfoForObject(LinkContext &Context);
/// FoldingSet that uniques the abbreviations.
FoldingSet<DIEAbbrev> AbbreviationsSet;
diff --git a/llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h b/llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
index 5386f9ca68674e..5d735abab419e6 100644
--- a/llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
+++ b/llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h
@@ -60,10 +60,6 @@ class AddressesMap {
virtual bool applyValidRelocs(MutableArrayRef<char> Data, uint64_t BaseOffset,
bool IsLittleEndian) = 0;
- /// Returns all valid functions address ranges (i.e., those ranges
- /// which points to sections with code).
- virtual RangesTy &getValidAddressRanges() = 0;
-
/// Erases all data.
virtual void clear() = 0;
};
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index a954a1adb45be7..ceeb26acee6a05 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -565,9 +565,8 @@ unsigned DWARFLinker::shouldKeepVariableDIE(AddressesMap &RelocMgr,
/// Check if a function describing DIE should be kept.
/// \returns updated TraversalFlags.
unsigned DWARFLinker::shouldKeepSubprogramDIE(
- AddressesMap &RelocMgr, RangesTy &Ranges, const DWARFDie &DIE,
- const DWARFFile &File, CompileUnit &Unit, CompileUnit::DIEInfo &MyInfo,
- unsigned Flags) {
+ AddressesMap &RelocMgr, const DWARFDie &DIE, const DWARFFile &File,
+ CompileUnit &Unit, CompileUnit::DIEInfo &MyInfo, unsigned Flags) {
Flags |= TF_InFunctionScope;
auto LowPc = dwarf::toAddress(DIE.find(dwarf::DW_AT_low_pc));
@@ -622,16 +621,14 @@ unsigned DWARFLinker::shouldKeepSubprogramDIE(
}
// Replace the debug map range with a more accurate one.
- Ranges.insert({*LowPc, *HighPc}, MyInfo.AddrAdjust);
Unit.addFunctionRange(*LowPc, *HighPc, MyInfo.AddrAdjust);
return Flags;
}
/// Check if a DIE should be kept.
/// \returns updated TraversalFlags.
-unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
- const DWARFDie &DIE, const DWARFFile &File,
- CompileUnit &Unit,
+unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, const DWARFDie &DIE,
+ const DWARFFile &File, CompileUnit &Unit,
CompileUnit::DIEInfo &MyInfo,
unsigned Flags) {
switch (DIE.getTag()) {
@@ -640,8 +637,7 @@ unsigned DWARFLinker::shouldKeepDIE(AddressesMap &RelocMgr, RangesTy &Ranges,
return shouldKeepVariableDIE(RelocMgr, DIE, MyInfo, Flags);
case dwarf::DW_TAG_subprogram:
case dwarf::DW_TAG_label:
- return shouldKeepSubprogramDIE(RelocMgr, Ranges, DIE, File, Unit, MyInfo,
- Flags);
+ return shouldKeepSubprogramDIE(RelocMgr, DIE, File, Unit, MyInfo, Flags);
case dwarf::DW_TAG_base_type:
// DWARF Expressions may reference basic types, but scanning them
// is expensive. Basic types are tiny, so just keep all of them.
@@ -861,7 +857,7 @@ void DWARFLinker::lookForParentDIEsToKeep(
///
/// The return value indicates whether the DIE is incomplete.
void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
- RangesTy &Ranges, const UnitListTy &Units,
+ const UnitListTy &Units,
const DWARFDie &Die, const DWARFFile &File,
CompileUnit &Cu, unsigned Flags) {
// LIFO work list.
@@ -916,8 +912,8 @@ void DWARFLinker::lookForDIEsToKeep(AddressesMap &AddressesMap,
continue;
if (!(Current.Flags & TF_DependencyWalk))
- Current.Flags = shouldKeepDIE(AddressesMap, Ranges, Current.Die, File,
- Current.CU, MyInfo, Current.Flags);
+ Current.Flags = shouldKeepDIE(AddressesMap, Current.Die, File, Current.CU,
+ MyInfo, Current.Flags);
// We need to mark context for the canonical die in the end of normal
// traversing(not TF_DependencyWalk) or after normal traversing if die
@@ -2097,8 +2093,6 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
LineTable.Sequences = LT->Sequences;
} else {
- RangesTy &Ranges = ObjFile.Addresses->getValidAddressRanges();
-
// This vector is the output line table.
std::vector<DWARFDebugLine::Row> NewRows;
NewRows.reserve(LT->Rows.size());
@@ -2127,27 +2121,12 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
// it is marked as end_sequence in the input (because in that
// case, the relocation offset is accurate and that entry won't
// serve as the start of another function).
- if (!CurrRange || !CurrRange->Range.contains(Row.Address.Address) ||
- (Row.Address.Address == CurrRange->Range.end() &&
- !Row.EndSequence)) {
+ if (!CurrRange || !CurrRange->Range.contains(Row.Address.Address)) {
// We just stepped out of a known range. Insert a end_sequence
// corresponding to the end of the range.
uint64_t StopAddress =
CurrRange ? CurrRange->Range.end() + CurrRange->Value : -1ULL;
CurrRange = FunctionRanges.getRangeThatContains(Row.Address.Address);
- if (!CurrRange) {
- if (StopAddress != -1ULL) {
- // Try harder by looking in the Address ranges map.
- // There are corner cases where this finds a
- // valid entry. It's unclear if this is right or wrong, but
- // for now do as dsymutil.
- // FIXME: Understand exactly what cases this addresses and
- // potentially remove it along with the Ranges map.
- if (std::optional<AddressRangeValuePair> Range =
- Ranges.getRangeThatContains(Row.Address.Address))
- StopAddress = Row.Address.Address + (*Range).Value;
- }
- }
if (StopAddress != -1ULL && !Seq.empty()) {
// Insert end sequence row with the computed end address, but
// the same line as the previous one.
@@ -2236,14 +2215,20 @@ void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) {
/// This is actually pretty easy as the data of the CIEs and FDEs can
/// be considered as black boxes and moved as is. The only thing to do
/// is to patch the addresses in the headers.
-void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
- RangesTy &Ranges,
- DWARFContext &OrigDwarf,
- unsigned AddrSize) {
+void DWARFLinker::patchFrameInfoForObject(LinkContext &Context) {
+ DWARFContext &OrigDwarf = *Context.File.Dwarf;
+ unsigned SrcAddrSize = OrigDwarf.getDWARFObj().getAddressSize();
+
StringRef FrameData = OrigDwarf.getDWARFObj().getFrameSection().Data;
if (FrameData.empty())
return;
+ RangesTy AllUnitsRanges;
+ for (std::unique_ptr<CompileUnit> &Unit : Context.CompileUnits) {
+ for (auto CurRange : Unit->getFunctionRanges())
+ AllUnitsRanges.insert(CurRange.Range, CurRange.Value);
+ }
+
DataExtractor Data(FrameData, OrigDwarf.isLittleEndian(), 0);
uint64_t InputOffset = 0;
@@ -2255,7 +2240,7 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
uint64_t EntryOffset = InputOffset;
uint32_t InitialLength = Data.getU32(&InputOffset);
if (InitialLength == 0xFFFFFFFF)
- return reportWarning("Dwarf64 bits no supported", File);
+ return reportWarning("Dwarf64 bits no supported", Context.File);
uint32_t CIEId = Data.getU32(&InputOffset);
if (CIEId == 0xFFFFFFFF) {
@@ -2267,14 +2252,14 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
continue;
}
- uint64_t Loc = Data.getUnsigned(&InputOffset, AddrSize);
+ uint64_t Loc = Data.getUnsigned(&InputOffset, SrcAddrSize);
// Some compilers seem to emit frame info that doesn't start at
// the function entry point, thus we can't just lookup the address
// in the debug map. Use the AddressInfo's range map to see if the FDE
// describes something that we can relocate.
std::optional<AddressRangeValuePair> Range =
- Ranges.getRangeThatContains(Loc);
+ AllUnitsRanges.getRangeThatContains(Loc);
if (!Range) {
// The +4 is to account for the size of the InitialLength field itself.
InputOffset = EntryOffset + InitialLength + 4;
@@ -2285,7 +2270,8 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
// Have we already emitted a corresponding CIE?
StringRef CIEData = LocalCIES[CIEId];
if (CIEData.empty())
- return reportWarning("Inconsistent debug_frame content. Dropping.", File);
+ return reportWarning("Inconsistent debug_frame content. Dropping.",
+ Context.File);
// Look if we already emitted a CIE that corresponds to the
// referenced one (the CIE data is the key of that lookup).
@@ -2301,8 +2287,8 @@ void DWARFLinker::patchFrameInfoForObject(const DWARFFile &File,
// Emit the FDE with updated address and CIE pointer.
// (4 + AddrSize) is the size of the CIEId + initial_location
// fields that will get reconstructed by emitFDE().
- unsigned FDERemainingBytes = InitialLength - (4 + AddrSize);
- TheDwarfEmitter->emitFDE(IteratorInserted.first->getValue(), AddrSize,
+ unsigned FDERemainingBytes = InitialLength - (4 + SrcAddrSize);
+ TheDwarfEmitter->emitFDE(IteratorInserted.first->getValue(), SrcAddrSize,
Loc + Range->Value,
FrameData.substr(InputOffset, FDERemainingBytes));
InputOffset += FDERemainingBytes;
@@ -2868,9 +2854,7 @@ Error DWARFLinker::link() {
copyInvariantDebugSection(*OptContext.File.Dwarf);
} else {
for (auto &CurrentUnit : OptContext.CompileUnits) {
- lookForDIEsToKeep(*OptContext.File.Addresses,
- OptContext.File.Addresses->getValidAddressRanges(),
- OptContext.CompileUnits,
+ lookForDIEsToKeep(*OptContext.File.Addresses, OptContext.CompileUnits,
CurrentUnit->getOrigUnit().getUnitDIE(),
OptContext.File, *CurrentUnit, 0);
#ifndef NDEBUG
@@ -2895,10 +2879,7 @@ Error DWARFLinker::link() {
}
if ((TheDwarfEmitter != nullptr) && !OptContext.CompileUnits.empty() &&
LLVM_LIKELY(!Options.Update))
- patchFrameInfoForObject(
- OptContext.File, OptContext.File.Addresses->getValidAddressRanges(),
- *OptContext.File.Dwarf,
- OptContext.CompileUnits[0]->getOrigUnit().getAddressByteSize());
+ patchFrameInfoForObject(OptContext);
// Clean-up before starting working on the next object.
cleanupAuxiliarryData(OptContext);
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.h b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
index 61737bea084176..230f569a6988c3 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.h
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.h
@@ -92,8 +92,6 @@ class DwarfLinkerForBinary {
std::vector<ValidReloc> ValidDebugAddrRelocs;
/// }
- RangesTy AddressRanges;
-
StringRef SrcFileName;
/// Returns list of valid relocations from \p Relocs,
@@ -120,31 +118,6 @@ class DwarfLinkerForBinary {
const DebugMapObject &DMO)
: Linker(Linker), SrcFileName(DMO.getObjectFilename()) {
findValidRelocsInDebugSections(Obj, DMO);
-
- // Iterate over the debug map entries and put all the ones that are
- // functions (because they have a size) into the Ranges map. This map is
- // very similar to the FunctionRanges that are stored in each unit, with 2
- // notable
diff erences:
- //
- // 1. Obviously this one is global, while the other ones are per-unit.
- //
- // 2. This one contains not only the functions described in the DIE
- // tree, but also the ones that are only in the debug map.
- //
- // The latter information is required to reproduce dsymutil's logic while
- // linking line tables. The cases where this information matters look like
- // bugs that need to be investigated, but for now we need to reproduce
- // dsymutil's behavior.
- // FIXME: Once we understood exactly if that information is needed,
- // maybe totally remove this (or try to use it to do a real
- // -gline-tables-only on Darwin.
- for (const auto &Entry : DMO.symbols()) {
- const auto &Mapping = Entry.getValue();
- if (Mapping.Size && Mapping.ObjectAddress)
- AddressRanges.insert(
- {*Mapping.ObjectAddress, *Mapping.ObjectAddress + Mapping.Size},
- int64_t(Mapping.BinaryAddress) - *Mapping.ObjectAddress);
- }
}
~AddressManager() override { clear(); }
@@ -188,10 +161,7 @@ class DwarfLinkerForBinary {
bool applyValidRelocs(MutableArrayRef<char> Data, uint64_t BaseOffset,
bool IsLittleEndian) override;
- RangesTy &getValidAddressRanges() override { return AddressRanges; }
-
void clear() override {
- AddressRanges.clear();
ValidDebugInfoRelocs.clear();
ValidDebugAddrRelocs.clear();
}
diff --git a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
index cd719f21016799..47a23e8448cc49 100644
--- a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
+++ b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
@@ -59,18 +59,26 @@ class ObjFileAddressMap : public AddressMapBase {
for (std::unique_ptr<DWARFUnit> &CU : Context.compile_units()) {
Expected<llvm::DWARFAddressRangesVector> ARanges =
CU->getUnitDIE().getAddressRanges();
- if (ARanges) {
- for (auto &Range : *ARanges) {
- if (!isDeadAddressRange(Range.LowPC, Range.HighPC, CU->getVersion(),
- Options.Tombstone, CU->getAddressByteSize()))
- DWARFAddressRanges.insert({Range.LowPC, Range.HighPC}, 0);
+ if (!ARanges) {
+ llvm::consumeError(ARanges.takeError());
+ continue;
+ }
+
+ for (auto &Range : *ARanges) {
+ if (!isDeadAddressRange(Range.LowPC, Range.HighPC, CU->getVersion(),
+ Options.Tombstone, CU->getAddressByteSize())) {
+ HasValidAddressRanges = true;
+ break;
}
}
+
+ if (HasValidAddressRanges)
+ break;
}
}
// should be renamed into has valid address ranges
- bool hasValidRelocs() override { return !DWARFAddressRanges.empty(); }
+ bool hasValidRelocs() override { return HasValidAddressRanges; }
std::optional<int64_t>
getSubprogramRelocAdjustment(const DWARFDie &DIE) override {
@@ -127,9 +135,7 @@ class ObjFileAddressMap : public AddressMapBase {
return false;
}
- RangesTy &getValidAddressRanges() override { return DWARFAddressRanges; };
-
- void clear() override { DWARFAddressRanges.clear(); }
+ void clear() override {}
protected:
// returns true if specified address range is inside address ranges
@@ -197,9 +203,9 @@ class ObjFileAddressMap : public AddressMapBase {
}
private:
- RangesTy DWARFAddressRanges;
AddressRanges TextAddressRanges;
const Options &Opts;
+ bool HasValidAddressRanges = false;
};
static bool knownByDWARFUtil(StringRef SecName) {
More information about the llvm-commits
mailing list