[PATCH] D63807: [llvm-objcopy] Add --only-keep-debug for ELF

Wed Jun 26 00:20:56 PDT 2019

abrachet marked an inline comment as done.
abrachet added a comment.

I'll want to add more tests later, along with making the current test better, its pretty bare right now. I'd like to test the keep debug, remove debug and add them back in with --add-gnu-debuglink too. But there are many things that I would like to iron out first, and just get the patch initially looked at, hopefully I'm not missing why this wasn't here before.

================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:590
+      if (Sec.Flags & (SHF_ALLOC | SHF_GROUP) && Sec.Type != SHT_NOTE) {
+        if (Sec.Type == SHT_SYMTAB) {
+          Sec.Flags &= ~SHF_ALLOC;
----------------
This is temporary!

I've been looking at how GNU objcopy deals with --only-keep-debug, I cannot figure out what is different about an SHT_SYMTAB with SHF_ALLOC flag, but it treats them differently. I need to look into this further but would love some help because I really can't figure it out. 

The code can be found [[ https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=binutils/objcopy.c;h=28b9d3bf9291518bffe93e98aa9b9b50a8fff3e2;hb=refs/heads/master#l3799 | here ]]. It seems to me like find_section_list will always return nullptr, meaning the if I have on 589 should be enough. Clearly this isn't the case, though. 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63807/new/

https://reviews.llvm.org/D63807