[llvm-objcopy] Implement --only-keep-debug for ELF

[MaskRay]

Summary

--only-keep-debug is used to create separate debug files. It removes
contents of sections that are not useful for debugging purposes
(SHF_ALLOC sections that are not SHT_NOTE), by changing their section
types to SHT_NOBITS.

The intended use case is:

llvm-objcopy --only-keep-debug a a.dbg
llvm-objcopy --strip-debug a b
llvm-objcopy --add-gnu-debuglink=a.dbg b

The current layout algorithm is incapable of deleting contents and
shrinking segments, so it is not suitable for implementing the
functionality.

This patch adds a new algorithm which assigns sh_offset to sections
first, then modifies p_offset/p_filesz of program headers. It bears a
resemblance to lld/ELF/Writer.cpp.

[MaskRay]

Reclaim

Discussed with Manjoy, labath and others, we need a solution that rewrites p_offset/p_filesz so that we can do some symbolization without access to the original executable.

[jakehehrlich]

This looks highly promising to me BTW.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

574–577

I think this should happen last since it might affect the behavoir of other parts.

llvm/tools/llvm-objcopy/ELF/Object.cpp

1963

Funny enough this looks almost identical to the original layout algorithm used and is on of the primary reasons firstSection was exposed to begin with. Just a funny historical detail.

1976

Why not use Sec.ParentSegment->Align instead of MaxPageSize?

1986–1987

Can you add a comment explaining that FirstSec is nullptr generally means that we're in a non-allocated section.

1991

Flags not Type

1991–1994

I think only the non-allocated branch is needed here.

The first branch only makes sense if we already know that Sec.Type != SHT_NOBITS which we do because of the previous check for that above. The second branch makes complete since in both the allocated and non-allocated cases within the same segment.

2020–2026

This whole check feels really strange to me. Why would this happen? Why do we need to increase the size of the headers like this?

llvm/tools/llvm-objcopy/ELF/Object.h

344

Please add a note here that this selects a layout algorithm and is *not* an instance of the config leaking into the ELFWriter which should never occur.

[jakehehrlich]

llvm/tools/llvm-objcopy/ELF/Object.cpp

2009

In the case where a Segment doesn't have a section this seems likely to go very poorly. For instance image a file with all the debug information after the allocated stuff but with a section-less segment in between the allocated content and the debug information. That would I think cause the offset of this segment to overlap with debug information which would be *very* bad. Perhaps we can add all of the section-less segments to a list and then assign them all MaxOffset with a FileSize of zero?

2020–2026

Ah I see why this is to handle the PT_LOAD covering the PT_PHDR. Ok this makes sense.

[MaskRay]

Address review comments

[MaskRay]

llvm/tools/llvm-objcopy/ELF/Object.cpp

1976

Thanks. Deleting MaxPageSize can simplify the code a bit.

1991–1994

I agree. For non-first sections in a segment, we can just use its relative sh_offset distance to the first section.

I added it probably to emulate GNU objcopy. Reading it again, their choice looks weird and I don't think an executable that is linked by a linker (instead of hand-crafted) may need Off = Sec.Addr - FirstSec->Addr + FirstSec->Offset. I'll simplify the logic here.

2009

GNU objcopy seems to set p_offset of such PT_LOAD to 0. Rewriting p_offset and p_filesz is to make symbolization tools that rely on file offsets happy. I think such segments are not useful to the tool. p_filesz=MaxOffset probably is not better than p_filesz=old_p_filesz, so I will leave them unchanged.

Added another test.

[MaskRay]

Forgot to update the second test

[jhenderson]

For clarity, is this change supposed to be instead of D67090? If so, please make sure the docs and help text are updated as part of it!

Also, for my own benefit, would you mind resummarising the expected behaviour of --only-keep-debug. It might be useful to add this as a comment in the code somewhere too.

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

9

It's not strictly needed, but it would be helpful to include the column headers here too, to make it easier to see which column is which.

50

If you specify an AddressAlign, you can ensure that this address will remain correct, even if more program headers are added or yaml2obj changes layout slightly. For example, you could specify AddressAlign: 0x400, and then Address: 0x400.

57

What happens if we have a progbits section before the note sections in this segment? NOBITS sections are usually at the end of segments.

62

Do we need to consider the weird layout of .tbss sections at all?

67

Any reason for this arbitrary size?

73

Ditto.

79

Does this matter?

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

591–592

If the symbol or string tables have SHF_ALLOC set, this will make them SHT_NOBITS. I'm not sure if that matters or not, but wanted to point it out.

llvm/tools/llvm-objcopy/ELF/Object.cpp

1973

Maybe worth updating this comment to say that maximum page size is the segment alignment.

Is this comment strictly true? What if there's a hole in the segment before the first section?

2001

Nit: sh_offset are -> sh_offset values have been

2020

I think the first part only needs to be Seg->Offset < HdrOffset. A segment at the end of the headers doesn't need to be related to them in any way.

2056

