This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy]Don't implicitly strip sections in segments
ClosedPublic

Authored by jhenderson on Mar 13 2019, 4:31 AM.

Details

Summary

This patch changes llvm-objcopy's behaviour to not strip sections that are in segments, if they otherwise would be due to a stripping operation (--strip-all, --strip-sections, --strip-non-alloc). This preserves the segment contents. It does not change the behaviour of --strip-all-gnu (although we could choose to do so), because GNU objcopy's behaviour in this case seems to be to strip the section, nor does it prevent removing of sections in segments with --remove-section (if a user REALLY wants to remove a section, we should probably let them, although I could be persuaded that warning might be appropriate). Tests have been added to show this latter behaviour.

I haven't modified --strip-debug's behaviour (i.e. a debug section in a segment will be stripped), but I'd be happy to.

This fixes https://bugs.llvm.org/show_bug.cgi?id=41006.

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.Mar 13 2019, 4:31 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson edited the summary of this revision. (Show Details)Mar 13 2019, 4:33 AM
grimar accepted this revision.Mar 13 2019, 5:38 AM

I approve, but please wait for somebody's else opinion too.

This revision is now accepted and ready to land.Mar 13 2019, 5:38 AM
jakehehrlich accepted this revision.Mar 13 2019, 1:43 PM

This isn't precisely what I had in mind but I think its a strict improvement. Tests look good and document the specific behavior well. I think the behavior with zeroing out sections in explicit cases or where we're looking to match GNU behavior precisely is a good idea (and would have been harder to impossible with what I had in mind). The only one I question is --strip-non-alloc but that was something I invented that turned out not to be useful. We could probably remove that and no one would care. There's not standard or use case to guide what should happen there. The guiding principal should probably be "when it doesn't make people scratch their heads, don't remove stuff in segments" which this change adhears to.

LGTM.

rupprecht accepted this revision.Mar 13 2019, 2:42 PM
rupprecht added inline comments.
test/tools/llvm-objcopy/ELF/strip-all-gnu.test
10 ↗(On Diff #190396)

Does od work on windows?

tools/llvm-objcopy/StripOpts.td
34 ↗(On Diff #190396)

The spacing around quotes is inconsistent; clang-format past rC356099 should be able to automatically fix this

jakehehrlich added inline comments.Mar 13 2019, 2:45 PM
test/tools/llvm-objcopy/ELF/strip-all-gnu.test
10 ↗(On Diff #190396)

We've been using in practice so...I think so?

jhenderson marked an inline comment as done.Mar 14 2019, 3:06 AM
jhenderson added inline comments.
test/tools/llvm-objcopy/ELF/strip-all-gnu.test
10 ↗(On Diff #190396)

Yes, od is part of GnuWin32, which is a pre-requisite for running the lit tests on Windows.

This revision was automatically updated to reflect the committed changes.