[LLD] [COFF] Fix up missing stdcall decorations in MinGW mode

[mstorsjo]

Summary

If linking directly against a DLL without an import library, the
DLL export symbols might not contain stdcall decorations.

If we have an undefined symbol with decoration, and we happen to have
a matching undecorated symbol (which either is lazy and can be loaded,
or already defined), then alias it against that instead.

This matches what's done in reverse, when we have a def file
declaring to export a symbol without decoration, but we only have
a defined decorated symbol. In that case we do a fuzzy match
(SymbolTable::findMangle). This case is more straightforward; if we
have a decorated undefined symbol, just strip the decoration and look
for the corresponding undecorated symbol name.

Add warnings and options for either silencing the warning or disabling
the whole feature, corresponding to how ld.bfd does it.

(This feature works for any symbol decoration mismatch, not only when
linking against a DLL directly; ld.bfd also tolerates it anywhere,
and also fixes up mismatches in the other direction, like
SymbolTable::findMangle, for any symbol, not only exports. But in
practice, at least for lld, it would primarily end up used for linking
against DLLs.)

[mstorsjo]

Updated to rerun CI after fixing a test in the base patch

[rnk]

lgtm

lld/test/COFF/link-dll-stdcall.s

80

Test case looks good, thanks!

lld/COFF/Config.h

struct Configuration {
   bool largeAddressAware = false;
   bool highEntropyVA = false;
   bool appContainer = false;
   bool mingw = false;
   bool warnMissingOrderSymbol = true;
   bool warnLocallyDefinedImported = true;
   bool warnDebugInfoUnusable = true;
   bool warnLongSectionNames = true;
+  bool warnStdcallFixup = true;
   bool incremental = true;
   bool integrityCheck = false;
   bool killAt = false;
   bool repro = false;
   bool swaprunCD = false;
   bool swaprunNet = false;
   bool thinLTOEmitImportsFiles;
   bool thinLTOIndexOnly;
   bool autoImport = false;
   bool pseudoRelocs = false;
+  bool stdcallFixup = false;
 };
 
 extern Configuration *config;
 
 } // namespace coff
 } // namespace lld
 
 #endif

lld/COFF/Driver.cpp

void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   config->debugGHashes = debug == DebugKind::GHash || debug == DebugKind::Full;
   config->debugSymtab = debug == DebugKind::Symtab;
   config->autoImport =
       args.hasFlag(OPT_auto_import, OPT_auto_import_no, config->mingw);
   config->pseudoRelocs = args.hasFlag(
       OPT_runtime_pseudo_reloc, OPT_runtime_pseudo_reloc_no, config->mingw);
   config->callGraphProfileSort = args.hasFlag(
       OPT_call_graph_profile_sort, OPT_call_graph_profile_sort_no, true);
+  config->stdcallFixup =
+      args.hasFlag(OPT_stdcall_fixup, OPT_stdcall_fixup_no, config->mingw);
+  config->warnStdcallFixup = !args.hasArg(OPT_stdcall_fixup);
 
   // Don't warn about long section names, such as .debug_info, for mingw or
   // when -debug:dwarf is requested.
   if (config->mingw || config->debugDwarf)
     config->warnLongSectionNames = false;
 
   config->lldmapFile = getMapFile(args, OPT_lldmap, OPT_lldmap_file);
   config->mapFile = getMapFile(args, OPT_map, OPT_map_file);
void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   }
 
   // Create wrapped symbols for -wrap option.
   std::vector<WrappedSymbol> wrapped = addWrappedSymbols(args);
   // Load more object files that might be needed for wrapped symbols.
   if (!wrapped.empty())
     while (run());
 
