[PATCH] D42872: Fix handling of zero-size segments in llvm-objcopy

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D42872#1001677, @vit9696 wrote:

> More importantly, if you have a PT_LOAD "marker" segment with all zero values:
>
> - will gnu binutils objcopy also corrupt such an ELF?
> - will a Linux/BSD-based system load such an ELF? If in both cases the answer is no, then we should probably just special-case indeed, but I still cannot say that silently overwriting the header is any good. Perhaps we should print an error and abort?


I wouldn't define any PT_LOAD segment as a "marker" segment. Each segment type has a defined purpose, as defined by its type, so marker segments should only be those with types defined as marker types (i.e. PT_GNU_STACK and PT_OPENBSD_WXNEEDED).

That being said, I think we may be talking at cross-purposes here. Clearly overwriting the header is wrong, but a PT_LOAD segment with all zero values won't overwrite the header, since by definition it has zero size, and segments like this can and do occur, if they are empty. Furthermore, it is legal for the ELF header and program header table to be nested inside a segment, as long as that segment has an appropriate file offset. However, as we've seen here, the problem is that the layout code doesn't take account of the elf and program headers when laying out the remaining segments. Perhaps worrying about which segments should be treated as marker segments or not is the wrong approach, and really we should also treat the elf header and program header table as special in the LayoutSegments code. This would suggest a more correct approach to me: treat the ELF header and program header table as pseudo-segments for layout purposes, with an OriginalOffset of 0 and sizeof(ElfHeader) respectively. If they end up nested inside another segment, that's fine, as the segment by definition must be already in the right place. If they end up with segments nested inside them, that is also fine for the same reasoning. Zero-sized segments at offset 0 will still be placed at offset 0, as desired, but the ELF header segment will "follow" (at offset 0) immediately after it, since it will be the first non-empty segment in the ordered list (or will be nested inside another segment that is similarly at offset 0).

How does this sound as an approach? It feels more robust and avoids any special casing, since everything should just naturally work with the existing layout scheme (the only thing we'd have to do is write these pseudo-segments differently).


Repository:
  rL LLVM

https://reviews.llvm.org/D42872