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

[Seiya Nuta via Phabricator via llvm-commits]

seiya marked an inline comment as done.
seiya added inline comments.


================
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();
----------------
seiya wrote:
> jakehehrlich wrote:
> > 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.
> > 
> > 
> I think that's not sufficient. If both --rename-section and --prefix-alloc-sections are given, GNU objcopy renames sections by  --rename-section first and then adds the prefix specified by --prefix-alloc-sections.
> 
> For example, `--rename-section=.text=.text2 --prefix-alloc-sections=.prefix` renames .rel.text to .rel.prefix.text2.
> 
> It looks that the current patch only renames correctly if the target section comes //before// its relocation section by the way.
> 
> I have two ideas:
> - Iterate sections twice: rename sections first and then add the prefix.
> - Add `Renamed` flag to `SectionBase`: check the flag here and rename the section if needed.
> 
> I would like to hear your thoughts on this @jakehehrlich .
Or add `OriginalName` to SectionBase and use `Config.SectionsToRename` here:
```lang=cpp
  const auto Iter = Config.SectionsToRename.find(TargetSec->OriginalName);
  if (Iter != Config.SectionsToRename.end()) {
    // Both `--rename-section` and `--prefix-alloc-sections` are given.
    const SectionRename &SR = Iter->second;
    Sec.Name = (prefix + Config.AllocSectionsPrefix + SR.NewName).str();
  } else {
    Sec.Name = (prefix + Config.AllocSectionsPrefix + TargetSec->Name).str();
  }
```


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

https://reviews.llvm.org/D60042