This is an archive of the discontinued LLVM Phabricator instance.

[llvm-strip] Add --strip-symbol
ClosedPublic

Authored by evgeny777 on Jan 30 2019, 2:12 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

evgeny777 created this revision.Jan 30 2019, 2:12 AM
jhenderson added inline comments.Jan 30 2019, 2:24 AM
test/tools/llvm-objcopy/ELF/strip-symbol.test
2 ↗(On Diff #184256)

Please use a different output file for each test case. Probably easiest is to put this after the llvm-objcopy case and use %t3 instead.

tools/llvm-objcopy/CopyConfig.cpp
437 ↗(On Diff #184256)

I'm not sure I like checking InputArgs here, since it looks weird compared to the other cases. Perhaps the better thing to do would be to move this entire if clause to after the for loops, and then check if the SymbolsToRemove vector is empty.

This option exists in objcopy too, can you add it to ObjcopyOpts.td and handle it in the other args parsing block too?

tools/llvm-objcopy/StripOpts.td
55 ↗(On Diff #184256)

Nit: looks like this isn't indented right, it should align w/ Eq

Note: clang-format (from trunk) can fix these simple tablegen files, with the exception that it puts in spaces around ["-"] that you have to revert -- try git-clang-format --extensions td --binary ~/build/clang-format master (I'm trying to submit a couple patches to fix that)

LGTM, with @rupprecht's fixes.

jhenderson accepted this revision.Jan 30 2019, 7:00 AM
This revision is now accepted and ready to land.Jan 30 2019, 7:00 AM
rupprecht accepted this revision.Jan 30 2019, 11:57 AM

This option exists in objcopy too, can you add it to ObjcopyOpts.td and handle it in the other args parsing block too?

I just noticed this is already handled by objcopy, and was just missing from strip.

This revision was automatically updated to reflect the committed changes.