[elf2bin] Exclude zero-length segments from --bincombined. (#481)

(And also from --vhxcombined, which uses the same underlying code, but
that didn't fit in a sensible-length subject line.)

Last week I found Arm Toolchain had generated an ELF image in which
three segments had p_paddr in the bottom few Kb of the address space,
but there was also a segment with zero p_filesz (but nonzero p_memsz),
with p_paddr > 0x20000000. Running that through `elf2bin --physical
--bincombined`, it didn't notice that the segment at a high address had
length 0, so it padded the output file to half a Gb in size to reach the
start address, and then didn't actually write anything there.

I worked around it at the time by selecting a subset of the segments to
put in the --bincombined output file. But I think a better answer is to
discard the zero-length segment completely when writing --bincombined
output, because the padding in the output file is only there to get the
file position to the place where some actually important data needs to
be written. If there _is_ no important data to put at that position,
there's no need to go there at all.

(Incidentally, I think I know why my image came out like that. The
target memory map had Flash at 0, and RAM at 0x20000000. The code
segments were in Flash, and the initialized data segment had its p_paddr
in Flash but its p_vaddr at 0x20000000, representing respectively where
the startup code would memcpy it from and to. The segment with
p_filesz==0 represented the program's zero-initialized data, so its
p_vaddr was in RAM, representing where the data would be at run time –
but since it didn't need to be memcpy()ed from anywhere, the linker had
left its p_paddr the same as its p_vaddr, and not bothered to allocate
it a different one. In other words, the linker felt that if the segment
was 0 bytes long then it didn't need a meaningful p_paddr at all.)

The new behavior adapts appropriately if you use the --zi option: in
that situation, a segment with zero p_filesz but nonzero p_memsz doesn't
count as empty, because the user _does_ want zero bytes in the
--bincombined output file at its address.

Author: statham-arm

arm-software/shared/elf2bin/bin.cpp

@@ -210,8 +210,19 @@ static std::unique_ptr<BinaryDataStream>
 combined_prepare(const InputObject &inobj,
                  const std::vector<Segment> &segments_orig, bool include_zi,
                  std::optional<uint64_t> baseaddr) {
+  // Discard segments that contribute no actual bytes to the file, in case one
+  // of those appears at the highest address in the list and causes trailing
+  // padding of the binary output file.
+  std::vector<Segment> filtered;
+  std::copy_if(segments_orig.begin(), segments_orig.end(),
+               std::back_inserter(filtered),
+               [include_zi](const Segment &seg) {
+                 uint64_t size = include_zi ? seg.memsize : seg.filesize;
+                 return size != 0;
+               });
+
   // Sort the segments by base address, in case they weren't already.
-  std::vector<Segment> sorted = segments_orig;
+  std::vector<Segment> sorted = std::move(filtered);
   std::stable_sort(sorted.begin(), sorted.end(),
                    [](const Segment &a, const Segment &b) {
                      return a.baseaddr < b.baseaddr;

arm-software/shared/elf2bin/test/test.py

@@ -556,6 +556,64 @@ def testBaseAddr(self):
                 "0x1235",
             )
 
+    def testZeroLength(self):
+        "Test that a trailing length-0 segment doesn't cause trailing padding."
+
+        segment1_contents = bytes(range(0x10))
+
+        for bigend in False, True:
+            for sixtyfour in False, True:
+                with self.tempsubdir():
+                    makeelf(
+                        "input.elf",
+                        bigend,
+                        sixtyfour,
+                        [
+                            # A segment that is definitely not empty
+                            SegmentDesc(1, 0x1000, segment1_contents),
+
+                            # A segment that has no initialized contents but
+                            # does have ZI contents, so it's normally empty,
+                            # but stops counting as empty if you say
+                            # --zi
+                            SegmentDesc(1, 0x1100, b'', pad=0x80),
+
+                            # A completely empty segment
+                            SegmentDesc(1, 0x1200, b''),
+                        ],
+                    )
+
+                    # Without --zi: the first segment is the only non-empty
+                    # one, so we expect the output file to contain nothing but
+                    # the bytes in segment1_contents.
+                    self.elf2bin(
+                        "--bincombined", "-o", "output.bin", "input.elf"
+                    )
+                    with open("output.bin", "rb") as fh:
+                        bindata = fh.read()
+                    self.assertEqual(
+                        bindata,
+                        segment1_contents,
+                    )
+
+                    # With --zi: the second segment also counts as non-empty,
+                    # containing 0x80 zero bytes. So the output file contains
+                    # the 0x10 bytes of segment1_contents, the 0xF0 bytes of
+                    # padding to get from there to the second segment's
+                    # address, and the 0x80 ZI bytes from that segment. But we
+                    # stop there, and don't go on to the third segment, which
+                    # is still completely empty.
+                    self.elf2bin(
+                        "--bincombined", "-o", "output.bin", "input.elf",
+                        "--zi"
+                    )
+                    with open("output.bin", "rb") as fh:
+                        bindata = fh.read()
+                    self.assertEqual(
+                        bindata,
+                        segment1_contents + b'\0' * (0xF0 + 0x80),
+                    )
+
 
 class vhx(Base):
     def testOneSegment(self):