[PATCH] D67137: [llvm-objcopy] Support --only-keep-debug

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:585-586
+    for (auto &Sec : Obj.sections())
+      if (Sec.Flags & SHF_ALLOC && Sec.Type != SHT_NOTE)
+        Sec.Type = SHT_NOBITS;
+
----------------
jhenderson wrote:
> 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.
It matters. Thanks for bringing that up.


================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:2044
+  if (OnlyKeepDebug) {
+    uint64_t HdrOffset =
+        sizeof(Elf_Ehdr) + llvm::size(Obj.segments()) * sizeof(Elf_Phdr);
----------------
jhenderson wrote:
> 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".
+1 this was really confusing to me as well. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67137