[PATCH] D60042: [llvm-objcopy] Add --prefix-alloc-sections
Jake Ehrlich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 17 10:47:06 PDT 2019
jakehehrlich added a comment.
A few comments.
================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:607
+ // .rela.prefix.plt since GNU objcopy does so.
+ } else if (isa<RelocationSectionBase>(&Sec)) {
+ auto *TargetSec = dyn_cast<RelocationSectionBase>(&Sec)->getSection();
----------------
You can do `if (auto *RelocSec = dyn_cast<RelocationSectionBase>(&Sec))` and then `SectionBase *TargetSec = RelocSec->getSection()`
================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:607
+ // .rela.prefix.plt since GNU objcopy does so.
+ } else if (isa<RelocationSectionBase>(&Sec)) {
+ auto *TargetSec = dyn_cast<RelocationSectionBase>(&Sec)->getSection();
----------------
jakehehrlich wrote:
> You can do `if (auto *RelocSec = dyn_cast<RelocationSectionBase>(&Sec))` and then `SectionBase *TargetSec = RelocSec->getSection()`
I forget. Does this apply to dynamic relocation sections as well? Does this flag rename the dynamic relocation section? My hunch is that it doesn't.
================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:626
+ // section comes *before* the target section, we add the prefix.
+ if (TargetSec->Name.find(Config.AllocSectionsPrefix) == 0)
+ Sec.Name = (prefix + TargetSec->Name).str();
----------------
This is not a good way to do this.
I think the right way is to have the "OriginalName" and do this in a non-conditional way. I tried thinking about a bunch of alternatives but they all have sticky issues. Your best bet is to just add OriginalName to SectionBase and unconditionally assign the section name here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60042/new/
https://reviews.llvm.org/D60042
More information about the llvm-commits
mailing list