mstorsjo created this revision.
mstorsjo added reviewers: phosek, MaskRay.
Herald added subscribers: pengfei, kristof.beyls.
mstorsjo requested review of this revision.
Herald added a project: clang.

For x86, most contempory mingw toolchains use i686 as 32 bit
x86 arch target.

As long as the target triple is set to the right form, this works
fine, either as the compiler's default target, or via e.g.
a triple prefix like i686-w64-mingw32-clang.

However, if the unprefixed toolchain targets x86_64, but the user
tries to switch it to target 32 bit by adding the -m32 option, the
computeTargetTriple function in Clang, together with
Triple::get32BitArchVariant, sets the arch to i386. This causes
the right sysroot to not be found.

When targeting an arch where there are potential spelling ambiguities
with respect to the sysroots (i386 and arm), check if the driver can
find a sysroot with the arch name - if not, try a couple other
candidates.

Other design ideas considered: It would fit in better with the design
to do this kind of arch name fixups in computeTargetTriple (it
already has a couple other OS/arch specific cases), but the
heuristics for detecting the potential right choice ties in quite
closely with how the toolchain looks for the implicit sysroot to use,
so I'd prefer to keep that logic in the toolchain::MinGW class/file.

This is done as a wrapper function before invoking the toolchain::MinGW()
constructor, because the MinGW() constructor can't run code of its own
until the superclass ToolChain() constructor returns. The fixups need to
be done before the ToolChain() constructor, because it uses the Triple
for per-target runtime directories.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===================================================================
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, this usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|\\\\}}i686-w64-mingw32{{/|\\\\}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===================================================================
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -56,10 +56,15 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY MinGW : public ToolChain {
-public:
+private:
   MinGW(const Driver &D, const llvm::Triple &Triple,
         const llvm::opt::ArgList &Args);
 
+public:
+  static std::unique_ptr<MinGW> Create(const Driver &D,
+                                       const llvm::Triple &Triple,
+                                       const llvm::opt::ArgList &Args);
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +108,6 @@
   mutable std::unique_ptr<tools::gcc::Preprocessor> Preprocessor;
   mutable std::unique_ptr<tools::gcc::Compiler> Compiler;
   void findGccLibDir();
-  llvm::ErrorOr<std::string> findGcc();
-  llvm::ErrorOr<std::string> findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===================================================================
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr<std::string> toolchains::MinGW::findGcc() {
+static llvm::ErrorOr<std::string> findGcc(const llvm::Triple &T) {
   llvm::SmallVector<llvm::SmallString<32>, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr<std::string> toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr<std::string>
+findClangRelativeSysroot(const Driver &D, const llvm::Triple &T,
+                         std::string &SysrootName) {
   llvm::SmallVector<llvm::SmallString<32>, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-      llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
     if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
       RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
     Base = getDriver().SysRoot;
   // Look for <clang-bin>/../<triplet>; if found, use <clang-bin>/.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr<std::string> TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr<std::string> TargetSubdir =
+               findClangRelativeSysroot(getDriver(), getTriple(), SysrootName))
     Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
-  else if (llvm::ErrorOr<std::string> GPPName = findGcc())
+  else if (llvm::ErrorOr<std::string> GPPName = findGcc(getTriple()))
     Base = std::string(llvm::sys::path::parent_path(
         llvm::sys::path::parent_path(GPPName.get())));
   else
@@ -625,3 +629,58 @@
     break;
   }
 }
+
+static bool testTriple(const Driver &D, const llvm::Triple &Triple,
+                       const ArgList &Args) {
+  // If an explicit sysroot is set, that will be used and we shouldn't try to
+  // detect anything else.
+  std::string SysrootName;
+  if (D.SysRoot.size())
+    return true;
+  if (llvm::ErrorOr<std::string> TargetSubdir =
+          findClangRelativeSysroot(D, Triple, SysrootName))
+    return true;
+  if (llvm::ErrorOr<std::string> GPPName = findGcc(Triple))
+    return true;
+  // If we neither found a colocated sysroot or a matching gcc executable,
+  // conclude that we can't know if this is the correct spelling of the triple.
+  return false;
+}
+
+static llvm::Triple adjustTriple(const Driver &D, const llvm::Triple &Triple,
+                                 const ArgList &Args) {
+  // First test if the original triple can find a sysroot with the triple
+  // name.
+  if (testTriple(D, Triple, Args))
+    return Triple;
+  llvm::SmallVector<llvm::StringRef, 3> Archs;
+  // If not, test a couple other possible arch names that might be what was
+  // intended.
+  if (Triple.getArch() == llvm::Triple::x86) {
+    Archs.emplace_back("i386");
+    Archs.emplace_back("i586");
+    Archs.emplace_back("i686");
+  } else if (Triple.getArch() == llvm::Triple::arm ||
+             Triple.getArch() == llvm::Triple::thumb) {
+    Archs.emplace_back("armv7");
+  }
+  for (auto A : Archs) {
+    llvm::Triple TestTriple(Triple);
+    TestTriple.setArchName(A);
+    if (testTriple(D, TestTriple, Args))
+      return TestTriple;
+  }
+  // If none was found, just proceed with the original value.
+  return Triple;
+}
+
+std::unique_ptr<toolchains::MinGW>
+toolchains::MinGW::Create(const Driver &D, const llvm::Triple &Triple,
+                          const ArgList &Args) {
+  llvm::Triple AdjustedTriple(Triple);
+  if (Triple.getArch() == llvm::Triple::x86 ||
+      Triple.getArch() == llvm::Triple::arm ||
+      Triple.getArch() == llvm::Triple::thumb)
+    AdjustedTriple = adjustTriple(D, Triple, Args);
+  return std::unique_ptr<MinGW>(new MinGW(D, AdjustedTriple, Args));
+}
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5342,7 +5342,7 @@
           TC = std::make_unique<toolchains::Generic_GCC>(*this, Target, Args);
         break;
       case llvm::Triple::GNU:
-        TC = std::make_unique<toolchains::MinGW>(*this, Target, Args);
+        TC = toolchains::MinGW::Create(*this, Target, Args);
         break;
       case llvm::Triple::Itanium:
         TC = std::make_unique<toolchains::CrossWindowsToolChain>(*this, Target,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to