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

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added a comment.

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).

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.



================
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);
----------------
@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?


================
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.
----------------
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.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67090