[PATCH] D60042: [llvm-objcopy] Add --prefix-alloc-sections

[Seiya Nuta via Phabricator via llvm-commits]

seiya marked 5 inline comments as done.
seiya added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/prefix-alloc-sections.test:54
+# CHECK: Name: .alloc_prefix.plt
+# CHECK: Name: .alloc_prefix.rela.plt
+# CHECK: ]
----------------
This `.rela.plt` is a testcase what I mean.


================
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:
> seiya wrote:
> > jakehehrlich wrote:
> > > 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.
> > Yes. This flag renames dynamic relocation sections too. It seems that GNU objcopy simply checks whether ALLOC flag is set: 
> > 
> > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=binutils/objcopy.c;h=330b93c1ad3f87a0f5777ac88dc4af1398959bbe;hb=HEAD#l3772
> Can you add a test case for this. We have a binary checked in that you can use. I want to make sure it isn't double renamed and all that. It's just a slightly more tricky real world use case.
We have a test case which renames dynamic relocation sections. Should I clarify that in a comment or add a test case against a binary file, say, `test/tools/llvm-objcopy/ELF/Inputs/dynrel.elf`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60042/new/

https://reviews.llvm.org/D60042