-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[lld-macho] Fix crash with DWARF section-relative relocations #168075
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?
Conversation
DWARF sections with section-relative relocations (e.g., DW_FORM_strp) caused the linker to crash. Fixed by adding DWARF sections to subsections for relocation processing while marking them live=false to preserve MachO's traditional STABS-only debug output behavior.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-lld-macho @llvm/pr-subscribers-lld Author: Joel Reymont (joelreymont) ChangesDWARF sections with section-relative relocations (e.g., Fixed by adding DWARF sections to subsections for relocation processing while marking them Full diff: https://github.com/llvm/llvm-project/pull/168075.diff 2 Files Affected:
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index d0128d03a9eab..2324a83868eeb 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -411,10 +411,14 @@ void ObjFile::parseSections(ArrayRef<SectionHeader> sectionHeaders) {
auto *isec = make<ConcatInputSection>(section, data, align);
if (isDebugSection(isec->getFlags()) &&
isec->getSegName() == segment_names::dwarf) {
- // Instead of emitting DWARF sections, we emit STABS symbols to the
- // object files that contain them. We filter them out early to avoid
- // parsing their relocations unnecessarily.
+ // Keep debug sections in debugSections for diagnostic purposes
debugSections.push_back(isec);
+ // Add DWARF sections to subsections so their relocations are processed,
+ // but mark them dead so they aren't emitted (MachO uses STABS, not DWARF).
+ // This fixes crashes with section-relative relocations (e.g., DW_FORM_strp)
+ // while maintaining MachO's traditional STABS-only debug output.
+ isec->live = false;
+ section.subsections.push_back({0, isec});
} else {
section.subsections.push_back({0, isec});
}
diff --git a/lld/test/MachO/dwarf-strp-relocations.s b/lld/test/MachO/dwarf-strp-relocations.s
new file mode 100644
index 0000000000000..bc873d6c50a0c
--- /dev/null
+++ b/lld/test/MachO/dwarf-strp-relocations.s
@@ -0,0 +1,63 @@
+# REQUIRES: x86
+# RUN: rm -rf %t; split-file %s %t
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: %lld -dylib %t/test.o -o %t/test.dylib
+# RUN: llvm-objdump --section-headers %t/test.dylib | FileCheck %s
+
+## Test that lld can handle section-relative relocations in DWARF sections,
+## specifically DW_FORM_strp which creates X86_64_RELOC_UNSIGNED relocations
+## to the __debug_str section. This previously caused linker crashes with
+## "malformed relocation" errors on macOS.
+##
+## The test verifies that:
+## 1. The link completes successfully without crashing (key requirement)
+## 2. DWARF sections are processed for relocations but not emitted to output
+## (MachO traditionally uses STABS, not DWARF, for debug info)
+##
+## Negative checks ensure DWARF sections are NOT in the final binary, preventing
+## regression where they might be accidentally emitted.
+
+# CHECK-NOT: __debug_info
+# CHECK-NOT: __debug_abbrev
+# CHECK-NOT: __debug_str
+
+#--- test.s
+.section __TEXT,__text,regular,pure_instructions
+.globl _main
+_main:
+ movl $42, %eax
+ retq
+
+.section __DWARF,__debug_abbrev,regular,debug
+Labbrev_begin:
+ .byte 1 ## Abbrev code
+ .byte 17 ## DW_TAG_compile_unit
+ .byte 1 ## DW_CHILDREN_yes
+ .byte 37 ## DW_AT_producer
+ .byte 14 ## DW_FORM_strp (string table pointer!)
+ .byte 3 ## DW_AT_name
+ .byte 14 ## DW_FORM_strp
+ .byte 0 ## End attributes
+ .byte 0
+ .byte 0 ## End abbrev table
+
+.section __DWARF,__debug_info,regular,debug
+Linfo_begin:
+ .long Linfo_end - Linfo_begin - 4 ## Length
+ .short 4 ## DWARF version 4
+ .long 0 ## Abbrev offset
+ .byte 8 ## Address size
+ .byte 1 ## Abbrev code
+ ## These .long directives create section-relative relocations (X86_64_RELOC_UNSIGNED)
+ ## to the __debug_str section. This is the critical test case.
+ .long Lproducer - Ldebug_str ## DW_AT_producer (section-relative!)
+ .long Lfilename - Ldebug_str ## DW_AT_name (section-relative!)
+Linfo_end:
+
+.section __DWARF,__debug_str,regular,debug
+Ldebug_str:
+Lproducer:
+ .asciz "Test Producer 1.0"
+Lfilename:
+ .asciz "test.c"
|
int3
left a comment
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.
thanks for the fix! just some minor changes
| // This fixes crashes with section-relative relocations (e.g., DW_FORM_strp) | ||
| // while maintaining MachO's traditional STABS-only debug output. |
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.
"fixes crashes" here will not make sense to future readers who did not know how the code looked like before. let's explain it directly here; my understanding is that we cannot simply drop the sections here because it would result in a "dangling reference" issue if other sections contain relocations that point to this one
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.
(if that's not right, please explain a bit more what the crash is. what's the stack trace?)
| // Keep debug sections in debugSections for diagnostic purposes | ||
| debugSections.push_back(isec); |
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.
it is not used for diagnostic purposes, it's used to initialize the DwarfObject so we can emit STABS properly. I actually think we can remove debugSections entirely now that we are keeping references to the DWARF InputSections in sections. We can have DwarfObject::create re-traverse sections in order to find the relevant ones.
|
I wonder if this is AI created pull request based on his other pull requests to many different projects. |
|
I'll get back to this next week and provide more details. This was something I hit as part of adding DWARF information to the OCaml compiler on macOS. |
DWARF sections with section-relative relocations (e.g.,
DW_FORM_strp) caused the linker to crash.Fixed by adding DWARF sections to subsections for relocation processing while marking them
live=falseto preserve MachO's traditional STABS-only debug output behavior.