[Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

[phosek]

Summary

This introduces a support for multilibs to Fuchsia driver. Unlike the
existing multilibs that are used primarily for handling different
architecture variants, we use multilibs to handle different variants
of Clang runtime libraries: -fsanitize=address and -fno-exceptions
are the two we support initially. This replaces the existing support
for sanitized runtimes libraries that was only used by Fuchsia driver
and it also refactors some of the logic to allow sharing between GNU
and Fuchsia drivers.

[jroelofs]

clang/lib/Driver/ToolChains/CommonArgs.h

122 ↗

(On Diff #196335)

Can we enforce this precondition with an assert? The '-'-prefix-not-there part is easy, but what about the "it's a driver flag" part?

clang/test/Driver/fuchsia.cpp

53 ↗

(On Diff #196335)

What about a test for -fsanitize=address + -fno-exceptions?

[phosek]

clang/lib/Driver/ToolChains/CommonArgs.h

122 ↗

(On Diff #196335)

We could, but we would need to pass in the reference to Driver so we can use getOpts().ParseOneArg to parse the flag. However, we're not doing that in Multilib either at the moment even though the comment says it has to be a valid flag. I'd prefer to keep this as a pure move and then add the additional checking both here and to Multilib as a separate change, would that be fine with you?

[mcgrathr]

lgtm

clang/lib/Driver/ToolChains/CommonArgs.cpp

1507 ↗

(On Diff #196335)

I'd have reduced the duplication and just used Enabled ? "+" : "-"

clang/lib/Driver/ToolChains/Fuchsia.cpp

197 ↗

(On Diff #196335)

These two lines repeating the path construction logic could be CSEd into a lambda.

201 ↗

(On Diff #196335)

This merits a comment about the order they're being inserted.

clang/test/Driver/fuchsia.cpp

60 ↗

(On Diff #196335)

Might be worth adding a test that noexcept does *not* appear by default (or explicit -fexceptions).

[mcgrathr]

clang/lib/Driver/ToolChains/Fuchsia.cpp

179 ↗

(On Diff #196335)

Add a comment about the priorities.

lib/Driver/ToolChains/CommonArgs.h

 //===--- CommonArgs.h - Args handling for multiple toolchains ---*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONARGS_H
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONARGS_H
 
 #include "InputInfo.h"
 #include "clang/Driver/Driver.h"
+#include "clang/Driver/Multilib.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 #include "llvm/Support/CodeGen.h"
 
 namespace clang {
 namespace driver {
 namespace tools {
 
 void addPathIfExists(const Driver &D, const Twine &Path,
                      ToolChain::path_list &Paths);
 
 void AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs,
                      const llvm::opt::ArgList &Args,
                      llvm::opt::ArgStringList &CmdArgs, const JobAction &JA);
 
 void claimNoWarnArgs(const llvm::opt::ArgList &Args);
 
 bool addSanitizerRuntimes(const ToolChain &TC, const llvm::opt::ArgList &Args,
                           llvm::opt::ArgStringList &CmdArgs);
 
-void addSanitizerPathLibArgs(const ToolChain &TC,
-                             const llvm::opt::ArgList &Args,
-                             llvm::opt::ArgStringList &CmdArgs);
-
 void linkSanitizerRuntimeDeps(const ToolChain &TC,
                               llvm::opt::ArgStringList &CmdArgs);
 
 bool addXRayRuntime(const ToolChain &TC, const llvm::opt::ArgList &Args,
                     llvm::opt::ArgStringList &CmdArgs);
 
 void linkXRayRuntimeDeps(const ToolChain &TC,
                          llvm::opt::ArgStringList &CmdArgs);
void handleTargetFeaturesGroup(const llvm::opt::ArgList &Args,
                                std::vector<StringRef> &Features,
                                llvm::opt::OptSpecifier Group);
 
 /// Handles the -save-stats option and returns the filename to save statistics
 /// to.
 SmallString<128> getStatsFileName(const llvm::opt::ArgList &Args,
                                   const InputInfo &Output,
                                   const InputInfo &Input, const Driver &D);
+
+/// \p Flag must be a flag accepted by the driver with its leading '-' removed,
+//     otherwise '-print-multi-lib' will not emit them correctly.
+void addMultilibFlag(bool Enabled, const char *const Flag,
+                     Multilib::flags_list &Flags);
+
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_COMMONARGS_H

lib/Driver/ToolChains/CommonArgs.cpp

static bool addSanitizerDynamicList(const ToolChain &TC, const ArgList &Args,
   SmallString<128> SanRT(TC.getCompilerRT(Args, Sanitizer));
   if (llvm::sys::fs::exists(SanRT + ".syms")) {
     CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + SanRT + ".syms"));
     return true;
   }
   return false;
 }
 
-static void addSanitizerLibPath(const ToolChain &TC, const ArgList &Args,
-                                ArgStringList &CmdArgs, StringRef Name) {
-  for (const auto &LibPath : TC.getLibraryPaths()) {
-    if (!LibPath.empty()) {
-      SmallString<128> P(LibPath);
-      llvm::sys::path::append(P, Name);
-      if (TC.getVFS().exists(P))
-        CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + P));
-    }
-  }
-}
-
-void tools::addSanitizerPathLibArgs(const ToolChain &TC, const ArgList &Args,
-                                    ArgStringList &CmdArgs) {
-  const SanitizerArgs &SanArgs = TC.getSanitizerArgs();
-  if (SanArgs.needsAsanRt()) {
-    addSanitizerLibPath(TC, Args, CmdArgs, "asan");
-  }
-  if (SanArgs.needsHwasanRt()) {
-    addSanitizerLibPath(TC, Args, CmdArgs, "hwasan");
-  }
-  if (SanArgs.needsLsanRt()) {
-    addSanitizerLibPath(TC, Args, CmdArgs, "lsan");
-  }
-  if (SanArgs.needsMsanRt()) {
-    addSanitizerLibPath(TC, Args, CmdArgs, "msan");
-  }
-  if (SanArgs.needsTsanRt()) {
-    addSanitizerLibPath(TC, Args, CmdArgs, "tsan");
-  }
-}
-
-
-
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
                                      ArgStringList &CmdArgs) {
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
       !TC.getTriple().isAndroid()) {
if (SaveStats == "obj" && Output.isFilename()) {
     return {};
   }
 
   StringRef BaseName = llvm::sys::path::filename(Input.getBaseInput());
   llvm::sys::path::append(StatsFile, BaseName);
   llvm::sys::path::replace_extension(StatsFile, "stats");
   return StatsFile;
 }
+
+void tools::addMultilibFlag(bool Enabled, const char *const Flag,
+                            Multilib::flags_list &Flags) {
+  Flags.push_back(std::string(Enabled ? "+" : "-") + Flag);
+}

lib/Driver/ToolChains/Fuchsia.cpp

 #include "CommonArgs.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
+using tools::addMultilibFlag;
+
 void fuchsia::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                    const InputInfo &Output,
                                    const InputInfoList &Inputs,
                                    const ArgList &Args,
                                    const char *LinkingOutput) const {
   const toolchains::Fuchsia &ToolChain =
       static_cast<const toolchains::Fuchsia &>(getToolChain());
   const Driver &D = ToolChain.getDriver();
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
     if (!Args.hasArg(options::OPT_shared)) {
       CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("Scrt1.o")));
     }
   }
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_u);
 
-  addSanitizerPathLibArgs(ToolChain, Args, CmdArgs);
-
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   if (D.isUsingLTO()) {
     assert(!Inputs.empty() && "Must have at least one input.");
     AddGoldPlugin(ToolChain, Args, CmdArgs, Output, Inputs[0],
                   D.getLTOMode() == LTOK_Thin);
   }
 
Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
   if (getDriver().getInstalledDir() != D.Dir)
     getProgramPaths().push_back(D.Dir);
 
   if (!D.SysRoot.empty()) {
     SmallString<128> P(D.SysRoot);
     llvm::sys::path::append(P, "lib");
     getFilePaths().push_back(P.str());
   }
+
+  auto RuntimeDirs = [&](const Multilib &M) -> std::vector<std::string> {
+    SmallString<128> P;
+    std::vector<std::string> RD;
+
+    P.assign(D.ResourceDir);
+    llvm::sys::path::append(P, D.getTargetTriple(), "lib", M.gccSuffix());
+    if (getVFS().exists(P))
+      RD.push_back(P.str());
+
+    P.assign(D.ResourceDir);
+    llvm::sys::path::append(P, Triple.str(), "lib", M.gccSuffix());
+    if (getVFS().exists(P))
+      RD.push_back(P.str());
+
+    return RD;
+  };
+
+  Multilibs.push_back(Multilib());
+  // Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
+  Multilibs.push_back(Multilib("noexcept", {}, {}, 1)
+                          .flag("-fexceptions")
+                          .flag("+fno-exceptions"));
+  // ASan has higher priority because we always want the instrumentated version.
+  Multilibs.push_back(Multilib("asan", {}, {}, 2)
+                          .flag("+fsanitize=address"));
+  Multilibs.FilterOut([&](const Multilib &M) {
+    std::vector<std::string> RD = RuntimeDirs(M);
+    return std::all_of(RD.begin(), RD.end(), [&](std::string P) {
+      return !getVFS().exists(P);
+    });
+  });
+
+  Multilib::flags_list Flags;
+  addMultilibFlag(
+      Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, true),
+      "fexceptions", Flags);
+  addMultilibFlag(getSanitizerArgs().needsAsanRt(), "fsanitize=address", Flags);
+  Multilibs.setFilePathsCallback(RuntimeDirs);
+
+  if (Multilibs.select(Flags, SelectedMultilib))
+    if (!SelectedMultilib.isDefault())
+      if (const auto &PathsCallback = Multilibs.filePathsCallback())
+        for (const auto &Path : PathsCallback(SelectedMultilib))
+          // We need to prepend the multilib path to ensure it takes precedence.
+          getLibraryPaths().insert(getLibraryPaths().begin(), Path);
 }
 
 std::string Fuchsia::ComputeEffectiveClangTriple(const ArgList &Args,
                                                  types::ID InputType) const {
   llvm::Triple Triple(ComputeLLVMTriple(Args, InputType));
   return (Triple.getArchName() + "-" + Triple.getOSName()).str();
 }
 

