Skip to content

[BOLT] Fix debug line emission for functions in multiple compilation units #151230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions bolt/include/bolt/Core/BinaryContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,12 @@ class BinaryContext {
/// overwritten, but it is okay to re-generate debug info for them.
std::set<const DWARFUnit *> ProcessedCUs;

/// DWARF-related container to manage lifecycle of groups of rows from line
/// tables associated with instructions. Since binary functions can span
/// multiple compilation units, instructions may reference debug line
/// information from multiple CUs.
ClusteredRowsContainer ClusteredRows;

// Setup MCPlus target builder
void initializeTarget(std::unique_ptr<MCPlusBuilder> TargetBuilder) {
MIB = std::move(TargetBuilder);
Expand Down
27 changes: 18 additions & 9 deletions bolt/include/bolt/Core/BinaryFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ class BinaryFunction {
/// Original LSDA type encoding
unsigned LSDATypeEncoding{dwarf::DW_EH_PE_omit};

/// Containing compilation unit for the function.
DWARFUnit *DwarfUnit{nullptr};
/// All compilation units this function belongs to.
SmallVector<DWARFUnit *, 1> DwarfUnitVec;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DenseSet, if they don't need to be ordered?


/// Last computed hash value. Note that the value could be recomputed using
/// different parameters by every pass.
Expand Down Expand Up @@ -2414,15 +2414,24 @@ class BinaryFunction {
void
computeBlockHashes(HashFunction HashFunction = HashFunction::Default) const;

void setDWARFUnit(DWARFUnit *Unit) { DwarfUnit = Unit; }
void addDWARFUnit(DWARFUnit *Unit) { DwarfUnitVec.push_back(Unit); }

/// Return DWARF compile unit for this function.
DWARFUnit *getDWARFUnit() const { return DwarfUnit; }
void removeDWARFUnit(DWARFUnit *Unit) {
auto *It = std::find(DwarfUnitVec.begin(), DwarfUnitVec.end(), Unit);
// If found, erase it
if (It != DwarfUnitVec.end()) {
DwarfUnitVec.erase(It);
}
}

/// Return DWARF compile units for this function.
const SmallVector<DWARFUnit *, 1>& getDWARFUnits() const {
return DwarfUnitVec;
}

/// Return line info table for this function.
const DWARFDebugLine::LineTable *getDWARFLineTable() const {
return getDWARFUnit() ? BC.DwCtx->getLineTableForUnit(getDWARFUnit())
: nullptr;
const DWARFDebugLine::LineTable *
getDWARFLineTableForUnit(DWARFUnit *Unit) const {
return BC.DwCtx->getLineTableForUnit(Unit);
}

/// Finalize profile for the function.
Expand Down
102 changes: 81 additions & 21 deletions bolt/include/bolt/Core/DebugData.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,6 @@ struct DebugLineTableRowRef {
uint32_t DwCompileUnitIndex;
uint32_t RowIndex;

const static DebugLineTableRowRef NULL_ROW;

bool operator==(const DebugLineTableRowRef &Rhs) const {
return DwCompileUnitIndex == Rhs.DwCompileUnitIndex &&
RowIndex == Rhs.RowIndex;
Expand All @@ -145,24 +143,6 @@ struct DebugLineTableRowRef {
bool operator!=(const DebugLineTableRowRef &Rhs) const {
return !(*this == Rhs);
}

static DebugLineTableRowRef fromSMLoc(const SMLoc &Loc) {
union {
decltype(Loc.getPointer()) Ptr;
DebugLineTableRowRef Ref;
} U;
U.Ptr = Loc.getPointer();
return U.Ref;
}

SMLoc toSMLoc() const {
union {
decltype(SMLoc().getPointer()) Ptr;
DebugLineTableRowRef Ref;
} U;
U.Ref = *this;
return SMLoc::getFromPointer(U.Ptr);
}
};

/// Common buffer vector used for debug info handling.
Expand Down Expand Up @@ -210,7 +190,7 @@ class DebugRangesSectionWriter {
static bool classof(const DebugRangesSectionWriter *Writer) {
return Writer->getKind() == RangesWriterKind::DebugRangesWriter;
}

/// Append a range to the main buffer.
void appendToRangeBuffer(const DebugBufferVector &CUBuffer);

Expand Down Expand Up @@ -852,6 +832,86 @@ class DwarfLineTable {
// Returns DWARF Version for this line table.
uint16_t getDwarfVersion() const { return DwarfVersion; }
};

/// ClusteredRows represents a collection of debug line table row references.
///
/// MEMORY LAYOUT AND DESIGN:
/// This class uses a flexible array member pattern to store all
/// DebugLineTableRowRef elements in a single contiguous memory allocation.
/// The memory layout is:
///
/// +------------------+
/// | ClusteredRows | <- Object header (Size + first element)
/// | - Size |
/// | - Rows (element) | <- First DebugLineTableRowRef element
/// +------------------+
/// | element[1] | <- Additional DebugLineTableRowRef elements
/// | element[2] | stored immediately after the object
/// | ... |
/// | element[Size-1] |
/// +------------------+
///
/// The 'Rows' member serves as both the first element storage and the base
/// address for pointer arithmetic to access subsequent elements.
class ClusteredRows {
public:
ArrayRef<DebugLineTableRowRef> getRows() const {
return ArrayRef<DebugLineTableRowRef>(beginPtrConst(), Size);
}
uint64_t size() const { return Size; }
static const ClusteredRows *fromSMLoc(const SMLoc &Loc) {
return reinterpret_cast<const ClusteredRows *>(Loc.getPointer());
}
SMLoc toSMLoc() const {
return SMLoc::getFromPointer(reinterpret_cast<const char *>(this));
}

template <typename T> void populate(const T Vec) {
assert(Vec.size() == Size && "");
DebugLineTableRowRef *CurRawPtr = beginPtr();
for (DebugLineTableRowRef RowRef : Vec) {
*CurRawPtr = RowRef;
++CurRawPtr;
}
}

private:
uint64_t Size;
DebugLineTableRowRef Rows;

ClusteredRows(uint64_t Size) : Size(Size) {}
static uint64_t getTotalSize(uint64_t Size) {
assert(Size > 0 && "Size must be greater than 0");
return sizeof(ClusteredRows) + (Size - 1) * sizeof(DebugLineTableRowRef);
}
const DebugLineTableRowRef *beginPtrConst() const {
return reinterpret_cast<const DebugLineTableRowRef *>(&Rows);
}
DebugLineTableRowRef *beginPtr() {
return reinterpret_cast<DebugLineTableRowRef *>(&Rows);
}

friend class ClusteredRowsContainer;
};

/// ClusteredRowsContainer manages the lifecycle of ClusteredRows objects.
class ClusteredRowsContainer {
public:
ClusteredRows *createClusteredRows(uint64_t Size) {
auto *CR = new (std::malloc(ClusteredRows::getTotalSize(Size)))
ClusteredRows(Size);
Clusters.push_back(CR);
return CR;
}
~ClusteredRowsContainer() {
for (auto *CR : Clusters)
std::free(CR);
}

private:
std::vector<ClusteredRows *> Clusters;
};

} // namespace bolt
} // namespace llvm

Expand Down
73 changes: 44 additions & 29 deletions bolt/lib/Core/BinaryContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1568,23 +1568,19 @@ unsigned BinaryContext::addDebugFilenameToUnit(const uint32_t DestCUID,
DWARFCompileUnit *SrcUnit = DwCtx->getCompileUnitForOffset(SrcCUID);
const DWARFDebugLine::LineTable *LineTable =
DwCtx->getLineTableForUnit(SrcUnit);
const std::vector<DWARFDebugLine::FileNameEntry> &FileNames =
LineTable->Prologue.FileNames;
const DWARFDebugLine::FileNameEntry &FileNameEntry =
LineTable->Prologue.getFileNameEntry(FileIndex);
Comment on lines +1571 to +1572
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe some of the refactoring changes could be separate PRs that come before your big change?
Like this example here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created refactoring PR: #151401

// Dir indexes start at 1, as DWARF file numbers, and a dir index 0
// means empty dir.
assert(FileIndex > 0 && FileIndex <= FileNames.size() &&
"FileIndex out of range for the compilation unit.");
StringRef Dir = "";
if (FileNames[FileIndex - 1].DirIdx != 0) {
if (FileNameEntry.DirIdx != 0) {
if (std::optional<const char *> DirName = dwarf::toString(
LineTable->Prologue
.IncludeDirectories[FileNames[FileIndex - 1].DirIdx - 1])) {
LineTable->Prologue.IncludeDirectories[FileNameEntry.DirIdx - 1])) {
Dir = *DirName;
}
}
StringRef FileName = "";
if (std::optional<const char *> FName =
dwarf::toString(FileNames[FileIndex - 1].Name))
if (std::optional<const char *> FName = dwarf::toString(FileNameEntry.Name))
FileName = *FName;
assert(FileName != "");
DWARFCompileUnit *DstUnit = DwCtx->getCompileUnitForOffset(DestCUID);
Expand Down Expand Up @@ -1697,22 +1693,41 @@ void BinaryContext::preprocessDebugInfo() {

auto It = llvm::partition_point(
AllRanges, [=](CURange R) { return R.HighPC <= FunctionAddress; });
if (It != AllRanges.end() && It->LowPC <= FunctionAddress)
Function.setDWARFUnit(It->Unit);
if (It == AllRanges.end() || It->LowPC > FunctionAddress) {
continue;
}
Function.addDWARFUnit(It->Unit);

// Go forward and add all units from ranges that cover the function
while (++It != AllRanges.end()) {
if (It->LowPC <= FunctionAddress && FunctionAddress < It->HighPC) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplify logic

Function.addDWARFUnit(It->Unit);
} else {
break;
}
}
}

// Discover units with debug info that needs to be updated.
for (const auto &KV : BinaryFunctions) {
const BinaryFunction &BF = KV.second;
if (shouldEmit(BF) && BF.getDWARFUnit())
ProcessedCUs.insert(BF.getDWARFUnit());
if (shouldEmit(BF) && !BF.getDWARFUnits().empty())
for (const DWARFUnit *Unit : BF.getDWARFUnits())
ProcessedCUs.insert(Unit);
}

// Clear debug info for functions from units that we are not going to process.
for (auto &KV : BinaryFunctions) {
BinaryFunction &BF = KV.second;
if (BF.getDWARFUnit() && !ProcessedCUs.count(BF.getDWARFUnit()))
BF.setDWARFUnit(nullptr);
// Collect units to remove to avoid iterator invalidation
SmallVector<DWARFUnit *, 1> UnitsToRemove;
for (auto *Unit : BF.getDWARFUnits()) {
if (!ProcessedCUs.count(Unit))
UnitsToRemove.push_back(Unit);
}
// Remove the collected units
for (auto *Unit : UnitsToRemove) {
BF.removeDWARFUnit(Unit);
}
}

if (opts::Verbosity >= 1) {
Expand Down Expand Up @@ -1907,25 +1922,25 @@ bool BinaryContext::isMarker(const SymbolRef &Symbol) const {
static void printDebugInfo(raw_ostream &OS, const MCInst &Instruction,
const BinaryFunction *Function,
DWARFContext *DwCtx) {
DebugLineTableRowRef RowRef =
DebugLineTableRowRef::fromSMLoc(Instruction.getLoc());
if (RowRef == DebugLineTableRowRef::NULL_ROW)
const ClusteredRows *LineTableRows =
ClusteredRows::fromSMLoc(Instruction.getLoc());
if (LineTableRows == nullptr)
return;

const DWARFDebugLine::LineTable *LineTable;
if (Function && Function->getDWARFUnit() &&
Function->getDWARFUnit()->getOffset() == RowRef.DwCompileUnitIndex) {
LineTable = Function->getDWARFLineTable();
} else {
LineTable = DwCtx->getLineTableForUnit(
DwCtx->getCompileUnitForOffset(RowRef.DwCompileUnitIndex));
}
assert(LineTable && "line table expected for instruction with debug info");
// File name and line number should be the same for all CUs.
// So it is sufficient to check the first one.
DebugLineTableRowRef RowRef = LineTableRows->getRows().front();
const DWARFDebugLine::LineTable *LineTable = DwCtx->getLineTableForUnit(
DwCtx->getCompileUnitForOffset(RowRef.DwCompileUnitIndex));

if (!LineTable)
return;

const DWARFDebugLine::Row &Row = LineTable->Rows[RowRef.RowIndex - 1];
StringRef FileName = "";

if (std::optional<const char *> FName =
dwarf::toString(LineTable->Prologue.FileNames[Row.File - 1].Name))
dwarf::toString(LineTable->Prologue.getFileNameEntry(Row.File).Name))
FileName = *FName;
OS << " # debug line " << FileName << ":" << Row.Line;
if (Row.Column)
Expand Down
Loading
Loading