From 8abf8d124ae346016c56209de7f57b85671d4367 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Wed, 24 Jan 2024 08:53:36 -0800 Subject: [PATCH] [ELF] Don't resolve relocations referencing SHN_ABS to tombstone in non-SHF_ALLOC sections (#79238) A SHN_ABS symbol has never been considered for InputSection::relocateNonAlloc. Before #74686, the code did made it work in the absence of `-z dead-reloc-in-nonalloc=`. There is now a report about such SHN_ABS uses (https://github.com/llvm/llvm-project/pull/74686#issuecomment-1904101711) and I think it makes sense for non-SHF_ALLOC to support SHN_ABS, like SHF_ALLOC sections do. ``` // clang -g __attribute__((weak)) int symbol; int *foo() { return &symbol; } 0x00000023: DW_TAG_variable [2] (0x0000000c) ... DW_AT_location [DW_FORM_exprloc] (DW_OP_addrx 0x0) ``` .debug_addr references `symbol`, which can be redefined by a symbol assignment or --defsym to become a SHN_ABS symbol. The problem is that `!sym.getOutputSection()` cannot discern SHN_ABS from a symbol whose section has been discarded. Since commit 1981b1b6b92f7579a30c9ed32dbdf3bc749c1b40, a symbol relative to a discarded section is changed to `Undefined`, so the `SHN_ABS` check become trivial. We currently apply tombstone for a relocation referencing `SharedSymbol`. This patch does not change the behavior. --- lld/ELF/InputSection.cpp | 13 ++++++------- lld/test/ELF/dead-reloc-in-nonalloc.s | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp index c728dd6c6306aa..0e0b9783bd88a0 100644 --- a/lld/ELF/InputSection.cpp +++ b/lld/ELF/InputSection.cpp @@ -961,12 +961,11 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef rels) { // vector. The computed value is st_value plus a non-negative offset. // Negative values are invalid, so -1 can be used as the tombstone value. // - // If the referenced symbol is discarded (made Undefined), or the - // section defining the referenced symbol is garbage collected, - // sym.getOutputSection() is nullptr. `ds->folded` catches the ICF folded - // case. However, resolving a relocation in .debug_line to -1 would stop - // debugger users from setting breakpoints on the folded-in function, so - // exclude .debug_line. + // If the referenced symbol is relative to a discarded section (due to + // --gc-sections, COMDAT, etc), it has been converted to a Undefined. + // `ds->folded` catches the ICF folded case. However, resolving a + // relocation in .debug_line to -1 would stop debugger users from setting + // breakpoints on the folded-in function, so exclude .debug_line. // // For pre-DWARF-v5 .debug_loc and .debug_ranges, -1 is a reserved value // (base address selection entry), use 1 (which is used by GNU ld for @@ -974,7 +973,7 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef rels) { // // TODO To reduce disruption, we use 0 instead of -1 as the tombstone // value. Enable -1 in a future release. - if (!sym.getOutputSection() || (ds && ds->folded && !isDebugLine)) { + if (!ds || (ds->folded && !isDebugLine)) { // If -z dead-reloc-in-nonalloc= is specified, respect it. uint64_t value = SignExtend64(*tombstone); // For a 32-bit local TU reference in .debug_names, X86_64::relocate diff --git a/lld/test/ELF/dead-reloc-in-nonalloc.s b/lld/test/ELF/dead-reloc-in-nonalloc.s index 145604eb883a9a..b675fc50fc2ea2 100644 --- a/lld/test/ELF/dead-reloc-in-nonalloc.s +++ b/lld/test/ELF/dead-reloc-in-nonalloc.s @@ -17,7 +17,7 @@ # AA: Contents of section .debug_info: # AA-NEXT: 0000 [[ADDR]] 00000000 aaaaaaaa 00000000 # AA: Contents of section .not_debug: -# AA-NEXT: 0000 bbbbbbbb bbbbbbbb 00000000 . +# AA-NEXT: 0000 bbbbbbbb 2a000000 00000000 . ## Specifying zero can get a behavior similar to GNU ld. # RUN: ld.lld --icf=all -z dead-reloc-in-nonalloc=.debug_info=0 %t.o %tabs.o -o %tzero