[LLD][COFF] Implement /filealign parameter

[thrimbor]

Summary

This adds the /filealign parameter to lld, which allows to specify the section alignment in the output file (as it does on Microsoft's link.exe).
This is required to be able to load dynamically linked libraries on the original Xbox, where the debugger monitor expects the section alignment in the file to be the same as in memory.

[ruiu]

Can you write a patch description? Please mention that Microsoft link.exe has this option.

Could you tell me a little bit about why you wanted this option?

It needs a testcase.

lld/COFF/Config.h

183

We use the same name for a command line option and a Config member name, so FileAlign.

[thrimbor]

Changed Config member name, added a test.

The test currently only checks the FileAlignment header field - I wanted to check the individual section offsets, too, but llvm-objdump doesn't expose this information.

[ruiu]

Did you personally need this? Or are you adding this just for completeness?

Either case is fine, but I'd like to know what is a use case of this feature.

[thrimbor]

In D62124#1509410, @ruiu wrote:

Did you personally need this? Or are you adding this just for completeness?

Either case is fine, but I'd like to know what is a use case of this feature.

I didn't need this myself, but the need for it came up during the work of another XboxDev member who worked on adding support for debug extensions to our toolchain targeting the original Xbox (called nxdk), and I decided to try and add this feature. Due to the lack of /filealign-support we currently have a workaround in place: https://github.com/JayFoxRox/nxdk/pull/10/commits/1822465478b04ca18d5544c103bfe743ad47876c#diff-6ffdcfc2a5d215955b49510d4547fc8cR19

Ideally we would be able to have filealign==align (which this patch makes possible, removing the need for the workaround) and be able to choose a small value for this (which would require hooking up /align, which I do plan to do, too) to minimize memory usage (the Xbox only has 64MiB of RAM and there has to be enough left for the application, so our debug extensions aren't allowed to consume much space).

[ruiu]

Th

lld/COFF/Driver.cpp

1190

Thank you for your explanation. One last thing -- can you add a check to make sure that a parsed value is a power of two?

[thrimbor]

I updated the patch to include a power-of-two check.

[ruiu]

LGTM

[ruiu]

Submitted as https://reviews.llvm.org/rL361634.

lld/COFF/Config.h

   std::string MapFile;
 
   uint64_t ImageBase = -1;
+  uint64_t FileAlign = 512;
   uint64_t StackReserve = 1024 * 1024;
   uint64_t StackCommit = 4096;
   uint64_t HeapReserve = 1024 * 1024;

lld/COFF/Driver.cpp

   if (auto *Arg = Args.getLastArg(OPT_base))
     parseNumbers(Arg->getValue(), &Config->ImageBase);
 
+  // Handle /filealign
+  if (auto *Arg = Args.getLastArg(OPT_filealign)) {
+    parseNumbers(Arg->getValue(), &Config->FileAlign);
+    if (!isPowerOf2_64(Config->FileAlign))
+      error("/filealign: not a power of two: " + Twine(Config->FileAlign));
+  }
+
   // Handle /stack
   if (auto *Arg = Args.getLastArg(OPT_stack))
     parseNumbers(Arg->getValue(), &Config->StackReserve, &Config->StackCommit);

lld/COFF/Options.td

 def export  : P<"export", "Export a function">;
 // No help text because /failifmismatch is not intended to be used by the user.
 def failifmismatch : P<"failifmismatch", "">;
+def filealign : P<"filealign", "Section alignment in the output file">;
 def functionpadmin : F<"functionpadmin">;
 def functionpadmin_opt : P<"functionpadmin", "Prepares an image for hotpatching">;
 def guard   : P<"guard", "Control flow guard">;

lld/COFF/Writer.cpp

 static_assert(sizeof(DOSProgram) % 8 == 0,
               "DOSProgram size must be multiple of 8");
 
-static const int SectorSize = 512;
 static const int DOSStubSize = sizeof(dos_header) + sizeof(DOSProgram);
 static_assert(DOSStubSize % 8 == 0, "DOSStub size must be multiple of 8");
 

   PointerToSymbolTable = FileOff;
   FileOff += OutputSymtab.size() * sizeof(coff_symbol16);
   FileOff += 4 + Strtab.size();
-  FileSize = alignTo(FileOff, SectorSize);
+  FileSize = alignTo(FileOff, Config->FileAlign);
 }
 
 void Writer::mergeSections() {

                   sizeof(coff_section) * OutputSections.size();
   SizeOfHeaders +=
       Config->is64() ? sizeof(pe32plus_header) : sizeof(pe32_header);
-  SizeOfHeaders = alignTo(SizeOfHeaders, SectorSize);
+  SizeOfHeaders = alignTo(SizeOfHeaders, Config->FileAlign);
   uint64_t RVA = PageSize; // The first page is kept unmapped.
   FileSize = SizeOfHeaders;
 

       C->finalizeContents();
       VirtualSize += C->getSize();
       if (C->hasData())
-        RawSize = alignTo(VirtualSize, SectorSize);
+        RawSize = alignTo(VirtualSize, Config->FileAlign);
     }
     if (VirtualSize > UINT32_MAX)
       error("section larger than 4 GiB: " + Sec->Name);

     if (RawSize != 0)
       Sec->Header.PointerToRawData = FileSize;
     RVA += alignTo(VirtualSize, PageSize);
-    FileSize += alignTo(RawSize, SectorSize);
+    FileSize += alignTo(RawSize, Config->FileAlign);
   }
   SizeOfImage = alignTo(RVA, PageSize);
 }

 
   PE->ImageBase = Config->ImageBase;
   PE->SectionAlignment = PageSize;
-  PE->FileAlignment = SectorSize;
+  PE->FileAlignment = Config->FileAlign;
   PE->MajorImageVersion = Config->MajorImageVersion;
   PE->MinorImageVersion = Config->MinorImageVersion;
   PE->MajorOperatingSystemVersion = Config->MajorOSVersion;

lld/test/COFF/filealign.test

+# RUN: yaml2obj < %s > %t.obj
+
+# RUN: lld-link /out:%t.exe /entry:main %t.obj
+# RUN: llvm-readobj --file-headers %t.exe | FileCheck -check-prefix=DEFAULT-HEADER %s
+
+# DEFAULT-HEADER:    FileAlignment: 512
+
+# RUN: lld-link /out:%t.exe /entry:main %t.obj /filealign:4096
+# RUN: llvm-readobj --file-headers %t.exe | FileCheck -check-prefix=FILEALIGN-HEADER %s
+
+# FILEALIGN-HEADER: FileAlignment: 4096
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4096
+    SectionData:     0000000000000000
+    Relocations:
+      - VirtualAddress:  0
+        SymbolName:      __ImageBase
+        Type:            IMAGE_REL_AMD64_ADDR64
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          8
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            main
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            __ImageBase
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...