-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[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
base: main
Are you sure you want to change the base?
Changes from all commits
74747d1
885937d
abcd695
9391b3a
41edb2b
44bf8bb
04663a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -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) | ||
|
There was a problem hiding this comment.
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?