Here and elsewhere, could you rename this to HdrEnd or similar please? I thought it was the start of the headers when reading before, which would be the normal situation for an "Offset".

[jakehehrlich]

I had another thought. Right now the sections first layout algorithm is actually pretty general and doesn't make too much use of being specific to a particular flag. That's a good thing. That bad bit is that we're changing the section type.

As a consequence SHT_NOBITS has to be set for allocated sections in HandleArgs. However there are many allocated section types that require casting and this makes the casting break. In many contexts this winds up being valid but its a non-local contract that SHT_NOBITS is special for the sake of --only-keep debug and all casts have to consider it as potentially special. There are at least two ways out of this

1. Add a separate type information field to SectionBase that remains unchanged...perhaps OrigionalType so that we can mutate Type. Then change all the classof functions to use OrigionalType. For consistency OrigionalFlags might be nice as well so that we don't have to do this again later.
2. Remove the mutation on Type and leave it all functioning as is then add a check on OnlyKeepDebug in section header writing code so that the Type is always written out as SHT_NOBITS if the section is allocated. Additionally inside of the new layout algorithm you'll have to say "if the section is nobits or unallocated" rather than just checking for SHT_NOBITS as we did before.

I prefer #2 personally but it does make the new layout algorithm --only-keep-debug specific and probably won't allow it to be used for any future unforeseen flags that are as difficult as --only-keep-debug

[jakehehrlich]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

591–592

It matters. Thanks for bringing that up.

llvm/tools/llvm-objcopy/ELF/Object.cpp

2056

+1 this was really confusing to me as well.

[MaskRay]

Add more tests

[MaskRay]

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

57

Added

## Only the tail of a segment can be trimmed. .text still occupies space because
## it is followed by .note which is not SHT_NOBITS.
# CHECK2:      Name        Type     Address          Off    Size   ES Flg Lk Inf Al
# CHECK2:      .text       NOBITS   0000000000000200 000200 000001 00  AX  0   0 512
# CHECK2-NEXT: .note       NOTE     0000000000000201 000201 000001 00   A  0   0  0

62

Added .tbss to demonstrate that .tbss can be handled as well.

67

This arbitrary non-zero size is used to show that p_filesz of the containg PT_TLS gets rewritten to 0.

67

Added a comment.

73

Added a comment.

79

Added a comment.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

591–592

For SHF_ALLOC .shstrtab, we will get: error: e_shstrndx field value 5 in elf header is not a string table

GNU objcopy handles the section by dropping SHF_ALLOC. I haven't ever seen a tool that can make e_shstrndx SHF_ALLOC, so it probably does not matter.

---

Added a test for SHF_ALLOC .symtab and .strtab. The behavior is consistent with GNU objcopy. The ELF spec says this is allowed but I don't think GNU ld or lld can create such components.

llvm/tools/llvm-objcopy/ELF/Object.cpp

1973

This looks stale after I removed MaxPageSize.

Updated the comment.

2056

Thanks for the suggestion. I agree HdrEnd is a much better name.

[MaskRay]

reword a comment

[jhenderson]

Only two minor nits from me currently, plus these comments which I made out-of-line:

please make sure the docs and help text are updated

Also, for my own benefit, would you mind resummarising the expected behaviour of --only-keep-debug. It might be useful to add this as a comment in the code somewhere too.

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

69

You use hex for AddressAlign above and also for Addresses. I think it would make sense to make it hex here and throughout the rest of the file too.

llvm/tools/llvm-objcopy/ELF/Object.cpp

1973

Nit: "equals to" -> "equals" or "is equal to"

[MaskRay]

Address review comments

llvm/tools/llvm-objcopy/ELF/Object.cpp

1973

Thanks!

[jhenderson]

Thanks. The existing changes look good, but the docs/help text still need updating, and I think the whole --only-keep-debug feature could do with an explanation somewhere in the code (i.e. what it does).

[MaskRay]

Add comment in Object.cpp: assignOffsets

Add documentation

[jakehehrlich]

I'd still like to avoid changing the section type for the reason mentioned, at least until the appropriate point in finalization. I don't want this dangling assumption that casts don't always do what you think they should do.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

591–592

The issue is more the dynamic string table. The dynamic string table *should* be SHT_NOBITS but what matters it that the type information changes. Similar things need to happen for .dynamic and such. At the very least I don't want to carry around the assumption that the type information can change.

[MaskRay]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

591–592

If changing the section type here does not look good, should I do it in ELFObject.cpp, after

if (!Config.SetSectionFlags.empty()) {
  for (auto &Sec : Obj.sections()) {
    const auto Iter = Config.SetSectionFlags.find(Sec.Name);
    if (Iter != Config.SetSectionFlags.end()) {
      const SectionFlagsUpdate &SFU = Iter->second;
      setSectionFlagsAndType(Sec, SFU.NewFlags);
    }
  }
}

?

[MaskRay]

Move SHT_NOBITS code to handleArgs.

[jhenderson]

