elf utils implements a particularly extreme form of stripping that I'd like to support. eu-strip has an option called "strip-sections" that removes all section headers and leaves only program headers and the segment data. I have implemented this option partly as a test but mainly because in Fuchsia we would like to use this option to minimize the size of our executables. The other strip options that are on my list include --strip-all and --strip-debug. This is a preliminary implementation that I'd like to start using in Fuchsia builds if possible. This change implements such a stripping option for llvm-objcopy
Details
- Reviewers
jhenderson phosek - Commits
- rGf03384dce7cf: Reland "[llvm-objcopy] Add support for --strip-sections to remove all section…
rGb5152447baba: [llvm-objcopy] Add support for --strip-sections to remove all section headers…
rL315484: Reland "[llvm-objcopy] Add support for --strip-sections to remove all section…
rL315412: [llvm-objcopy] Add support for --strip-sections to remove all section headers…
Diff Detail
- Repository
- rL LLVM
Event Timeline
I definitely support the new switch, but there are a few issues with this change as it stands:
As mentioned inline, I think quite a few of these changes are not specific to removing all section headers, but rather should be in D38260.
Also, I'm not convinced that removing all sections is the same as removing all section headers, i.e. -R on every section is subtly different from --strip-sections. This is because removing sections one-by-one should still leave the null section header, in my opinion, whereas --strip-sections shouldn't. Essentially there's a slight semantic difference in an ELF without a section header table, and one with no sections.
Finally, there's currently no test for trying to strip the section header string table directly, if there are any remaining sections. I'm not sure it should be an error anyway - it looks like GNU binutils at the very least can handle an ELF with no section header string table, but with section headers, without too much problem, although I'd expect the e_shstrndx field to be set to 0 in this case.
test/tools/llvm-objcopy/strip-sections.test | ||
---|---|---|
2 | This test needs to test the ELF header fields as well. | |
tools/llvm-objcopy/Object.cpp | ||
618–627 | Full stop. Why do we need the first loop here? What was wrong with the previous behaviour? If the answer is because the section data should be preserved in the PT_LOAD segment regardless of whether a section is stripped or not, then shouldn't this change also be present in D38260? | |
644–645 | This sounds like it's not limited to this change? What's stopping us using -R to strip the .symtab (as long as we also strip any other sections referring to it)? | |
646–654 | Ditto - should this not be part of D38260? |
After some thought I agree. Here's the plan I propose
- Update D38260 to add the supported features (including targeted tests that don't just remove everything)
- rebase those changes into this change
- Add a Boolean for weather or not to output a section table
- Have --strip-sections remove all sections *and* set that boolean.
An interesting option from there might be to implement a flag to toggle weather or not the section table is output from there. "llvm-objcopy --strip-sections" would then become equivalent to "llvm-objcopy --no-section-headers -R *" if we were implement globs for section names in section removal.
- Added boolean flag so that removing sections isn't tied to not having a section header.
- Moved some of these changes into the -R change where they were more appropriate
- This change should be the official change where writing out files without section headers is added.
I would have no problem with this.
test/tools/llvm-objcopy/strip-sections.test | ||
---|---|---|
2 | For clarity, I mean the e_shnum, e_shoff, and e_shstrndx fields. | |
tools/llvm-objcopy/Object.cpp | ||
743 | We don't need to do this if there is no section header table. | |
tools/llvm-objcopy/llvm-objcopy.cpp | ||
79–86 | It feels like there's a lot of overlap here. Would it be possible to fold the two cases together for this bit? I.e. the predicate becomes "if StripSections or in ToRemove list". |
- rebased latest remove section changes
- Made sure SHOffset wasn't bumped too far if no section header was going to be output
I realize I haven't added the ELF header stuff to the test. I need to go now but I'll do that tomorrow.
LGTM.
test/tools/llvm-objcopy/strip-sections.test | ||
---|---|---|
29 | FWIW, I don't think you need to check all the fields, just the section header ones, but if you think it's useful, I'm not opposed to it. |
test/tools/llvm-objcopy/strip-sections.test | ||
---|---|---|
29 | To be perfectly honest I just copy and paste the headers for %t and then make the changes that should be made so that they match what %t2 should be. So this is me just being lazy not me covering bases. I don't think it's harming anything so I'll just leave it. |
This test needs to test the ELF header fields as well.