COFF: Give a section with no file bytes an address of its own

cle/backends/coff.py

@@ -430,6 +430,22 @@ def value(self):
     },
 }
 
+#: ``IMAGE_SCN_ALIGN_*`` is a four-bit field holding one plus the log2 of the alignment. Zero states none,
+#: for which the linker's own default is 16 bytes.
+_ALIGN_MASK = 0x00F00000
+_ALIGN_SHIFT = 20
+
+
+def _section_alignment(section: CoffSectionTableEntry) -> int:
+    encoded = (section.Characteristics & _ALIGN_MASK) >> _ALIGN_SHIFT
+    return 1 << (encoded - 1) if encoded else 16
+
+
+#: ``SizeOfRawData`` is 32 bits wide, and for a section with no bytes in the file nothing else in the file
+#: bounds it. This caps the zero-filled space such a section is given.
+MAX_IMAGE_SIZE = 0x10000000
+
+
 COFF_MACHINE_TO_ARCH_NAME = {
     IMAGE_FILE_MACHINE.I386: "x86",
     IMAGE_FILE_MACHINE.AMD64: "AMD64",
@@ -459,26 +475,52 @@ def __init__(self, *args, **kwargs):
 
         # FIXME: Currently we just map the whole object file for convenience. Create a better memory map, discard object
         # file structure data.
-        self._image_vmem = self._data
+        image = bytearray(self._data)
 
         # Add each section
+        self._section_addrs: list[int] = []
+        next_uninitialized_vaddr = len(image)
         for section_idx, section in enumerate(self._coff.sections):
             section_name = self._coff.get_section_name(section_idx)
-            vaddr = section.PointerToRawData
             vsize = section.SizeOfRawData
-            self.segments.append(Segment(section.PointerToRawData, vaddr, section.SizeOfRawData, vsize))
+            if section.PointerToRawData or not vsize or not section.Characteristics & IMAGE_SCN.CNT_UNINITIALIZED_DATA:
+                vaddr = section.PointerToRawData
+                filesize = vsize
+            else:
+                # A section marked IMAGE_SCN_CNT_UNINITIALIZED_DATA has no bytes in the file and states
+                # PointerToRawData 0, which in the layout above is the file header. Placing it there lays it over
+                # the header and, once it is longer than the section table, over the sections that follow: a mingw
+                # object whose .bss is 0x1300 bytes long covers the first 0x11fc bytes of its own .text. Give it
+                # zero-filled space of its own past the image instead.
+                alignment = _section_alignment(section)
+                vaddr = (next_uninitialized_vaddr + alignment - 1) & ~(alignment - 1)
+                next_uninitialized_vaddr = vaddr + vsize
+                if next_uninitialized_vaddr <= MAX_IMAGE_SIZE:
+                    image.extend(bytes(next_uninitialized_vaddr - len(image)))
+                else:
+                    log.warning(
+                        "Section %s states %#x bytes of uninitialized data, which would take the image past "
+                        "%#x. Leaving it unbacked.",
+                        section_name,
+                        vsize,
+                        MAX_IMAGE_SIZE,
+                    )
+                filesize = 0
+            self._section_addrs.append(vaddr)
+            self.segments.append(Segment(section.PointerToRawData, vaddr, filesize, vsize))
             self.sections.append(
                 CoffSection(
                     section_name,
                     section.PointerToRawData,
-                    section.SizeOfRawData,
+                    filesize,
                     vaddr,
                     vsize,
                     section,
                 )
             )
 
-        self.memory.add_backer(0, bytes(self._image_vmem))
+        self._image_vmem = bytes(image)
+        self.memory.add_backer(0, self._image_vmem)
         self.mapped_base = self.linked_base = 0
         self.pic = True
         # assume windows, this can be wrong, but is more often right.
@@ -501,11 +543,11 @@ def _add_defined_symbols(self) -> None:
                 self.symbols.add(self.get_symbol(sym_name))
 
     def _add_relocs(self) -> None:
-        for section_idx, section in enumerate(self._coff.sections):
+        for section_idx in range(len(self._coff.sections)):
             for reloc in self._coff.relocations[section_idx]:
                 sym = self._coff.symbols[reloc.SymbolTableIndex]
                 sym_name = self._coff.get_symbol_name(reloc.SymbolTableIndex)
-                patch_offset = section.PointerToRawData + reloc.VirtualAddress
+                patch_offset = self._section_addrs[section_idx] + reloc.VirtualAddress
 
                 if sym.StorageClass in {
                     IMAGE_SYM_CLASS.STATIC,
@@ -542,7 +584,7 @@ def get_symbol(self, name: str, produce_extern_symbols: bool = False) -> Symbol
         }:
             symbol_type = SymbolType.TYPE_FUNCTION if sym.Type == 0x20 else SymbolType.TYPE_OTHER
             if sym.SectionNumber > 0:
-                sym_addr = self._coff.sections[sym.SectionNumber - 1].PointerToRawData + sym.Value
+                sym_addr = self._section_addrs[sym.SectionNumber - 1] + sym.Value
                 return Symbol(self, name, sym_addr, 1, symbol_type)
             elif sym.SectionNumber == 0:
                 if produce_extern_symbols:

tests/test_coff.py

@@ -74,6 +74,56 @@ def test_rel32_relocation_encodes_a_negative_displacement(self):
         field_addr = section_vaddr(ld.main_object, ".text") + field_offset
         assert ld.memory.load(field_addr, 4) == struct.pack("<i", expected)
 
+    def test_uninitialized_section_gets_space_of_its_own(self):
+        # .bss states no PointerToRawData because it has no bytes in the file, and this object's
+        # is 0x1000 long -- far past the 0xb4 where .text begins. Mapped at the file offset it
+        # states, it lies over the file header and the whole of .text.
+        exe = os.path.join(TEST_BASE, "tests", "x86", "coff_bss.obj")
+        ld = cle.Loader(exe, auto_load_libs=False)
+        obj = ld.main_object
+        bss = next(section for section in obj.sections if section.name == ".bss")
+        text = next(section for section in obj.sections if section.name == ".text")
+
+        assert bss.memsize == 0x1000
+        assert bss.vaddr >= text.vaddr + text.memsize
+        assert obj.find_section_containing(text.vaddr) is text
+        # Uninitialized data reads as zero, not as whatever the file happens to hold there.
+        assert ld.memory.load(bss.vaddr, bss.memsize) == bytes(bss.memsize)
+        buffer_symbol = obj.get_symbol("_buffer")
+        assert buffer_symbol is not None
+        assert buffer_symbol.rebased_addr == bss.vaddr
+
+    def test_a_section_with_no_file_bytes_and_no_flag_gets_no_space(self):
+        # The section states PointerToRawData 0 with SizeOfRawData 0x4000000 and marks itself
+        # code, not uninitialized data. Its size is under MAX_IMAGE_SIZE, so the ceiling would
+        # not stop it; only the IMAGE_SCN_CNT_UNINITIALIZED_DATA condition does.
+        exe = os.path.join(TEST_BASE, "tests", "x86", "coff_bss_no_flag.obj")
+        ld = cle.Loader(exe, auto_load_libs=False)
+        obj = ld.main_object
+        bss = next(section for section in obj.sections if section.name == ".bss")
+
+        assert bss.memsize == 0x4000000
+        assert not bss.only_contains_uninitialized_data
+        # It keeps the address its header states rather than getting space of its own.
+        assert bss.vaddr == obj.mapped_base
+        assert sum(len(backer) for _, backer in obj.memory.backers()) == os.path.getsize(exe)
+
+    def test_an_uninitialized_section_past_the_ceiling_is_not_materialized(self):
+        # .bss states 0x20000000 bytes and the file is 580 long. Nothing in the file bounds
+        # SizeOfRawData, so zero-filling whatever it says turns a header field into an allocation.
+        exe = os.path.join(TEST_BASE, "tests", "x86", "coff_huge_bss.obj")
+        ld = cle.Loader(exe, auto_load_libs=False)
+        obj = ld.main_object
+        bss = next(section for section in obj.sections if section.name == ".bss")
+        text = next(section for section in obj.sections if section.name == ".text")
+
+        assert bss.memsize == 0x20000000
+        assert bss.vaddr >= text.vaddr + text.memsize
+        # The image is the file and nothing more, so it does not grow with the field.
+        assert sum(len(backer) for _, backer in obj.memory.backers()) == os.path.getsize(exe)
+        with self.assertRaises(KeyError):
+            ld.memory.load(bss.vaddr, 1)
+
 
 if __name__ == "__main__":
     unittest.main()