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

[arichardson]

Summary

This implementation removes much more than either GNU binutils or
elftoolchain. The only downside seems to be that GDB prints lots of
warnings like this: warning: section .init not found in ../clang.debug
when loading the file but debugging seems to work just fine.

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

[jakehehrlich]

Hmmm, I don't know enough about how debuggers to know if this is ok. I spoke with Roland McGrath about this a while back and he seemed to think the NOBITS sections were needed. This is coming from the guy who championed the --strip-sections feature which is a pretty aggressive form of stripping. If this more aggressive approach is ok then we should do it this way but otherwise I'll have to reject this change unless it does the conversion to NOBITS. We should probably get rid of the warnings in GDB in that case.

There might be a better way to do this now that we have the Writer setup. Instead of converting to NOBITS we can just not write those sections out and we can just write things out a little bit differently.

[jhenderson]

For reference, Jake's original version was here: https://reviews.llvm.org/D40523

Not being someone who knows much about GDB or other debuggers, I can't really comment on the validity of this patch. What I will say is that the predicate here is significantly different (and more complex) than the original predicate Jake's version used (which marked only allocatable segments for removal/replacement). It's also quite a bit different from the inverse of the --strip-debug switch, although that might just indicate a problem with that switch, rather than with the implementation here.

[arichardson]

It seems like changing all alloc sections to NOBITS is probably the correct way of doing this. So I guess D40523 adjusted to always keep the named debug sections is better.
I don't think any of them are SHF_ALLOC anyway but checking the names shouldn't do any harm.

Looking at the source code it seems like bfd keeps sections starting with: ".debug", ".zdebug", ".gdb_index", ".line", ".stab", ".gnu.linkonce.wi."
and elftoolchian keeps: ".apple_", ".debug", ".gnu.linkonce.wi.", ".line", ".stab"

[jakehehrlich]

I can ask some people about this stuff and get back to everyone on this. Like James I have no clue if this is valid or why the old tools converted allocated sections to NOBITS. As for --strip-debug, we should do some testing to see if .zdebug, .gdb_index, etc... are removed by it. Making --strip-debug more aggressive where possible is probably a good thing. Funny anecdote: my main use case for --strip-debug is to tell other people to use it so that they can send me large binaries via email and I can still see the symbol table and relocations.

The following is speculation:
I think there are two use cases for --only-keep-debug that require the NOBITS sections.

1. If you pre-link the stripped binary then things might be relocated and thus the debug information would need to have access to the original sections in order to translate. I vaguely remember Roland talking about a case like this.
2. If you use --strip-all/--strip-sections with the right --keep options you might still be able to debug the stripped binary due to extra information in the debug binary. If this is possible then --strip-debug should become as aggressive as it can be without actually preventing this from happening.

[jakehehrlich]

So in my cursory findings I have confirmed that prelinking is a known use case for this. Also the fact that debuggers emit warnings on this is reason enough to not accept this sort of change. It seems like there should in theory be ways to make something like this work but it would a) need debugger support and b) would need a design proposal. I'm happy to participate in new inventions but we can't just make --only-keep-debug do something which is known to behave differently from GNU objcopy in ways that are trivially visible.

So the basic method here will have to be rejected. Paths to resolve this include at least the following:

1. Propose a new standard for how debugging information should be collected by the debugger. Get community approval. Decide on a new flag name, and implement that.
2. Modify llvm-objcopy to properly handle the conversion to nobits. I think this can now be done more simply by not performing the conversion in memory but instead laying out the file differently and writing out the section header table a bit differently.

[arichardson]

This approach is wrong and I currently don't have time to work on a correct solution.

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 %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .dynstr
+    Type:            SHT_STRTAB
+    Flags:           [ SHF_ALLOC ]
+  - Name:            .symtab.dyn
+    Type:            SHT_SYMTAB
+    Flags:           [ SHF_ALLOC ]
+    Type:            SHT_NOBITS
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Size:            4
+  - Name:            .debug_info
+    Type:            SHT_PROGBITS
+    Flags:           [ ]
+    AddressAlign:    0x1
+    Size:            4
+  - Name:            .debug_loc
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x1
+    Size:            4
+  - Name:            .zdebug_foo
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+  - Name:            .line
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+  - Name:            .stab
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+  - Name:            .gnu.linkonce.wi.1
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+  - Name:            .comment
+    Type:            SHT_PROGBITS
+  - Name:            .random_section_name
+    Type:            SHT_PROGBITS
+  - Name:            .debug_not_a_real_debug_section
+    Type:            SHT_PROGBITS
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Info:            .text
+  - Name:            .rela.text
+    Type:            SHT_RELA
+    Info:            .text
+
+
+# CHECK: SectionHeaderCount: 11
+
+# CHECK:      Name: .debug_info
+# CHECK:      Name: .debug_loc
+# CHECK:      Name: .zdebug_foo
+# CHECK:      Name: .line
+# CHECK:      Name: .stab
+# CHECK:      Name: .gnu.linkonce.wi.1
+# CHECK:      Name: .debug_not_a_real_debug_section
+# CHECK:      Name: .symtab
+# CHECK:      Name: .strtab
+# CHECK:      Name: .shstrtab

tools/llvm-objcopy/llvm-objcopy.cpp

StripAllGNU("strip-all-gnu",
                 cl::desc("Removes symbol, relocation, and debug information"));
 static cl::list<std::string> Keep("keep", cl::desc("Keep <section>"),
                                   cl::value_desc("section"));
 static cl::list<std::string> OnlyKeep("only-keep",
                                       cl::desc("Remove all but <section>"),
                                       cl::value_desc("section"));
 static cl::alias OnlyKeepA("j", cl::desc("Alias for only-keep"),
                            cl::aliasopt(OnlyKeep));
+static cl::opt<bool>
+    OnlyKeepDebug("only-keep-debug",
+                  cl::desc("Removes all but debug information"));
 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> ExtractDWO(
RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
       if (Obj.SymbolTable == &Sec || Obj.SymbolTable->getStrTab() == &Sec)
         return false;
 
       // Remove everything else.
       return true;
     };
   }
 
+  if (OnlyKeepDebug) {
+    RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
+      // Allow all implicit removes.
+      if (RemovePred(Sec))
+        return true;
+
+      // Only keep debug sections and the string + symbol table.
+      // This is much more aggressive than either GNU binutils or elftoolchain
+      // but GDB accepts this just fine
+      if (Obj.SectionNames == &Sec || Obj.SymbolTable == &Sec ||
+          Obj.SymbolTable->getStrTab() == &Sec)
+        return false;
+      for (const char *S : {".debug", ".zdebug", ".gdb_index", ".line", ".stab",
+                            ".gnu.linkonce.wi."})
+        if (Sec.Name.startswith(S))
+          return false;
+
+      // Remove everything else.
+      return true;
+    };
+  }
+
   if (!Keep.empty()) {
     RemovePred = [RemovePred](const SectionBase &Sec) {
       // Explicitly keep these sections regardless of previous removes.
       if (std::find(std::begin(Keep), std::end(Keep), Sec.Name) !=
           std::end(Keep))
         return false;
       // Otherwise defer to RemovePred.
       return RemovePred(Sec);