llvm/docs/CommandGuide/llvm-objcopy.rst

72

I don't think "separate" should be used here, as it implies to me that the output of this operation is not the normal output file. Perhaps change this sentence to "Produce a debug file as the output that only preserves contents of sections useful for debugging purposes."

75–76

I'd add "by making them SHT_NOBITS and shrinking the program headers where possible."

llvm/tools/llvm-objcopy/CommonOpts.td

89–90

Same comment as the documentation.

[MaskRay]

Update documentation

[jakehehrlich]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

591–592

I'd like to avoid changing Type all together. See my original two proposals for how to fix it. One of them is a very minor tweak to the existing code that would only require changing a few lines of this patch.

[MaskRay]

Rebase on D69739

[MaskRay]

In D67137#1725678, @jakehehrlich wrote:

I had another thought. Right now the sections first layout algorithm is actually pretty general and doesn't make too much use of being specific to a particular flag. That's a good thing. That bad bit is that we're changing the section type.

As a consequence SHT_NOBITS has to be set for allocated sections in HandleArgs. However there are many allocated section types that require casting and this makes the casting break. In many contexts this winds up being valid but its a non-local contract that SHT_NOBITS is special for the sake of --only-keep debug and all casts have to consider it as potentially special. There are at least two ways out of this

1. Add a separate type information field to SectionBase that remains unchanged...perhaps OrigionalType so that we can mutate Type. Then change all the classof functions to use OrigionalType. For consistency OrigionalFlags might be nice as well so that we don't have to do this again later.
2. Remove the mutation on Type and leave it all functioning as is then add a check on OnlyKeepDebug in section header writing code so that the Type is always written out as SHT_NOBITS if the section is allocated. Additionally inside of the new layout algorithm you'll have to say "if the section is nobits or unallocated" rather than just checking for SHT_NOBITS as we did before.

I prefer #2 personally but it does make the new layout algorithm --only-keep-debug specific and probably won't allow it to be used for any future unforeseen flags that are as difficult as --only-keep-debug

Sorry, I missed this comment. I understand your concern regarding the mutability of llvm::objcopy::elf::SectionBase::Type. Type is used by various static bool classof(const SectionBase *) overloads and mutating Type can change its result in different stages. My feeling is that we may actually need OriginalTypes and OriginalFlags, because Flags is also used in classof methods.

#2 requires us to do:

