[PATCH] D36558: [llvm-objcopy] Add support for nested and overlapping segments

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

One of the renamed tests needs another tweak to the name, and another of the tests still has a typo in its name.



================
Comment at: test/tools/llvm-objcopy/identical-segment.test:1
+# This test tests that if two possible parent segments have the same offset that
+# they're disambiguated based in their original index. This ensures that cycles
----------------
Sorry, should be identical-segments.test (plural).


================
Comment at: test/tools/llvm-objcopy/identical-segment.test:2
+# This test tests that if two possible parent segments have the same offset that
+# they're disambiguated based in their original index. This ensures that cycles
+# do not occur.
----------------
based in -> based on.


================
Comment at: test/tools/llvm-objcopy/tripple-overlap.test:3
+# Importantly if two segments could be the parent segment of a segment this test
+# should cover the case which a new parent replaces the old parent and the case
+# where an old parent is not replaced by a new parent.
----------------
the case which -> the case where


================
Comment at: tools/llvm-objcopy/Object.cpp:432
+  for (auto &Segment : OrderedSegments) {
+    if (Segment->Type == PT_GNU_STACK)
+      continue;
----------------
jakehehrlich wrote:
> jhenderson wrote:
> > Why is PT_GNU_STACK special?
> For the purposes of skipping it here it is special because it has no alignment and it's offset must be zero. In general segments should have non-zero alignment.. It's more generally special however. All of it's fields are zero (including address, offset, and size) except for the flags. It isn't nested in another loadable segment but it also doesn't cover any section.
> 
> I skip it here because it needs to maintain a zero offset and it causes align to fail (when I originally wrote this diff, the new layout algorithm wasn't a part of it so I didn't have this issue)
LLD can emit other segments with zero in every field. I have a linker script, for example, that requests a PT_INTERP segment, but LLD does not assign anything to it, so the segment is empty. Every field apart from the type is zero. Empty segments don't need an alignment or address, so I think the check should be for any empty segments at offset zero.


Repository:
  rL LLVM

https://reviews.llvm.org/D36558