This is an archive of the discontinued LLVM Phabricator instance.

[llvm-strip]Add --no-strip-all to disable --strip-all behaviour (including default stripping)
ClosedPublic

Authored by jhenderson on May 1 2019, 7:05 AM.

Details

Summary

If certain switches are not specified, llvm-strip behaves as if --strip-all were specified. This means that for testing, when we don't want the stripping behaviour, we have to specify one of these switches, which can be confusing. This change adds --no-strip-all to allow an alternative way of suppressing the default stripping, in a less confusing manner.

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.May 1 2019, 7:05 AM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay added inline comments.May 1 2019, 7:50 AM
test/tools/llvm-objcopy/ELF/no-strip-all.test
7 ↗(On Diff #197540)

Replace cp %t.o %t1.o with -o %t1.o?

jhenderson marked an inline comment as done.May 1 2019, 8:15 AM
jhenderson added inline comments.
test/tools/llvm-objcopy/ELF/no-strip-all.test
7 ↗(On Diff #197540)

No, because each of the test cases makes a copy from the pristine version before llvm-strip modifies it.

MaskRay added inline comments.May 1 2019, 8:34 AM
test/tools/llvm-objcopy/ELF/no-strip-all.test
7 ↗(On Diff #197540)

llvm-strip %t.o -o %t1.o

jhenderson marked an inline comment as done.May 1 2019, 8:37 AM
jhenderson added inline comments.
test/tools/llvm-objcopy/ELF/no-strip-all.test
7 ↗(On Diff #197540)

Oh, I see what you mean. Yes, that should work. For some reason, I'd forgotten about -o!

jakehehrlich accepted this revision.May 1 2019, 12:51 PM

Good idea! The code looks good as well as testing.

This revision is now accepted and ready to land.May 1 2019, 12:51 PM
jhenderson updated this revision to Diff 197732.May 2 2019, 3:30 AM

Use -o instead of cp.

MaskRay accepted this revision.May 2 2019, 3:37 AM
MaskRay added inline comments.
tools/llvm-objcopy/CopyConfig.cpp
722 ↗(On Diff #197732)

Nit. auto * is more common, but I see the auto Arg below doesn't have the *... so you can ignore me.

This revision was automatically updated to reflect the committed changes.