--- a/llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -70,7 +70,10 @@ template <class ELFT> void ELFWriter<ELFT>::writeShdr(const SectionBase &Sec) {
   uint8_t *B = Buf.getBufferStart() + Sec.HeaderOffset;
   Elf_Shdr &Shdr = *reinterpret_cast<Elf_Shdr *>(B);
   Shdr.sh_name = Sec.NameIndex;
-  Shdr.sh_type = Sec.Type;
+  if (OnlyKeepDebug && (Sec.Flags & SHF_ALLOC) && Sec.Type != SHT_NOTE)
+    Shdr.sh_type = SHT_NOBITS;
+  else
+    Shdr.sh_type = Sec.Type;

and add some code to both layoutSectionsForOnlyKeepDebug and layoutSegmentsForOnlyKeepDebug.

#1 as this patch currently does, can remain simple if we introduce OriginalTypes and OriginalFlags (D69739).

[jakehehrlich]

LGTM

You did it!!!

[jhenderson]

LGTM.

[grimar]

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

218

There is an issue revealed by D95364 here. This should be SHT_SYMTAB, right?

llvm-objcopy reports an error when I change it:
error: Symbol table has link index of 4 which is not a string table

I guess the desired behavior is to drop the sh_link value as the new section is of SHT_NOBITS type?

llvm/docs/CommandGuide/llvm-objcopy.rst

 
  Enable deterministic mode when copying archives, i.e. use 0 for archive member
  header UIDs, GIDs and timestamp fields. On by default.
 
 .. option:: --help, -h
 
  Print a summary of command line options.
 
+.. option:: --only-keep-debug
+
+ Produce a separate debug file that only preserves contents of sections useful
+ for debugging purposes.
+
+ For ELF objects, this removes the contents of `SHF_ALLOC` sections that are
+ not `SHT_NOTE`.
+
 .. option:: --only-section <section>, -j
 
  Remove all sections from the output, except for sections named ``<section>``.
  Can be specified multiple times to keep multiple sections.
 
  For MachO objects, ``<section>`` must be formatted as
  ``<segment name>,<section name>``.
 

 
 COFF-SPECIFIC OPTIONS
 ---------------------
 
 The following options are implemented only for COFF objects. If used with other
 objects, :program:`llvm-objcopy` will either emit an error or silently ignore
 them.
 
-.. option:: --only-keep-debug
-
- Remove the contents of non-debug sections from the output, but keep the section
- headers.
-
 ELF-SPECIFIC OPTIONS
 --------------------
 
 The following options are implemented only for ELF objects. If used with other
 objects, :program:`llvm-objcopy` will either emit an error or silently ignore
 them.
 
 .. option:: --add-symbol <name>=[<section>:]<value>[,<flags>]

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

-# NOTE: This test is only intended to be valid as long as --only-keep-debug is
-#       implemented as a NOP. This test should fail when that changes and you
-#       will need to update this test.
-
-# RUN: yaml2obj %s > %t
-# RUN: llvm-objcopy %t %t2
-# RUN: llvm-objcopy --only-keep-debug %t %t3
-# RUN: cmp %t2 %t3
-# RUN: llvm-strip --only-keep-debug --no-strip-all %t -o %t4
-# RUN: cmp %t2 %t4
-
-!ELF
-FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_EXEC
-  Machine:         EM_X86_64
-Sections:
-  - Name:            .text
-    Type:            SHT_PROGBITS
-    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
-    Content:         "DEADBEEF"

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

+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-objcopy --only-keep-debug %t1 %t1.dbg
+# RUN: llvm-readelf -S -l -x .note1 -x .note2 -x .debug_abbrev -x .debug_frame -x .debug_info %t1.dbg | FileCheck %s
+
+## Check that SHT_NOTE and .debug* are kept, but others are changed to SHT_NOBITS.
+## SHT_NOBITS sections do not occupy space in the output.
+
+# CHECK:      [Nr] Name          Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK:      [ 1] .note1        NOTE     0000000000000400 000400 000001 00   A  0   0 1024
+# CHECK-NEXT: [ 2] .note2        NOTE     0000000000000401 000401 000001 00   A  0   0  0
+# CHECK-NEXT: [ 3] .text         NOBITS   0000000000000402 000402 000001 00  AX  0   0  0
+# CHECK-NEXT: [ 4] .tdata        NOBITS   0000000000001480 000480 000007 00 WAT  0   0 128
+# CHECK-NEXT: [ 5] .tbss         NOBITS   0000000000001487 000480 000005 00 WAT  0   0  0
+# CHECK-NEXT: [ 6] .bss          NOBITS   00000000000014a0 000480 00003f 00  WA  0   0 32
+## objcopy sets sh_offset to 0x402. We don't do this to keep sh_offset non-decreasing.
+# CHECK-NEXT: [ 7] .debug_abbrev PROGBITS 0000000000000000 000480 000001 00      0   0  0
+# CHECK-NEXT: [ 8] .debug_frame  PROGBITS 0000000000000000 000488 000001 00      0   0  8
+# CHECK-NEXT: [ 9] .debug_info   PROGBITS 0000000000000000 000489 000001 00      0   0  0
+# CHECK-NEXT: [10] .strtab       STRTAB   0000000000000000 00048a 000001 00      0   0  1
+# CHECK-NEXT: [11] .shstrtab     STRTAB   0000000000000000 00048b 000060 00      0   0  1
+
+# CHECK:      Type Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+# CHECK-NEXT: LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000402 0x000403 R E 0x1000
+# CHECK-NEXT: LOAD 0x000480 0x0000000000001480 0x0000000000000000 0x000000 0x00005f RW  0x1000
+# CHECK-NEXT: TLS  0x000480 0x0000000000001480 0x0000000000000000 0x000000 0x00000c RW  0x80
+# CHECK-NEXT: NOTE 0x000400 0x0000000000000400 0x0000000000000000 0x000002 0x000002     0x400
+
+## Contents of SHT_NOTE and .debug* are kept.
+
+# CHECK:      Hex dump of section '.note1':
+# CHECK-NEXT: 0x00000400 01
+# CHECK:      Hex dump of section '.note2':
+# CHECK-NEXT: 0x00000401 02
+# CHECK:      Hex dump of section '.debug_abbrev':
+# CHECK-NEXT: 0x00000000 03
+# CHECK:      Hex dump of section '.debug_frame':
+# CHECK-NEXT: 0x00000000 04
+# CHECK:      Hex dump of section '.debug_info':
+# CHECK-NEXT: 0x00000000 05
+
+--- !ELF
+FileHeader:
+  Class:     ELFCLASS64
+  Data:      ELFDATA2LSB
+  Type:      ET_DYN
+  Machine:   EM_X86_64
+Sections:
+  - Name:         .note1
+    Type:         SHT_NOTE
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x400
+    AddressAlign: 0x400
+    Content:      01
+  - Name:         .note2
+    Type:         SHT_NOTE
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x401
+    Content:      02
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x402
+    Content:      c3
+  - Name:         .tdata
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE, SHF_TLS ]
+    Address:      0x1480          # Ensure Address=0x1000+Offset
+    AddressAlign: 0x80
+    # An arbitrary non-zero Size tests that .tdata does not occupy space
+    # and we can rewrite p_filesz of PT_TLS.
+    Size:         7
+  - Name:         .tbss
+    Type:         SHT_NOBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE, SHF_TLS ]
+    Address:      0x1487          # Ensure Address=0x1000+Offset
+    Size:         5
+  - Name:         .bss
+    Type:         SHT_NOBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE ]
+    Address:      0x14a0          # Ensure Address=0x1000+Offset
+    AddressAlign: 0x20
+    # An arbitrary non-zero Size tests that .bss does not occupy space.
+    Size:         63
+  - Name:         .debug_abbrev
+    Type:         SHT_PROGBITS
+    Content:      03
+  - Name:         .debug_frame
+    Type:         SHT_PROGBITS
+    # AddressAlign tests the file offset assignment leaves a gap.
+    AddressAlign: 0x8
+    Content:      04
+  - Name:         .debug_info
+    Type:         SHT_PROGBITS
+    Content:      05
+ProgramHeaders:
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_X ]
+    Offset:   0
+    Align:    0x1000
+    Sections:
+      - Section: .note1
+      - Section: .note2
+      - Section: .text
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_W ]
+    VAddr:    0x1480    # Ensure Offset=VAddr (mod Align) if Offset changes
+    Align:    0x1000
+    Sections:
+      - Section: .tdata
+      - Section: .bss
+  - Type:     PT_TLS
+    Flags:    [ PF_R, PF_W ]
+    VAddr:    0x1480    # Ensure Offset=VAddr (mod Align) if Offset changes
+    Sections:
+      - Section: .tdata
+      - Section: .tbss
+  - Type:     PT_NOTE
+    VAddr:    0x400
+    Sections:
+      - Section: .note1
+      - Section: .note2
+...
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-objcopy --only-keep-debug %t2 %t2.dbg
+# RUN: llvm-readelf -S -l %t2.dbg | FileCheck --check-prefix=CHECK2 %s
+
+## Only the tail of a segment can be trimmed. .text still occupies space because
+## it is followed by .note which is not SHT_NOBITS.
+# CHECK2:      [Nr] Name        Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK2:      [ 1] .text       NOBITS   0000000000000200 000200 000001 00  AX  0   0 512
+# CHECK2-NEXT: [ 2] .note       NOTE     0000000000000201 000201 000001 00   A  0   0  0
+# CHECK2-NEXT: [ 3] .debug_info PROGBITS 0000000000000000 000220 000001 00      0   0 32
+# CHECK2-NEXT: [ 4] .strtab     STRTAB   0000000000000000 000221 000001 00      0   0  1
+# CHECK2-NEXT: [ 5] .shstrtab   STRTAB   0000000000000000 000222 00002b 00      0   0  1
+
+## Check that p_offset or p_filesz of empty segments or PT_PHDR are not modified.
+# CHECK2:      Type Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+# CHECK2-NEXT: PHDR 0x000040 0x0000000000000040 0x0000000000000000 0x0000a8 0x0000a8 R   0x8
+# CHECK2-NEXT: LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000202 0x000202 R E 0x1000
+# CHECK2-NEXT: LOAD 0x000202 0x0000000000000202 0x0000000000000000 0x00000e 0x00000e RW  0x1
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x200
+    AddressAlign: 0x200
+    Content:      c3
+  - Name:         .note
+    Type:         SHT_NOTE
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x201
+    Content:      01
+  - Name:         .debug_info
+    Type:         SHT_PROGBITS
+    AddressAlign: 0x20
+    Content:      02
+ProgramHeaders:
+  - Type:     PT_PHDR
+    Flags:    [ PF_R ]
+    Offset:   0x40
+    VAddr:    0x40
+    # 3 * sizeof(Elf64_Phdr) = 0xa8
+    FileSize: 0xa8
+    MemSize:  0xa8
+    Align:    8
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_X ]
+    Offset:   0
+    Align:    4096
+    Sections:
+      - Section: .text
+      - Section: .note
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_W ]
+    Offset:   0x202
+    VAddr:    0x202
+    FileSize: 14
+    MemSize:  14
+...
+
+## If .symtab or .strtab has the SHF_ALLOC flag, it will be changed to SHT_NOBITS.
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-objcopy --only-keep-debug %t3 %t3.dbg
+# RUN: llvm-readelf -S -l %t3.dbg | FileCheck --check-prefix=CHECK3 %s
+
+# CHECK3:      [Nr] Name          Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK3:      [ 1] .dynsym       NOBITS   0000000000000000 000040 000018 18   A  2   1 1024
+# CHECK3-NEXT: [ 2] .dynstr       NOBITS   0000000000000000 000040 000001 00   A  0   0  0
+# CHECK3-NEXT: [ 3] .symtab       NOBITS   0000000000000000 000040 000018 18   A  4   1  0
+# CHECK3-NEXT: [ 4] .strtab       NOBITS   0000000000000000 000040 000001 00   A  0   0  0
+# CHECK3-NEXT: [ 5] .shstrtab     STRTAB   0000000000000000 000040 00002b 00      0   0  1
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:  .dynsym
+    Type:  SHT_DYNSYM
+    Flags: [ SHF_ALLOC ]
+    Link:  .dynstr
+    AddressAlign: 0x400
+  - Name:  .dynstr
+    Type:  SHT_STRTAB
+    Flags: [ SHF_ALLOC ]
+  - Name:  .symtab
+    Type:  SHT_STRTAB
+    Flags: [ SHF_ALLOC ]
+    Link:  .strtab
+  - Name:  .strtab
+    Type:  SHT_STRTAB
+    Flags: [ SHF_ALLOC ]
+DynamicSymbols: []
+Symbols: []

