[PATCH] D39132: [llvm-objcopy] Fix bug in how segment alignment was being handled

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

You should probably have a test for the BinaryObject case as well.

> In fact yaml2obj can't even produce a valid situation which causes llvm-objcopy to produce incorrect results because alignment and offset were both inherited from the sections the program header covers

Could you explain a bit more what you mean by this? The p_align field for a segment should be completely independent of the align field of its sections.



================
Comment at: test/tools/llvm-objcopy/check-addr-offset-align.test:15
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x1000
+    AddressAlign:    0x0000000000000008
----------------
This might be related to the problem below, but this Address (and that of .data) does not lie within the corresponding segment, according to the VAddr field you specify for the segment, so I'm not sure this makes any sense.


================
Comment at: test/tools/llvm-objcopy/check-addr-offset-align.test:43-44
+#CHECK-NEXT:    Type: PT_LOAD
+#CHECK-NEXT:    Offset: 0x230
+#CHECK-NEXT:    VirtualAddress: 0x1030
+#CHECK-NEXT:    PhysicalAddress: 0x1030
----------------
Isn't this the wrong behaviour here?

p_offset = 0x230, p_vaddr = 0x1030, p_align = 0x1000, so (p_offset % p_align) != (p_vaddr % p_align).

I'd expect offset to be 0x1030 in this case.


Repository:
  rL LLVM

https://reviews.llvm.org/D39132