Skip to content

Conversation

@MaskRay
Copy link
Member

@MaskRay MaskRay commented Mar 14, 2024

#69295 demoted Defined symbols relative to discarded sections.
If such a symbol is unreferenced, the desired behavior is to
eliminate it from .symtab just like --gc-sections discarded
definitions.
Linux kernel's CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y configuration expects
that the unreferenced unused is not emitted to .symtab
(ClangBuiltLinux/linux#2006).

For relocations referencing demoted symbols, the symbol index restores
to 0 like older lld (R_X86_64_64 0 in discard-section.s).

Fix #85048

@llvmbot
Copy link
Member

llvmbot commented Mar 14, 2024

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

#69295 demoted Defined symbols relative to discarded sections.
If such a symbol is unreferenced, the desired behavior is to
eliminate it from .symtab just like --gc-sections discarded
definitions.

Fix #85048


Full diff: https://github.com/llvm/llvm-project/pull/85167.diff

2 Files Affected:

  • (modified) lld/ELF/Writer.cpp (+3)
  • (modified) lld/test/ELF/linkerscript/discard-section.s (+15-9)
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp index a9292b3b1a2241..d8782affe879ba 100644 --- a/lld/ELF/Writer.cpp +++ b/lld/ELF/Writer.cpp @@ -261,6 +261,9 @@ static void demoteDefined(Defined &sym, DenseMap<SectionBase *, size_t> &map) { Undefined(sym.file, sym.getName(), binding, sym.stOther, sym.type, /*discardedSecIdx=*/map.lookup(sym.section)) .overwrite(sym); + // Eliminate from the symbol table, otherwise we would leave an undefined + // symbol if the symbol is unreferenced in the absence of GC. + sym.isUsedInRegularObj = false; } // If all references to a DSO happen to be weak, the DSO is not added to diff --git a/lld/test/ELF/linkerscript/discard-section.s b/lld/test/ELF/linkerscript/discard-section.s index 24f3b2b73e991f..24edda2feeaade 100644 --- a/lld/test/ELF/linkerscript/discard-section.s +++ b/lld/test/ELF/linkerscript/discard-section.s @@ -9,6 +9,9 @@ # RUN: ld.lld -r -T a.lds a.o b.o -o a.ro 2>&1 | FileCheck %s --check-prefix=WARNING --implicit-check-not=warning: # RUN: llvm-readelf -r -s a.ro | FileCheck %s --check-prefix=RELOC +# RUN: ld.lld -r --gc-sections -T a.lds a.o b.o -o a.gc.ro --no-fatal-warnings +# RUN: llvm-readelf -r -s a.gc.ro | FileCheck %s --check-prefix=RELOC-GC + # LOCAL: error: relocation refers to a discarded section: .aaa # LOCAL-NEXT: >>> defined in a.o # LOCAL-NEXT: >>> referenced by a.o:(.bbb+0x0) @@ -32,16 +35,17 @@ # WARNING: warning: relocation refers to a discarded section: .aaa # WARNING-NEXT: >>> referenced by a.o:(.rela.bbb+0x0) +## GNU ld reports "defined in discarded secion" errors even in -r mode. # RELOC: Relocation section '.rela.bbb' at offset {{.*}} contains 1 entries: # RELOC-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend # RELOC-NEXT: 0000000000000000 0000000000000000 R_X86_64_NONE 0 # RELOC-EMPTY: # RELOC-NEXT: Relocation section '.rela.data' at offset {{.*}} contains 4 entries: # RELOC-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend -# RELOC-NEXT: 0000000000000000 0000000500000001 R_X86_64_64 0000000000000000 global + 0 -# RELOC-NEXT: 0000000000000008 0000000700000001 R_X86_64_64 0000000000000000 weak + 0 -# RELOC-NEXT: 0000000000000010 0000000600000001 R_X86_64_64 0000000000000000 weakref1 + 0 -# RELOC-NEXT: 0000000000000018 0000000800000001 R_X86_64_64 0000000000000000 weakref2 + 0 +# RELOC-NEXT: 0000000000000000 0000000000000001 R_X86_64_64 0 +# RELOC-NEXT: 0000000000000008 0000000000000001 R_X86_64_64 0 +# RELOC-NEXT: 0000000000000010 0000000000000001 R_X86_64_64 0 +# RELOC-NEXT: 0000000000000018 0000000000000001 R_X86_64_64 0 # RELOC: Num: Value Size Type Bind Vis Ndx Name # RELOC-NEXT: 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND @@ -49,23 +53,25 @@ # RELOC-NEXT: 2: 0000000000000000 0 SECTION LOCAL DEFAULT 2 .bbb # RELOC-NEXT: 3: 0000000000000000 0 SECTION LOCAL DEFAULT 4 .data # RELOC-NEXT: 4: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 1 _start -# RELOC-NEXT: 5: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND global -# RELOC-NEXT: 6: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND weakref1 -# RELOC-NEXT: 7: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND weak -# RELOC-NEXT: 8: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND weakref2 # RELOC-EMPTY: +# RELOC-GC: There are no relocations in this file. + #--- a.s .globl _start _start: .section .aaa,"a" -.globl global, weakref1 +.globl global, weakref1, unused .weak weak, weakref2 global: weak: weakref1: weakref2: +## Eliminate `unused` just like GC discarded definitions. +## Linux kernel's CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y configuration expects +## that the unreferenced `unused` is not emitted to .symtab. +unused: .quad 0 .section .bbb,"aw" 
@MaskRay MaskRay changed the title [ELF] Don't add symbols demoted due to /DISCARD/ to .symtab [ELF] Eliminate symbols demoted due to /DISCARD/ discarded sections Mar 14, 2024
 llvm#69295 demoted Defined symbols relative to discarded sections. If such a symbol is unreferenced, the desired behavior is to eliminate it from .symtab just like --gc-sections discarded definitions. Linux kernel's CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y configuration expects that the unreferenced `unused` is not emitted to .symtab (ClangBuiltLinux/linux#2006). For relocations referencing demoted symbols, the symbol index restores to 0 like older lld (`R_X86_64_64 0` in `discard-section.s`). Fix llvm#85048
Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I agree that if the symbols from discarding the section are unreferenced then removing them lowers the risk of downstream ELF processing tools not accepting the undefined symbols.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that this fixes the original kernel build issue.

@MaskRay MaskRay merged commit 8fe3e70 into llvm:main Mar 14, 2024
@MaskRay MaskRay deleted the lld-discard branch March 14, 2024 16:51
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 14, 2024
…lvm#85167) llvm#69295 demoted Defined symbols relative to discarded sections. If such a symbol is unreferenced, the desired behavior is to eliminate it from .symtab just like --gc-sections discarded definitions. Linux kernel's CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y configuration expects that the unreferenced `unused` is not emitted to .symtab (ClangBuiltLinux/linux#2006). For relocations referencing demoted symbols, the symbol index restores to 0 like older lld (`R_X86_64_64 0` in `discard-section.s`). Fix llvm#85048 (cherry picked from commit 8fe3e70)
@pointhex pointhex mentioned this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants