[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#1000361, @vit9696 wrote:

> @jhenderson, thanks, NOBITS makes a lot more sense to me than your previous example. Yet I am unaware of any software or even any tool that would rely on such on offset, and to my eyes the linkers are simply doing the wrong thing by not setting the offset of 0 for whatever reason. In my opinion being able to copy more ELF files, which will be most likely valid and loaded on a target system, is more important, than trying to support some shady offset to a non-existent memory nobody knows how to calculate.


I can't claim to know what all the different consumers of ELF might do, but I think there's a non-zero chance of this change as-is breaking a system/program relying on the output produced by the linker. All I'm asking for is the check to be extended to test both MemSize and FileSize against 0. This should detect all marker segments safely, without potentially adversely affecting segments with non-zero MemSize. Also, not updating the offset will result in inconsistent behaviour beteween llvm-objcopy and linkers.

> I was able to produce the test case based on program-header-nobits.yaml, but I am not sure whether I should actually include it. Because as I stated above, whatever the offset is, it is undefined.

I think calling it "undefined" is a bit of a stretch. It must follow the usual alignment requirements of p_vaddr % p_align == p_offset % p_align (or % page_size, depending on segment type). Certainly, leaving the p_offset unchanged will maintain this, assuming it was correct in the input, but, for example, setting it to zero (which "undefined" implies is valid) may not.


Repository:
  rL LLVM

https://reviews.llvm.org/D42872