llvm/tools/llvm-objcopy/CommonOpts.td

def K : JoinedOrSeparate<["-"], "K">,
         Alias<keep_symbol>,
         HelpText<"Alias for --keep-symbol">;
 
 def keep_file_symbols : Flag<["--"], "keep-file-symbols">,
                         HelpText<"Do not remove file symbols">;
 
 def only_keep_debug
     : Flag<["--"], "only-keep-debug">,
-      HelpText<"Clear sections that would not be stripped by --strip-debug. "
-               "Currently only implemented for COFF.">;
+      HelpText<"Produce a separate debug file that only preserves contents of "
+               "sections useful for debugging purposes">;
 
 def discard_locals : Flag<["--"], "discard-locals">,
                      HelpText<"Remove compiler-generated local symbols, (e.g. "
                               "symbols starting with .L)">;
 def X : Flag<["-"], "X">,
         Alias<discard_locals>,
         HelpText<"Alias for --discard-locals">;
 

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

 }
 
 static std::unique_ptr<Writer> createELFWriter(const CopyConfig &Config,
                                                Object &Obj, Buffer &Buf,
                                                ElfType OutputElfType) {
   // Depending on the initial ELFT and OutputFormat we need a different Writer.
   switch (OutputElfType) {
   case ELFT_ELF32LE:
-    return std::make_unique<ELFWriter<ELF32LE>>(Obj, Buf,
-                                                 !Config.StripSections);
+    return std::make_unique<ELFWriter<ELF32LE>>(Obj, Buf, !Config.StripSections,
+                                                Config.OnlyKeepDebug);
   case ELFT_ELF64LE:
-    return std::make_unique<ELFWriter<ELF64LE>>(Obj, Buf,
-                                                 !Config.StripSections);
+    return std::make_unique<ELFWriter<ELF64LE>>(Obj, Buf, !Config.StripSections,
+                                                Config.OnlyKeepDebug);
   case ELFT_ELF32BE:
-    return std::make_unique<ELFWriter<ELF32BE>>(Obj, Buf,
-                                                 !Config.StripSections);
+    return std::make_unique<ELFWriter<ELF32BE>>(Obj, Buf, !Config.StripSections,
+                                                Config.OnlyKeepDebug);
   case ELFT_ELF64BE:
-    return std::make_unique<ELFWriter<ELF64BE>>(Obj, Buf,
-                                                 !Config.StripSections);
+    return std::make_unique<ELFWriter<ELF64BE>>(Obj, Buf, !Config.StripSections,
+                                                Config.OnlyKeepDebug);
   }
   llvm_unreachable("Invalid output format");
 }
 
 static std::unique_ptr<Writer> createWriter(const CopyConfig &Config,
                                             Object &Obj, Buffer &Buf,
                                             ElfType OutputElfType) {
   switch (Config.OutputFormat) {
if ((!Config.SymbolsToKeep.empty() || Config.KeepFileSymbols) &&
       Obj.SymbolTable && !Obj.SymbolTable->empty()) {
     RemovePred = [&Obj, RemovePred](const SectionBase &Sec) {
       if (&Sec == Obj.SymbolTable || &Sec == Obj.SymbolTable->getStrTab())
         return false;
       return RemovePred(Sec);
     };
   }
 
   if (Config.CompressionType != DebugCompressionType::None)
     replaceDebugSections(Obj, RemovePred, isCompressable, 
                          [&Config, &Obj](const SectionBase *S) {
                            return &Obj.addSection<CompressedSection>(
                                 *S, Config.CompressionType);
                         });
   else if (Config.DecompressDebugSections)
     replaceDebugSections(
         Obj, RemovePred,
         [](const SectionBase &S) { return isa<CompressedSection>(&S); },
         [&Obj](const SectionBase *S) {
           auto CS = cast<CompressedSection>(S);
           return &Obj.addSection<DecompressedSection>(*CS);
         });
 
+  if (Config.OnlyKeepDebug)
+    for (auto &Sec : Obj.sections())
+      if (Sec.Flags & SHF_ALLOC && Sec.Type != SHT_NOTE)
+        Sec.Type = SHT_NOBITS;
+
   return Obj.removeSections(Config.AllowBrokenLinks, RemovePred);
 }
 
 // This function handles the high level operations of GNU objcopy including
 // handling command line options. It's important to outline certain properties
 // we expect to hold of the command line operations. Any operation that "keeps"
 // should keep regardless of a remove. Additionally any removal should respect
 // any previous removals. Lastly whether or not something is removed shouldn't

llvm/tools/llvm-objcopy/ELF/Object.h

private:
 
   std::unique_ptr<ELFSectionWriter<ELFT>> SecWriter;
 
   size_t totalSize() const;
 
 public:
   virtual ~ELFWriter() {}
   bool WriteSectionHeaders;
 
+  // For --only-keep-debug, select an alternative section/segment layout
+  // algorithm.
+  bool OnlyKeepDebug;
+
   Error finalize() override;
   Error write() override;
-  ELFWriter(Object &Obj, Buffer &Buf, bool WSH);
+  ELFWriter(Object &Obj, Buffer &Buf, bool WSH, bool OnlyKeepDebug);
 };
 
 class BinaryWriter : public Writer {
 private:
   std::unique_ptr<BinarySectionWriter> SecWriter;
 
   uint64_t TotalSize;
 
bool operator()(const SectionBase *Lhs, const SectionBase *Rhs) const {
       // fix this we can use the lexicographic ordering on ->Addr and the
       // address of the actully stored section.
       if (Lhs->OriginalOffset == Rhs->OriginalOffset)
         return Lhs < Rhs;
       return Lhs->OriginalOffset < Rhs->OriginalOffset;
     }
   };
 
