[PATCH] D150859: [llvm-objcopy][ELF] Preserve sh_link to .symtab when applicable
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 25 11:00:50 PDT 2023
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.
Sorry for the delay. This looks good me. Is the intention to make `ld.lld --icf=safe` work after certain `llvm-objcopy` operations? If so, it'd be good to state the motivation in the summary.
I think the change is good for other `sh_link->SHT_SYMTAB` sections.
I have a comment whether we should make new symbols (while existing symbol indices are unchanged) reset `sh_link` to zero.
I wonder whether we still consider changing the design of ELF `SHT_LLVM_ADDRSIG`. Mach-O uses relocation-based `__addrsig` (https://discourse.llvm.org/t/problems-with-mach-o-address-significance-table-generation/63392).
================
Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:690
uint32_t Index = 0;
- for (auto &Sym : Symbols)
- Sym->Index = Index++;
+ if (IndicesChanged)
+ for (auto &Sym : Symbols)
----------------
Add `{}` for `if` and `else`. See https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
If the manual loop unswitching necessary? Can we just keep the `else` part?
================
Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:2537
+ // If the .symtab indices have not been changed restore the sh_link to .symtab
+ // for sections that were linked to .symtab.
----------------
================
Comment at: llvm/test/tools/llvm-objcopy/ELF/symtab-link.test:21
+# RUN: llvm-readobj --sections %t7 | FileCheck %s --check-prefix=LINK-0
+## Remove .text section
+# RUN: llvm-objcopy --remove-section=.text %t %t8
----------------
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150859/new/
https://reviews.llvm.org/D150859
More information about the llvm-commits
mailing list