[DWARFLinker] Fix matching logic to remove type 1 missing offsets #149618
+592
−311
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… duplicated offsets
Signed-off-by: Peter Rong <[email protected]>
|
@llvm/pr-subscribers-debuginfo Author: Peter Rong (DataCorrupted) ChangesSecond attempt to fix #143656 Context: the sequence offset to row index we parsed may not be complete. And we need to add manual matching to it. #143656 attempts to do trivial 1:1 matching, however, sometimes they don't line up perfectly, as shown below: In this case, we need to use the mapping we read from the line table as a ground truth and organize them properly to prevent duplicated offset/missing offset. Test: Updated the test case Patch is 68.27 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149618.diff 2 Files Affected:
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
index 222dc88098102..572b2a252afc5 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -2297,8 +2297,129 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
// Create a map of stmt sequence offsets to original row indices.
DenseMap<uint64_t, unsigned> SeqOffToOrigRow;
- for (const DWARFDebugLine::Sequence &Seq : LT->Sequences)
- SeqOffToOrigRow[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
+ DenseMap<uint64_t, unsigned> LineTableMapping;
+ // The DWARF parser's discovery of sequences can be incomplete. To
+ // ensure all DW_AT_LLVM_stmt_sequence attributes can be patched, we
+ // build a map from both the parser's results and a manual
+ // reconstruction.
+ if (!LT->Rows.empty()) {
+ // First, trust the sequences that the DWARF parser did identify.
+ for (const DWARFDebugLine::Sequence &Seq : LT->Sequences) {
+ LineTableMapping[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
+ }
+
+ // Second, manually find sequence boundaries and match them to the
+ // sorted attributes to handle sequences the parser might have missed.
+ auto StmtAttrs = Unit.getStmtSeqListAttributes();
+ llvm::sort(StmtAttrs,
+ [](const PatchLocation &A, const PatchLocation &B) {
+ return A.get() < B.get();
+ });
+
+ std::vector<size_t> SeqStartRows;
+ SeqStartRows.push_back(0);
+ for (size_t i = 0; i < LT->Rows.size() - 1; ++i)
+ if (LT->Rows[i].EndSequence)
+ SeqStartRows.push_back(i + 1);
+
+ // While SeqOffToOrigRow parsed from CU could be the ground truth,
+ // e.g.
+ //
+ // SeqOff Row
+ // 0x08 9
+ // 0x14 15
+ //
+ // The StmtAttrs and SeqStartRows may not match perfectly, e.g.
+ //
+ // StmtAttrs SeqStartRows
+ // 0x04 3
+ // 0x08 5
+ // 0x10 9
+ // 0x12 11
+ // 0x14 15
+ //
+ // In this case, we don't want to assign 5 to 0x08, since we know 0x08
+ // maps to 9. If we do a dummy 1:1 mapping 0x10 will be mapped to 9
+ // which is incorrect. The expected behavior is ignore 5, realign the
+ // table based on the result from the line table:
+ //
+ // StmtAttrs SeqStartRows
+ // 0x04 3
+ // -- 5
+ // 0x08 9 <- LineTableMapping ground truth
+ // 0x10 11
+ // 0x12 --
+ // 0x14 15 <- LineTableMapping ground truth
+
+ // Dummy last element to make sure StmtAttrIdx and SeqStartIdx always
+ // run out first. Can't directly use TombstoneKey/TombstoneVal, that's
+ // preserved.
+ constexpr size_t DummyKey = UINT64_MAX - 2;
+ constexpr unsigned DummyVal = UINT32_MAX - 2;
+ LineTableMapping[DummyKey] = DummyVal;
+ SmallVector<uint64_t> SortedLineTableKeys(LineTableMapping.keys());
+ llvm::sort(SortedLineTableKeys);
+
+ size_t StmtAttrIdx = 0, SeqStartIdx = 0;
+ size_t NextSeqOff = 0;
+ unsigned NextRow = 0;
+
+ auto StmtIdxValidAndSmallerThanNext = [&]() {
+ return StmtAttrIdx < StmtAttrs.size() &&
+ StmtAttrs[StmtAttrIdx].get() < NextSeqOff;
+ };
+
+ auto SeqStartIdxValidAndSmallerThanNext = [&]() {
+ return SeqStartIdx < SeqStartRows.size() &&
+ SeqStartRows[SeqStartIdx] < NextRow;
+ };
+ for (size_t i = 0; i < SortedLineTableKeys.size(); ++i) {
+ NextSeqOff = SortedLineTableKeys[i];
+ NextRow = LineTableMapping[NextSeqOff];
+ // If both StmtAttrs and SeqStartRows points to value not in
+ // the LineTableMapping yet, we do a dummy one to one mapping and
+ // move the pointer.
+ while (StmtIdxValidAndSmallerThanNext() &&
+ SeqStartIdxValidAndSmallerThanNext()) {
+ SeqOffToOrigRow[StmtAttrs[StmtAttrIdx].get()] =
+ SeqStartRows[SeqStartIdx];
+ ++StmtAttrIdx;
+ ++SeqStartIdx;
+ }
+ // One of the pointer points to the value at or past Next in the
+ // LineTableMapping, We move the pointer to re-align with the
+ // LineTableMapping
+ while (StmtIdxValidAndSmallerThanNext()) {
+ ++StmtAttrIdx;
+ }
+ while (SeqStartIdxValidAndSmallerThanNext()) {
+ ++SeqStartIdx;
+ }
+ // Use the LineTableMapping's result as the ground truth and move
+ // on.
+ if (NextSeqOff != DummyKey) {
+ SeqOffToOrigRow[NextSeqOff] = NextRow;
+ }
+ // It is possible that the first StmtAttrIdx/SeqStartIdx point to
+ // later entries in LineTableMapping. Therefore we only increment
+ // the pointers after we validate they are pointing to the `Next`
+ // entry. e.g. LineTableMapping SeqOff Row 0x08 9 <-
+ // NextSeqOff/NextRow 0x14 15
+ //
+ // StmtAttrs SeqStartRows
+ // 0x14 13 <- StmtAttrIdx/SeqStartIdx
+ // 0x16 15
+ // -- 17
+ if (StmtAttrIdx < StmtAttrs.size() &&
+ StmtAttrs[StmtAttrIdx].get() == NextSeqOff) {
+ ++StmtAttrIdx;
+ }
+ if (SeqStartIdx < SeqStartRows.size() &&
+ SeqStartRows[SeqStartIdx] == NextRow) {
+ ++SeqStartIdx;
+ }
+ }
+ }
// Create a map of original row indices to new row indices.
DenseMap<size_t, size_t> OrigRowToNewRow;
diff --git a/llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test b/llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test
index f2fe794e1b484..db223cda43247 100644
--- a/llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test
+++ b/llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test
@@ -5,7 +5,13 @@
# RUN: yaml2obj %t/stmt_seq_macho.o.yaml -o %t/stmt_seq_macho.o
# RUN: dsymutil --flat --verify-dwarf=none -oso-prepend-path %t %t/stmt_seq_macho.exe -o %t/stmt_seq_macho.dSYM
# RUN: llvm-dwarfdump --debug-info --debug-line -v %t/stmt_seq_macho.dSYM | sort | FileCheck %s -check-prefix=CHECK_DSYM
+# RUN: llvm-dwarfdump --debug-info --debug-line -v %t/stmt_seq_macho.dSYM > %t/stmt_seq_macho.dSYM.txt
+# RUN: cat %t/stmt_seq_macho.dSYM.txt | sort | FileCheck %s -check-prefix=CHECK_DSYM
+# RUN: cat %t/stmt_seq_macho.dSYM.txt | FileCheck %s -check-prefix=CHECK_NO_INVALID_OFFSET
+# RUN: cat stmt_seq_macho.dSYM.txt | grep DW_AT_LLVM_stmt_sequence | sort | uniq -d | wc -l | FileCheck %s -check-prefix=CHECK_NO_DUPLICATES
+# CHECK_NO_DUPLICATES: 0
+# CHECK_NO_INVALID_OFFSET-NOT: DW_AT_LLVM_stmt_sequence{{.*}}0xfffffff
# CHECK_DSYM: DW_AT_LLVM_stmt_sequence [DW_FORM_sec_offset] ([[OFFSET1:(0x[0-9a-f]+)]])
# CHECK_DSYM: DW_AT_LLVM_stmt_sequence [DW_FORM_sec_offset] ([[OFFSET2:(0x[0-9a-f]+)]])
# CHECK_DSYM: DW_AT_LLVM_stmt_sequence [DW_FORM_sec_offset] ([[OFFSET3:(0x[0-9a-f]+)]])
@@ -18,6 +24,9 @@
#--- stmt_seq_macho.cpp
#define ATTRIB extern "C" __attribute__((noinline))
+ATTRIB int function1_copy1(int a) {
+ return ++a;
+}
ATTRIB int function3_copy1(int a) {
int b = a + 3;
@@ -51,6 +60,7 @@ int main() {
sum += function2_copy2(3);
sum += function3_copy2(41);
sum += function2_copy1(11);
+ sum += function1_copy1(42);
length_error e("test");
return sum;
}
@@ -108,9 +118,9 @@ LoadCommands:
cmdsize: 1032
segname: ''
vmaddr: 0
- vmsize: 2793
+ vmsize: 3125
fileoff: 1208
- filesize: 2793
+ filesize: 3125
maxprot: 7
initprot: 7
nsects: 12
@@ -119,18 +129,18 @@ LoadCommands:
- sectname: __text
segname: __TEXT
addr: 0x0
- size: 128
+ size: 148
offset: 0x4B8
align: 2
- reloff: 0xFA8
- nreloc: 7
+ reloff: 0x10F0
+ nreloc: 8
flags: 0x80000400
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
- content: 00100011C0035FD600580051C0035FD600100011C0035FD600580051C0035FD6FFC300D1F44F01A9FD7B02A9FD8300916000805200000094F30300AA20058052000000941400130B6001805200000094F30300AA0100009021000091E03F0091000000948002130BFD7B42A9F44F41A9FFC30091C0035FD600000014C0035FD6
+ content: 00040011C0035FD600100011C0035FD600580051C0035FD600100011C0035FD600580051C0035FD6FFC300D1F44F01A9FD7B02A9FD8300916000805200000094F30300AA20058052000000941400130B6001805200000094F30300AA40058052000000947302000B0100009021000091E03F0091000000948002130BFD7B42A9F44F41A9FFC30091C0035FD600000014C0035FD6
relocations:
- - address: 0x78
+ - address: 0x8C
symbolnum: 4
pcrel: true
length: 2
@@ -138,7 +148,7 @@ LoadCommands:
type: 2
scattered: false
value: 0
- - address: 0x60
+ - address: 0x74
symbolnum: 3
pcrel: true
length: 2
@@ -146,7 +156,7 @@ LoadCommands:
type: 2
scattered: false
value: 0
- - address: 0x58
+ - address: 0x6C
symbolnum: 1
pcrel: false
length: 2
@@ -154,7 +164,7 @@ LoadCommands:
type: 4
scattered: false
value: 0
- - address: 0x54
+ - address: 0x68
symbolnum: 1
pcrel: true
length: 2
@@ -162,7 +172,7 @@ LoadCommands:
type: 3
scattered: false
value: 0
- - address: 0x4C
+ - address: 0x60
symbolnum: 5
pcrel: true
length: 2
@@ -170,16 +180,24 @@ LoadCommands:
type: 2
scattered: false
value: 0
- - address: 0x40
- symbolnum: 8
+ - address: 0x54
+ symbolnum: 6
pcrel: true
length: 2
extern: true
type: 2
scattered: false
value: 0
- - address: 0x34
- symbolnum: 6
+ - address: 0x48
+ symbolnum: 9
+ pcrel: true
+ length: 2
+ extern: true
+ type: 2
+ scattered: false
+ value: 0
+ - address: 0x3C
+ symbolnum: 7
pcrel: true
length: 2
extern: true
@@ -188,9 +206,9 @@ LoadCommands:
value: 0
- sectname: __cstring
segname: __TEXT
- addr: 0x80
+ addr: 0x94
size: 5
- offset: 0x538
+ offset: 0x54C
align: 0
reloff: 0x0
nreloc: 0
@@ -201,9 +219,9 @@ LoadCommands:
content: '7465737400'
- sectname: __debug_loc
segname: __DWARF
- addr: 0x85
+ addr: 0x99
size: 412
- offset: 0x53D
+ offset: 0x551
align: 0
reloff: 0x0
nreloc: 0
@@ -211,12 +229,12 @@ LoadCommands:
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
- content: 00000000000000000400000000000000010050040000000000000008000000000000000400A301509F0000000000000000000000000000000000000000000000000400000000000000030070039F0000000000000000000000000000000008000000000000000C000000000000000100500C0000000000000010000000000000000400A301509F0000000000000000000000000000000010000000000000001400000000000000010050140000000000000018000000000000000400A301509F0000000000000000000000000000000010000000000000001400000000000000030070039F0000000000000000000000000000000018000000000000001C000000000000000100501C0000000000000020000000000000000400A301509F000000000000000000000000000000001C0000000000000020000000000000000100500000000000000000000000000000000030000000000000003C00000000000000030011009F3C0000000000000048000000000000000100634800000000000000540000000000000001006400000000000000000000000000000000
+ content: 08000000000000000C000000000000000100500C0000000000000010000000000000000400A301509F0000000000000000000000000000000008000000000000000C00000000000000030070039F0000000000000000000000000000000010000000000000001400000000000000010050140000000000000018000000000000000400A301509F0000000000000000000000000000000018000000000000001C000000000000000100501C0000000000000020000000000000000400A301509F0000000000000000000000000000000018000000000000001C00000000000000030070039F0000000000000000000000000000000020000000000000002400000000000000010050240000000000000028000000000000000400A301509F00000000000000000000000000000000240000000000000028000000000000000100500000000000000000000000000000000038000000000000004400000000000000030011009F4400000000000000500000000000000001006350000000000000005C0000000000000001006400000000000000000000000000000000
- sectname: __debug_abbrev
segname: __DWARF
- addr: 0x221
- size: 359
- offset: 0x6D9
+ addr: 0x235
+ size: 372
+ offset: 0x6ED
align: 0
reloff: 0x0
nreloc: 0
@@ -226,18 +244,34 @@ LoadCommands:
reserved3: 0x0
- sectname: __debug_info
segname: __DWARF
- addr: 0x388
- size: 686
- offset: 0x840
+ addr: 0x3A9
+ size: 747
+ offset: 0x861
align: 0
- reloff: 0xFE0
- nreloc: 14
+ reloff: 0x1130
+ nreloc: 16
flags: 0x2000000
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
relocations:
- - address: 0x26A
+ - address: 0x2A7
+ symbolnum: 1
+ pcrel: false
+ length: 3
+ extern: false
+ type: 0
+ scattered: false
+ value: 0
+ - address: 0x28E
+ symbolnum: 1
+ pcrel: false
+ length: 3
+ extern: false
+ type: 0
+ scattered: false
+ value: 0
+ - address: 0x253
symbolnum: 1
pcrel: false
length: 3
@@ -245,7 +279,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x251
+ - address: 0x1F5
symbolnum: 1
pcrel: false
length: 3
@@ -253,7 +287,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x216
+ - address: 0x1E1
symbolnum: 1
pcrel: false
length: 3
@@ -261,7 +295,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1B8
+ - address: 0x1CE
symbolnum: 1
pcrel: false
length: 3
@@ -269,7 +303,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1A5
+ - address: 0x1BA
symbolnum: 1
pcrel: false
length: 3
@@ -277,7 +311,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x191
+ - address: 0x1A7
symbolnum: 1
pcrel: false
length: 3
@@ -285,7 +319,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x17E
+ - address: 0x169
symbolnum: 1
pcrel: false
length: 3
@@ -293,7 +327,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x140
+ - address: 0x12D
symbolnum: 1
pcrel: false
length: 3
@@ -301,7 +335,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x104
+ - address: 0xF1
symbolnum: 1
pcrel: false
length: 3
@@ -309,7 +343,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0xC8
+ - address: 0xC4
symbolnum: 1
pcrel: false
length: 3
@@ -317,7 +351,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x9B
+ - address: 0x88
symbolnum: 1
pcrel: false
length: 3
@@ -351,9 +385,9 @@ LoadCommands:
value: 0
- sectname: __debug_str
segname: __DWARF
- addr: 0x636
- size: 239
- offset: 0xAEE
+ addr: 0x694
+ size: 400
+ offset: 0xB4C
align: 0
reloff: 0x0
nreloc: 0
@@ -363,9 +397,9 @@ LoadCommands:
reserved3: 0x0
- sectname: __apple_names
segname: __DWARF
- addr: 0x725
- size: 260
- offset: 0xBDD
+ addr: 0x824
+ size: 288
+ offset: 0xCDC
align: 0
reloff: 0x0
nreloc: 0
@@ -373,12 +407,12 @@ LoadCommands:
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
- content: 485341480100000008000000080000000C000000000000000100000001000600000000000200000005000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF90D9F86F88CB36CF4908311CD1125E5389CB36CF4A08311C522B70536A7F9A7C8000000094000000A4000000B4000000C4000000D4000000E4000000F40000008A0000000200000015020000690200000000000055000000010000009A0000000000000045000000010000005E00000000000000A3000000010000001502000000000000750000000100000003010000...
[truncated]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Second attempt to fix https://discourse.llvm.org/t/rfc-new-dwarf-attribute-for-symbolication-of-merged-functions/79434/29?u=alx32
(First attempt: #143656)
Context: the sequence offset to row index we parsed may not be complete. And we need to add manual matching to it.
#143656 attempts to do trivial 1:1 matching, however, sometimes they don't line up perfectly, as shown below:
In this case, we need to use the mapping we read from the line table as a ground truth and organize them properly to prevent duplicated offset/missing offset.
Test:
Updated the test case