-  std::set<const SectionBase *, SectionCompare> Sections;
-
 public:
   uint32_t Type;
   uint32_t Flags;
   uint64_t Offset;
   uint64_t VAddr;
   uint64_t PAddr;
   uint64_t FileSize;
   uint64_t MemSize;
   uint64_t Align;
 
   uint32_t Index;
   uint64_t OriginalOffset;
   Segment *ParentSegment = nullptr;
   ArrayRef<uint8_t> Contents;
+  std::set<const SectionBase *, SectionCompare> Sections;
 
   explicit Segment(ArrayRef<uint8_t> Data) : Contents(Data) {}
   Segment() {}
 
   const SectionBase *firstSection() const {
     if (!Sections.empty())
       return *Sections.begin();
     return nullptr;

llvm/tools/llvm-objcopy/ELF/Object.cpp

for (SectionBase &Sec : Obj.sections())
     // section data if the section is not in a segment. Note that this renders
     // sections in segments effectively immutable.
     if (Sec.ParentSegment == nullptr)
       Sec.accept(*SecWriter);
 }
 
 template <class ELFT> void ELFWriter<ELFT>::writeSegmentData() {
   for (Segment &Seg : Obj.segments()) {
-    uint8_t *B = Buf.getBufferStart() + Seg.Offset;
-    assert(Seg.FileSize == Seg.getContents().size() &&
-           "Segment size must match contents size");
-    std::memcpy(B, Seg.getContents().data(), Seg.FileSize);
+    size_t Size = std::min<size_t>(Seg.FileSize, Seg.getContents().size());
+    std::memcpy(Buf.getBufferStart() + Seg.Offset, Seg.getContents().data(),
+                Size);
   }
 
   // Iterate over removed sections and overwrite their old data with zeroes.
   for (auto &Sec : Obj.removedSections()) {
     Segment *Parent = Sec.ParentSegment;
     if (Parent == nullptr || Sec.Type == SHT_NOBITS || Sec.Size == 0)
       continue;
     uint64_t Offset =
         Sec.OriginalOffset - Parent->OriginalOffset + Parent->Offset;
     std::memset(Buf.getBufferStart() + Offset, 0, Sec.Size);
   }
 }
 
 template <class ELFT>