-  if (config->autoImport) {
+  if (config->autoImport || config->stdcallFixup) {
     // MinGW specific.
     // Load any further object files that might be needed for doing automatic
-    // imports.
+    // imports, and do stdcall fixups.
     //
     // For cases with no automatically imported symbols, this iterates once
     // over the symbol table and doesn't do anything.
     //
     // For the normal case with a few automatically imported symbols, this
     // should only need to be run once, since each new object file imported
     // is an import library and wouldn't add any new undefined references,
     // but there's nothing stopping the __imp_ symbols from coming from a
     // normal object file as well (although that won't be used for the
     // actual autoimport later on). If this pass adds new undefined references,
     // we won't iterate further to resolve them.
-    symtab->loadMinGWAutomaticImports();
+    //
+    // If stdcall fixups only are needed for loading import entries from
+    // a DLL without import library, this also just needs running once.
+    // If it ends up pulling in more object files from static libraries,
+    // (and maybe doing more stdcall fixups along the way), this would need
+    // to loop these two calls.
+    symtab->loadMinGWSymbols();
     run();
   }
 
   // At this point, we should not have any symbols that cannot be resolved.
   // If we are going to do codegen for link-time optimization, check for
   // unresolvable symbols first, so we don't spend time generating code that
   // will fail to link anyway.
   if (!BitcodeFile::instances.empty() && !config->forceUnresolved)

lld/COFF/InputFiles.cpp

if (!dwarf) {
     dwarf = make<DWARFCache>(DWARFContext::create(*getCOFFObj()));
     if (!dwarf)
       return None;
   }
 
   return dwarf->getDILineInfo(offset, sectionIndex);
 }
 
-static StringRef ltrim1(StringRef s, const char *chars) {
-  if (!s.empty() && strchr(chars, s[0]))
-    return s.substr(1);
-  return s;
-}
-
 void ImportFile::parse() {
   const char *buf = mb.getBufferStart();
   const auto *hdr = reinterpret_cast<const coff_import_header *>(buf);
 
   // Check if the total size is valid.
   if (mb.getBufferSize() != sizeof(*hdr) + hdr->SizeOfData)
     fatal("broken import library");
 

lld/COFF/Options.td

 def osversion : P_priv<"osversion">;
 def output_def : Joined<["/", "-", "/?", "-?"], "output-def:">;
 def pdb_source_path : P<"pdbsourcepath",
     "Base path used to make relative source file path absolute in PDB">;
 def rsp_quoting : Joined<["--"], "rsp-quoting=">,
   HelpText<"Quoting style for response files, 'windows' (default) or 'posix'">;
 def start_lib : F<"start-lib">,
   HelpText<"Start group of objects treated as if they were in a library">;
+defm stdcall_fixup : B_priv<"stdcall-fixup">;
 def thinlto_emit_imports_files :
     F<"thinlto-emit-imports-files">,
     HelpText<"Emit .imports files with -thinlto-index-only">;
 def thinlto_index_only :
     F<"thinlto-index-only">,
     HelpText<"Instead of linking, emit ThinLTO index files">;
 def thinlto_index_only_arg : P<
     "thinlto-index-only",

lld/COFF/SymbolTable.h

public:
   // Emit errors for symbols that cannot be resolved.
   void reportUnresolvable();
 
   // Try to resolve any undefined symbols and update the symbol table
   // accordingly, then print an error message for any remaining undefined
   // symbols and warn about imported local symbols.
   void resolveRemainingUndefines();
 
-  void loadMinGWAutomaticImports();
+  // Load lazy objects that are needed for MinGW automatic import and for
+  // doing stdcall fixups.
+  void loadMinGWSymbols();
   bool handleMinGWAutomaticImport(Symbol *sym, StringRef name);
 
   // Returns a list of chunks of selected symbols.
   std::vector<Chunk *> getChunks();
 
   // Returns a symbol for a given name. Returns a nullptr if not found.
   Symbol *find(StringRef name);
   Symbol *findUnderscore(StringRef name);
private:
   llvm::DenseMap<llvm::CachedHashStringRef, Symbol *> symMap;
   std::unique_ptr<BitcodeCompiler> lto;
 };
 
 extern SymbolTable *symtab;
 
 std::vector<std::string> getSymbolLocations(ObjFile *file, uint32_t symIndex);
 
