[llvm] [BOLT][DWARF][NFC] Refactor address writers (PR #98094)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 8 17:03:06 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-bolt
Author: Sayhaan Siddiqui (sayhaan)
<details>
<summary>Changes</summary>
Refactors address writers to create an instance for each CU and its DWO CU.
---
Patch is 20.86 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/98094.diff
4 Files Affected:
- (modified) bolt/include/bolt/Core/DebugData.h (+39-14)
- (modified) bolt/include/bolt/Rewrite/DWARFRewriter.h (+5)
- (modified) bolt/lib/Core/DebugData.cpp (+34-37)
- (modified) bolt/lib/Rewrite/DWARFRewriter.cpp (+40-23)
``````````diff
diff --git a/bolt/include/bolt/Core/DebugData.h b/bolt/include/bolt/Core/DebugData.h
index 144433ac78a37..05217c699f0dd 100644
--- a/bolt/include/bolt/Core/DebugData.h
+++ b/bolt/include/bolt/Core/DebugData.h
@@ -344,14 +344,34 @@ class DebugAddrWriter {
uint32_t getIndexFromAddress(uint64_t Address, DWARFUnit &CU);
/// Write out entries in to .debug_addr section for CUs.
- virtual void update(DIEBuilder &DIEBlder, DWARFUnit &CUs);
+ virtual void update();
/// Return buffer with all the entries in .debug_addr already writen out using
/// update(...).
virtual AddressSectionBuffer &finalize() { return *Buffer; }
/// Returns False if .debug_addr section was created..
- bool isInitialized() const { return !AddressMaps.empty(); }
+ bool isInitialized() { return Map.empty; }
+
+ /// Updates address base with the given Offset.
+ virtual void updateAddrBase(DIEBuilder &DIEBlder, DWARFUnit &CU,
+ const uint64_t Offset);
+
+ /// Appends an AddressSectionBuffer to the address writer buffer for the given
+ /// CU.
+ void appendToAddressBuffer(const AddressSectionBuffer &Buffer) {
+ *AddressStream << Buffer;
+ }
+
+ /// Sets AddressByteSize for the CU.
+ void setAddressByteSize(uint8_t AddressByteSize) {
+ this->AddressByteSize = AddressByteSize;
+ }
+
+ /// Sets AddrOffsetSectionBase for the CU.
+ void setAddrOffsetSectionBase(std::optional<uint64_t> AddrOffsetSectionBase) {
+ this->AddrOffsetSectionBase = AddrOffsetSectionBase;
+ }
protected:
class AddressForDWOCU {
@@ -396,6 +416,8 @@ class DebugAddrWriter {
void dump();
+ bool empty = false;
+
private:
AddressToIndexMap AddressToIndex;
IndexToAddressMap IndexToAddress;
@@ -408,14 +430,16 @@ class DebugAddrWriter {
}
BinaryContext *BC;
- /// Maps DWOID to AddressForDWOCU.
- std::unordered_map<uint64_t, AddressForDWOCU> AddressMaps;
+ /// Address for the DWO CU associated with the address writer.
+ AddressForDWOCU Map;
+ uint8_t AddressByteSize;
+ std::optional<uint64_t> AddrOffsetSectionBase;
/// Mutex used for parallel processing of debug info.
std::mutex WriterMutex;
std::unique_ptr<AddressSectionBuffer> Buffer;
std::unique_ptr<raw_svector_ostream> AddressStream;
/// Used to track sections that were not modified so that they can be re-used.
- DenseMap<uint64_t, uint64_t> UnmodifiedAddressOffsets;
+ static DenseMap<uint64_t, uint64_t> UnmodifiedAddressOffsets;
};
class DebugAddrWriterDwarf5 : public DebugAddrWriter {
@@ -424,7 +448,10 @@ class DebugAddrWriterDwarf5 : public DebugAddrWriter {
DebugAddrWriterDwarf5(BinaryContext *BC) : DebugAddrWriter(BC) {}
/// Write out entries in to .debug_addr section for CUs.
- virtual void update(DIEBuilder &DIEBlder, DWARFUnit &CUs) override;
+ virtual void update() override;
+
+ virtual void updateAddrBase(DIEBuilder &DIEBlder, DWARFUnit &CU,
+ const uint64_t Offset) override;
protected:
/// Given DWARFUnit \p Unit returns either DWO ID or it's offset within
@@ -584,12 +611,10 @@ class DebugLoclistWriter : public DebugLocWriter {
public:
~DebugLoclistWriter() {}
DebugLoclistWriter() = delete;
- DebugLoclistWriter(DWARFUnit &Unit, uint8_t DV, bool SD)
- : DebugLocWriter(DV, LocWriterKind::DebugLoclistWriter), CU(Unit),
- IsSplitDwarf(SD) {
- assert(DebugLoclistWriter::AddrWriter &&
- "Please use SetAddressWriter to initialize "
- "DebugAddrWriter before instantiation.");
+ DebugLoclistWriter(DWARFUnit &Unit, uint8_t DV, bool SD,
+ DebugAddrWriter *AddrW)
+ : DebugLocWriter(DV, LocWriterKind::DebugLoclistWriter),
+ AddrWriter(AddrW), CU(Unit), IsSplitDwarf(SD) {
if (DwarfVersion >= 5) {
LocBodyBuffer = std::make_unique<DebugBufferVector>();
LocBodyStream = std::make_unique<raw_svector_ostream>(*LocBodyBuffer);
@@ -601,7 +626,7 @@ class DebugLoclistWriter : public DebugLocWriter {
}
}
- static void setAddressWriter(DebugAddrWriter *AddrW) { AddrWriter = AddrW; }
+ void setAddressWriter(DebugAddrWriter *AddrW) { AddrWriter = AddrW; }
/// Stores location lists internally to be written out during finalize phase.
virtual void addList(DIEBuilder &DIEBldr, DIE &Die, DIEValue &AttrInfo,
@@ -631,7 +656,7 @@ class DebugLoclistWriter : public DebugLocWriter {
/// Writes out locations in to a local buffer and applies debug info patches.
void finalizeDWARF5(DIEBuilder &DIEBldr, DIE &Die);
- static DebugAddrWriter *AddrWriter;
+ DebugAddrWriter *AddrWriter;
DWARFUnit &CU;
bool IsSplitDwarf{false};
// Used for DWARF5 to store location lists before being finalized.
diff --git a/bolt/include/bolt/Rewrite/DWARFRewriter.h b/bolt/include/bolt/Rewrite/DWARFRewriter.h
index 4f576eaa95576..49cec64ce4cea 100644
--- a/bolt/include/bolt/Rewrite/DWARFRewriter.h
+++ b/bolt/include/bolt/Rewrite/DWARFRewriter.h
@@ -93,6 +93,10 @@ class DWARFRewriter {
std::unordered_map<uint64_t, std::unique_ptr<DebugRangesSectionWriter>>
LegacyRangesWritersByCU;
+ /// Stores address writer for each CU.
+ std::unordered_map<uint64_t, std::unique_ptr<DebugAddrWriter>>
+ AddressWritersByCU;
+
std::mutex LocListDebugInfoPatchesMutex;
/// Dwo id specific its RangesBase.
@@ -115,6 +119,7 @@ class DWARFRewriter {
void updateUnitDebugInfo(DWARFUnit &Unit, DIEBuilder &DIEBldr,
DebugLocWriter &DebugLocWriter,
DebugRangesSectionWriter &RangesSectionWriter,
+ DebugAddrWriter &AddressWriter,
std::optional<uint64_t> RangesBase = std::nullopt);
/// Patches the binary for an object's address ranges to be updated.
diff --git a/bolt/lib/Core/DebugData.cpp b/bolt/lib/Core/DebugData.cpp
index 08d4c45aac791..52df5d5e55707 100644
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -393,6 +393,7 @@ void DebugARangesSectionWriter::writeARangesSection(
DebugAddrWriter::DebugAddrWriter(BinaryContext *BC) : BC(BC) {
Buffer = std::make_unique<AddressSectionBuffer>();
AddressStream = std::make_unique<raw_svector_ostream>(*Buffer);
+ Map = AddressForDWOCU();
}
void DebugAddrWriter::AddressForDWOCU::dump() {
@@ -405,11 +406,8 @@ void DebugAddrWriter::AddressForDWOCU::dump() {
}
uint32_t DebugAddrWriter::getIndexFromAddress(uint64_t Address, DWARFUnit &CU) {
std::lock_guard<std::mutex> Lock(WriterMutex);
- const uint64_t CUID = getCUID(CU);
- if (!AddressMaps.count(CUID))
- AddressMaps[CUID] = AddressForDWOCU();
-
- AddressForDWOCU &Map = AddressMaps[CUID];
+ if (Map.begin() == Map.end())
+ Map.empty = true;
auto Entry = Map.find(Address);
if (Entry == Map.end()) {
auto Index = Map.getNextIndex();
@@ -449,25 +447,20 @@ static void updateAddressBase(DIEBuilder &DIEBlder, DebugAddrWriter &AddrWriter,
}
}
-void DebugAddrWriter::update(DIEBuilder &DIEBlder, DWARFUnit &CU) {
- // Handling the case where debug information is a mix of Debug fission and
- // monolithic.
- if (!CU.getDWOId())
- return;
- const uint64_t CUID = getCUID(CU);
- auto AM = AddressMaps.find(CUID);
- // Adding to map even if it did not contribute to .debug_addr.
- // The Skeleton CU might still have DW_AT_GNU_addr_base.
- uint64_t Offset = Buffer->size();
- // If does not exist this CUs DWO section didn't contribute to .debug_addr.
- if (AM == AddressMaps.end())
+void DebugAddrWriter::updateAddrBase(DIEBuilder &DIEBlder, DWARFUnit &CU,
+ const uint64_t Offset) {
+ updateAddressBase(DIEBlder, *this, CU, Offset);
+}
+
+void DebugAddrWriter::update() {
+ if (Map.indexToAddressBegin() == Map.indexToAdddessEnd())
return;
- std::vector<IndexAddressPair> SortedMap(AM->second.indexToAddressBegin(),
- AM->second.indexToAdddessEnd());
+ std::vector<IndexAddressPair> SortedMap(Map.indexToAddressBegin(),
+ Map.indexToAdddessEnd());
// Sorting address in increasing order of indices.
llvm::sort(SortedMap, llvm::less_first());
- uint8_t AddrSize = CU.getAddressByteSize();
+ uint8_t AddrSize = AddressByteSize;
uint32_t Counter = 0;
auto WriteAddress = [&](uint64_t Address) -> void {
++Counter;
@@ -490,10 +483,23 @@ void DebugAddrWriter::update(DIEBuilder &DIEBlder, DWARFUnit &CU) {
WriteAddress(0);
WriteAddress(Val.second);
}
- updateAddressBase(DIEBlder, *this, CU, Offset);
}
-void DebugAddrWriterDwarf5::update(DIEBuilder &DIEBlder, DWARFUnit &CU) {
+void DebugAddrWriterDwarf5::updateAddrBase(DIEBuilder &DIEBlder, DWARFUnit &CU,
+ const uint64_t Offset) {
+ /// Doesn't update address base if the CU doesn't access .debug_addr.
+ if (Map.indexToAddressBegin() == Map.indexToAdddessEnd()) {
+ std::optional<uint64_t> BaseOffset = CU.getAddrOffsetSectionBase();
+ if (!BaseOffset)
+ return;
+ }
+ /// Header for DWARF5 has size 8, so we add it to the offset.
+ updateAddressBase(DIEBlder, *this, CU, Offset + 8);
+}
+
+DenseMap<uint64_t, uint64_t> DebugAddrWriter::UnmodifiedAddressOffsets;
+
+void DebugAddrWriterDwarf5::update() {
// Need to layout all sections within .debug_addr
// Within each section sort Address by index.
const endianness Endian = BC->DwCtx->isLittleEndian()
@@ -505,14 +511,11 @@ void DebugAddrWriterDwarf5::update(DIEBuilder &DIEBlder, DWARFUnit &CU) {
DWARFDebugAddrTable AddrTable;
DIDumpOptions DumpOpts;
constexpr uint32_t HeaderSize = 8;
- const uint64_t CUID = getCUID(CU);
- const uint8_t AddrSize = CU.getAddressByteSize();
- auto AMIter = AddressMaps.find(CUID);
+ const uint8_t AddrSize = AddressByteSize;
// A case where CU has entry in .debug_addr, but we don't modify addresses
// for it.
- if (AMIter == AddressMaps.end()) {
- AMIter = AddressMaps.insert({CUID, AddressForDWOCU()}).first;
- std::optional<uint64_t> BaseOffset = CU.getAddrOffsetSectionBase();
+ if (Map.indexToAddressBegin() == Map.indexToAdddessEnd()) {
+ std::optional<uint64_t> BaseOffset = AddrOffsetSectionBase;
if (!BaseOffset)
return;
// Address base offset is to the first entry.
@@ -520,7 +523,6 @@ void DebugAddrWriterDwarf5::update(DIEBuilder &DIEBlder, DWARFUnit &CU) {
uint64_t Offset = *BaseOffset - HeaderSize;
auto Iter = UnmodifiedAddressOffsets.find(Offset);
if (Iter != UnmodifiedAddressOffsets.end()) {
- updateAddressBase(DIEBlder, *this, CU, Iter->getSecond());
return;
}
UnmodifiedAddressOffsets[Offset] = Buffer->size() + HeaderSize;
@@ -529,16 +531,13 @@ void DebugAddrWriterDwarf5::update(DIEBuilder &DIEBlder, DWARFUnit &CU) {
DumpOpts.RecoverableErrorHandler(std::move(Err));
return;
}
-
uint32_t Index = 0;
for (uint64_t Addr : AddrTable.getAddressEntries())
- AMIter->second.insert(Addr, Index++);
+ Map.insert(Addr, Index++);
}
- updateAddressBase(DIEBlder, *this, CU, Buffer->size() + HeaderSize);
-
- std::vector<IndexAddressPair> SortedMap(AMIter->second.indexToAddressBegin(),
- AMIter->second.indexToAdddessEnd());
+ std::vector<IndexAddressPair> SortedMap(Map.indexToAddressBegin(),
+ Map.indexToAdddessEnd());
// Sorting address in increasing order of indices.
llvm::sort(SortedMap, llvm::less_first());
// Writing out Header
@@ -789,8 +788,6 @@ void DebugLoclistWriter::finalize(DIEBuilder &DIEBldr, DIE &Die) {
finalizeDWARF5(DIEBldr, Die);
}
-DebugAddrWriter *DebugLoclistWriter::AddrWriter = nullptr;
-
static std::string encodeLE(size_t ByteSize, uint64_t NewValue) {
std::string LE64(ByteSize, 0);
for (size_t I = 0; I < ByteSize; ++I) {
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 1f426d0adfc61..22209f6d62303 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -615,7 +615,6 @@ void DWARFRewriter::updateDebugInfo() {
if (BC.isDWARF5Used()) {
AddrWriter = std::make_unique<DebugAddrWriterDwarf5>(&BC);
RangeListsSectionWriter = std::make_unique<DebugRangeListsSectionWriter>();
- DebugRangeListsSectionWriter::setAddressWriter(AddrWriter.get());
} else {
AddrWriter = std::make_unique<DebugAddrWriter>(&BC);
}
@@ -623,8 +622,6 @@ void DWARFRewriter::updateDebugInfo() {
if (BC.isDWARFLegacyUsed())
LegacyRangesSectionWriter = std::make_unique<DebugRangesSectionWriter>();
- DebugLoclistWriter::setAddressWriter(AddrWriter.get());
-
uint32_t CUIndex = 0;
std::mutex AccessMutex;
// Needs to be invoked in the same order as CUs are processed.
@@ -632,8 +629,16 @@ void DWARFRewriter::updateDebugInfo() {
std::lock_guard<std::mutex> Lock(AccessMutex);
const uint16_t DwarfVersion = CU.getVersion();
if (DwarfVersion >= 5) {
- LocListWritersByCU[CUIndex] =
- std::make_unique<DebugLoclistWriter>(CU, DwarfVersion, false);
+ auto AddrW = std::make_unique<DebugAddrWriterDwarf5>(&BC);
+ AddressWritersByCU[CU.getOffset()] = std::move(AddrW);
+ DebugRangeListsSectionWriter::setAddressWriter(
+ AddressWritersByCU[CU.getOffset()].get());
+ AddressWritersByCU[CU.getOffset()]->setAddressByteSize(
+ CU.getAddressByteSize());
+ AddressWritersByCU[CU.getOffset()]->setAddrOffsetSectionBase(
+ CU.getAddrOffsetSectionBase());
+ LocListWritersByCU[CUIndex] = std::make_unique<DebugLoclistWriter>(
+ CU, DwarfVersion, false, AddressWritersByCU[CU.getOffset()].get());
if (std::optional<uint64_t> DWOId = CU.getDWOId()) {
assert(RangeListsWritersByCU.count(*DWOId) == 0 &&
@@ -645,6 +650,10 @@ void DWARFRewriter::updateDebugInfo() {
}
} else {
+ auto AddrW = std::make_unique<DebugAddrWriter>(&BC);
+ AddressWritersByCU[CU.getOffset()] = std::move(AddrW);
+ AddressWritersByCU[CU.getOffset()]->setAddressByteSize(
+ CU.getAddressByteSize());
LocListWritersByCU[CUIndex] = std::make_unique<DebugLocWriter>();
if (std::optional<uint64_t> DWOId = CU.getDWOId()) {
assert(LegacyRangesWritersByCU.count(*DWOId) == 0 &&
@@ -677,6 +686,7 @@ void DWARFRewriter::updateDebugInfo() {
DebugRangesSectionWriter *RangesSectionWriter =
Unit->getVersion() >= 5 ? RangeListsSectionWriter.get()
: LegacyRangesSectionWriter.get();
+ auto &AddressWriter = AddressWritersByCU[Unit->getOffset()];
// Skipping CUs that failed to load.
if (SplitCU) {
DIEBuilder DWODIEBuilder(BC, &(*SplitCU)->getContext(), DebugNamesTable,
@@ -697,7 +707,8 @@ void DWARFRewriter::updateDebugInfo() {
DWODIEBuilder.updateDWONameCompDirForTypes(DWOStrOffstsWriter,
DWOStrWriter, **SplitCU,
DwarfOutputPath, DWOName);
- DebugLoclistWriter DebugLocDWoWriter(*Unit, Unit->getVersion(), true);
+ DebugLoclistWriter DebugLocDWoWriter(*Unit, Unit->getVersion(), true,
+ AddressWriter.get());
DebugRangesSectionWriter *TempRangesSectionWriter = RangesSectionWriter;
if (Unit->getVersion() >= 5) {
TempRangesSectionWriter = RangeListsWritersByCU[*DWOId].get();
@@ -708,7 +719,7 @@ void DWARFRewriter::updateDebugInfo() {
}
updateUnitDebugInfo(*(*SplitCU), DWODIEBuilder, DebugLocDWoWriter,
- *TempRangesSectionWriter);
+ *TempRangesSectionWriter, *AddressWriter.get());
DebugLocDWoWriter.finalize(DWODIEBuilder,
*DWODIEBuilder.getUnitDIEbyUnit(**SplitCU));
if (Unit->getVersion() >= 5)
@@ -727,11 +738,10 @@ void DWARFRewriter::updateDebugInfo() {
}
updateUnitDebugInfo(*Unit, *DIEBlder, *DebugLocWriter, *RangesSectionWriter,
- RangesBase);
+ *AddressWriter.get(), RangesBase);
DebugLocWriter->finalize(*DIEBlder, *DIEBlder->getUnitDIEbyUnit(*Unit));
if (Unit->getVersion() >= 5)
RangesSectionWriter->finalizeSection();
- AddrWriter->update(*DIEBlder, *Unit);
};
DIEBuilder DIEBlder(BC, BC.DwCtx.get(), DebugNamesTable);
@@ -781,7 +791,7 @@ void DWARFRewriter::updateDebugInfo() {
void DWARFRewriter::updateUnitDebugInfo(
DWARFUnit &Unit, DIEBuilder &DIEBldr, DebugLocWriter &DebugLocWriter,
DebugRangesSectionWriter &RangesSectionWriter,
- std::optional<uint64_t> RangesBase) {
+ DebugAddrWriter &AddressWriter, std::optional<uint64_t> RangesBase) {
// Cache debug ranges so that the offset for identical ranges could be reused.
std::map<DebugAddressRangesVector, uint64_t> CachedRanges;
@@ -815,7 +825,7 @@ void DWARFRewriter::updateUnitDebugInfo(
if (FormLowPC == dwarf::DW_FORM_addrx ||
FormLowPC == dwarf::DW_FORM_GNU_addr_index)
- LowPC = AddrWriter->getIndexFromAddress(LowPC, Unit);
+ LowPC = AddressWriter.getIndexFromAddress(LowPC, Unit);
if (LowPCVal)
DIEBldr.replaceValue(Die, AttrLowPC, FormLowPC, DIEInteger(LowPC));
@@ -979,7 +989,7 @@ void DWARFRewriter::updateUnitDebugInfo(
if (AttrVal.getForm() == dwarf::DW_FORM_addrx) {
const uint32_t Index =
- AddrWriter->getIndexFromAddress(UpdatedAddress, Unit);
+ AddressWriter.getIndexFromAddress(UpdatedAddress, Unit);
DIEBldr.replaceValue(Die, AttrVal.getAttribute(), AttrVal.getForm(),
DIEInteger(Index));
} else if (AttrVal.getForm() == dwarf::DW_FORM_addr) {
@@ -1196,7 +1206,7 @@ void DWARFRewriter::updateUnitDebugInfo(
assert(EntryAddress && "Address is not found.");
assert(Index <= std::numeric_limits<uint32_t>::max() &&
"Invalid Operand Index.");
- const uint32_t AddrIndex = AddrWriter->getIndexFromAddress(
+ const uint32_t AddrIndex = AddressWriter.getIndexFromAddress(
EntryAddress->Address, Unit);
// update Index into .debug_address section for DW_AT_location.
// The Size field is not stored in IR, we need to minus 1 in
@@ -1248,7 +1258,7 @@ void DWARFRewriter::updateUnitDebugInfo(
std::lock_guard<std::mutex> Lock(DWARFRewriterMutex);
if (Form == dwarf::DW_FORM_addrx ||
Form == dwarf::DW_FORM_GNU_addr_index) {
- const uint32_t Index = AddrWriter->getIndexFromAddress(
+ const uint32_t Index = AddressWriter.getIndexFromAddress(
NewAddress ? NewAddress : Address, Unit);
DIEBldr.replaceValue(Die, LowPCAttrInfo.getAttribute(),
LowPCAttrInfo.getForm(), DIEInteger(Index));
@@ -1567,14 +1577,10 @@ void DWARFRewriter::finalizeDebugSections(
LocationListSectionContents->size());
}
- // AddrWriter should be finalized after debug_loc since more addresses can be
- // added there.
- if (AddrWriter->isInitialized()) {
- AddressSectionBuffer AddressSectionContents = AddrWriter->finalize();
- BC.registerOrUpdateNoteSection(".debug_addr",
- copyByteArray(AddressSectionContents),
- AddressSectionContents.size());
- }
+ AddressSectionBuffer AddressSectionContents = AddrWriter->finalize();
+ BC.registerOrUpdateNoteSection(".debug_addr",
+ copyByteArray(AddressSectionContents),
+ AddressSectionContents.size());
Streamer.emitAbbrevs(DIEBlder.getAbbrevs(), BC.DwCtx->getMaxVersion());
Streamer.finish();
@@ -1628,6 +1634,16 @@ void DWARFRewriter::finalizeCompileUnits(DIEBuilder &DIEBlder,
CUOffsetMap &CUMap,
const std::list<DWARFUnit *> &CUs) {
for (DWARFUnit *CU : CUs) {
+ auto AddressWriterIterator = AddressWritersByCU.find(CU->getOffset());
+ assert(AddressWriterIterator != AddressWritersByCU.end() &&
+ "AddressWriter does not exist for CU");
+ auto &AddressWriter = AddressWriterIterator->second;
+ AddressWriter->u...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/98094
More information about the llvm-commits
mailing list