Skip to content

Commit aa89b84

Browse files
DataCorruptedDataCorrupted
committed
Address reviewer's comment
1 parent 54a86d7 commit aa89b84

File tree

1 file changed

+128
-123
lines changed

1 file changed

+128
-123
lines changed

‎llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp‎

Lines changed: 128 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,132 @@ static bool isTlsAddressCode(uint8_t DW_OP_Code) {
413413
DW_OP_Code == dwarf::DW_OP_GNU_push_tls_address;
414414
}
415415

416+
static void constructSeqOffsettoOrigRowMapping(
417+
CompileUnit &Unit, const DWARFDebugLine::LineTable *LT,
418+
DenseMap<size_t, unsigned> &SeqOffToOrigRow) {
419+
420+
assert(LT && "Line table is null");
421+
422+
// Use std::map for ordered iteration.
423+
std::map<uint64_t, unsigned> LineTableMapping;
424+
425+
// First, trust the sequences that the DWARF parser did identify.
426+
for (const DWARFDebugLine::Sequence &Seq : LT->Sequences)
427+
LineTableMapping[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
428+
429+
// Second, manually find sequence boundaries and match them to the
430+
// sorted attributes to handle sequences the parser might have missed.
431+
auto StmtAttrs = Unit.getStmtSeqListAttributes();
432+
llvm::sort(StmtAttrs, [](const PatchLocation &A, const PatchLocation &B) {
433+
return A.get() < B.get();
434+
});
435+
436+
std::vector<size_t> SeqStartRows;
437+
SeqStartRows.push_back(0);
438+
for (size_t I = 0; I < LT->Rows.size() - 1; ++I)
439+
if (LT->Rows[I].EndSequence)
440+
SeqStartRows.push_back(I + 1);
441+
442+
// While SeqOffToOrigRow parsed from CU could be the ground truth,
443+
// e.g.
444+
//
445+
// SeqOff Row
446+
// 0x08 9
447+
// 0x14 15
448+
//
449+
// The StmtAttrs and SeqStartRows may not match perfectly, e.g.
450+
//
451+
// StmtAttrs SeqStartRows
452+
// 0x04 3
453+
// 0x08 5
454+
// 0x10 9
455+
// 0x12 11
456+
// 0x14 15
457+
//
458+
// In this case, we don't want to assign 5 to 0x08, since we know 0x08
459+
// maps to 9. If we do a dummy 1:1 mapping 0x10 will be mapped to 9
460+
// which is incorrect. The expected behavior is ignore 5, realign the
461+
// table based on the result from the line table:
462+
//
463+
// StmtAttrs SeqStartRows
464+
// 0x04 3
465+
// -- 5
466+
// 0x08 9 <- LineTableMapping ground truth
467+
// 0x10 11
468+
// 0x12 --
469+
// 0x14 15 <- LineTableMapping ground truth
470+
471+
// Dummy last element to make sure StmtAttrIdx and SeqStartIdx always
472+
// run out first.
473+
constexpr size_t DummyKey = UINT64_MAX;
474+
constexpr unsigned DummyVal = UINT32_MAX;
475+
LineTableMapping[DummyKey] = DummyVal;
476+
477+
size_t StmtAttrIdx = 0, SeqStartIdx = 0;
478+
size_t NextSeqOff = 0;
479+
unsigned NextRow = 0;
480+
481+
auto StmtIdxValidAndSmallerThanNext = [&]() {
482+
return StmtAttrIdx < StmtAttrs.size() &&
483+
StmtAttrs[StmtAttrIdx].get() < NextSeqOff;
484+
};
485+
486+
auto SeqStartIdxValidAndSmallerThanNext = [&]() {
487+
return SeqStartIdx < SeqStartRows.size() &&
488+
SeqStartRows[SeqStartIdx] < NextRow;
489+
};
490+
491+
for (auto It : LineTableMapping) {
492+
// More verbosed setup to make sure closure capture works.
493+
NextSeqOff = It.first;
494+
NextRow = It.second;
495+
496+
// If both StmtAttrs and SeqStartRows points to value not in
497+
// the LineTableMapping yet, we do a dummy one to one mapping and
498+
// move the pointer.
499+
while (StmtIdxValidAndSmallerThanNext() &&
500+
SeqStartIdxValidAndSmallerThanNext()) {
501+
SeqOffToOrigRow[StmtAttrs[StmtAttrIdx].get()] = SeqStartRows[SeqStartIdx];
502+
++StmtAttrIdx;
503+
++SeqStartIdx;
504+
}
505+
// One of the pointer points to the value at or past Next in the
506+
// LineTableMapping, We move the pointer to re-align with the
507+
// LineTableMapping
508+
while (StmtIdxValidAndSmallerThanNext()) {
509+
++StmtAttrIdx;
510+
}
511+
while (SeqStartIdxValidAndSmallerThanNext()) {
512+
++SeqStartIdx;
513+
}
514+
// Use the LineTableMapping's result as the ground truth and move
515+
// on.
516+
if (NextSeqOff != DummyKey) {
517+
SeqOffToOrigRow[NextSeqOff] = NextRow;
518+
}
519+
// It is possible that the first StmtAttrIdx/SeqStartIdx point to
520+
// later entries in LineTableMapping. Therefore we only increment
521+
// the pointers after we validate they are pointing to the `Next`
522+
// entry. e.g.
523+
//
524+
// LineTableMapping
525+
// SeqOff Row
526+
// 0x08 9 <- NextSeqOff/NextRow
527+
// 0x14 15
528+
//
529+
// StmtAttrs SeqStartRows
530+
// 0x14 13 <- StmtAttrIdx/SeqStartIdx
531+
// 0x16 15
532+
// -- 17
533+
if (StmtAttrIdx < StmtAttrs.size() &&
534+
StmtAttrs[StmtAttrIdx].get() == NextSeqOff)
535+
++StmtAttrIdx;
536+
if (SeqStartIdx < SeqStartRows.size() &&
537+
SeqStartRows[SeqStartIdx] == NextRow)
538+
++SeqStartIdx;
539+
}
540+
}
541+
416542
std::pair<bool, std::optional<int64_t>>
417543
DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
418544
const DWARFDie &DIE) {
@@ -2297,133 +2423,12 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
22972423

22982424
// Create a map of stmt sequence offsets to original row indices.
22992425
DenseMap<uint64_t, unsigned> SeqOffToOrigRow;
2300-
DenseMap<uint64_t, unsigned> LineTableMapping;
23012426
// The DWARF parser's discovery of sequences can be incomplete. To
23022427
// ensure all DW_AT_LLVM_stmt_sequence attributes can be patched, we
23032428
// build a map from both the parser's results and a manual
23042429
// reconstruction.
2305-
if (!LT->Rows.empty()) {
2306-
// First, trust the sequences that the DWARF parser did identify.
2307-
for (const DWARFDebugLine::Sequence &Seq : LT->Sequences) {
2308-
LineTableMapping[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
2309-
}
2310-
2311-
// Second, manually find sequence boundaries and match them to the
2312-
// sorted attributes to handle sequences the parser might have missed.
2313-
auto StmtAttrs = Unit.getStmtSeqListAttributes();
2314-
llvm::sort(StmtAttrs,
2315-
[](const PatchLocation &A, const PatchLocation &B) {
2316-
return A.get() < B.get();
2317-
});
2318-
2319-
std::vector<size_t> SeqStartRows;
2320-
SeqStartRows.push_back(0);
2321-
for (size_t i = 0; i < LT->Rows.size() - 1; ++i)
2322-
if (LT->Rows[i].EndSequence)
2323-
SeqStartRows.push_back(i + 1);
2324-
2325-
// While SeqOffToOrigRow parsed from CU could be the ground truth,
2326-
// e.g.
2327-
//
2328-
// SeqOff Row
2329-
// 0x08 9
2330-
// 0x14 15
2331-
//
2332-
// The StmtAttrs and SeqStartRows may not match perfectly, e.g.
2333-
//
2334-
// StmtAttrs SeqStartRows
2335-
// 0x04 3
2336-
// 0x08 5
2337-
// 0x10 9
2338-
// 0x12 11
2339-
// 0x14 15
2340-
//
2341-
// In this case, we don't want to assign 5 to 0x08, since we know 0x08
2342-
// maps to 9. If we do a dummy 1:1 mapping 0x10 will be mapped to 9
2343-
// which is incorrect. The expected behavior is ignore 5, realign the
2344-
// table based on the result from the line table:
2345-
//
2346-
// StmtAttrs SeqStartRows
2347-
// 0x04 3
2348-
// -- 5
2349-
// 0x08 9 <- LineTableMapping ground truth
2350-
// 0x10 11
2351-
// 0x12 --
2352-
// 0x14 15 <- LineTableMapping ground truth
2353-
2354-
// Dummy last element to make sure StmtAttrIdx and SeqStartIdx always
2355-
// run out first. Can't directly use TombstoneKey/TombstoneVal, that's
2356-
// preserved.
2357-
constexpr size_t DummyKey = UINT64_MAX - 2;
2358-
constexpr unsigned DummyVal = UINT32_MAX - 2;
2359-
LineTableMapping[DummyKey] = DummyVal;
2360-
SmallVector<uint64_t> SortedLineTableKeys(LineTableMapping.keys());
2361-
llvm::sort(SortedLineTableKeys);
2362-
2363-
size_t StmtAttrIdx = 0, SeqStartIdx = 0;
2364-
size_t NextSeqOff = 0;
2365-
unsigned NextRow = 0;
2366-
2367-
auto StmtIdxValidAndSmallerThanNext = [&]() {
2368-
return StmtAttrIdx < StmtAttrs.size() &&
2369-
StmtAttrs[StmtAttrIdx].get() < NextSeqOff;
2370-
};
2371-
2372-
auto SeqStartIdxValidAndSmallerThanNext = [&]() {
2373-
return SeqStartIdx < SeqStartRows.size() &&
2374-
SeqStartRows[SeqStartIdx] < NextRow;
2375-
};
2376-
for (size_t i = 0; i < SortedLineTableKeys.size(); ++i) {
2377-
NextSeqOff = SortedLineTableKeys[i];
2378-
NextRow = LineTableMapping[NextSeqOff];
2379-
// If both StmtAttrs and SeqStartRows points to value not in
2380-
// the LineTableMapping yet, we do a dummy one to one mapping and
2381-
// move the pointer.
2382-
while (StmtIdxValidAndSmallerThanNext() &&
2383-
SeqStartIdxValidAndSmallerThanNext()) {
2384-
SeqOffToOrigRow[StmtAttrs[StmtAttrIdx].get()] =
2385-
SeqStartRows[SeqStartIdx];
2386-
++StmtAttrIdx;
2387-
++SeqStartIdx;
2388-
}
2389-
// One of the pointer points to the value at or past Next in the
2390-
// LineTableMapping, We move the pointer to re-align with the
2391-
// LineTableMapping
2392-
while (StmtIdxValidAndSmallerThanNext()) {
2393-
++StmtAttrIdx;
2394-
}
2395-
while (SeqStartIdxValidAndSmallerThanNext()) {
2396-
++SeqStartIdx;
2397-
}
2398-
// Use the LineTableMapping's result as the ground truth and move
2399-
// on.
2400-
if (NextSeqOff != DummyKey) {
2401-
SeqOffToOrigRow[NextSeqOff] = NextRow;
2402-
}
2403-
// It is possible that the first StmtAttrIdx/SeqStartIdx point to
2404-
// later entries in LineTableMapping. Therefore we only increment
2405-
// the pointers after we validate they are pointing to the `Next`
2406-
// entry. e.g.
2407-
//
2408-
// LineTableMapping
2409-
// SeqOff Row
2410-
// 0x08 9 <- NextSeqOff/NextRow
2411-
// 0x14 15
2412-
//
2413-
// StmtAttrs SeqStartRows
2414-
// 0x14 13 <- StmtAttrIdx/SeqStartIdx
2415-
// 0x16 15
2416-
// -- 17
2417-
if (StmtAttrIdx < StmtAttrs.size() &&
2418-
StmtAttrs[StmtAttrIdx].get() == NextSeqOff) {
2419-
++StmtAttrIdx;
2420-
}
2421-
if (SeqStartIdx < SeqStartRows.size() &&
2422-
SeqStartRows[SeqStartIdx] == NextRow) {
2423-
++SeqStartIdx;
2424-
}
2425-
}
2426-
}
2430+
if (!LT->Rows.empty())
2431+
constructSeqOffsettoOrigRowMapping(Unit, LT, SeqOffToOrigRow);
24272432

24282433
// Create a map of original row indices to new row indices.
24292434
DenseMap<size_t, size_t> OrigRowToNewRow;

0 commit comments

Comments
 (0)