Revert "[DWARFLinker] Fix matching logic to remove type 1 missing off… #151424
+311
−585
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
…sets (llvm#149618)" This reverts commit ed940d7.
|
@llvm/pr-subscribers-debuginfo Author: Peter Rong (DataCorrupted) Changes…sets (#149618)" This reverts commit ed940d7. Patch is 67.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/151424.diff 2 Files Affected:
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
index f4c2f603cd109..222dc88098102 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -413,116 +413,6 @@ static bool isTlsAddressCode(uint8_t DW_OP_Code) {
DW_OP_Code == dwarf::DW_OP_GNU_push_tls_address;
}
-static void constructSeqOffsettoOrigRowMapping(
- CompileUnit &Unit, const DWARFDebugLine::LineTable <,
- DenseMap<size_t, unsigned> &SeqOffToOrigRow) {
-
- // Use std::map for ordered iteration.
- std::map<uint64_t, unsigned> LineTableMapping;
-
- // 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 (auto [I, Row] : llvm::enumerate(ArrayRef(LT.Rows).drop_back()))
- if (Row.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
-
- ArrayRef StmtAttrsRef(StmtAttrs);
- ArrayRef SeqStartRowsRef(SeqStartRows);
-
- // Dummy last element to make sure StmtAttrsRef and SeqStartRowsRef always
- // run out first.
- constexpr size_t DummyKey = UINT64_MAX;
- constexpr unsigned DummyVal = UINT32_MAX;
- LineTableMapping[DummyKey] = DummyVal;
-
- for (auto [NextSeqOff, NextRow] : LineTableMapping) {
- auto StmtAttrSmallerThanNext = [NextSeqOff](const PatchLocation &SA) {
- return SA.get() < NextSeqOff;
- };
- auto SeqStartSmallerThanNext = [NextRow](const size_t &Row) {
- return Row < NextRow;
- };
-
- // 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 (!StmtAttrsRef.empty() && !SeqStartRowsRef.empty() &&
- StmtAttrSmallerThanNext(StmtAttrsRef.front()) &&
- SeqStartSmallerThanNext(SeqStartRowsRef.front())) {
- SeqOffToOrigRow[StmtAttrsRef.consume_front().get()] =
- SeqStartRowsRef.consume_front();
- }
- // 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
- StmtAttrsRef = StmtAttrsRef.drop_while(StmtAttrSmallerThanNext);
- SeqStartRowsRef = SeqStartRowsRef.drop_while(SeqStartSmallerThanNext);
- // Use the LineTableMapping's result as the ground truth and move
- // on.
- if (NextSeqOff != DummyKey) {
- SeqOffToOrigRow[NextSeqOff] = NextRow;
- }
- // Move the pointers if they are pointed at Next.
- // It is possible that they 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 <- StmtAttrsRef.front() / SeqStartRowsRef.front()
- // 0x16 15
- // -- 17
- if (!StmtAttrsRef.empty() && StmtAttrsRef.front().get() == NextSeqOff)
- StmtAttrsRef.consume_front();
- if (!SeqStartRowsRef.empty() && SeqStartRowsRef.front() == NextRow)
- SeqStartRowsRef.consume_front();
- }
-}
-
std::pair<bool, std::optional<int64_t>>
DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
const DWARFDie &DIE) {
@@ -2407,12 +2297,8 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
// Create a map of stmt sequence offsets to original row indices.
DenseMap<uint64_t, unsigned> SeqOffToOrigRow;
- // 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())
- constructSeqOffsettoOrigRowMapping(Unit, *LT, SeqOffToOrigRow);
+ for (const DWARFDebugLine::Sequence &Seq : LT->Sequences)
+ SeqOffToOrigRow[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
// 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 db223cda43247..f2fe794e1b484 100644
--- a/llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test
+++ b/llvm/test/tools/dsymutil/ARM/stmt-seq-macho.test
@@ -5,13 +5,7 @@
# 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]+)]])
@@ -24,9 +18,6 @@
#--- 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;
@@ -60,7 +51,6 @@ int main() {
sum += function2_copy2(3);
sum += function3_copy2(41);
sum += function2_copy1(11);
- sum += function1_copy1(42);
length_error e("test");
return sum;
}
@@ -118,9 +108,9 @@ LoadCommands:
cmdsize: 1032
segname: ''
vmaddr: 0
- vmsize: 3125
+ vmsize: 2793
fileoff: 1208
- filesize: 3125
+ filesize: 2793
maxprot: 7
initprot: 7
nsects: 12
@@ -129,18 +119,18 @@ LoadCommands:
- sectname: __text
segname: __TEXT
addr: 0x0
- size: 148
+ size: 128
offset: 0x4B8
align: 2
- reloff: 0x10F0
- nreloc: 8
+ reloff: 0xFA8
+ nreloc: 7
flags: 0x80000400
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
- content: 00040011C0035FD600100011C0035FD600580051C0035FD600100011C0035FD600580051C0035FD6FFC300D1F44F01A9FD7B02A9FD8300916000805200000094F30300AA20058052000000941400130B6001805200000094F30300AA40058052000000947302000B0100009021000091E03F0091000000948002130BFD7B42A9F44F41A9FFC30091C0035FD600000014C0035FD6
+ content: 00100011C0035FD600580051C0035FD600100011C0035FD600580051C0035FD6FFC300D1F44F01A9FD7B02A9FD8300916000805200000094F30300AA20058052000000941400130B6001805200000094F30300AA0100009021000091E03F0091000000948002130BFD7B42A9F44F41A9FFC30091C0035FD600000014C0035FD6
relocations:
- - address: 0x8C
+ - address: 0x78
symbolnum: 4
pcrel: true
length: 2
@@ -148,7 +138,7 @@ LoadCommands:
type: 2
scattered: false
value: 0
- - address: 0x74
+ - address: 0x60
symbolnum: 3
pcrel: true
length: 2
@@ -156,7 +146,7 @@ LoadCommands:
type: 2
scattered: false
value: 0
- - address: 0x6C
+ - address: 0x58
symbolnum: 1
pcrel: false
length: 2
@@ -164,7 +154,7 @@ LoadCommands:
type: 4
scattered: false
value: 0
- - address: 0x68
+ - address: 0x54
symbolnum: 1
pcrel: true
length: 2
@@ -172,7 +162,7 @@ LoadCommands:
type: 3
scattered: false
value: 0
- - address: 0x60
+ - address: 0x4C
symbolnum: 5
pcrel: true
length: 2
@@ -180,24 +170,16 @@ LoadCommands:
type: 2
scattered: false
value: 0
- - address: 0x54
- symbolnum: 6
- pcrel: true
- length: 2
- extern: true
- type: 2
- scattered: false
- value: 0
- - address: 0x48
- symbolnum: 9
+ - address: 0x40
+ symbolnum: 8
pcrel: true
length: 2
extern: true
type: 2
scattered: false
value: 0
- - address: 0x3C
- symbolnum: 7
+ - address: 0x34
+ symbolnum: 6
pcrel: true
length: 2
extern: true
@@ -206,9 +188,9 @@ LoadCommands:
value: 0
- sectname: __cstring
segname: __TEXT
- addr: 0x94
+ addr: 0x80
size: 5
- offset: 0x54C
+ offset: 0x538
align: 0
reloff: 0x0
nreloc: 0
@@ -219,9 +201,9 @@ LoadCommands:
content: '7465737400'
- sectname: __debug_loc
segname: __DWARF
- addr: 0x99
+ addr: 0x85
size: 412
- offset: 0x551
+ offset: 0x53D
align: 0
reloff: 0x0
nreloc: 0
@@ -229,12 +211,12 @@ LoadCommands:
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
- content: 08000000000000000C000000000000000100500C0000000000000010000000000000000400A301509F0000000000000000000000000000000008000000000000000C00000000000000030070039F0000000000000000000000000000000010000000000000001400000000000000010050140000000000000018000000000000000400A301509F0000000000000000000000000000000018000000000000001C000000000000000100501C0000000000000020000000000000000400A301509F0000000000000000000000000000000018000000000000001C00000000000000030070039F0000000000000000000000000000000020000000000000002400000000000000010050240000000000000028000000000000000400A301509F00000000000000000000000000000000240000000000000028000000000000000100500000000000000000000000000000000038000000000000004400000000000000030011009F4400000000000000500000000000000001006350000000000000005C0000000000000001006400000000000000000000000000000000
+ content: 00000000000000000400000000000000010050040000000000000008000000000000000400A301509F0000000000000000000000000000000000000000000000000400000000000000030070039F0000000000000000000000000000000008000000000000000C000000000000000100500C0000000000000010000000000000000400A301509F0000000000000000000000000000000010000000000000001400000000000000010050140000000000000018000000000000000400A301509F0000000000000000000000000000000010000000000000001400000000000000030070039F0000000000000000000000000000000018000000000000001C000000000000000100501C0000000000000020000000000000000400A301509F000000000000000000000000000000001C0000000000000020000000000000000100500000000000000000000000000000000030000000000000003C00000000000000030011009F3C0000000000000048000000000000000100634800000000000000540000000000000001006400000000000000000000000000000000
- sectname: __debug_abbrev
segname: __DWARF
- addr: 0x235
- size: 372
- offset: 0x6ED
+ addr: 0x221
+ size: 359
+ offset: 0x6D9
align: 0
reloff: 0x0
nreloc: 0
@@ -244,34 +226,18 @@ LoadCommands:
reserved3: 0x0
- sectname: __debug_info
segname: __DWARF
- addr: 0x3A9
- size: 747
- offset: 0x861
+ addr: 0x388
+ size: 686
+ offset: 0x840
align: 0
- reloff: 0x1130
- nreloc: 16
+ reloff: 0xFE0
+ nreloc: 14
flags: 0x2000000
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
relocations:
- - 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
+ - address: 0x26A
symbolnum: 1
pcrel: false
length: 3
@@ -279,7 +245,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1F5
+ - address: 0x251
symbolnum: 1
pcrel: false
length: 3
@@ -287,7 +253,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1E1
+ - address: 0x216
symbolnum: 1
pcrel: false
length: 3
@@ -295,7 +261,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1CE
+ - address: 0x1B8
symbolnum: 1
pcrel: false
length: 3
@@ -303,7 +269,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1BA
+ - address: 0x1A5
symbolnum: 1
pcrel: false
length: 3
@@ -311,7 +277,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x1A7
+ - address: 0x191
symbolnum: 1
pcrel: false
length: 3
@@ -319,7 +285,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x169
+ - address: 0x17E
symbolnum: 1
pcrel: false
length: 3
@@ -327,7 +293,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x12D
+ - address: 0x140
symbolnum: 1
pcrel: false
length: 3
@@ -335,7 +301,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0xF1
+ - address: 0x104
symbolnum: 1
pcrel: false
length: 3
@@ -343,7 +309,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0xC4
+ - address: 0xC8
symbolnum: 1
pcrel: false
length: 3
@@ -351,7 +317,7 @@ LoadCommands:
type: 0
scattered: false
value: 0
- - address: 0x88
+ - address: 0x9B
symbolnum: 1
pcrel: false
length: 3
@@ -385,9 +351,9 @@ LoadCommands:
value: 0
- sectname: __debug_str
segname: __DWARF
- addr: 0x694
- size: 400
- offset: 0xB4C
+ addr: 0x636
+ size: 239
+ offset: 0xAEE
align: 0
reloff: 0x0
nreloc: 0
@@ -397,9 +363,9 @@ LoadCommands:
reserved3: 0x0
- sectname: __apple_names
segname: __DWARF
- addr: 0x824
- size: 288
- offset: 0xCDC
+ addr: 0x725
+ size: 260
+ offset: 0xBDD
align: 0
reloff: 0x0
nreloc: 0
@@ -407,12 +373,12 @@ LoadCommands:
reserved1: 0x0
reserved2: 0x0
reserved3: 0x0
- content: 485341480100000009000000090000000C00000000000000010000000100060000000000FFFFFFFFFFFFFFFF0100000003000000040000000600000007000000080000004A08311CC78E3C8288CB36CF89CB36CFD1125E53522B705390D9F86F6A7F9A7C4908311C8C0000009C000000AC000000BC000000CC000000DC000000EC00000000010000100100000601000001000000F000000000000000D6000000010000005E00000000000000F600000001000000C30000000000000016010000010000002C01000000000000440100000100000052020000000000005C01000001000000A6020000000000002B0100000200000052020000A60200000000000026010000010000006801000000000000E6000000010000008700000000000000
+ content: 485341480100000008000000080000000C000000000000000100000001000600000000000200000005000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF90D9F86F88CB36CF4908311CD1125E5389CB36CF4A08311C522B70536A7F9A7C8000000094000000A4000000B4000000C4000000D4000000E4000000F40000008A0000000200000015020000690200000000000055000000010000009A0000000000000045000000010000005E00000000000000A3000000010000001502000000000000750000000100000003010000000...
[truncated]
|
ellishg
approved these changes
Jul 30, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/10545 Here is the relevant piece of the build log for the reference |
DataCorrupted
added a commit
to DataCorrupted/llvm-project
that referenced
this pull request
Jul 31, 2025
…f… (llvm#151424) This reverts commit 6c3d62a.
DataCorrupted
added a commit
that referenced
this pull request
Jul 31, 2025
…1427) Reverts the [revert](#151424) and fixed some typos. Original PR description: 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: // 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 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 --------- Signed-off-by: Peter Rong <PeterRong@meta.com> Co-authored-by: Ellis Hoag <ellis.sparky.hoag@gmail.com>
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jul 31, 2025
…offset (#151427) Reverts the [revert](llvm/llvm-project#151424) and fixed some typos. Original PR description: Second attempt to fix https://discourse.llvm.org/t/rfc-new-dwarf-attribute-for-symbolication-of-merged-functions/79434/29?u=alx32 (First attempt: llvm/llvm-project#143656) Context: the sequence offset to row index we parsed may not be complete. And we need to add manual matching to it. llvm/llvm-project#143656 attempts to do trivial 1:1 matching, however, sometimes they don't line up perfectly, as shown below: // 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 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 --------- Signed-off-by: Peter Rong <PeterRong@meta.com> Co-authored-by: Ellis Hoag <ellis.sparky.hoag@gmail.com>
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.
…sets (#149618)"
This reverts commit ed940d7.