lib/Driver/ToolChains/Gnu.cpp

 #include "llvm/Support/VirtualFileSystem.h"
 #include <system_error>
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace clang;
 using namespace llvm::opt;
 
+using tools::addMultilibFlag;
+
 void tools::GnuTool::anchor() {}
 
 static bool forwardToGCC(const Option &O) {
   // Don't forward inputs from the original command line.  They are added from
   // InputInfoList.
   return O.getKind() != Option::InputClass &&
          !O.hasFlag(options::DriverOption) && !O.hasFlag(options::LinkerInput);
 }
static bool isSoftFloatABI(const ArgList &Args) {
   if (!A)
     return false;
 
   return A->getOption().matches(options::OPT_msoft_float) ||
          (A->getOption().matches(options::OPT_mfloat_abi_EQ) &&
           A->getValue() == StringRef("soft"));
 }
 
-/// \p Flag must be a flag accepted by the driver with its leading '-' removed,
-//     otherwise '-print-multi-lib' will not emit them correctly.
-static void addMultilibFlag(bool Enabled, const char *const Flag,
-                            std::vector<std::string> &Flags) {
-  if (Enabled)
-    Flags.push_back(std::string("+") + Flag);
-  else
-    Flags.push_back(std::string("-") + Flag);
-}
-
 static bool isArmOrThumbArch(llvm::Triple::ArchType Arch) {
   return Arch == llvm::Triple::arm || Arch == llvm::Triple::thumb;
 }
 
 static bool isMipsEL(llvm::Triple::ArchType Arch) {
   return Arch == llvm::Triple::mipsel || Arch == llvm::Triple::mips64el;
 }
 

