[PATCH] D67090: [llvm-objcopy][llvm-strip] Support --only-keep-debug

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 3 inline comments as done.
MaskRay added inline comments.


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1767
+    // may be taken by subsequent non-ALLOC sections.
+    if (Sec.ParentSegment == nullptr || !(Sec.Flags & SHF_ALLOC))
       Sec.accept(*SecWriter);
----------------
jakehehrlich wrote:
> @jhenderson I seem to recall you had a tricky non-allocated data in a segment case. Is this still kosher? Perhaps in that case the sections can write themselves and we don't have to worry about the interstitial data?
preserve-segment-contents.test is the tricky test. For the new approach for --only-keep-debug, program headers are deleted and sections' sh_offset are rewritten, `writeSegmentData` should not be called.


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1923
+// This function finds a consistent layout for a list of segments starting from
+// an Offset. It assumes that Segments have been sorted by OrderSegments and
+// returns an Offset one past the end of the last segment.
----------------
jakehehrlich wrote:
> I just realized this comment wasn't correctly updated when we fixed the naming convention. OrderSegments should be orderSegments. Would be nice if you could fix this for us but it isn't a requirement for this change.
There are a few others. Fixed in rL370838.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67090