[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/Object.cpp:1997
+      continue;
+    uint64_t Offset = Seg->Offset;
+    if (const SectionBase *Sec = Seg->firstSection())
----------------
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?


================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:2008-2014
+    // If the segment includes EHDR or PHDR, don't make it smaller than the
+    // headers.
+    if (Seg->Offset <= HdrOffset && HdrOffset <= Seg->Offset + Seg->FileSize) {
+      FileSize += Offset - Seg->Offset;
+      Offset = Seg->Offset;
+      FileSize = std::max(FileSize, HdrOffset - Offset);
+    }
----------------
jakehehrlich wrote:
> 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?
Ah I see why this is to handle the PT_LOAD covering the PT_PHDR. Ok this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67137