-ELFWriter<ELFT>::ELFWriter(Object &Obj, Buffer &Buf, bool WSH)
-    : Writer(Obj, Buf), WriteSectionHeaders(WSH && Obj.HadShdrs) {}
+ELFWriter<ELFT>::ELFWriter(Object &Obj, Buffer &Buf, bool WSH,
+                           bool OnlyKeepDebug)
+    : Writer(Obj, Buf), WriteSectionHeaders(WSH && Obj.HadShdrs),
+      OnlyKeepDebug(OnlyKeepDebug) {}
 
 Error Object::removeSections(bool AllowBrokenLinks,
     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 (Sec.ParentSegment != nullptr) {
       Sec.Offset = Offset;
       if (Sec.Type != SHT_NOBITS)
         Offset += Sec.Size;
     }
   }
   return Offset;
 }
 
+// Rewrite sh_offset after some sections are changed to SHT_NOBITS and thus
+// occupy no space in the file.
+static uint64_t layoutSectionsForOnlyKeepDebug(Object &Obj, uint64_t Off) {
+  uint32_t Index = 1;
+  for (auto &Sec : Obj.sections()) {
+    Sec.Index = Index++;
+
+    auto *FirstSec = Sec.ParentSegment && Sec.ParentSegment->Type == PT_LOAD
+                         ? Sec.ParentSegment->firstSection()
+                         : nullptr;
+
+    // The first section in a PT_LOAD has to have congruent offset and address
+    // modulo the alignment, which usually equals the maximum page size.
+    if (FirstSec && FirstSec == &Sec)
+      Off = alignTo(Off, Sec.ParentSegment->Align, Sec.Addr);
+
+    // sh_offset is not significant for SHT_NOBITS sections, but the congruence
+    // rule must be followed if it is the first section in a PT_LOAD. Do not
+    // advance Off.
+    if (Sec.Type == SHT_NOBITS) {
+      Sec.Offset = Off;
+      continue;
+    }
+
+    if (!FirstSec) {
+      // FirstSec being nullptr generally means that Sec does not have the
+      // SHF_ALLOC flag.
+      Off = Sec.Align ? alignTo(Off, Sec.Align) : Off;
+    } else if (FirstSec != &Sec) {
+      // The offset is relative to the first section in the PT_LOAD segment. Use
+      // sh_offset for non-SHF_ALLOC sections.
+      Off = Sec.OriginalOffset - FirstSec->OriginalOffset + FirstSec->Offset;
+    }
+    Sec.Offset = Off;
+    Off += Sec.Size;
+  }
+  return Off;
+}
+
+// Rewrite p_offset and p_filesz of non-empty non-PT_PHDR segments after
+// sh_offset values have been updated.
+static uint64_t layoutSegmentsForOnlyKeepDebug(std::vector<Segment *> &Segments,
+                                               uint64_t HdrEnd) {
+  uint64_t MaxOffset = 0;
+  for (Segment *Seg : Segments) {
+    const SectionBase *Sec = Seg->firstSection();
+    if (Seg->Type == PT_PHDR || !Sec)
+      continue;
+
+    uint64_t Offset = Sec->Offset;
+    uint64_t FileSize = 0;
+    for (const SectionBase *Sec : Seg->Sections) {
+      uint64_t Size = Sec->Type == SHT_NOBITS ? 0 : Sec->Size;
+      if (Sec->Offset + Size > Offset)
+        FileSize = std::max(FileSize, Sec->Offset + Size - Offset);
+    }
+
+    // If the segment includes EHDR and program headers, don't make it smaller
+    // than the headers.
+    if (Seg->Offset < HdrEnd && HdrEnd <= Seg->Offset + Seg->FileSize) {
+      FileSize += Offset - Seg->Offset;
+      Offset = Seg->Offset;
+      FileSize = std::max(FileSize, HdrEnd - Offset);
+    }
+
+    Seg->Offset = Offset;
+    Seg->FileSize = FileSize;
+    MaxOffset = std::max(MaxOffset, Offset + FileSize);
+  }
+  return MaxOffset;
+}
+
 template <class ELFT> void ELFWriter<ELFT>::initEhdrSegment() {
   Segment &ElfHdr = Obj.ElfHdrSegment;
   ElfHdr.Type = PT_PHDR;
   ElfHdr.Flags = 0;
   ElfHdr.VAddr = 0;
   ElfHdr.PAddr = 0;
   ElfHdr.FileSize = ElfHdr.MemSize = sizeof(Elf_Ehdr);
   ElfHdr.Align = 0;
 }
 
 template <class ELFT> void ELFWriter<ELFT>::assignOffsets() {
   // We need a temporary list of segments that has a special order to it
   // so that we know that anytime ->ParentSegment is set that segment has
   // already had its offset properly set.
   std::vector<Segment *> OrderedSegments;
   for (Segment &Segment : Obj.segments())
     OrderedSegments.push_back(&Segment);
   OrderedSegments.push_back(&Obj.ElfHdrSegment);
   OrderedSegments.push_back(&Obj.ProgramHdrSegment);
   orderSegments(OrderedSegments);
+
+  uint64_t Offset;
+  if (OnlyKeepDebug) {
+    // For --only-keep-debug, the sections that did not preserve contents were
+    // changed to SHT_NOBITS. We now rewrite sh_offset fields of sections, and
+    // then rewrite p_offset/p_filesz of program headers.
+    uint64_t HdrEnd =
+        sizeof(Elf_Ehdr) + llvm::size(Obj.segments()) * sizeof(Elf_Phdr);
+    Offset = layoutSectionsForOnlyKeepDebug(Obj, HdrEnd);
+    Offset = std::max(Offset,
+                      layoutSegmentsForOnlyKeepDebug(OrderedSegments, HdrEnd));
+  } else {
     // Offset is used as the start offset of the first segment to be laid out.
     // Since the ELF Header (ElfHdrSegment) must be at the start of the file,
     // we start at offset 0.
-  uint64_t Offset = 0;
-  Offset = layoutSegments(OrderedSegments, Offset);
+    Offset = layoutSegments(OrderedSegments, 0);
     Offset = layoutSections(Obj.sections(), Offset);
+  }
   // If we need to write the section header table out then we need to align the
   // Offset so that SHOffset is valid.
   if (WriteSectionHeaders)
     Offset = alignTo(Offset, sizeof(Elf_Addr));
   Obj.SHOff = Offset;
 }
 
 template <class ELFT> size_t ELFWriter<ELFT>::totalSize() const {