[llvm-objcopy] [COFF] Always set PointerToRawData when writing a COFF file

[mstorsjo]

Summary

If we don't want to set PointerToRawData, for an empty section,
we do must set it to zero explicitly. Some object file generators
do set it to zero for empty sections, while others set a nonzero
value pointing at the end of the previous section.

If the value was nonzero on input, we need to update it - either
setting it to zero, or to a valid offset in the output file (not
out of bounds)

This fixes https://github.com/mstorsjo/llvm-mingw/issues/313.

Testing this is tricky, because we can't use yaml2obj, since that
doesn't produce object files with nonzero PointerToRawData for
empty sections. We can use llvm-mc to assemble a small file
(assuming that LLVM's MC layer keeps this behaviour), or bundle
a small binary object file. I opted for using llvm-mc for now here
(with a test that it actually does keep this property), but I don't
mind changing it to a canned object file to make the test less brittle.

[jhenderson]

This looks fine, but you could always look to see how hard it would be to parameterise the PointerToRawData field in yaml2obj. (I'm not particularly familiar with the COFF side of yaml2obj, so don't know whether it's a simple task or not).

[mstorsjo]

In D138783#3952885, @jhenderson wrote:

This looks fine, but you could always look to see how hard it would be to parameterise the PointerToRawData field in yaml2obj. (I'm not particularly familiar with the COFF side of yaml2obj, so don't know whether it's a simple task or not).

I guess that could be useful, for letting the yaml format encapsulate more details of the COFF object files. Although, I'm not sure in how many other cases that would be useful... In any case - I think I'll refrain from taking on that right now, but wouldn't mind if someone else does.

llvm/lib/ObjCopy/COFF/COFFWriter.cpp

Error COFFWriter::finalizeSymbolContents() {
   }
   return Error::success();
 }
 
 void COFFWriter::layoutSections() {
   for (auto &S : Obj.getMutableSections()) {
     if (S.Header.SizeOfRawData > 0)
       S.Header.PointerToRawData = FileSize;
+    else
+      S.Header.PointerToRawData = 0;
     FileSize += S.Header.SizeOfRawData; // For executables, this is already
                                         // aligned to FileAlignment.
     if (S.Relocs.size() >= 0xffff) {
       S.Header.Characteristics |= COFF::IMAGE_SCN_LNK_NRELOC_OVFL;
       S.Header.NumberOfRelocations = 0xffff;
       S.Header.PointerToRelocations = FileSize;
       FileSize += sizeof(coff_relocation);
     } else {

llvm/test/tools/llvm-objcopy/COFF/empty-sections.s

+## Check how the PointerToRawData field is set for empty sections.
+##
+## Some tools may leave it set to a nonzero value, while others
+## set it to zero. Currently, the LLVM MC layer produces a .data
+## section with a nonzero PointerToRawData value, even if no
+## .data section needs to be emitted. Other tools, such as yaml2obj
+## writes a zero field if there's no data.
+##
+## When llvm-objcopy copies object files, it either needs to
+## update the value to a valid value (within the bounds of the
+## file, even if the section is empty) or zero. Some tools
+## (such as obj2yaml or llvm-objcopy) can error out if the value
+## is out of bounds.
+##
+## Check that our input file has got a nonzero field, and that
+## it is set to zero after a run with llvm-objcopy.
+
+# REQUIRES: x86-registered-target
+
+# RUN: llvm-mc -triple x86_64-pc-win32 -filetype=obj %s -o %t.in.obj
+# RUN: llvm-readobj --sections %t.in.obj | FileCheck %s --check-prefix=INPUT
+# RUN: llvm-objcopy --remove-section=.bss %t.in.obj %t.out.obj
+# RUN: llvm-readobj --sections %t.out.obj | FileCheck %s --check-prefix=OUTPUT
+
+# INPUT:          Name: .data
+# INPUT-NEXT:     VirtualSize: 0x0
+# INPUT-NEXT:     VirtualAddress: 0x0
+# INPUT-NEXT:     RawDataSize: 0
+# INPUT-NEXT:     PointerToRawData: 0x8D
+
+# OUTPUT:         Name: .data
+# OUTPUT-NEXT:    VirtualSize: 0x0
+# OUTPUT-NEXT:    VirtualAddress: 0x0
+# OUTPUT-NEXT:    RawDataSize: 0
+# OUTPUT-NEXT:    PointerToRawData: 0x0{{$}}
+
+    .text
+    .globl func
+func:
+    ret