Page MenuHomePhabricator

[llvm-objcopy] Add --prefix-sections option
Needs RevisionPublic

Authored by paulsemel on Aug 8 2018, 10:23 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

paulsemel created this revision.Aug 8 2018, 10:23 AM
This revision is now accepted and ready to land.Aug 8 2018, 10:47 PM
jhenderson requested changes to this revision.EditedAug 9 2018, 2:04 AM

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>"

This revision now requires changes to proceed.Aug 9 2018, 2:04 AM

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).

tools/llvm-objcopy/llvm-objcopy.cpp
274

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).

568

I'm wondering if GNU objcopy's behaviour is section-name specific rather than type specific. Have you checked?

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>"

Hmm ok.. I just tried on the "chromium" binary on my laptop and it appeard the the prefix is at the beggining of the section, not between '.rela' and the rest of the section name.. am I missing smth ?

I don't think I want this option to do anything more than literally add a prefix to all section names. It isn't clear that those precise semantics are being relied on anywhere and the behavior of making relocation names and not renaming symtab etc... is a result of how bfd reconstructs the output binary without consideration for what the user initially put in which is opposite the stated goal of llvm-objcopy.

tools/llvm-objcopy/llvm-objcopy.cpp
274

+1 on all of these points actually. The lowercase thing is my fault but we should begin changing it now.

I don't think I want this option to do anything more than literally add a prefix to all section names. It isn't clear that those precise semantics are being relied on anywhere and the behavior of making relocation names and not renaming symtab etc... is a result of how bfd reconstructs the output binary without consideration for what the user initially put in which is opposite the stated goal of llvm-objcopy.

I can't think of any semantic reason why names should matter for the cases discussed, with the exception of a tiny number of sections (usually to do with debug information or similar such things), names of sections shouldn't matter (and indeed, this has been discussed on the ELF gABI mailing list as a stated goal of ELF). This option WILL break DWARF sections though, which rely on the section naming. I'm not sure if this is something we should care about though?

Also, blanket renaming sections seems like a bad thing for linking, since it will affect semantics of where sections get placed. Is there actually a use-case for it?