[LLD] [MinGW] Fall back to using default target if no -m flag given. #134700

jeremyd2019 · 2025-04-07T18:09:41Z

On Cygwin at least, GCC is not passing any -m flag to the linker, so fall back to the default target triple to determine if we need to apply i386-specific behaviors.

Fixes #134558

llvmbot · 2025-04-07T18:10:21Z

@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-lld

Author: None (jeremyd2019)

Changes

On Cygwin at least, GCC is not pasing any -m flag to the linker, so fall back to the default target triple to determine if we need to apply i386-specific behaviors.

Fixes #134558

Full diff: https://github.com/llvm/llvm-project/pull/134700.diff

1 Files Affected:

(modified) lld/MinGW/Driver.cpp (+13-2)

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index a77d86b443a6c..abe88cab1b334 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -171,6 +171,15 @@ searchLibrary(StringRef name, ArrayRef<StringRef> searchPaths, bool bStatic) {
   return "";
 }
 
+static bool
+isI386Target(const opt::InputArgList &args, const Triple &defaultTarget)
+{
+  auto *a = args.getLastArg(OPT_m);
+  if (a)
+    return StringRef(a->getValue()) == "i386pe";
+  return defaultTarget.getArch() == Triple::x86;
+}
+
 namespace lld {
 namespace coff {
 bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
@@ -216,6 +225,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     return false;
   }
 
+  Triple defaultTarget(Triple::normalize(sys::getDefaultTargetTriple()));
+
   std::vector<std::string> linkArgs;
   auto add = [&](const Twine &s) { linkArgs.push_back(s.str()); };
 
@@ -224,7 +235,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
 
   if (auto *a = args.getLastArg(OPT_entry)) {
     StringRef s = a->getValue();
-    if (args.getLastArgValue(OPT_m) == "i386pe" && s.starts_with("_"))
+    if (isI386Target(args, defaultTarget) && s.starts_with("_"))
       add("-entry:" + s.substr(1));
     else if (!s.empty())
       add("-entry:" + s);
@@ -521,7 +532,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
   for (auto *a : args.filtered(OPT_Xlink))
     add(a->getValue());
 
-  if (args.getLastArgValue(OPT_m) == "i386pe")
+  if (isI386Target(args, defaultTarget))
     add("-alternatename:__image_base__=___ImageBase");
   else
     add("-alternatename:__image_base__=__ImageBase");

github-actions · 2025-04-07T18:11:59Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- lld/MinGW/Driver.cpp

View the diff from clang-format here.

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index abe88cab1..222ee3a93 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -171,9 +171,8 @@ searchLibrary(StringRef name, ArrayRef<StringRef> searchPaths, bool bStatic) {
   return "";
 }
 
-static bool
-isI386Target(const opt::InputArgList &args, const Triple &defaultTarget)
-{
+static bool isI386Target(const opt::InputArgList &args,
+                         const Triple &defaultTarget) {
   auto *a = args.getLastArg(OPT_m);
   if (a)
     return StringRef(a->getValue()) == "i386pe";

On Cygwin at least, GCC is not pasing any -m flag to the linker, so fall back to the default target triple to determine if we need to apply i386-specific behaviors. Fixes llvm#134558

jeremyd2019 · 2025-04-07T18:14:18Z

Compared to what I was playing with in #134558 (comment) I decided to create a Triple variable in mingw::link since this isI386Target function is now called twice. Also, more cygwin-related hackery I'm playing with is going to have to depend on the target being isWindowsCygwinEnvironment so having the target there as a variable will be helpful for that too.

jeremyd2019 · 2025-04-08T17:08:30Z

@mstorsjo I don't know if I was supposed to ping you on this. Sorry for the noise if not.

mstorsjo · 2025-04-08T20:28:09Z

@mstorsjo I don't know if I was supposed to ping you on this. Sorry for the noise if not.

No problem, I don't mind getting CC:d. (If you add the lld:COFF tag on an issue or PR, it should notify a group of people including me, so that usually works as well, but I don't mind getting mentioned just to be sure.)

Edit: lld:COFF isn't perhaps exactly the right one here, but it's close enough, so hopefully the other subscribers to that tag don't mind.

mstorsjo

For context here; normally if one invokes ld.lld without an -m flag, it would run the ELF linker. However, if LLD is built with the CMake option LLD_DEFAULT_LD_LLD_IS_MINGW enabled, then ld.lld without an -m flag invokes the MinGW linker instead. In such a setup, this PR allows using the default target triple for checking whether we should assume i386 specific behaviours, when no -m option is present.

I believe this is a reasonable thing to do, so I'll approve the change, and can merge it after leaving some time for others to comment/disagree on it. If there's no disagreement, I can merge it after a day or two.

jeremyd2019 · 2025-04-09T00:47:47Z

This particular case isn't too bad, but for Cygwin there are others. Unfortunately, it looks like GNU ld "knows" the target at build time and does some different things between Cygwin and MinGW.

for example, @mati865's patch

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 131bacdf1468..94acd68abd12 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -546,6 +546,10 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     add("-exclude-symbols:" + StringRef(a->getValue()));

   std::vector<StringRef> searchPaths;
+  #ifdef __CYGWIN__
+    searchPaths.push_back("/usr/lib");
+    searchPaths.push_back("/usr/lib/w32api");
+  #endif
   for (auto *a : args.filtered(OPT_L)) {
     searchPaths.push_back(a->getValue());
     add("-libpath:" + StringRef(a->getValue()));
-- 
2.47.1.windows.2

and my patch (which also includes the changes in this PR)

--- a/lld/MinGW/Driver.cpp	2025-04-07 14:55:18.420801800 -0700
+++ b/lld/MinGW/Driver.cpp	2025-04-07 17:27:27.318509600 -0700
@@ -177,6 +177,24 @@
   return "";
 }
 
+static inline bool isI386Target(const opt::InputArgList &args,
+                                const Triple &defaultTarget) {
+  auto *a = args.getLastArg(OPT_m);
+  if (a)
+    return StringRef(a->getValue()) == "i386pe";
+  return defaultTarget.getArch() == Triple::x86;
+}
+
+static inline bool is64bitTarget(const opt::InputArgList &args,
+                                 const Triple &defaultTarget) {
+  auto *a = args.getLastArg(OPT_m);
+  if (a) {
+    StringRef s = a->getValue();
+    return s == "i386pep" || s == "arm64pe" || s == "arm64ecpe";
+  }
+  return defaultTarget.isArch64Bit();
+}
+
 namespace lld {
 namespace coff {
 bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
@@ -222,6 +240,9 @@
     return false;
   }
 
+  Triple defaultTarget(Triple::normalize(sys::getDefaultTargetTriple()));
+  bool isCygwinTarget = defaultTarget.isWindowsCygwinEnvironment();
+
   std::vector<std::string> linkArgs;
   auto add = [&](const Twine &s) { linkArgs.push_back(s.str()); };
 
@@ -230,7 +251,7 @@
 
   if (auto *a = args.getLastArg(OPT_entry)) {
     StringRef s = a->getValue();
-    if (args.getLastArgValue(OPT_m) == "i386pe" && s.starts_with("_"))
+    if (isI386Target(args, defaultTarget) && s.starts_with("_"))
       add("-entry:" + s.substr(1));
     else if (!s.empty())
       add("-entry:" + s);
@@ -287,6 +308,26 @@
     add("-output-def:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_image_base))
     add("-base:" + StringRef(a->getValue()));
+  else if (isCygwinTarget) {
+    if (args.hasFlag(OPT_enable_auto_image_base, OPT_disable_auto_image_base,
+          false) && args.hasArg(OPT_shared)) {
+      hash_code outhash;
+      uint64_t autobase;
+      if (auto *a = args.getLastArg(OPT_o))
+        outhash = hash_value(StringRef(a->getValue()));
+      else
+        outhash = hash_value(StringRef("a.dll"));
+      if (is64bitTarget(args, defaultTarget))
+        autobase =
+          0x400000000ULL + (((uint64_t)outhash << 16) & 0x1ffff0000ULL);
+      else
+        autobase =
+          (uint32_t)(0x61500000 + (((uint32_t)outhash << 16) & 0x0FFC0000));
+      add("-base:0x" + Twine::utohexstr(autobase));
+    } else if (is64bitTarget(args, defaultTarget)) {
+      add(args.hasArg(OPT_shared) ? "-base:0x400000000" : "-base:0x100400000");
+    }
+  }
   if (auto *a = args.getLastArg(OPT_map))
     add("-lldmap:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_reproduce))
@@ -373,14 +414,17 @@
   if (args.hasFlag(OPT_no_seh, OPT_disable_no_seh, false))
     add("-noseh");
 
-  if (args.getLastArgValue(OPT_m) != "thumb2pe" &&
-      args.getLastArgValue(OPT_m) != "arm64pe" &&
-      args.getLastArgValue(OPT_m) != "arm64ecpe" &&
-      args.hasFlag(OPT_disable_dynamicbase, OPT_dynamicbase, false))
+  if ((args.getLastArgValue(OPT_m) != "thumb2pe" &&
+       args.getLastArgValue(OPT_m) != "arm64pe" &&
+       args.getLastArgValue(OPT_m) != "arm64ecpe" &&
+       args.hasFlag(OPT_disable_dynamicbase, OPT_dynamicbase, false)) ||
+      (isCygwinTarget && !args.hasArg(OPT_dynamicbase)))
     add("-dynamicbase:no");
-  if (args.hasFlag(OPT_disable_high_entropy_va, OPT_high_entropy_va, false))
+  if (args.hasFlag(OPT_disable_high_entropy_va, OPT_high_entropy_va, false) ||
+      (isCygwinTarget && !args.hasArg(OPT_high_entropy_va)))
     add("-highentropyva:no");
-  if (args.hasFlag(OPT_disable_nxcompat, OPT_nxcompat, false))
+  if (args.hasFlag(OPT_disable_nxcompat, OPT_nxcompat, false) ||
+      (isCygwinTarget && !args.hasArg(OPT_nxcompat)))
     add("-nxcompat:no");
   if (args.hasFlag(OPT_disable_tsaware, OPT_tsaware, false))
     add("-tsaware:no");
@@ -527,7 +568,7 @@
   for (auto *a : args.filtered(OPT_Xlink))
     add(a->getValue());
 
-  if (args.getLastArgValue(OPT_m) == "i386pe")
+  if (isI386Target(args, defaultTarget))
     add("-alternatename:__image_base__=___ImageBase");
   else
     add("-alternatename:__image_base__=__ImageBase");
--- a/lld/MinGW/Options.td	2025-04-07 14:55:18.388986300 -0700
+++ b/lld/MinGW/Options.td	2025-04-07 17:03:20.201184300 -0700
@@ -59,6 +59,7 @@
 defm demangle: B<"demangle",
     "Demangle symbol names (default)",
     "Do not demangle symbol names">;
+def disable_auto_image_base: F<"disable-auto-image-base">;
 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">,
@@ -68,6 +69,7 @@
 defm dll_search_prefix: EEq<"dll-search-prefix",
     "Prefer DLL with this prefix if importlib is missing">;
 defm dynamicbase: B_disable<"dynamicbase", "Enable ASLR", "Disable ASLR">;
+def enable_auto_image_base: F<"enable-auto-image-base">;
 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">,
@@ -238,8 +240,6 @@
 // Ignored options
 def: Joined<["-"], "O">;
 def: F<"as-needed">;
-def: F<"disable-auto-image-base">;
-def: F<"enable-auto-image-base">;
 def: F<"end-group">;
 def: Flag<["--"], "full-shutdown">;
 defm: EqNoHelp<"major-image-version">;

…lvm#134700) On Cygwin at least, GCC is not passing any -m flag to the linker, so fall back to the default target triple to determine if we need to apply i386-specific behaviors. Fixes llvm#134558

llvmbot added the lld label Apr 7, 2025

[LLD] [MinGW] Fall back to using default target if no -m flag given.

2fe7deb

On Cygwin at least, GCC is not pasing any -m flag to the linker, so fall back to the default target triple to determine if we need to apply i386-specific behaviors. Fixes llvm#134558

jeremyd2019 force-pushed the lld-mingw-i386-no-m branch from 4b0c68a to 2fe7deb Compare April 7, 2025 18:12

mstorsjo added the lld:COFF label Apr 8, 2025

mstorsjo approved these changes Apr 8, 2025

View reviewed changes

mstorsjo merged commit e4a79b7 into llvm:main Apr 10, 2025
8 checks passed

jeremyd2019 deleted the lld-mingw-i386-no-m branch April 10, 2025 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLD] [MinGW] Fall back to using default target if no -m flag given. #134700

[LLD] [MinGW] Fall back to using default target if no -m flag given. #134700

Uh oh!

jeremyd2019 commented Apr 7, 2025 •

edited by mstorsjo

Loading

Uh oh!

llvmbot commented Apr 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

jeremyd2019 commented Apr 7, 2025 •

edited

Loading

Uh oh!

jeremyd2019 commented Apr 8, 2025

Uh oh!

mstorsjo commented Apr 8, 2025 •

edited

Loading

Uh oh!

mstorsjo left a comment

Uh oh!

jeremyd2019 commented Apr 9, 2025

Uh oh!

Uh oh!

Uh oh!

[LLD] [MinGW] Fall back to using default target if no -m flag given. #134700

[LLD] [MinGW] Fall back to using default target if no -m flag given. #134700

Uh oh!

Conversation

jeremyd2019 commented Apr 7, 2025 • edited by mstorsjo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

jeremyd2019 commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeremyd2019 commented Apr 8, 2025

Uh oh!

mstorsjo commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstorsjo left a comment

Choose a reason for hiding this comment

Uh oh!

jeremyd2019 commented Apr 9, 2025

Uh oh!

Uh oh!

Uh oh!

jeremyd2019 commented Apr 7, 2025 •

edited by mstorsjo

Loading

llvmbot commented Apr 7, 2025 •

edited

Loading

jeremyd2019 commented Apr 7, 2025 •

edited

Loading

mstorsjo commented Apr 8, 2025 •

edited

Loading