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

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 1 23:49:41 PDT 2021


Author: Martin Storsjö
Date: 2021-07-02T09:49:14+03:00
New Revision: ce211c505b82e5bbb68b936968d9b54608285416

URL: https://github.com/llvm/llvm-project/commit/ce211c505b82e5bbb68b936968d9b54608285416
DIFF: https://github.com/llvm/llvm-project/commit/ce211c505b82e5bbb68b936968d9b54608285416.diff

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

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.)

Differential Revision: https://reviews.llvm.org/D104532

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

Modified: 
    lld/COFF/Config.h
    lld/COFF/Driver.cpp
    lld/COFF/InputFiles.cpp
    lld/COFF/Options.td
    lld/COFF/SymbolTable.cpp
    lld/COFF/SymbolTable.h
    lld/MinGW/Driver.cpp
    lld/MinGW/Options.td
    lld/test/MinGW/driver.test

Removed: 
    


################################################################################
diff  --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 71ba27e19f069..df883b779ee4a 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -263,6 +263,7 @@ struct Configuration {
   bool warnLocallyDefinedImported = true;
   bool warnDebugInfoUnusable = true;
   bool warnLongSectionNames = true;
+  bool warnStdcallFixup = true;
   bool incremental = true;
   bool integrityCheck = false;
   bool killAt = false;
@@ -273,6 +274,7 @@ struct Configuration {
   bool thinLTOIndexOnly;
   bool autoImport = false;
   bool pseudoRelocs = false;
+  bool stdcallFixup = false;
 };
 
 extern Configuration *config;

diff  --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 937f605590bc8..6ebd80f741808 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1773,6 +1773,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
       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.
@@ -2106,10 +2109,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   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.
@@ -2121,7 +2124,13 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     // 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();
   }
 

diff  --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 7488ed947698b..ef37e203d7edf 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -957,12 +957,6 @@ Optional<DILineInfo> ObjFile::getDILineInfo(uint32_t offset,
   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);

diff  --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index 33d560902f78d..2ce145520ea89 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -221,6 +221,7 @@ 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">;

diff  --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 3e0741839757c..536f343507243 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -28,6 +28,12 @@ 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;
@@ -249,7 +255,7 @@ static void reportUndefinedSymbol(const UndefinedDiag &undefDiag) {
   errorOrWarn(os.str());
 }
 
-void SymbolTable::loadMinGWAutomaticImports() {
+void SymbolTable::loadMinGWSymbols() {
   for (auto &i : symMap) {
     Symbol *sym = i.second;
     auto *undef = dyn_cast<Undefined>(sym);
@@ -260,17 +266,50 @@ void SymbolTable::loadMinGWAutomaticImports() {
 
     StringRef name = undef->getName();
 
-    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;
+    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);
+      log("Loading lazy " + l->getName() + " from " + l->getFile()->getName() +
+          " for automatic import");
+      forceLazy(l);
+    }
   }
 }
 

diff  --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h
index 2d3ec65eda236..e88002c883101 100644
--- a/lld/COFF/SymbolTable.h
+++ b/lld/COFF/SymbolTable.h
@@ -57,7 +57,9 @@ class SymbolTable {
   // 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.
@@ -135,6 +137,8 @@ extern SymbolTable *symtab;
 
 std::vector<std::string> getSymbolLocations(ObjFile *file, uint32_t symIndex);
 
+StringRef ltrim1(StringRef s, const char *chars);
+
 } // namespace coff
 } // namespace lld
 

diff  --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 27cb508403f63..4a3a9ef9be030 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -289,6 +289,11 @@ bool mingw::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   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))

diff  --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index e27c1365ab1f3..e1a505240cb82 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -35,11 +35,15 @@ 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">;

diff  --git a/lld/test/COFF/link-dll-stdcall.s b/lld/test/COFF/link-dll-stdcall.s
new file mode 100644
index 0000000000000..d9a6dbb342240
--- /dev/null
+++ b/lld/test/COFF/link-dll-stdcall.s
@@ -0,0 +1,88 @@
+# 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 at 8
+  .globl  @fastcall at 8
+  .globl  vectorcall@@8
+  .globl  __underscored
+_stdcall at 8:
+  movl    8(%esp), %eax
+  addl    4(%esp), %eax
+  retl    $8
+ at 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 at 8
+  movl    $1, %ecx
+  movl    $2, %edx
+  calll   @fastcall at 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 at 8 by linking to _fastcall
+# LOG-DAG: Resolving _stdcall at 8 by linking to _stdcall
+
+# ERROR-DAG: undefined symbol: _stdcall at 8
+# ERROR-DAG: undefined symbol: @fastcall at 8
+# ERROR-DAG: undefined symbol: vectorcall@@8

diff  --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index cb164df6ecce1..ab5ca4c5c791c 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -303,3 +303,12 @@ RUN: ld.lld -### -m i386pep foo.o 2>&1 | FileCheck -check-prefix NO-FATAL_WARNIN
 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{{ }}


        


More information about the llvm-commits mailing list