[PATCH] D50463: [llvm-objcopy] Add --prefix-sections option
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 9 02:56:12 PDT 2018
jhenderson added a comment.
In https://reviews.llvm.org/D50463#1193520, @jhenderson wrote:
> I've got some comments that I'll send through a bit later today, but as things stand, this doesn't match GNU objcopy's behaviour for relocation sections at the very least - the "prefix" is placed between the end of ".rela" and the start of the section name so that they have the name ".rela<prefixed relocated section name>"
It turns out that this is a wider bug in llvm-objcopy that is related to, but not specific to this issue: if you do --rename-section in GNU objcopy, you also implicitly rename relocation sections. Meanwhile, if you do --rename-section on relocation sections or strtab or symtab sections, nothing happens (if less concerned about this, because you requested something specific for those sections, but still, it is a behaviour difference).
================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:274
+static inline bool StartsWith(std::string S1, StringRef S2) {
+ return std::equal(S2.begin(), S2.end(), S1.begin());
----------------
This should take a pair of StringRefs, not a std::string and a StringRef. And then you can use StringRef::startswith.
Also `inline` has no purpose here. Compilers generally ignore it when it comes to function inlining. It does have a completely separate purpose in header files though.
It should also be named with lower-case (@jakehehrlich and I discussed this offline, and I think that somewhere we started following a non-LLVM style for functions, contrary to the style guide, and we'd like to get back to the style it should be).
================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:568
+ for (auto &Sec : Obj.sections()) {
+ if (Sec.Type != SHT_STRTAB && Sec.Type != SHT_SYMTAB)
+ Sec.Name = Config.SectionsPrefix.str() + Sec.Name;
----------------
I'm wondering if GNU objcopy's behaviour is section-name specific rather than type specific. Have you checked?
Repository:
rL LLVM
https://reviews.llvm.org/D50463
More information about the llvm-commits
mailing list