This change adds support for the --only-keep option and the -j alias as well. A common use case for these being used together is to dump a specific section's data. Additionally the --keep option is added (GNU objcopy doesn't have this) to avoid removing a bunch of things. This allows people to err on the side of stripping aggressively and then to keep the specific bits that they need for their application.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Diff context is missing.
I noticed that gnu objcopy also keeps the symbol table when using -j, along with all the symbols in the kept section(s). Do you think we should match this behaviour as well?
Reading the documentation, gnu objcopy also allows -j to match using wildcards. Is this a use case you intend to support? (I'm happy for it to be a separate change, if you plan on doing it later).
Why can't we just use the standard BinaryObject class for the -O binary/-j use case?
As discussed in person at the LLVM conference last week, I'd support an alias for -O binary/-j to dump a single section raw to a file, e.g. something like "--copy-section". This can be done later though.
tools/llvm-objcopy/llvm-objcopy.cpp | ||
---|---|---|
78–79 | Rather than call this "Strange exception", I think it might be more helpful to explain what this behaviour actually is (i.e. copies just the one section). Also missing a full stop! | |
82 | togethor -> together |
I didn't realize this. I think it should be to match though. I'll make that change.
Reading the documentation, gnu objcopy also allows -j to match using wildcards. Is this a use case you intend to support? (I'm happy for it to be a separate change, if you plan on doing it later).
Eventually I want to add section patterns for all the list like options but there's not a strong push for it from anyone right now so I'll wait. It should be easy enough to implement in a separate change however. I'm not sure if LLVM already has a wildcard patterns library but if not I can translate the pattern to a regex and match against that.
Why can't we just use the standard BinaryObject class for the -O binary/-j use case?
-O bianry outputs per segment in llvm-objcopy not per section. GNU objcopy seems to reconstruct the program headers from the remaining sections. We would have to change -O binary to be more section oriented. It's not really clear how -O binary should work in llvm-objcopy in the presence of multiple sections with some removed however. For instance say that .foo and .bar are seperated by a few sections but are in the same segment. Should llvm-objcopy -j .foo -j .bar -O binary produce a file that contains them one right after the other? Or should it dump them separated by the proper amount with zeros in between (that seems more sensible to me)? How should alignment be handled when these two sections being kept? The first section can just be at the top because 0x0 has maximal alignment but if we separate the second section from the first by the desired amount then it's alignment might be wrong. Does this mean we should pad the front with zeros until we can fit both in the output? That last option (padding before and between) would be fully consistent with what -O binary does now but seems to produce a bizarre result. Another (possibly exactly same?) way of handeling it would be to say that -O binary should just output the ELF minus all headers and without the data from any non-allocated section. That would produce the strange gap of zeros at the end and be consistent and everything, it just doesn't seem like sensible output to me. I could be persuaded to make the change though.
As discussed in person at the LLVM conference last week, I'd support an alias for -O binary/-j to dump a single section raw to a file, e.g. something like "--copy-section". This can be done later though.
I think GNU objcopy has the --dump-section option for precisely this purpose. I plan on implementing that.
- Fixed typos
- Changed --only-keep so that it keeps the symbol table and string table for the symbol table
- Fixed comments
Without thinking about it too much, the way I'd expect -j and -O binary to work is for it to be identical to -O binary with --remove-section for every other section, complete with padding being used to preserve alignment and relative positions etc (quick experiments shows this to be the case with GNU objcopy). That being said, I see that GNU objcopy has some unexpected behaviour when emitting binary output - the second of a pair of kept sections was neither aligned as it was in the input, nor positioned at the same relative distance to the first section, even though it was in the same segment.
Tangentially related, I also see that we don't have any tests for what happens when you combine --remove-section etc with -O binary, and I doubt from my initial observation of GNU's behaviour that we match its behaviour. My initial instinct is therefore that the problem is with our binary output format, and that ultimately, we should be using the BinaryObject class in this case, after we've fixed it to match GNU's behaviour.
test/tools/llvm-objcopy/basic-only-keep.test | ||
---|---|---|
3 | I've probably been inconsistent in requesting this for other switches in the past, but we should really test both aliases explicitly. A simple diff of the output when using -j and --only-keep would suffice for this. |
Hmmm. I definitely agree that we don't match GNU's behavior here. I'm also not sure we should but my primary hope would be that there is some sensible (e.g. everything is handled uniformly) behavior which is consistent in the kernel dumb case (if you have a bunch of contiguous segments output them contiguously) and in the section dump case. I also agree that whatever -j does it would be ideal if doing a --remove-section on everything else (sans .shstrtab, .symtab, and .strtab) did the same thing. That isn't currently the case, in fact -O basically just doesn't interact with the other options because it reads from the segment data directly. So that should likely change. I'll try and investigate what -O binary is doing precisely tomorrow (by looking at the code) so we can add a better account of what that's doing that what we get from treating it as a magical black box. I haven't a clue what it's doing but I'm not particularly convinced that it's sensible.
More concise summary of my thoughts on this
- -O binary dosn't interact with other options in any way but it likely should
- Matching GNU objcopy is favorable if GNU objcopy's behavior makes some degree of sense and isn't completely magical.
- Even if we don't exactly match GNU objcopy I would like to find some sensible and consistent behavior for -O binary so that running -O binary on a single reasonably normal ELF agrees with GNU objcopy *and* when used in combination with -j or a bunch of -R options that do the same thing produces the same result. I can't seem to find an answer to that however.
- I'll try and respond with precisely what GNU objcopy is doing in this case to see if that helps any.
test/tools/llvm-objcopy/basic-only-keep.test | ||
---|---|---|
3 | Sounds good to me. |
Can you show me which inputs were not preserving relative offsets of sections within the same segment? I gave up trying to read the code and went back to empirical testing. After getting many more data points my theory about the behavior is the following
The first segment that contains a non-removed section is treated specially for some reason. The first section behaves in the following manner:
- Trailing and initial removed sections of a segment are not copied to the output file.
- Relative offsets of sections with the segment are preserved
- Removed sections are overwritten with zeros if they still have allocated space due to relative offsets being preserved
The remaining segments are placed at locations that preserve the segment alignment. Only trailing removed sections seem to be removed in this case.
So alignment is trashed if the first section is removed but relative offsets seem to work correctly. In particular If the first section of a segment is never removed then -O binary seems to produce sensible output. Also if alignment is the only thing we sacrifice and we only sacrifice it in the case that the first section is removed then I think this is the most sensible behavior that can exist. It seems to me the following requirements are contradictory and one of them must be dropped
- -O binary -j <some section> should dump the contents of <some section> (without padding before or after)
- alignment of section offsets should be preserved
- relative offsets of sections should be preserved
If we drop 3) we can maintain alignment in all but one case and be consistent with GNU objcopy in every case I have thus far found. The only strange behavior we have to add is removing leading zero space from the first segment.
Never mind - I apparently failed at maths. I can't reproduce what I saw yesterday, so I'm 99% certain I just calculated the difference wrong.
The first segment that contains a non-removed section is treated specially for some reason. The first section behaves in the following manner:
- Trailing and initial removed sections of a segment are not copied to the output file.
- Relative offsets of sections with the segment are preserved
- Removed sections are overwritten with zeros if they still have allocated space due to relative offsets being preserved
The remaining segments are placed at locations that preserve the segment alignment. Only trailing removed sections seem to be removed in this case.
So alignment is trashed if the first section is removed but relative offsets seem to work correctly. In particular If the first section of a segment is never removed then -O binary seems to produce sensible output. Also if alignment is the only thing we sacrifice and we only sacrifice it in the case that the first section is removed then I think this is the most sensible behavior that can exist. It seems to me the following requirements are contradictory and one of them must be dropped
- -O binary -j <some section> should dump the contents of <some section> (without padding before or after)
- alignment of section offsets should be preserved
- relative offsets of sections should be preserved
If we drop 3) we can maintain alignment in all but one case and be consistent with GNU objcopy in every case I have thus far found. The only strange behavior we have to add is removing leading zero space from the first segment.
When you talk about relative offsets of sections, are you talking about relative to the segment start? If so, I agree that appears to be the objcopy behaviour, and I'm happy for this to be done in llvm-objcopy. With that change, we should be able to drop the SectionDump class, as the BinaryObject class should give us the functionality we expect for -j, along with the other cases. I think it might be wise to make these changes in a separate pre-requisite review, since it's really a separate issue (e.g. the behaviour we currently have for -R is different to gnu objcopy's, if the first or last section is stripped).
When you talk about relative offsets of sections, are you talking about relative to the segment start? If so, I agree that appears to be the objcopy behaviour, and I'm happy for this to be done in llvm-objcopy. With that change, we should be able to drop the SectionDump class, as the BinaryObject class should give us the functionality we expect for -j, along with the other cases. I think it might be wise to make these changes in a separate pre-requisite review, since it's really a separate issue (e.g. the behaviour we currently have for -R is different to gnu objcopy's, if the first or last section is stripped).
Well I meant the relative offset between any two sections. So .text and .text3 if both in the same segment would be the same distance away from each other in the ELF file as they were in the binary file. But yes, I agree I should add another pre-req and then add this
Okay, I think that makes sense. Also, if the first section remaining was at offset 0 in the segment, all other sections will naturally remain at the same relative offset in the segment. Obviously, if the first section is removed (or presumably is not at offset 0), then things change.
I'll wait on your changes to the binary object writing before reviewing this further.
Yikes, here's an update on what GNU objcopy does. It interprets "keep" and "remove" differently. Namely, "keep" is not "remove all but". If you "remove" and "keep" a section then an error is thrown. --only-keep appears to be "remove all but" plus "keep". I don't think this behavior is so terrible to implement. We would implement --only-keep so that it, in an order independent way, removes all sections but the named one (and certain special sections obviously) then additionally adds the section to a keep list. We then, before performing removal, iterate though the keep list to see if we're about to remove anything on the keep list. If we detect that we are, we throw an error. This a general way of implementing this keep functionality so as new options come up there will be a standard way of handling it.
Ah crap this isn't right either because this dosn't do what GNU objcopy does in the -j case where multiple sections are kept. I'll investigate more.
Ok the following behavior seems consistent with my testing. There are 3 things that can happen to a section (and a 4th default thing sort of)
- Explicit remove
- Explicit copy
- Implicit remove
Implicit copy is an obvious 4th missing part and it's what happens by default but no option has the effect of making this occur.
GNU objcopy throws an error anytime you explicitly remove a section AND explicitly copy that section. The following behavoirs appear to be true
- No option implicitly copies a section (it's not a possible operation) so if a section is implicitly removed another option can't contradict that (so only explicit contradictions can occur). Implicit removals will never cause errors.
- -j explicitly copies the named section (or named sections in the case of patterns) and implicitly removes all other sections
- -R explicitly removes sections
- Faced with a contradiction between an implicit and an explicit remove/copy you do the explicit one.
- Faced with an explicit-explicit contradiction you throw an error
- stripping options are some how magical and ignore these rules above and simply remove what they want to (e.g. they don't respect explicit copies and no errors are thrown)
I propose that llvm-objcopy behave in the following manner:
- -R explicitly removes sections
- -j explicitly copies sections and implicitly removes all other sections
- we add a -keep that explicitly copies a section (the reason this doesn't exist in objcopy is because stripping doesn't respect explicit copies)
- we should resolve implicit-explicit contradictions the same way
- we should resolve explicit-explicit contradictions by copying *not* by throwing an error.
- striping operations should implicitly remove sections
This is still a drop in replacement for objcopy because if you used -j and a stripping option in conjunction in a way that was a contradiction with GNU objcopy you get out a basically empty file. So we're strictly making objcopy more sensible be behaving 6 behave this way. Both GNU objcopy and my proposed behavior for llvm-objcopy have order invariance which is nice.
I feel strongly about 6. What GNU objcopy does there makes very little sense to me and copying what they do can't possibly help anyone. Additionally I can imagine it might be helpful to implement one of the more extreme stripping options but keep just what you need. For instance --strip-non-alloc -keep .gnu.warning or --strip-non-alloc -keep .comment etc... etc... is a generically useful thing to be able to do and not something you can do with GNU objcopy (GNU objcopy isn't very aggressive in stripping though so it gets away with that).
I could budge on point 5 and throw an error in that case. I just think a) not throwing an error is much easier, b) it doesn't break anyone and c) very little is gained by throwing an error. If we don't check for explicit explicit contradictions we just have to be a bit careful about the order we check the arguments.
How does this sound?
I like the sound of this plan. It's pretty clear what the semantics are from your explanation, although we should somehow document this, maybe in a big comment block somewhere until llvm-objcopy has its own documentation, and I agree with your arguments for the differences in behaviour.
- I implemented the semantics I presented above. Also note that my proposal for llvm-objcopy simplifies to "copies have priority over removes". I describe the behavior this way in the comment I added for this behavior.
- I added a -keep operation
- All the SectionDump non-sense is gone
Could we have test cases for the following please:
- --only-keep + some way of stripping each of .symtab, .strtab, and .shstrtab.
- Show that --keep overrides --only-keep (i.e. sections not specified in --only-keep will still be kept if specified by --keep).
Two other tests mentioned inline, that are --only-keep equivalents to --keep.
As an aside, 2) might be a slightly surprising behaviour to people due to the name of the switches, but I don't think that's a big issue.
Also lots of small nits in the comments.
test/tools/llvm-objcopy/basic-keep.test | ||
---|---|---|
1 ↗ | (On Diff #123095) | Could we have an -only-keep version of this test, please. This will demonstrate that --only-keep=.test has priority over e.g. --strip-non-alloc. |
test/tools/llvm-objcopy/explicit-keep-remove.test | ||
2 ↗ | (On Diff #123095) | Could you add an -only-keep version of this test as well, please. |
test/tools/llvm-objcopy/keep-multi.test | ||
1 ↗ | (On Diff #123095) | Could you rename this test, please, to "keep-many.test" for consistency with "only-keep-many.test" (or vice versa, I don't mind which). |
tools/llvm-objcopy/llvm-objcopy.cpp | ||
156 | handeling -> handling | |
159 | weather -> whether | |
241 | Explicitlly -> Explicitly | |
245 | Full stop. | |
245–248 | Please add a new line before and after this if clause (with its comment), to clearly indicate what the comment refers to. | |
249–254 | I think it would be nice to have a comment around these two ifs to say something like "keep special sections." | |
255 | Full stop. | |
262 | Explicitlly -> Explicitly. Also full stop. | |
266 | Full stop. |
Almost ready to go in, but I've thought of one more test that I think would be worth having:
- --only-keep + --strip-sections. Test that the kept section contents are still in the ELF, but not those of .shstrtab. This is to show that the if statement at line 267 does not prevent stripping of the section header strings.
I was also going to suggest the following test. However, I realise that with the current behaviour of llvm-objcopy, it's not necessary, because IIRC, --strip-sections does not strip the .shstrtab contents, unless explicitly asked to do so. Is this correct? If so, I suspect that this behaviour needs to change. objcopy rebuilds the section header string table from scratch (and if necessary, splits it from the symbol string table, if the two are merged, like they are in clang's output), removing any redundant entries as it goes. If the section headers have been removed (e.g. via --strip-all), then it completely strips .shstrtab's contents. This is important, because in some cases, leaving the section header strings in the ELF could leak sensitive data to an end user.
- --keep .shstrtab + --strip-sections. Test that the section header strings are still present in the ELF somewhere, even though the section headers have been removed.
--only-keep + --strip-sections. Test that the kept section contents are still in the ELF, but not those of .shstrtab. This is to show that the if statement at line 267 does not prevent stripping of the section header strings.
Good idea. This also made me think of a funny case. If you were to run "llvm-objcopy --only-keep=.shstrtab --strip-sections" I think it would produce a file with an elf header and a bit of data containing "\0.shstrtab\0" and nothing else. This is a silly case and that behavior doesn't seem too bad but I found it funny nevertheless.
I was also going to suggest the following test. However, I realise that with the current behaviour of llvm-objcopy, it's not necessary, because IIRC, --strip-sections does not strip the .shstrtab contents, unless explicitly asked to do so. Is this correct? If so, I suspect that this behaviour needs to change. objcopy rebuilds the section header string table from scratch (and if necessary, splits it from the symbol string table, if the two are merged, like they are in clang's output), removing any redundant entries as it goes. If the section headers have been removed (e.g. via --strip-all), then it completely strips .shstrtab's contents. This is important, because in some cases, leaving the section header strings in the ELF could leak sensitive data to an end user.
--keep .shstrtab + --strip-sections. Test that the section header strings are still present in the ELF somewhere, even though the section headers have been removed.
--strip-sections removes .shstrtab and sets the flag to indicate that no section headers should be output. Unless --strip-sections is used the section header string table is rebuilt from scratch. The --keep .shstrtab + --strip-sections would include the string table in the output file but there would be no section header table. If --strip-sections is used then nothing from the string table should make it out. I'll defintl add the --keep .shstrtab + --strip-sections test case but should I also add a test to make sure that --strip-sections doesn't leak section names? Also why should we need to split string table and section header string table?
So, apparently, I'm unable to read code/remember what I've reviewed! I could have sworn I'd checked and found no rebuilding of the string table... Never mind - moving on! (For the record, there is no particular reason for llvm-objcopy to split apart the section header string table and the symbol name string table).
test/tools/llvm-objcopy/strip-sections-only-keep.test | ||
---|---|---|
1 ↗ | (On Diff #124681) | This test should also check that there is no ".test" string in the output. As noted, in the inline comments elsewhere, there's a clause that theoretically prevents stripping of .shstrtab, but we don't want it prevented in this case. A more general test illustrating that --strip-sections doesn't leak section names would be good, but doesn't need doing as part of this change. |
tools/llvm-objcopy/llvm-objcopy.cpp | ||
252 | Blank line before this comment. |
I've probably been inconsistent in requesting this for other switches in the past, but we should really test both aliases explicitly. A simple diff of the output when using -j and --only-keep would suffice for this.