[clang] Allow using -rtlib=platform to switching to the default rtlib on all targets

[mstorsjo]

Summary

Normally, passing -rtlib=platform overrides any earlier -rtlib
options, and overrides any hardcoded CLANG_DEFAULT_RTLIB option.
However, some targets, MSVC and Darwin, have custom logic for
disallowing specific -rtlib= option values; amend these checks for
allowing the -rtlib=platform option.

[thakis]

Do we have precedent for "platform" for this? For fuse-ld=, one is supposed to use -fuse-ld= (without anything after the =) to get the default ld. That's not great (...but it can't collide with actual linker names, i suppose).

Using "platform" (or any other self-descriptive name) for this seems easier to understand than passing an empty value. But it'd be nice if we could use this consistently in our various flags.

[mstorsjo]

In D132444#3742295, @thakis wrote:

Do we have precedent for "platform" for this? For fuse-ld=, one is supposed to use -fuse-ld= (without anything after the =) to get the default ld. That's not great (...but it can't collide with actual linker names, i suppose).

Using "platform" (or any other self-descriptive name) for this seems easier to understand than passing an empty value. But it'd be nice if we could use this consistently in our various flags.

Yes, "platform" is an existing option handled in a bunch of places already - see e.g. https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L832-L838. It's just that these cases hadn't been updated to take it into account.

[mstorsjo]

In D132444#3742305, @mstorsjo wrote:

In D132444#3742295, @thakis wrote:

Do we have precedent for "platform" for this? For fuse-ld=, one is supposed to use -fuse-ld= (without anything after the =) to get the default ld. That's not great (...but it can't collide with actual linker names, i suppose).

Using "platform" (or any other self-descriptive name) for this seems easier to understand than passing an empty value. But it'd be nice if we could use this consistently in our various flags.

Yes, "platform" is an existing option handled in a bunch of places already - see e.g. https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChain.cpp#L832-L838. It's just that these cases hadn't been updated to take it into account.

Then again, the quoted code comment says that "platform" only is meant to be used for overriding CLANG_DEFAULT_RTLIB in tests, but I don't see why one can't use it for overriding CLANG_DEFAULT_RTLIB (or an earlier -rtlib option on the command line) in real world uses too. (I had a user wanting to use my builds of clang for MSVC use cases, where it failed due to my Clang defaulting to -rtlib=compiler-rt, with no way of overriding it back to default, see https://github.com/mstorsjo/llvm-mingw/issues/267)

[thakis]

Makes sense to me. Maybe @hans has an opinion too.

WDYT about accepting the same string for -fuse-ld= to mean "platform linker" there as well?

[mstorsjo]

In D132444#3743020, @thakis wrote:

Makes sense to me. Maybe @hans has an opinion too.

WDYT about accepting the same string for -fuse-ld= to mean "platform linker" there as well?

Sounds reasonable to me - requiring the user to set an option to the empty string is kinda awkward.

[phosek]

LGTM

In D132444#3743051, @mstorsjo wrote:

In D132444#3743020, @thakis wrote:

Makes sense to me. Maybe @hans has an opinion too.

WDYT about accepting the same string for -fuse-ld= to mean "platform linker" there as well?

Sounds reasonable to me - requiring the user to set an option to the empty string is kinda awkward.

We currently support -fuse-ld= and -fuse-ld=ld, see https://github.com/llvm/llvm-project/blob/e29f9f7572d75c25cf06b080dce6bda9ebcf5008/clang/lib/Driver/ToolChain.cpp#L644. I think that -fuse-ld=platform would be less confusing (to distinguish from ld as the linker name) and it's also more consistent with other flags.

clang/lib/Driver/ToolChains/CommonArgs.cpp

case ToolChain::RLT_CompilerRT:
     CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
     AddUnwindLibrary(TC, D, CmdArgs, Args);
     break;
   case ToolChain::RLT_Libgcc:
     // Make sure libgcc is not used under MSVC environment by default
     if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
       // Issue error diagnostic if libgcc is explicitly specified
       // through command line as --rtlib option argument.
-      if (Args.hasArg(options::OPT_rtlib_EQ)) {
+      Arg *A = Args.getLastArg(options::OPT_rtlib_EQ);
+      if (A && A->getValue() != StringRef("platform")) {
         TC.getDriver().Diag(diag::err_drv_unsupported_rtlib_for_platform)
-            << Args.getLastArg(options::OPT_rtlib_EQ)->getValue() << "MSVC";
+            << A->getValue() << "MSVC";
       }
     } else
       AddLibgcc(TC, D, CmdArgs, Args);
     break;
   }
 
   // On Android, the unwinder uses dl_iterate_phdr (or one of
   // dl_unwind_find_exidx/__gnu_Unwind_Find_exidx on arm32) from libdl.so. For

clang/lib/Driver/ToolChains/Darwin.cpp

void DarwinClang::AddLinkSanitizerLibArgs(const ArgList &Args,
   auto RLO = RuntimeLinkOptions(RLO_AlwaysLink | (Shared ? RLO_AddRPath : 0U));
   AddLinkRuntimeLib(Args, CmdArgs, Sanitizer, RLO, Shared);
 }
 
 ToolChain::RuntimeLibType DarwinClang::GetRuntimeLibType(
     const ArgList &Args) const {
   if (Arg* A = Args.getLastArg(options::OPT_rtlib_EQ)) {
     StringRef Value = A->getValue();
-    if (Value != "compiler-rt")
+    if (Value != "compiler-rt" && Value != "platform")
       getDriver().Diag(clang::diag::err_drv_unsupported_rtlib_for_platform)
           << Value << "darwin";
   }
 
   return ToolChain::RLT_CompilerRT;
 }
 
 void DarwinClang::AddLinkRuntimeLibArgs(const ArgList &Args,

clang/test/Driver/msvc-compiler-rt.c

 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix MSVC-COMPILER-RT
+// RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt --rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix MSVC-DEFAULT
 // RUN: not %clang %s -target x86_64-pc-windows-msvc --rtlib=libgcc 2>&1 | FileCheck %s -check-prefix CHECK-ERROR
 
 // MSVC-COMPILER-RT: "{{.*}}clang_rt.builtins{{.*}}"
+// MSVC-DEFAULT-NOT: "{{.*}}clang_rt.builtins{{.*}}"
 // CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'MSVC'

clang/test/Driver/rtlib-darwin.c

+// RUN: %clang -target x86_64-apple-darwin -resource-dir=%S/Inputs/resource_dir --rtlib=compiler-rt -### %s 2>&1 | FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: %clang -target x86_64-apple-darwin -resource-dir=%S/Inputs/resource_dir --rtlib=platform -### %s 2>&1 | FileCheck %s -check-prefix DARWIN-COMPILER-RT
+// RUN: not %clang %s -target x86_64-apple-darwin --rtlib=libgcc 2>&1 | FileCheck %s -check-prefix CHECK-ERROR
+
+// DARWIN-COMPILER-RT: "{{.*}}clang_rt.osx{{.*}}"
+// CHECK-ERROR: unsupported runtime library 'libgcc' for platform 'darwin'