+StringRef ltrim1(StringRef s, const char *chars);
+
 } // namespace coff
 } // namespace lld
 
 #endif

lld/COFF/SymbolTable.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <utility>
 
 using namespace llvm;
 
 namespace lld {
 namespace coff {
 
+StringRef ltrim1(StringRef s, const char *chars) {
+  if (!s.empty() && strchr(chars, s[0]))
+    return s.substr(1);
+  return s;
+}
+
 static Timer ltoTimer("LTO", Timer::root());
 
 SymbolTable *symtab;
 
 void SymbolTable::addFile(InputFile *file) {
   log("Reading " + toString(file));
   file->parse();
 
for (const std::string &s : symbolLocations) {
       os << s;
     }
   }
   if (numDisplayedRefs < numRefs)
     os << "\n>>> referenced " << numRefs - numDisplayedRefs << " more times";
   errorOrWarn(os.str());
 }
 
-void SymbolTable::loadMinGWAutomaticImports() {
+void SymbolTable::loadMinGWSymbols() {
   for (auto &i : symMap) {
     Symbol *sym = i.second;
     auto *undef = dyn_cast<Undefined>(sym);
     if (!undef)
       continue;
     if (undef->getWeakAlias())
       continue;
 
     StringRef name = undef->getName();
 
+    if (config->machine == I386 && config->stdcallFixup) {
+      // Check if we can resolve an undefined decorated symbol by finding
+      // the indended target as an undecorated symbol (only with a leading
+      // underscore).
+      StringRef origName = name;
+      StringRef baseName = name;
+      // Trim down stdcall/fastcall/vectorcall symbols to the base name.
+      baseName = ltrim1(baseName, "_@");
+      baseName = baseName.substr(0, baseName.find('@'));
+      // Add a leading underscore, as it would be in cdecl form.
+      std::string newName = ("_" + baseName).str();
+      Symbol *l;
+      if (newName != origName && (l = find(newName)) != nullptr) {
+        // If we found a symbol and it is lazy; load it.
+        if (l->isLazy() && !l->pendingArchiveLoad) {
+          log("Loading lazy " + l->getName() + " from " +
+              l->getFile()->getName() + " for stdcall fixup");
+          forceLazy(l);
+        }
+        // If it's lazy or already defined, hook it up as weak alias.
+        if (l->isLazy() || isa<Defined>(l)) {
+          if (config->warnStdcallFixup)
+            warn("Resolving " + origName + " by linking to " + newName);
+          else
+            log("Resolving " + origName + " by linking to " + newName);
+          undef->weakAlias = l;
+          continue;
+        }
+      }
+    }
+
+    if (config->autoImport) {
       if (name.startswith("__imp_"))
         continue;
       // If we have an undefined symbol, but we have a lazy symbol we could
       // load, load it.
       Symbol *l = find(("__imp_" + name).str());
       if (!l || l->pendingArchiveLoad || !l->isLazy())
         continue;
 
       log("Loading lazy " + l->getName() + " from " + l->getFile()->getName() +
           " for automatic import");
       forceLazy(l);
     }
   }
+}
 
 Defined *SymbolTable::impSymbol(StringRef name) {
   if (name.startswith("__imp_"))
     return nullptr;
   return dyn_cast_or_null<Defined>(find(("__imp_" + name).str()));
 }
 
 bool SymbolTable::handleMinGWAutomaticImport(Symbol *sym, StringRef name) {

lld/MinGW/Driver.cpp

if (auto *a = args.getLastArg(OPT_pdb)) {
     add("-debug:dwarf");
   }
 
   if (args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false))
     add("-WX");
   else
     add("-WX:no");
 
+  if (args.hasFlag(OPT_enable_stdcall_fixup, OPT_disable_stdcall_fixup, false))
+    add("-stdcall-fixup");
+  else if (args.hasArg(OPT_disable_stdcall_fixup))
+    add("-stdcall-fixup:no");
+
   if (args.hasArg(OPT_shared))
     add("-dll");
   if (args.hasArg(OPT_verbose))
     add("-verbose");
   if (args.hasArg(OPT_exclude_all_symbols))
     add("-exclude-all-symbols");
   if (args.hasArg(OPT_export_all_symbols))
     add("-export-all-symbols");

lld/MinGW/Options.td

 def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries">;
 defm demangle: B<"demangle",
     "Demangle symbol names (default)",
     "Do not demangle symbol names">;
 def disable_auto_import: F<"disable-auto-import">,
     HelpText<"Don't automatically import data symbols from other DLLs without dllimport">;
 def disable_runtime_pseudo_reloc: F<"disable-runtime-pseudo-reloc">,
     HelpText<"Don't do automatic imports that require runtime fixups">;
+def disable_stdcall_fixup: F<"disable-stdcall-fixup">,
+    HelpText<"Don't resolve stdcall/fastcall/vectorcall to undecorated symbols">;
 defm dynamicbase: B<"dynamicbase", "Enable ASLR", "Disable ASLR">;
 def enable_auto_import: F<"enable-auto-import">,
     HelpText<"Automatically import data symbols from other DLLs where needed">;
 def enable_runtime_pseudo_reloc: F<"enable-runtime-pseudo-reloc">,
     HelpText<"Allow automatic imports that require runtime fixups">;
+def enable_stdcall_fixup: F<"enable-stdcall-fixup">,
+    HelpText<"Resolve stdcall/fastcall/vectorcall to undecorated symbols without warnings">;
 defm entry: Eq<"entry", "Name of entry point symbol">, MetaVarName<"<entry>">;
 def exclude_all_symbols: F<"exclude-all-symbols">,
     HelpText<"Don't automatically export any symbols">;
 def export_all_symbols: F<"export-all-symbols">,
     HelpText<"Export all symbols even if a def file or dllexport attributes are used">;
 defm fatal_warnings: B<"fatal-warnings",
     "Treat warnings as errors",
     "Do not treat warnings as errors (default)">;

lld/test/COFF/link-dll-stdcall.s

+# REQUIRES: x86
+
+## Test creating a DLL and linking against the DLL without using an import
+## library.
+
+## Test on i386 with stdcall/fastcall/vectorcall decorated symbols.
+
+## Check that we normally warn about these fixups. If -stdcall-fixup:no
+## (--disable-stdcall-fixup on the MinGW linker level) is passed, we don't
+## do these fixups. If we -stdcall-fixup (--enable-stdcall-fixup on the MinGW
+## linker level) is passed, we don't warn about it at all.
+
+# RUN: split-file %s %t.dir
+
+# RUN: llvm-mc -filetype=obj -triple=i386-windows-gnu %t.dir/lib.s -o %t.lib.o
+# RUN: lld-link -safeseh:no -noentry -dll -def:%t.dir/lib.def %t.lib.o -out:%t.lib.dll -implib:%t.implib.lib
+# RUN: llvm-mc -filetype=obj -triple=i386-windows-gnu %t.dir/main.s -o %t.main.o
+# RUN: lld-link -lldmingw %t.main.o -out:%t.main.exe %t.lib.dll -opt:noref 2>&1 | FileCheck --check-prefix=LOG %s
+# RUN: llvm-readobj --coff-imports %t.main.exe | FileCheck %s
+# RUN: not lld-link -lldmingw %t.main.o -out:%t.main.exe %t.lib.dll -opt:noref -stdcall-fixup:no 2>&1 | FileCheck --check-prefix=ERROR %s
+# RUN: lld-link -lldmingw %t.main.o -out:%t.main.exe %t.lib.dll -opt:noref -stdcall-fixup 2>&1 | count 0
+
+#--- lib.s
+  .text
+  .globl  _stdcall@8
+  .globl  @fastcall@8
+  .globl  vectorcall@@8
+  .globl  __underscored
+_stdcall@8:
+  movl    8(%esp), %eax
+  addl    4(%esp), %eax
+  retl    $8
+@fastcall@8:
+  movl    8(%esp), %eax
+  addl    4(%esp), %eax
+  retl    $8
+vectorcall@@8:
+  movl    8(%esp), %eax
+  addl    4(%esp), %eax
+  retl    $8
+__underscored:
+  ret
+
+#--- lib.def
+EXPORTS
+stdcall
+fastcall
+vectorcall
+_underscored
+
+#--- main.s
+.text
+.global _mainCRTStartup
+_mainCRTStartup:
+  pushl   $2
+  pushl   $1
+  calll   _stdcall@8
+  movl    $1, %ecx
+  movl    $2, %edx
+  calll   @fastcall@8
+  movl    $1, %ecx
+  movl    $2, %edx
+  calll   vectorcall@@8
+  pushl   $2
+  pushl   $1
+  calll   __underscored
+  addl    $8, %esp
+  xorl    %eax, %eax
+  popl    %ebp
+  retl
+
+# CHECK:      Import {
+# CHECK-NEXT:   Name: link-dll-stdcall.s.tmp.lib.dll
+# CHECK-NEXT:   ImportLookupTableRVA:
+# CHECK-NEXT:   ImportAddressTableRVA
+# CHECK-NEXT:   Symbol: _underscored
+# CHECK-NEXT:   Symbol: fastcall
+# CHECK-NEXT:   Symbol: stdcall
+# CHECK-NEXT:   Symbol: vectorcall
+# CHECK-NEXT: }
+
+# LOG-DAG: Resolving vectorcall@@8 by linking to _vectorcall
+# LOG-DAG: Resolving @fastcall@8 by linking to _fastcall
+# LOG-DAG: Resolving _stdcall@8 by linking to _stdcall
+
+# ERROR-DAG: undefined symbol: _stdcall@8
+# ERROR-DAG: undefined symbol: @fastcall@8
+# ERROR-DAG: undefined symbol: vectorcall@@8

lld/test/MinGW/driver.test

 RUN: ld.lld -### -m i386pep foo.o -fatal-warnings 2>&1 | FileCheck -check-prefix FATAL_WARNINGS %s
 RUN: ld.lld -### -m i386pep foo.o --fatal-warnings 2>&1 | FileCheck -check-prefix FATAL_WARNINGS %s
 FATAL_WARNINGS: -WX{{ }}
 
 RUN: ld.lld -### -m i386pep foo.o 2>&1 | FileCheck -check-prefix NO-FATAL_WARNINGS %s
 RUN: ld.lld -### -m i386pep foo.o -no-fatal-warnings 2>&1 | FileCheck -check-prefix NO-FATAL_WARNINGS %s
 RUN: ld.lld -### -m i386pep foo.o --no-fatal-warnings 2>&1 | FileCheck -check-prefix NO-FATAL_WARNINGS %s
 NO-FATAL_WARNINGS: -WX:no
+
+RUN: ld.lld -### -m i386pep foo.o 2>&1 | FileCheck -check-prefix NO-FIXUP %s
+NO-FIXUP-NOT: -stdcall-fixup
+
+RUN: ld.lld -### -m i386pep foo.o --enable-stdcall-fixup --disable-stdcall-fixup 2>&1 | FileCheck -check-prefix DISABLE-FIXUP %s
+DISABLE-FIXUP: -stdcall-fixup:no
+
+RUN: ld.lld -### -m i386pep foo.o --disable-stdcall-fixup --enable-stdcall-fixup 2>&1 | FileCheck -check-prefix ENABLE-FIXUP %s
+ENABLE-FIXUP: -stdcall-fixup{{ }}