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

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

> Also don't we already have an update sections loop somewhere in this already? I think this should go there to reduce the number of passes we make over the sections

It looks like we already have multiple loops over the sections so that some flags can apply on top of each other, though it's possible a few of the passes could be combined. O(n^2) is really the only real performance killer though, simply iterating over sections multiple times is a *much* less bigger deal.

Quick check:

  grep -c ": Obj.sections())" llvm/tools/llvm-objcopy/ELF/* | grep -v :0
  llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:6
  llvm/tools/llvm-objcopy/ELF/Object.cpp:11



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/prefix-alloc-sections.test:3
+# RUN: llvm-objcopy --prefix-alloc-sections=.alloc_prefix %t %t2
+# RUN: llvm-readobj --sections %t2 | FileCheck %s
+
----------------
It'd be good to check --relocs as well


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:618
+    // to .rel.prefix.text.
+    for (auto &Sec : SectionTable) {
+      if (Sec.Info == SHN_UNDEF)
----------------
Instead of the O(n^2) loop over the sections, you can do it one pass:

* If `Name == Config.AllocSectionsPrefix` then do the usual rename
* If `is_a<RelocationSectionBase>(&Sec)` and `dyn_cast<RelocationSectionbase>(&Sec)->getSection()->getName() == Config.AllocSectionsPrefix`, then change based on the .rel/.rela pattern below

`getSection()` is probably not well named since that makes it sound like a regular getter, it should probably be `getTargetSection()` or similar.


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

https://reviews.llvm.org/D60042