[LLD] [COFF] Link crtend.o as the last object file

[mstorsjo]

Summary

When faced with command line options such as "crtbegin.o appmain.o -lsomelib crtend.o", GNU ld pulls in all necessary object files from somelib before proceeding to crtend.o.

LLD operates differently, only loading object files from any referenced static libraries after processing all input object files.

This uses a similar hack as in the ELF linker. Here, it moves crtend.o to the end of the vector of object files. This makes sure that terminator chunks for sections such as .eh_frame gets ordered last, fixing DWARF exception handling for libgcc and gcc's crtend.o.

I copied the function for identifying the crtend object file from the ELF linker - should that maybe be moved to some shared file?

[ruiu]

This patches changes the order how input files are parsed. The other approach is to parse input files normally and then sort the list of object files so that crtend.o is at the end of the input list. I wonder if the latter is simpler. What do you think?

[mstorsjo]

In D60628#1466020, @ruiu wrote:

This patches changes the order how input files are parsed. The other approach is to parse input files normally and then sort the list of object files so that crtend.o is at the end of the input list. I wonder if the latter is simpler. What do you think?

I initially thought about something like that (although I mostly thought about sorting SectionChunks even later in the pipeline). My main concern was that one would end up doing a huge std::stable_sort over O(N) entries, when all I want to do is move one single entry to the end.

But I can give that a try, out of curiosity.

[ruiu]

I don't think you have to call std::stable_sort. Something like this should suffice.

for (size_t I = 0, E = InputFiles.size(); I != E; ++I) {
  InputFile *File = InputFiles[I];
  if (File.getName() != "crtend.o")
    continue;
  InputFiles.erase(i);
  InputFiles.push_back(File);
  break;
}

[mstorsjo]

In D60628#1466036, @ruiu wrote:

I don't think you have to call std::stable_sort. Something like this should suffice.

for (size_t I = 0, E = InputFiles.size(); I != E; ++I) {
  InputFile *File = InputFiles[I];
  if (File.getName() != "crtend.o")
    continue;
  InputFiles.erase(i);
  InputFiles.push_back(File);
  break;
}

Thanks, that actually is very sensible and straightforward.

[mstorsjo]

Reordering ObjFile::Instances instead of changing the order of parsing files, as suggested by Rui.

[ruiu]

COFF/Driver.cpp

95 ↗

(On Diff #195098)

Since Filename is always "crtend", I'd hardcode that and merge this function with isCrtend.

1685 ↗

(On Diff #195098)

Could you expand this comment a little to explain why that object file should be at the end of the input file list?

[mstorsjo]

Adjusted according to Rui's comments.

[ruiu]

LGTM

lld/trunk/COFF/Driver.cpp

 // Drop directory components and replace extension with ".exe" or ".dll".
 static std::string getOutputPath(StringRef Path) {
   auto P = Path.find_last_of("\\/");
   StringRef S = (P == StringRef::npos) ? Path : Path.substr(P + 1);
   const char* E = Config->DLL ? ".dll" : ".exe";
   return (S.substr(0, S.rfind('.')) + E).str();
 }
 
+// Returns true if S matches /crtend.?\.o$/.
+static bool isCrtend(StringRef S) {
+  if (!S.endswith(".o"))
+    return false;
+  S = S.drop_back(2);
+  if (S.endswith("crtend"))
+    return true;
+  return !S.empty() && S.drop_back().endswith("crtend");
+}
+
 // ErrorOr is not default constructible, so it cannot be used as the type
 // parameter of a future.
 // FIXME: We could open the file in createFutureForFile and avoid needing to
 // return an error here, but for the moment that would cost us a file descriptor
 // (a limited resource on Windows) for the duration that the future is pending.
 using MBErrPair = std::pair<std::unique_ptr<MemoryBuffer>, std::error_code>;
 
 // Create a std::future that opens and maps a file using the best strategy for
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (Args.hasFlag(OPT_safeseh, OPT_safeseh_no, false)) {
     for (ObjFile *File : ObjFile::Instances)
       if (!File->hasSafeSEH())
         error("/safeseh: " + File->getName() + " is not compatible with SEH");
     if (errorCount())
       return;
   }
 
+  if (Config->MinGW) {
     // In MinGW, all symbols are automatically exported if no symbols
     // are chosen to be exported.
-  if (Config->MinGW)
     maybeExportMinGWSymbols(Args);
 
+    // Make sure the crtend.o object is the last object file. This object
+    // file can contain terminating section chunks that need to be placed
+    // last. GNU ld processes files and static libraries explicitly in the
+    // order provided on the command line, while lld will pull in needed
+    // files from static libraries only after the last object file on the
+    // command line.
+    for (auto I = ObjFile::Instances.begin(), E = ObjFile::Instances.end();
+         I != E; I++) {
+      ObjFile *File = *I;
+      if (isCrtend(File->getName())) {
+        ObjFile::Instances.erase(I);
+        ObjFile::Instances.push_back(File);
+        break;
+      }
+    }
+  }
+
   // Windows specific -- when we are creating a .dll file, we also
   // need to create a .lib file.
   if (!Config->Exports.empty() || Config->DLL) {
     fixupExports();
     createImportLibrary(/*AsLib=*/false);
     assignExportOrdinals();
   }
 

lld/trunk/docs/ReleaseNotes.rst

 * lld-link now correctly reports duplicate symbol errors for obj files
   that were compiled with /Gy.
 
 * ...
 
 MinGW Improvements
 ------------------
 
-* ...
+* lld now correctly links crtend.o as the last object file, handling
+  terminators for the sections such as .eh_frame properly, fixing
+  DWARF exception handling with libgcc and gcc's crtend.o.
 
 MachO Improvements
 ------------------
 
 * Item 1.
 
 WebAssembly Improvements
 ------------------------
 
 * ...

lld/trunk/test/COFF/Inputs/eh_frame_terminator-crtend.s

+    .section .eh_frame,"dr"
+__FRAME_END__:
+    .byte 3

lld/trunk/test/COFF/Inputs/eh_frame_terminator-otherfunc.s

+    .text
+    .globl otherfunc
+otherfunc:
+    ret
+
+    .section .eh_frame,"dr"
+    .byte 2

lld/trunk/test/COFF/eh_frame_terminator.s

+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows-gnu %s -o %t.main.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows-gnu \
+// RUN:   %p/Inputs/eh_frame_terminator-otherfunc.s -o %t.otherfunc.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows-gnu \
+// RUN:   %p/Inputs/eh_frame_terminator-crtend.s -o %t.crtend.o
+// RUN: rm -f %t.otherfunc.lib
+// RUN: llvm-ar rcs %t.otherfunc.lib %t.otherfunc.o
+// RUN: lld-link -lldmingw %t.main.o %t.otherfunc.lib %t.crtend.o -out:%t.exe
+// RUN: llvm-objdump -s %t.exe | FileCheck %s
+
+    .text
+    .globl main
+main:
+    call otherfunc
+    ret
+
+    .globl mainCRTStartup
+mainCRTStartup:
+    call main
+
+    .section .eh_frame,"dr"
+    .byte 1
+
+// CHECK: Contents of section .eh_fram:
+// CHECK-NEXT: 140003000 010203