Skip to content

Commit 8c0f3b6

Browse files
authored
[BOLT] Fix debug line emission for functions in multiple compilation units (#151230)
This patch fixes a bug in BOLT's debug line emission where functions that belong to multiple compilation units (such as inline functions in header files) were not handled correctly. Previously, BOLT incorrectly assumed that a binary function could belong to only one compilation unit, leading to incomplete or incorrect debug line information. ### **Problem** When a function appears in multiple compilation units (common scenarios include): * Template instantiated functions * Inline functions defined in header files included by multiple source files BOLT would only emit debug line information for one compilation unit, losing debug information for other CUs where the function was compiled. This resulted in incomplete debugging information and could cause debuggers to fail to set breakpoints or show incorrect source locations. ### **Root Cause** The issue was in BOLT's assumption that each binary function maps to exactly one compilation unit. However, when the same function (e.g., an inline function from a header) is compiled into multiple object files, it legitimately belongs to multiple CUs in the final binary.
1 parent 299ba5d commit 8c0f3b6

File tree

12 files changed

+873
-159
lines changed

12 files changed

+873
-159
lines changed

bolt/include/bolt/Core/BinaryContext.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,12 @@ class BinaryContext {
288288
/// overwritten, but it is okay to re-generate debug info for them.
289289
std::set<const DWARFUnit *> ProcessedCUs;
290290

291+
/// DWARF-related container to manage lifecycle of groups of rows from line
292+
/// tables associated with instructions. Since binary functions can span
293+
/// multiple compilation units, instructions may reference debug line
294+
/// information from multiple CUs.
295+
ClusteredRowsContainer ClusteredRows;
296+
291297
// Setup MCPlus target builder
292298
void initializeTarget(std::unique_ptr<MCPlusBuilder> TargetBuilder) {
293299
MIB = std::move(TargetBuilder);

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "bolt/Core/JumpTable.h"
3636
#include "bolt/Core/MCPlus.h"
3737
#include "bolt/Utils/NameResolver.h"
38+
#include "llvm/ADT/DenseMap.h"
3839
#include "llvm/ADT/STLExtras.h"
3940
#include "llvm/ADT/SmallString.h"
4041
#include "llvm/ADT/StringRef.h"
@@ -423,8 +424,9 @@ class BinaryFunction {
423424
/// Original LSDA type encoding
424425
unsigned LSDATypeEncoding{dwarf::DW_EH_PE_omit};
425426

426-
/// Containing compilation unit for the function.
427-
DWARFUnit *DwarfUnit{nullptr};
427+
/// All compilation units this function belongs to.
428+
/// Maps DWARF unit offset to the unit pointer.
429+
DenseMap<uint64_t, DWARFUnit *> DwarfUnitMap;
428430

429431
/// Last computed hash value. Note that the value could be recomputed using
430432
/// different parameters by every pass.
@@ -2409,15 +2411,21 @@ class BinaryFunction {
24092411
void
24102412
computeBlockHashes(HashFunction HashFunction = HashFunction::Default) const;
24112413

2412-
void setDWARFUnit(DWARFUnit *Unit) { DwarfUnit = Unit; }
2414+
void addDWARFUnit(DWARFUnit *Unit) { DwarfUnitMap[Unit->getOffset()] = Unit; }
24132415

2414-
/// Return DWARF compile unit for this function.
2415-
DWARFUnit *getDWARFUnit() const { return DwarfUnit; }
2416+
void removeDWARFUnit(DWARFUnit *Unit) {
2417+
DwarfUnitMap.erase(Unit->getOffset());
2418+
}
2419+
2420+
/// Return DWARF compile units for this function.
2421+
/// Returns a reference to the map of DWARF unit offsets to units.
2422+
const DenseMap<uint64_t, DWARFUnit *> &getDWARFUnits() const {
2423+
return DwarfUnitMap;
2424+
}
24162425

2417-
/// Return line info table for this function.
2418-
const DWARFDebugLine::LineTable *getDWARFLineTable() const {
2419-
return getDWARFUnit() ? BC.DwCtx->getLineTableForUnit(getDWARFUnit())
2420-
: nullptr;
2426+
const DWARFDebugLine::LineTable *
2427+
getDWARFLineTableForUnit(DWARFUnit *Unit) const {
2428+
return BC.DwCtx->getLineTableForUnit(Unit);
24212429
}
24222430

24232431
/// Finalize profile for the function.

bolt/include/bolt/Core/DebugData.h

Lines changed: 92 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,6 @@ struct DebugLineTableRowRef {
135135
uint32_t DwCompileUnitIndex;
136136
uint32_t RowIndex;
137137

138-
const static DebugLineTableRowRef NULL_ROW;
139-
140138
bool operator==(const DebugLineTableRowRef &Rhs) const {
141139
return DwCompileUnitIndex == Rhs.DwCompileUnitIndex &&
142140
RowIndex == Rhs.RowIndex;
@@ -145,24 +143,6 @@ struct DebugLineTableRowRef {
145143
bool operator!=(const DebugLineTableRowRef &Rhs) const {
146144
return !(*this == Rhs);
147145
}
148-
149-
static DebugLineTableRowRef fromSMLoc(const SMLoc &Loc) {
150-
union {
151-
decltype(Loc.getPointer()) Ptr;
152-
DebugLineTableRowRef Ref;
153-
} U;
154-
U.Ptr = Loc.getPointer();
155-
return U.Ref;
156-
}
157-
158-
SMLoc toSMLoc() const {
159-
union {
160-
decltype(SMLoc().getPointer()) Ptr;
161-
DebugLineTableRowRef Ref;
162-
} U;
163-
U.Ref = *this;
164-
return SMLoc::getFromPointer(U.Ptr);
165-
}
166146
};
167147

168148
/// Common buffer vector used for debug info handling.
@@ -210,7 +190,7 @@ class DebugRangesSectionWriter {
210190
static bool classof(const DebugRangesSectionWriter *Writer) {
211191
return Writer->getKind() == RangesWriterKind::DebugRangesWriter;
212192
}
213-
193+
214194
/// Append a range to the main buffer.
215195
void appendToRangeBuffer(const DebugBufferVector &CUBuffer);
216196

@@ -852,6 +832,97 @@ class DwarfLineTable {
852832
// Returns DWARF Version for this line table.
853833
uint16_t getDwarfVersion() const { return DwarfVersion; }
854834
};
835+
836+
/// ClusteredRows represents a collection of debug line table row references.
837+
///
838+
/// MEMORY LAYOUT AND DESIGN:
839+
/// This class uses a flexible array member pattern to store all
840+
/// DebugLineTableRowRef elements in a single contiguous memory allocation.
841+
/// The memory layout is:
842+
///
843+
/// +------------------+
844+
/// | ClusteredRows | <- Object header (Size + first element)
845+
/// | - Size |
846+
/// | - Rows (element) | <- First DebugLineTableRowRef element
847+
/// +------------------+
848+
/// | element[1] | <- Additional DebugLineTableRowRef elements
849+
/// | element[2] | stored immediately after the object
850+
/// | ... |
851+
/// | element[Size-1] |
852+
/// +------------------+
853+
///
854+
/// The 'Rows' member serves as both the first element storage and the base
855+
/// address for pointer arithmetic to access subsequent elements.
856+
class ClusteredRows {
857+
public:
858+
ArrayRef<DebugLineTableRowRef> getRows() const {
859+
return ArrayRef<DebugLineTableRowRef>(beginPtrConst(), Size);
860+
}
861+
862+
/// Returns the number of elements in the array.
863+
uint64_t size() const { return Size; }
864+
865+
/// We re-purpose SMLoc inside MCInst to store the pointer
866+
/// to ClusteredRows. fromSMLoc() and toSMLoc() are helper
867+
/// functions to convert between SMLoc and ClusteredRows.
868+
869+
static const ClusteredRows *fromSMLoc(const SMLoc &Loc) {
870+
return reinterpret_cast<const ClusteredRows *>(Loc.getPointer());
871+
}
872+
SMLoc toSMLoc() const {
873+
return SMLoc::getFromPointer(reinterpret_cast<const char *>(this));
874+
}
875+
876+
/// Given a vector of DebugLineTableRowRef, this method
877+
/// copies the elements into pre-allocated memory.
878+
template <typename T> void populate(const T Vec) {
879+
assert(Vec.size() == Size && "Sizes must match");
880+
DebugLineTableRowRef *CurRawPtr = beginPtr();
881+
for (DebugLineTableRowRef RowRef : Vec) {
882+
*CurRawPtr = RowRef;
883+
++CurRawPtr;
884+
}
885+
}
886+
887+
private:
888+
uint64_t Size;
889+
DebugLineTableRowRef Rows;
890+
891+
ClusteredRows(uint64_t Size) : Size(Size) {}
892+
893+
/// Total size of the object including the array.
894+
static uint64_t getTotalSize(uint64_t Size) {
895+
assert(Size > 0 && "Size must be greater than 0");
896+
return sizeof(ClusteredRows) + (Size - 1) * sizeof(DebugLineTableRowRef);
897+
}
898+
const DebugLineTableRowRef *beginPtrConst() const {
899+
return reinterpret_cast<const DebugLineTableRowRef *>(&Rows);
900+
}
901+
DebugLineTableRowRef *beginPtr() {
902+
return reinterpret_cast<DebugLineTableRowRef *>(&Rows);
903+
}
904+
905+
friend class ClusteredRowsContainer;
906+
};
907+
908+
/// ClusteredRowsContainer manages the lifecycle of ClusteredRows objects.
909+
class ClusteredRowsContainer {
910+
public:
911+
ClusteredRows *createClusteredRows(uint64_t Size) {
912+
auto *CR = new (std::malloc(ClusteredRows::getTotalSize(Size)))
913+
ClusteredRows(Size);
914+
Clusters.push_back(CR);
915+
return CR;
916+
}
917+
~ClusteredRowsContainer() {
918+
for (auto *CR : Clusters)
919+
std::free(CR);
920+
}
921+
922+
private:
923+
std::vector<ClusteredRows *> Clusters;
924+
};
925+
855926
} // namespace bolt
856927
} // namespace llvm
857928

bolt/lib/Core/BinaryContext.cpp

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,22 +1693,39 @@ void BinaryContext::preprocessDebugInfo() {
16931693

16941694
auto It = llvm::partition_point(
16951695
AllRanges, [=](CURange R) { return R.HighPC <= FunctionAddress; });
1696-
if (It != AllRanges.end() && It->LowPC <= FunctionAddress)
1697-
Function.setDWARFUnit(It->Unit);
1696+
if (It == AllRanges.end() || It->LowPC > FunctionAddress) {
1697+
continue;
1698+
}
1699+
Function.addDWARFUnit(It->Unit);
1700+
1701+
// Go forward and add all units from ranges that cover the function.
1702+
while (++It != AllRanges.end()) {
1703+
if (It->LowPC > FunctionAddress || FunctionAddress >= It->HighPC)
1704+
break;
1705+
Function.addDWARFUnit(It->Unit);
1706+
}
16981707
}
16991708

17001709
// Discover units with debug info that needs to be updated.
17011710
for (const auto &KV : BinaryFunctions) {
17021711
const BinaryFunction &BF = KV.second;
1703-
if (shouldEmit(BF) && BF.getDWARFUnit())
1704-
ProcessedCUs.insert(BF.getDWARFUnit());
1712+
if (shouldEmit(BF) && !BF.getDWARFUnits().empty())
1713+
for (const auto &[_, Unit] : BF.getDWARFUnits())
1714+
ProcessedCUs.insert(Unit);
17051715
}
1706-
17071716
// Clear debug info for functions from units that we are not going to process.
17081717
for (auto &KV : BinaryFunctions) {
17091718
BinaryFunction &BF = KV.second;
1710-
if (BF.getDWARFUnit() && !ProcessedCUs.count(BF.getDWARFUnit()))
1711-
BF.setDWARFUnit(nullptr);
1719+
// Collect units to remove to avoid iterator invalidation
1720+
SmallVector<DWARFUnit *, 1> UnitsToRemove;
1721+
for (const auto &[_, Unit] : BF.getDWARFUnits()) {
1722+
if (!ProcessedCUs.count(Unit))
1723+
UnitsToRemove.push_back(Unit);
1724+
}
1725+
// Remove the collected units
1726+
for (auto *Unit : UnitsToRemove) {
1727+
BF.removeDWARFUnit(Unit);
1728+
}
17121729
}
17131730

17141731
if (opts::Verbosity >= 1) {
@@ -1903,23 +1920,23 @@ bool BinaryContext::isMarker(const SymbolRef &Symbol) const {
19031920
static void printDebugInfo(raw_ostream &OS, const MCInst &Instruction,
19041921
const BinaryFunction *Function,
19051922
DWARFContext *DwCtx) {
1906-
DebugLineTableRowRef RowRef =
1907-
DebugLineTableRowRef::fromSMLoc(Instruction.getLoc());
1908-
if (RowRef == DebugLineTableRowRef::NULL_ROW)
1923+
const ClusteredRows *LineTableRows =
1924+
ClusteredRows::fromSMLoc(Instruction.getLoc());
1925+
if (LineTableRows == nullptr)
19091926
return;
19101927

1911-
const DWARFDebugLine::LineTable *LineTable;
1912-
if (Function && Function->getDWARFUnit() &&
1913-
Function->getDWARFUnit()->getOffset() == RowRef.DwCompileUnitIndex) {
1914-
LineTable = Function->getDWARFLineTable();
1915-
} else {
1916-
LineTable = DwCtx->getLineTableForUnit(
1917-
DwCtx->getCompileUnitForOffset(RowRef.DwCompileUnitIndex));
1918-
}
1919-
assert(LineTable && "line table expected for instruction with debug info");
1928+
// File name and line number should be the same for all CUs.
1929+
// So it is sufficient to check the first one.
1930+
DebugLineTableRowRef RowRef = LineTableRows->getRows().front();
1931+
const DWARFDebugLine::LineTable *LineTable = DwCtx->getLineTableForUnit(
1932+
DwCtx->getCompileUnitForOffset(RowRef.DwCompileUnitIndex));
1933+
1934+
if (!LineTable)
1935+
return;
19201936

19211937
const DWARFDebugLine::Row &Row = LineTable->Rows[RowRef.RowIndex - 1];
19221938
StringRef FileName = "";
1939+
19231940
if (std::optional<const char *> FName =
19241941
dwarf::toString(LineTable->Prologue.getFileNameEntry(Row.File).Name))
19251942
FileName = *FName;

0 commit comments

Comments
 (0)