[PATCH] D67090: [llvm-objcopy] Add a new file offset assignment algorithm and support --only-keep-debug

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D67090#1656588 <https://reviews.llvm.org/D67090#1656588>, @jakehehrlich wrote:

> The desired behavior is to keep the program headers intact as much as possible. elf utils for instance just flatout ignores them in this case. Program headers are needed by many tools to matchup the virtual addresses with the sections. I think outside of virtual address and memory size there isn't much that's actully needed in practice however. Ideally we wouldn't even change the file size of segments but I think I've structured llvm-objcopy in a way that makes that difficult unfortunately. I'm 100% ok using a distinct layout algorithm for --only-keep-debug as its a special snowflake where the meaning of segments drastically changes and segments are critical to layout. I'm also ok changing the file size of segments as I don't think its likely to cause a problem with tools like debuggers (there also exist other tools that aren't debuggers that need to perform the same kinds of actions).


For a separate debug file created by --only-keep-debug, the program header is not used. I tried removing all insignificant sections but gdb would give warnings: .got .dynsym ... etc cannot be found. So I'll stick with the "making insignificant sections SHT_NOBITS" approach. Because sh_offset fields are now rewritten, they no longer match p_offset/p_filesz in program headers. To avoid confusion, I'll just delete program headers.

    if (Config.OnlyKeepDebug)
     RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
       if (RemovePred(Sec))
         return true;
       if (Obj.SectionNames == &Sec)
         return false;
       if (Obj.SymbolTable == &Sec ||
           (Obj.SymbolTable && Obj.SymbolTable->getStrTab() == &Sec))
         return false;
       return !(Sec.Type == SHT_NOTE || isDebugSection(Sec));
     };
  
  // gdb warns:
      warning: section .got not found in /tmp/a.dbg
      warning: section .bss.rel.ro not found in /tmp/c/ccls.dbg



> Proposed behavoir:
> 
> - Program headers are maintained exactly as they went in except their file sizes are set to zero. I think in the past I've pushed back on this but this flag is becoming such a pain point it might be the way forward.
> - All sections are maintained except that allocated sections are swapped to be SHT_NOBITS sections
> - To maintain the invariant that program headers are not touched elsewhere I'd ask that we only touch program headers in the --only-keep-debug case
> 
>   If we're willing to set FileSize to zero and we have a good way to replace allocated sections do we need a new layout algorithm? If we still do I'm 100% ok with using it in the --only-keep-debug case.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D67090