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

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 30 15:56:05 PDT 2019


jakehehrlich accepted this revision.
jakehehrlich added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:629
+            // section comes *before* the target section, we add the prefix.
+            if (PrefixedSections.find(TargetSec->Name) !=
+                PrefixedSections.end()) {
----------------
seiya wrote:
> jakehehrlich wrote:
> > I'm *really* against doing this kind of string searching when we can build a map. What if a section already contains the prefix being used. This won't be consistent in that case.
> > 
> > Also you should add a test case for that above.
> Let me clarify my understanding a bit. You mean a map which contains a pointer to a section like `DenseSet<SectionBase *> PrefixedSections`?
Ah whoops. I saw "find" and thought it hadn't changed. Big mistake on my part. Sorry. This LGTM.


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

https://reviews.llvm.org/D60042





More information about the llvm-commits mailing list