[llvm-objcopy] Add --only-keep-debug

[MaskRay]

Summary

GNU objcopy has two complimentary options to split files into debug and non-debug parts. --strip-debug is very simple. It's complementary option --only-keep-debug is a bit more complicated. --only-keep debug attempts to keep all section headers but remove any non-debug information it can. GNU objcopy appears to accomplish this by making every allocated section into a SHT_NOBITS section. The GNU objcopy man page explains that this is because the section headers have to be kept so that the debugger knows what parts of the file are what and can match the parts of the two files up. Note I plan on adding --add-gnu-debuglink and an option akin to -split-dwarf that will do all of these things at once for you in another commit.

[jhenderson]

Honestly, I'm a little confused by the complexity of this patch. It seems to me like changing a section to SHT_NOBITS should be pretty straightforward, since it's just changing the value of one field in a section header. Perhaps you could explain this a little more, and where necessary, add some code comments to the same effect.

Also, looking at the man page (I haven't tested this), but it seems to me like it's not just allocatable sections that get replaced, but any non-debug section. In other words, I'd expect the inverse of the predicate for --strip-debug to be used as the predicate for replacing. Is that not what you've observed?

test/tools/llvm-objcopy/replace-text-symtab.test

1 ↗

(On Diff #124465)

It's not clear to me what the purpose of the "symtab" bit is in the test name. Also, since this is testing the only-keep-debug switch, I think the test should say so in the name. The "replace" part isn't really relevant. Same sort of comment could apply to the other two tests.

tools/llvm-objcopy/Object.cpp

163

I don't think this case is tested at all.

166

Is the "this->" needed here?

246

Test needed for this error case.

327

Unnecessary "this->". Is this how clang-format formats this block? It's quite surprising if it is!

684

I don't think this should be called "ToRemove", since it isn't removing anything, just replacing.

686

Why do we do this extra sorting? Wouldn't it be easier (and probably faster) to just have an if(!ToRemove(*Sec)){continue;} inside the later for loop?

690

Test for this case?

691

And for these cases?

698–708

Why do we need to do all this? Can't we just have the one line: Sec->Type = SHT_NOBITS and not create a completely new section?

tools/llvm-objcopy/llvm-objcopy.cpp

185

Full stop. Why not do the replacing after the removal? Then there's less to replace, potentially.

[jakehehrlich]

Honestly, I'm a little confused by the complexity of this patch. It seems to me like changing a section to SHT_NOBITS should be pretty straightforward, since it's just changing the value of one field in a section header. Perhaps you could explain this a little more, and where necessary, add some code comments to the same effect.

The type is how we know how to cast things. It's effectively our RTTI (perhaps that wasn't a good choice). To be honest I think changing to SHT_NOBITS would probably be harmless but that wouldn't be the case for other section types and I'm not 100% sure it's safe. Moreover I don't know that I want to add the mental burdon of worrying about weather or not a class's section type changed or not at some point. Also when we call writeSection every writeSection would have to check if it was a NOBITS section or not. We could also check that case in the Object::writeSections. Layout would happen correctly I think so no changes would need to occur there. removeSectionReference could throw an error as when it should as well. In general it isn't clear to me what all the repercussions would be to just changing the type. If we create a new section then it's clear that there won't be any surprising issues.

Also, looking at the man page (I haven't tested this), but it seems to me like it's not just allocatable sections that get replaced, but any non-debug section. In other words, I'd expect the inverse of the predicate for --strip-debug to be used as the predicate for replacing. Is that not what you've observed?

Yeah that doesn't wind up being what it does in practice and I don't think we really want it to do that since there are other things like .note, .comment, .gnu.warning, etc might be nice to have. As near as I can tell it only makes the allocated sections into NOBITS sections. In general since the debug information is quite big I think keeping around extra possibly helpful information isn't too big of a deal. It also saves us from explicitly keeping a bunch of particular magic sections.

[jakehehrlich]

test/tools/llvm-objcopy/replace-text-symtab.test

1 ↗

(On Diff #124465)

So my intent here was a bit more subtle but certainly I haven't made that clear in the test. I was specifically trying to trigger the case where the symbol table needs to replace a section reference for a symbol. So maybe this should be "replace-text-ref-in-symtab.test"?

only-keep-debug just happens to be the option I used to do the replacing. Thinking about it, I never actually tried testing only-keep-debug. I only tested certain code paths. I should add that test.

tools/llvm-objcopy/Object.cpp

163

In practice this can't come up with the options we have now but I think it's important to throw an error if someone tries to replace the wrong thing in code. It's an artifact of a more general interface existing that can be used. I don't really like that being the case but having the error and never being able to test it seems better than not having the error and one day the code silently failing. I can be convinced otherwise though.

246

This is another one of those errors that I can't test with the current interface. There isn't a way to get yaml2obj to produce a relocation that dosn't have .symtab as its link and there isn't a way to make .symtab allocated so there isn't a way to get --only-keep-debug to trigger this error.

686

derp...Yeah that's way better. I was just blindly trying to refactor the code from removeSections and it pretty clearly wasn't all that relevant in the end.

691

This is another error case I can't test because yaml2obj can't produce an allocated .shstrtab and --only-keep-debug can't make a non-allocated section into a NOBITS section. Maybe I should add a hidden --make-nobits=<section> option for the sake of testing this stuff.

[jakehehrlich]

Fixed loop and ToRemove/ToReplace predicate

tools/llvm-objcopy/Object.cpp

690

Again, I can't make an allocated symbol table.

[jhenderson]

Maybe I should add a hidden --make-nobits=<section> option for the sake of testing this stuff.

I kind of like this idea, but if you wanted to do it, I might take it one step further and do something like --change-type=<section>,<new type> or similar. Maybe even something like --edit-shdr=<section index or name>,<new section properties> where <new section properties> is similar to the assembler .section directive, so takes the form <new name>,<flags>,<type>,..., and then I see no reason to make it hidden, since a tool that can effectively binary edit the section header fields is quite useful for wider testing, at the very least.

In D40523#937962, @jakehehrlich wrote:

The type is how we know how to cast things. It's effectively our RTTI (perhaps that wasn't a good choice). To be honest I think changing to SHT_NOBITS would probably be harmless but that wouldn't be the case for other section types and I'm not 100% sure it's safe. Moreover I don't know that I want to add the mental burdon of worrying about weather or not a class's section type changed or not at some point. Also when we call writeSection every writeSection would have to check if it was a NOBITS section or not. We could also check that case in the Object::writeSections. Layout would happen correctly I think so no changes would need to occur there. removeSectionReference could throw an error as when it should as well. In general it isn't clear to me what all the repercussions would be to just changing the type. If we create a new section then it's clear that there won't be any surprising issues.

Okay, that makes a lot of sense (although your function is "replaceWithNoBits", not "replace with arbitrary type"). I missed the fact that you are creating generic "Section" instances, rather than one of the various section sub-classes, and that it has no contents, so what you do there is reasonable. I do think it needs some code comments to explain some of this though.

Yeah that doesn't wind up being what it does in practice and I don't think we really want it to do that since there are other things like .note, .comment, .gnu.warning, etc might be nice to have. As near as I can tell it only makes the allocated sections into NOBITS sections. In general since the debug information is quite big I think keeping around extra possibly helpful information isn't too big of a deal. It also saves us from explicitly keeping a bunch of particular magic sections.

In other words - don't rely on the documentation being complete or accurate!

test/tools/llvm-objcopy/replace-text-symtab.test

1 ↗

(On Diff #124465)

OK. The test rename, and a new test specifically testing only-keep-debug would work for me. Please rename the other tests in a similar manner.

tools/llvm-objcopy/Object.cpp

163

Okay, that makes sense. I'd have probably written this as an assert, since we can't get into the case currently, but I'm happy with your approach here.

Once again though, this is something that we could unit-test if we figured out how to set up unit tests for llvm-objcopy.

685

Unnecessary blank line?

[jakehehrlich]

I kind of like this idea, but if you wanted to do it, I might take it one step further and do something like --change-type=<section>,<new type> or similar. Maybe even something like --edit-shdr=<section index or name>,<new section properties> where <new section properties> is similar to the assembler .section directive, so takes the form <new name>,<flags>,<type>,..., and then I see no reason to make it hidden, since a tool that can effectively binary edit the section header fields is quite useful for wider testing, at the very least.

I'm writing code for --add-gnu-debuglink right now which requires section adding. Section adding raises some interesting questions like "what should OriginalOffset be?" If I can resolve those issues in a consistent way then "--edit-shdr <section>=<stuff>" can be "-R <section> -add-section <section>=<stuff>". Adding a specific section like the debuglink section is a tad different but the major technical challenge is the same. That would let us add tests for these strange tests without modifying yaml2obj. In general it would let us get around all the cases where yaml2obj holds us back.

[jakehehrlich]

1. Added a test for the actual feature being added in this change
2. Added some comments in replaceWithNoBits
3. Removed that line
4. I feel like I did something else that was mentioned as a thing I should do but I can't remember what it was.

[jhenderson]

I just realised one fairly major aspect of this that isn't tested, namely the behaviour when there are segments in play. Could you add tests that demonstrate what the behaviour is here - I expect the contents to effectively be wiped, by changing the file size to zero (although I'm not sure if that is what actually happens here - could you point out where it does, if it does). GNU objcopy's behaviour is a little weird around here - the segment offsets in a simple case I just tried were outside the file! I don't think we need to match it, but I would expect something sensible.

In D40523#939690, @jakehehrlich wrote:

I'm writing code for --add-gnu-debuglink right now which requires section adding. Section adding raises some interesting questions like "what should OriginalOffset be?" If I can resolve those issues in a consistent way then "--edit-shdr <section>=<stuff>" can be "-R <section> -add-section <section>=<stuff>". Adding a specific section like the debuglink section is a tad different but the major technical challenge is the same. That would let us add tests for these strange tests without modifying yaml2obj. In general it would let us get around all the cases where yaml2obj holds us back.

Yes, I like this idea. However, we'd need the ability to explicitly specify where the new section is placed in the ELF. Otherwise, we end up moving the section at the same time as modifying it, which may not be desirable.

In D40523#939723, @jakehehrlich wrote:

4. I feel like I did something else that was mentioned as a thing I should do but I can't remember what it was.

I think it was the test renaming.

test/tools/llvm-objcopy/only-keep-debug.test

31

Could you make this symbol at a non-zero offset, and then test that the value is preserved, please.

38–47

You should probably check that the types of .debugfoo, .note, etc haven't been changed as well.

I would also recommend adding non-zero content to all sections, and testing that it is preserved for those that are not replaced.

tools/llvm-objcopy/Object.cpp

694

make whole -> make a whole

What does "delete the old way" mean?

tools/llvm-objcopy/llvm-objcopy.cpp

185

Ping.

[jakehehrlich]

So I spoke with Roland McGrath about what should be happening with the program headers and it's kind of strange. Basically they should be kept exactly as is but that shouldn't mean that all that space is consumed because of them which demands a more complex change that I desired. The program headers are sometimes needed to match the contents due to prelink.

This presents a complication because right now segments are always laid out where as in this case we really should just ignore their existence and output them without considering them during layout. It feels a bit hacky but I suppose we can add a boolean flag for "ignore program headers during layout". Honestly the whole behavoir of --only-keep-debug is quite exceptional; it dosn't really like obeying the rules.

[jhenderson]

In D40523#941536, @jakehehrlich wrote:

So I spoke with Roland McGrath about what should be happening with the program headers and it's kind of strange. Basically they should be kept exactly as is but that shouldn't mean that all that space is consumed because of them which demands a more complex change that I desired. The program headers are sometimes needed to match the contents due to prelink.

This presents a complication because right now segments are always laid out where as in this case we really should just ignore their existence and output them without considering them during layout. It feels a bit hacky but I suppose we can add a boolean flag for "ignore program headers during layout". Honestly the whole behavoir of --only-keep-debug is quite exceptional; it dosn't really like obeying the rules.

I agree that it is all a bit weird. Further investigation suggests that objcopy does the following:

1. All allocatable sections are converted to NOBITS.
2. The size of these sections is left unchanged.
3. The offset of these sections is moved to the start offset of the program headers, before the change.
4. The file size field is the only field of the program headers that changes, with the NOBITS sections no longer contributing to its size, as is normal. The address and offset remains the same.
5. Sections not in segments are moved as normal, i.e. based on the updated segment sizes from before. They may move to before segments, if there is space for them to fit. I assume (but I haven't experimented with this) that they cannot overlap the spot where a segment should be (e.g. if the segment is at offset two, a 2-byte-sized non-alloc section won't appear at offset 1).
6. The output file size does not include segments with their contents stripped.

I get the feeling that none of the above will happen with the current layout scheme... I don't have a good proposal for you at this point, other than to maybe consider a separate layout scheme entirely, that observes the above rules.

[jakehehrlich]

In D40523#941757, @jhenderson wrote:

In D40523#941536, @jakehehrlich wrote:

So I spoke with Roland McGrath about what should be happening with the program headers and it's kind of strange. Basically they should be kept exactly as is but that shouldn't mean that all that space is consumed because of them which demands a more complex change that I desired. The program headers are sometimes needed to match the contents due to prelink.

This presents a complication because right now segments are always laid out where as in this case we really should just ignore their existence and output them without considering them during layout. It feels a bit hacky but I suppose we can add a boolean flag for "ignore program headers during layout". Honestly the whole behavoir of --only-keep-debug is quite exceptional; it dosn't really like obeying the rules.

I agree that it is all a bit weird. Further investigation suggests that objcopy does the following:

1. All allocatable sections are converted to NOBITS.
2. The size of these sections is left unchanged.

Done and agreed.

3. The offset of these sections is moved to the start offset of the program headers, before the change.

I don't think there's any terribly good reason to do this honestly. I'd rather keep their index if possible.

4. The file size field is the only field of the program headers that changes, with the NOBITS sections no longer contributing to its size, as is normal. The address and offset remains the same.

This is tricky. I'm going to upload some code that changes the file size accordingly but please don't bother approving it as a) it's subject to change and b) I'm not sure it's right or that I understand all the moving parts here.

5. Sections not in segments are moved as normal, i.e. based on the updated segment sizes from before. They may move to before segments, if there is space for them to fit. I assume (but I haven't experimented with this) that they cannot overlap the spot where a segment should be (e.g. if the segment is at offset two, a 2-byte-sized non-alloc section won't appear at offset 1).

Yeah I think this is right and this property would hold right now (but the segments wouldn't be any smaller, they would just take up lots of space).

6. The output file size does not include segments with their contents stripped.

I'm confused how this is different from 4. Mind elaborating?

[jakehehrlich]

1. Added code to reduce the FileSize of program headers

[jhenderson]

In D40523#947252, @jakehehrlich wrote:

3. The offset of these sections is moved to the start offset of the program headers, before the change.

I don't think there's any terribly good reason to do this honestly. I'd rather keep their index if possible.

As long as consumers of this are fine with that, I have no issues either way, although a non-zero relative offset within a segment with file size zero doesn't make much sense (it implies that the section does not sit inside the segment). The relative order of the sections is still controlled by the address fields, which remain unchanged.

5. Sections not in segments are moved as normal, i.e. based on the updated segment sizes from before. They may move to before segments, if there is space for them to fit. I assume (but I haven't experimented with this) that they cannot overlap the spot where a segment should be (e.g. if the segment is at offset two, a 2-byte-sized non-alloc section won't appear at offset 1).

Yeah I think this is right and this property would hold right now (but the segments wouldn't be any smaller, they would just take up lots of space).

I think the point of reducing the file size field of the segments is so that they actually do not take up any space in the output file, reducing the overall file size by quite a bit potentially. See below.

4. The file size field is the only field of the program headers that changes, with the NOBITS sections no longer contributing to its size, as is normal. The address and offset remains the same.
5. The output file size does not include segments with their contents stripped.

I'm confused how this is different from 4. Mind elaborating?

4. deals with the file size field of a segment, and indeed, it's actual size on disk, whereas 6) is to do with the offsets of the segments and the overall object file size. Typically, NOBITS sections do not take up any disk footprint, so zero-sized segments make sense. However, usually a segment will always reside within the bounds of the output file, i.e. p_offset <= file size. In this case however, p_offset might be greater than file size, because the segments are not moved, but also require zero space on disk. objcopy appears to shrink the object to as small as it can, whilst still preserving the remaining non-replaced section contents at the location wherever that may be.

[jakehehrlich]

As long as consumers of this are fine with that, I have no issues either way, although a non-zero relative offset within a segment with file size zero doesn't make much sense (it implies that the section does not sit inside the segment). The relative order of the sections is still controlled by the address fields, which remain unchanged.

Sorry, I got confused with offset and index here. The behavior I'd like here is for the offset to be set by the existing layout algorithm which will frequently be the top but sometimes will be the bottom and occasionally will be somewhere in the middle depending on how we set segment sizes. In this current patch the size of PT_LOAD segments and their children segments will always be zero so everything will have the same offset (the only offset possible). I agree keeping the original offset doesn't make sense.

I think the point of reducing the file size field of the segments is so that they actually do not take up any space in the output file, reducing the overall file size by quite a bit potentially. See below.

Right. Roland's claim was that you should just copy the segments and then layout the sections as if the segments didn't matter. Since NOBITS sections don't really need to consider segments to be laid out and non-allocated sections generally are not in segments this is actually quite valid but the program headers become bizarrely special. Also this fails when you have non allocated sections in segments which is something we've supported so far (correctly so I'd say). Roland takes the stance that it isn't clear what would be wrong with changing the filesize but believes that not changing it is certainly correct.

As for this patch. I've slept on it and I'm ok with this going in if you think everything else is in order. Particularly look at how I change segment size BTW as I believe I'm being a bit more aggressive that even GNU objcopy is being (since it appears to keep around RELRO segments where as I am not)

[jhenderson]

As well as the new inline comments, there are a number of older points that need addressing still, when you next make changes.

It's not clear to me why you remove the new NOBITS sections from the segments. We don't remove other NOBITS sections from segments after all.

I did some thinking, and I'm pretty confident I understand more about the need for section offsets to change to the start of the segment (i.e. point 3 above). By not moving the section offset, but reducing the segment file size, the section to segment mapping (as displayed with readelf -l) no longer matches following stripping, whereas GNU objcopy behaviour (i.e. moving the section to the start offset of the segment) means that it continues to match (readelf appears to treat zero-sized sections at the end of a segment as being in that segment). Furthermore, if you consider the case where the sections were NOBITS in the linker input, they would be emitted at the start offsets. Not moving the sections is starting to feel simply wrong to me.

test/tools/llvm-objcopy/only-keep-debug-phdrs.test

1

Could you please either extend this test, or add a new test that demonstrates the behaviour when a non-alloc section is in a segment.

3

You might want to break this line over multiple lines using the '\' end-of-line mechanism.

15–16

Is this section supposed to be empty explicitly? If so, could you rename it .empty (to make the intention explicit) or similar, and if not, please add some content.

59

You should check the address and offsets of these sections, since we are messing about with layout with this switch.

83

You should check the value of these symbols, for the same reason as testing the section properties.

103–105

I think these fields need testing as well for the program headers, to show the behaviour.

tools/llvm-objcopy/Object.cpp

695

with the new -> with references to the new

[jakehehrlich]

I did some thinking, and I'm pretty confident I understand more about the need for section offsets to change to the start of the segment (i.e. point 3 above). By not moving the section offset, but reducing the segment file size, the section to segment mapping (as displayed with readelf -l) no longer matches following stripping, whereas GNU objcopy behaviour (i.e. moving the section to the start offset of the segment) means that it continues to match (readelf appears to treat zero-sized sections at the end of a segment as being in that segment). Furthermore, if you consider the case where the sections were NOBITS in the linker input, they would be emitted at the start offsets. Not moving the sections is starting to feel simply wrong to me.

Ah I just realized that I have to set the OriginalOffset to be the same as the parent segment. I was thinking for some reason that by changing the FileSize of the segments layout would handle it all for me but of course that's not right at all. Sorry about that.

[jakehehrlich]

It's not clear to me why you remove the new NOBITS sections from the segments. We don't remove other NOBITS sections from segments after all.

They don't affect anything by being in a segment or not being in a segment and I want the references gone. That all said I don't know why that method still exists because none of those things have been used in forever. I'm just going to get rid of all those empty methods in Segment since they're kind of vestiges of ideas that either weren't needed.

[MaskRay]

We committed https://reviews.llvm.org/D67137

test/tools/llvm-objcopy/only-keep-debug-phdrs.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -only-keep-debug %t %t2
+# RUN: llvm-readobj -file-headers -sections -symbols -program-headers %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .debugfoo
+    Type:            SHT_PROGBITS
+    Content:         "00000000"
+  - Name:            .blarg
+    Type:            SHT_PROGBITS
+  - Name:            .readonly
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "00000000"
+  - Name:            .note
+    Type:             SHT_NOTE
+    Flags:           [ SHF_ALLOC ]
+    Content:         "00000000"
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000010
+    Content:         "00000000"
+Symbols:
+  Global:
+    - Name: foo
+      Section: .text
+    - Name: debugfoo
+      Section: .debugfoo
+ProgramHeaders:
+  - Type: PT_LOAD
+    Flags: [PF_X, PF_R]
+    VAddr: 0x1000
+    PAddr: 0x1000
+    Sections:
+      - Section: .text
+  - Type: PT_LOAD
+    Flags: [PF_R]
+    VAddr: 0x2000
+    PAddr: 0x2000
+    Sections:
+      - Section: .readonly
+      - Section: .note
+  - Type: PT_NOTE
+    Flags: [PF_R]
+    VAddr: 0x2000
+    PAddr: 0x2000
+    Sections:
+      - Section: .note
+
+# CHECK: SectionHeaderCount: 9
+
+# CHECK: Name: .debugfoo
+# CHECK: Name: .blarg
+# CHECK: Name: .readonly
+# CHECK-NEXT: Type: SHT_NOBITS
+# CHECK: Name: .note
+# CHECK: Name: .text
+# CHECK-NEXT: Type: SHT_NOBITS
+# CHECK: Name: .symtab
+# CHECK: Name: .strtab
+# CHECK: Name: .shstrtab
+
+# Check that both symbols are copied
+# CHECK:      Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size:
+# CHECK-NEXT:     Binding:
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size:
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: .text
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: debugfoo
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size:
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: .debugfoo
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+# CHECK: ProgramHeader {
+# CHECK-NEXT: Type: PT_LOAD
+# CHECK-NEXT: Offset
+# CHECK-NEXT: Virtual
+# CHECK-NEXT: Physical
+# CHECK-NEXT: FileSize: 0
+# CHECK-NEXT: MemSize: 4
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   PF_R
+# CHECK-NEXT:   PF_X
+# CHECK-NEXT: ]
+
+# CHECK: ProgramHeader {
+# CHECK-NEXT: Type: PT_LOAD
+# CHECK-NEXT: Offset
+# CHECK-NEXT: Virtual
+# CHECK-NEXT: Physical
+# CHECK-NEXT: FileSize: 0
+# CHECK-NEXT: MemSize: 8
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   PF_R
+# CHECK-NEXT: ]
+
+# CHECK: ProgramHeader {
+# CHECK-NEXT: Type: PT_NOTE
+# CHECK-NEXT: Offset
+# CHECK-NEXT: Virtual
+# CHECK-NEXT: Physical
+# CHECK-NEXT: FileSize: 0
+# CHECK-NEXT: MemSize: 4
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   PF_R
+# CHECK-NEXT: ]

test/tools/llvm-objcopy/only-keep-debug.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -only-keep-debug %t %t2
+# RUN: llvm-readobj -file-headers -sections -symbols %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .debugfoo
+    Type:            SHT_PROGBITS
+    Content:         "00000000"
+  - Name:            .note
+    Type:            SHT_NOTE
+  - Name:            .blarg
+    Type:            SHT_PROGBITS
+  - Name:            .data
+    Type:             SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Content:         "00000000"
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000010
+    Content:         "00000000"
+Symbols:
+  Global:
+    - Name: foo
+      Section: .text
+    - Name: debugfoo
+      Section: .debugfoo
+
+# CHECK: SectionHeaderCount: 9
+
+# CHECK: Name: .debugfoo
+# CHECK: Name: .note
+# CHECK: Name: .blarg
+# CHECK: Name: .data
+# CHECK-NEXT: Type: SHT_NOBITS
+# CHECK: Name: .text
+# CHECK-NEXT: Type: SHT_NOBITS
+# CHECK: Name: .symtab
+# CHECK: Name: .strtab
+# CHECK: Name: .shstrtab
+
+# Check that both symbols are copied
+# CHECK:      Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size:
+# CHECK-NEXT:     Binding:
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: Undefined
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: foo
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size:
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: .text
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: debugfoo
+# CHECK-NEXT:     Value:
+# CHECK-NEXT:     Size:
+# CHECK-NEXT:     Binding: Global
+# CHECK-NEXT:     Type:
+# CHECK-NEXT:     Other:
+# CHECK-NEXT:     Section: .debugfoo
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]

test/tools/llvm-objcopy/replace-strtab-ref-in-dynamic-error.test

+# RUN: yaml2obj %s > %t
+# RUN: not llvm-objcopy -only-keep-debug %t %t2 2>&1 >/dev/null | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .mystrtab
+    Type:            SHT_STRTAB
+    Flags:           [ SHF_ALLOC ]
+    Size:            64
+  - Name:            .dynamic
+    Type:            SHT_DYNAMIC
+    Flags:           [ ]
+    Link:            .mystrtab
+    Info:            1
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000010
+    Content:         "00000000"
+
+# CHECK: String table .mystrtab cannot be replaced because it is referenced by the section .dynamic.

test/tools/llvm-objcopy/replace-strtab-ref-in-dynsym-error.test

+# RUN: yaml2obj %s > %t
+# RUN: not llvm-objcopy -only-keep-debug %t %t2 2>&1 >/dev/null | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .mystrtab
+    Type:            SHT_STRTAB
+    Flags:           [ SHF_ALLOC ]
+    Size:            64
+  - Name:            .dynsym
+    Type:            SHT_DYNSYM
+    Flags:           [ ]
+    Link:            .mystrtab
+    Info:            1
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000010
+    Content:         "00000000"
+
+# CHECK: String table .mystrtab cannot be replaced because it is referenced by the section .dynsym.

test/tools/llvm-objcopy/replace-text-ref-in-symtab.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -only-keep-debug %t %t2
+# RUN: llvm-readobj -symbols -file-headers -sections %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x1234
+    AddressAlign:    0x0000000000000010
+    Content:         "00000000"
+Symbols:
+  Global:
+    - Name:  _start
+      Type:  STT_FUNC
+      Value: 0x1000
+      Size:  4
+      Section: .text
+
+# CHECK: SectionHeaderCount: 5
+
+# CHECK:      Name: .text
+# CHECK-NEXT: Type: SHT_NOBITS
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   SHF_ALLOC
+# CHECK-NEXT:   SHF_EXECINSTR
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address: 0x1234
+# CHECK-NEXT: Offset:
+# CHECK-NEXT: Size: 4
+# CHECK-NEXT: Link:
+# CHECK-NEXT: Info:
+# CHECK-NEXT: AddressAlignment: 16
+
+# CHECK: .symtab
+# CHECK: .strtab
+# CHECK: .shstrtab
+
+#CHECK:         Name: _start
+#CHECK-NEXT:    Value: 0x1000
+#CHECK-NEXT:    Size: 4
+#CHECK-NEXT:    Binding: Global
+#CHECK-NEXT:    Type: Function (0x2)
+#CHECK-NEXT:    Other: 0
+#CHECK-NEXT:    Section: .text

tools/llvm-objcopy/Object.h

public:
   uint64_t Offset = 0;
   uint64_t Size = 0;
   uint64_t Type = ELF::SHT_NULL;
 
   virtual ~SectionBase() = default;
 
   virtual void initialize(SectionTableRef SecTable);
   virtual void finalize();
+  virtual void replaceSectionReferences(const SectionBase *Old,
+                                        const SectionBase *New);
   virtual void removeSectionReferences(const SectionBase *Sec);
   template <class ELFT> void writeHeader(FileOutputBuffer &Out) const;
   virtual void writeSection(FileOutputBuffer &Out) const = 0;
 };
 
 class Segment {
 private:
   struct SectionCompare {
enum SymbolShndxType {
   SYMBOL_HEXAGON_SCOMMON = ELF::SHN_HEXAGON_SCOMMON,
   SYMBOL_HEXAGON_SCOMMON_2 = ELF::SHN_HEXAGON_SCOMMON_2,
   SYMBOL_HEXAGON_SCOMMON_4 = ELF::SHN_HEXAGON_SCOMMON_4,
   SYMBOL_HEXAGON_SCOMMON_8 = ELF::SHN_HEXAGON_SCOMMON_8,
 };
 
 struct Symbol {
   uint8_t Binding;
-  SectionBase *DefinedIn = nullptr;
+  const SectionBase *DefinedIn = nullptr;
   SymbolShndxType ShndxType;
   uint32_t Index;
   StringRef Name;
   uint32_t NameIndex;
   uint64_t Size;
   uint8_t Type;
   uint64_t Value;
 

 public:
   void setStrTab(StringTableSection *StrTab) { SymbolNames = StrTab; }
   void addSymbol(StringRef Name, uint8_t Bind, uint8_t Type,
                  SectionBase *DefinedIn, uint64_t Value, uint16_t Shndx,
                  uint64_t Sz);
   void addSymbolNames();
   const SectionBase *getStrTab() const { return SymbolNames; }
   const Symbol *getSymbolByIndex(uint32_t Index) const;
+  void replaceSectionReferences(const SectionBase *Old,
+                                const SectionBase *New) override;
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_SYMTAB;
   }
 };

 private:
   SymTabType *Symbols = nullptr;
 
 protected:
   RelocSectionWithSymtabBase() = default;
 
 public:
   void setSymTab(SymTabType *StrTab) { Symbols = StrTab; }
+  void replaceSectionReferences(const SectionBase *Old,
+                                const SectionBase *New) override;
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 };
 
 template <class ELFT>
 class RelocationSection
     : public RelocSectionWithSymtabBase<SymbolTableSection> {

 class SectionWithStrTab : public Section {
 private:
   const SectionBase *StrTab = nullptr;
 
 public:
   SectionWithStrTab(ArrayRef<uint8_t> Data) : Section(Data) {}
 
   void setStrTab(const SectionBase *StringTable) { StrTab = StringTable; }
+  void replaceSectionReferences(const SectionBase *Old,
+                                const SectionBase *New) override;
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
   static bool classof(const SectionBase *S);
 };
 
 class DynamicSymbolTableSection : public SectionWithStrTab {
 public:
public:
   uint32_t Flags;
   bool WriteSectionHeaders = true;
 
   Object(const object::ELFObjectFile<ELFT> &Obj);
   virtual ~Object() = default;
 
   const SymbolTableSection *getSymTab() const { return SymbolTable; }
   const SectionBase *getSectionHeaderStrTab() const { return SectionNames; }
+  void replaceWithNoBits(std::function<bool(const SectionBase &)> ToReplace);
   void removeSections(std::function<bool(const SectionBase &)> ToRemove);
   virtual size_t totalSize() const = 0;
   virtual void finalize() = 0;
   virtual void write(FileOutputBuffer &Out) const = 0;
 };
 
 template <class ELFT> class ELFObject : public Object<ELFT> {
 private:

tools/llvm-objcopy/Object.cpp

 void Segment::writeSegment(FileOutputBuffer &Out) const {
   uint8_t *Buf = Out.getBufferStart() + Offset;
   // We want to maintain segments' interstitial data and contents exactly.
   // This lets us just copy segments directly.
   std::copy(std::begin(Contents), std::end(Contents), Buf);
 }
 
 void SectionBase::removeSectionReferences(const SectionBase *Sec) {}
+void SectionBase::replaceSectionReferences(const SectionBase *Old,
+                                           const SectionBase *New) {}
 void SectionBase::initialize(SectionTableRef SecTable) {}
 void SectionBase::finalize() {}
 
 template <class ELFT>
 void SectionBase::writeHeader(FileOutputBuffer &Out) const {
   uint8_t *Buf = Out.getBufferStart();
   Buf += HeaderOffset;
   typename ELFT::Shdr &Shdr = *reinterpret_cast<typename ELFT::Shdr *>(Buf);
void SymbolTableSection::addSymbol(StringRef Name, uint8_t Bind, uint8_t Type,
   }
   Sym.Value = Value;
   Sym.Size = Sz;
   Sym.Index = Symbols.size();
   Symbols.emplace_back(llvm::make_unique<Symbol>(Sym));
   Size += this->EntrySize;
 }
 
+void SymbolTableSection::replaceSectionReferences(const SectionBase *Old,
+                                                  const SectionBase *New) {
+  if (SymbolNames == Old) {
+    error("String table " + SymbolNames->Name +
+          " cannot be replaced because it is referenced by "
+          "the symbol table " +
+          Name);
+  }
+  for (auto &Sym : Symbols) {
+    if (Sym->DefinedIn == Old)
+      Sym->DefinedIn = New;
+  }
+}
+
 void SymbolTableSection::removeSectionReferences(const SectionBase *Sec) {
   if (SymbolNames == Sec) {
     error("String table " + SymbolNames->Name +
           " cannot be removed because it is referenced by the symbol table " +
-          this->Name);
+          Name);
   }
   auto Iter =
       std::remove_if(std::begin(Symbols), std::end(Symbols),
                      [=](const SymPtr &Sym) { return Sym->DefinedIn == Sec; });
   Size -= (std::end(Symbols) - Iter) * this->EntrySize;
   Symbols.erase(Iter, std::end(Symbols));
 }
 
for (auto &Symbol : Symbols) {
     Sym->setBinding(Symbol->Binding);
     Sym->setType(Symbol->Type);
     Sym->st_shndx = Symbol->getShndx();
     ++Sym;
   }
 }
 
 template <class SymTabType>
+void RelocSectionWithSymtabBase<SymTabType>::replaceSectionReferences(
+    const SectionBase *Old, const SectionBase *New) {
+  if (Symbols == Old) {
+    error("Symbol table " + Symbols->Name + " cannot be replaced because it is "
+                                            "referenced by the relocation "
+                                            "section " +
+          this->Name);
+  }
+}
+
+template <class SymTabType>
 void RelocSectionWithSymtabBase<SymTabType>::removeSectionReferences(
     const SectionBase *Sec) {
   if (Symbols == Sec) {
     error("Symbol table " + Symbols->Name +
           " cannot be removed because it is "
           "referenced by the relocation "
           "section " +
           this->Name);
else
     writeRel(reinterpret_cast<Elf_Rela *>(Buf));
 }
 
 void DynamicRelocationSection::writeSection(FileOutputBuffer &Out) const {
   std::copy(std::begin(Contents), std::end(Contents),
             Out.getBufferStart() + Offset);
 }
 
+void SectionWithStrTab::replaceSectionReferences(const SectionBase *Old,
+                                                 const SectionBase *New) {
+  if (StrTab == Old) {
+    error("String table " + StrTab->Name + " cannot be replaced because it is "
+                                           "referenced by the section " +
+          Name);
+  }
+}
+
 void SectionWithStrTab::removeSectionReferences(const SectionBase *Sec) {
   if (StrTab == Sec) {
     error("String table " + StrTab->Name +
           " cannot be removed because it is "
           "referenced by the section " +
-          this->Name);
+          Name);
   }
 }
 
 bool SectionWithStrTab::classof(const SectionBase *S) {
   return isa<DynamicSymbolTableSection>(S) || isa<DynamicSection>(S);
 }
 
 void SectionWithStrTab::initialize(SectionTableRef SecTable) {

 
 template <class ELFT>
 void Object<ELFT>::writeSectionData(FileOutputBuffer &Out) const {
   for (auto &Section : Sections)
     Section->writeSection(Out);
 }
 
 template <class ELFT>
+void Object<ELFT>::replaceWithNoBits(
+    std::function<bool(const SectionBase &)> ToReplace) {
+  if (SymbolTable != nullptr && ToReplace(*SymbolTable))
+    SymbolTable = nullptr;
+  if (SectionNames != nullptr && ToReplace(*SectionNames)) {
+    if (WriteSectionHeaders)
+      error("Cannot replace " + SectionNames->Name +
+            " because it is the section header string table.");
+    SectionNames = nullptr;
+  }
+  // It's not necessarily safe to just change the section type so instead we
+  // make whole new section and delete the old way. This means removing any
+  // remaining references and replacing them with the new one as well.
+  for (auto &Sec : Sections) {
+    if (!ToReplace(*Sec))
+      continue;
+    // Save the current pointer.
+    auto Tmp = std::move(Sec);
+    // Make a new empty section for the nobits section.
+    Sec = llvm::make_unique<Section>(ArrayRef<uint8_t>{});
+    // Copy in all the original information.
+    Sec->Name = Tmp->Name;
+    Sec->Type = SHT_NOBITS;
+    Sec->Flags = Tmp->Flags;
+    Sec->Addr = Tmp->Addr;
+    Sec->Offset = Tmp->Offset;
+    Sec->OriginalOffset = Tmp->OriginalOffset;
+    Sec->Size = Tmp->Size;
+    Sec->Align = Tmp->Align;
+    Sec->EntrySize = Tmp->EntrySize;
+    // NOBITS sections don't affect layout and don't logically need to be in
+    // the segment. We remove the old section from the segment.
+    for (auto &Seg : Segments)
+      Seg->removeSection(Tmp.get());
+    // Before we let Tmp go out of scope we need to remove existing references
+    // to it.
+    for (auto &KeptSection : Sections) {
+      if (ToReplace(*KeptSection))
+        continue;
+      KeptSection->replaceSectionReferences(Tmp.get(), Sec.get());
+    }
+  }
+  // Now all PT_LOAD segments and segments with PT_LOAD parent segments can
+  // have their FileSize set to zero.
+  for (auto &Seg : Segments) {
+    if (Seg->Type == PT_LOAD ||
+        (Seg->ParentSegment && Seg->ParentSegment->Type == PT_LOAD))
+      Seg->FileSize = 0;
+  }
+}
+
+template <class ELFT>
 void Object<ELFT>::removeSections(
     std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter = std::stable_partition(
       std::begin(Sections), std::end(Sections), [=](const SecPtr &Sec) {
         if (ToRemove(*Sec))
           return false;
         if (auto RelSec = dyn_cast<RelocationSectionBase>(Sec.get())) {

tools/llvm-objcopy/llvm-objcopy.cpp

 static cl::opt<bool> StripDebug("strip-debug",
                                 cl::desc("Removes all debug information"));
 static cl::opt<bool> StripSections("strip-sections",
                                    cl::desc("Remove all section headers"));
 static cl::opt<bool> StripNonAlloc("strip-non-alloc",
                                    cl::desc("Remove all non-allocated sections"));
 static cl::opt<bool>
     StripDWO("strip-dwo", cl::desc("Remove all DWARF .dwo sections from file"));
+static cl::opt<bool> OnlyKeepDebug(
+    "only-keep-debug",
+    cl::desc("Removes most of what is not removed by strip-debug. Section "
+             "headers of removed sections are kept as SHT_NOBITS sections"));
 static cl::opt<bool> ExtractDWO(
     "extract-dwo",
     cl::desc("Remove all sections that are not DWARF .dwo sections from file"));
 static cl::opt<std::string>
     SplitDWO("split-dwo",
              cl::desc("Equivalent to extract-dwo on the input file to "
                       "<dwo-file>, then strip-dwo on the input file"),
              cl::value_desc("dwo-file"));
void CopyBinary(const ELFObjectFile<ELFT> &ObjFile) {
   if (!OutputFormat.empty() && OutputFormat == "binary")
     Obj = llvm::make_unique<BinaryObject<ELFT>>(ObjFile);
   else
     Obj = llvm::make_unique<ELFObject<ELFT>>(ObjFile);
 
   if (!SplitDWO.empty())
     SplitDWOToFile<ELFT>(ObjFile, SplitDWO.getValue());
 
+  // Replace sections before removing sections
+  if (OnlyKeepDebug) {
+    Obj->replaceWithNoBits(
+        [](const SectionBase &Sec) { return (Sec.Flags & SHF_ALLOC) != 0; });
+  }
+
   SectionPred RemovePred = [](const SectionBase &) { return false; };
 
   // Removes:
 
   if (!ToRemove.empty()) {
     RemovePred = [&](const SectionBase &Sec) {
       return std::find(std::begin(ToRemove), std::end(ToRemove), Sec.Name) !=
              std::end(ToRemove);