test/Driver/fuchsia.cpp

 // CHECK-STATIC: "--pop-state"
 // CHECK-STATIC: "-lc"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia -nostdlib++ -fuse-ld=lld 2>&1 \
 // RUN:     | FileCheck %s -check-prefix=CHECK-NOSTDLIBXX
 // CHECK-NOSTDLIBXX-NOT: "-lc++"
 // CHECK-NOSTDLIBXX-NOT: "-lm"
 // CHECK-NOSTDLIBXX: "-lc"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:     -fuse-ld=lld 2>&1\
+// RUN:     | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
+// RUN: %clang %s -### --target=x86_64-fuchsia -fsanitize=address \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:     -fuse-ld=lld 2>&1\
+// RUN:     | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-ASAN-X86
+// RUN: %clang %s -### --target=x86_64-fuchsia -fno-exceptions \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:     -fuse-ld=lld 2>&1\
+// RUN:     | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-NOEXCEPT-X86
+// RUN: %clang %s -### --target=x86_64-fuchsia -fsanitize=address -fno-exceptions \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:     -fuse-ld=lld 2>&1\
+// RUN:     | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-ASAN-X86
+// CHECK-MULTILIB-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-MULTILIB-ASAN-X86: "-L[[RESOURCE_DIR]]{{/|\\\\}}x86_64-fuchsia{{/|\\\\}}lib{{/|\\\\}}asan"
+// CHECK-MULTILIB-NOEXCEPT-X86: "-L[[RESOURCE_DIR]]{{/|\\\\}}x86_64-fuchsia{{/|\\\\}}lib{{/|\\\\}}noexcept"
+// CHECK-MULTILIB-X86: "-L[[RESOURCE_DIR]]{{/|\\\\}}x86_64-fuchsia{{/|\\\\}}lib"