Page MenuHomePhabricator

[llvm-objcopy] Add --localize-hidden option
ClosedPublic

Authored by jakehehrlich on Jan 2 2018, 2:27 PM.

Details

Reviewers
phosek
jhenderson
Summary

This change adds support in llvm-objcopy for GNU objcopy's --localize-hidden option. This option changes every hidden or internal symbol into a local symbol.

Diff Detail

Repository
rL LLVM

Event Timeline

jakehehrlich created this revision.Jan 2 2018, 2:27 PM
phosek added inline comments.Jan 2 2018, 11:33 PM
tools/llvm-objcopy/llvm-objcopy.cpp
197

nit: This block should be probably above the RemovePred since RemovePred is related to lines 199-304.

jhenderson added inline comments.Jan 3 2018, 3:58 AM
test/tools/llvm-objcopy/localize-hidden.test
36

Why do you have so many different symbols here?

Also, you should have symbols with hidden visibility with both local and weak bindings, to show that a) local symbols remain unchanged, and b) weak symbols are changed.

Finally, a symbol with STV_PROTECTED visibility should also be included to show that it is not localized.

tools/llvm-objcopy/Object.cpp
184–188

I'd add blank lines either side of this block.

  1. Moved RemovePred decl as recommended
  2. Added spaces around a bit of code
  3. Made some big changes to the test. I added weak and local hidden symbols. I changed their names because I couldn't keep track of anything. I changed a symbol to be protected. I believe it should meet the requirements originally desired.
jhenderson added inline comments.Jan 4 2018, 4:58 AM
test/tools/llvm-objcopy/localize-hidden.test
48

Could we have a separate symbol for hidden Globals, please. Since that's going to be a relatively common case, I think it deserves explicit testing.

tools/llvm-objcopy/Object.cpp
191

indexs -> indexes

  1. Added hidden global symbol
  2. Fixed "indexes" typo
This revision is now accepted and ready to land.Jan 5 2018, 1:53 AM
jakehehrlich closed this revision.Jan 5 2018, 4:07 PM