[PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

[Justin Bogner via cfe-commits]

Chris Bieneman <beanz at apple.com> writes:
> beanz created this revision.
> beanz added reviewers: zaks.anna, kubabrecka, bogner.
> beanz added a subscriber: cfe-commits.
>
> The -nodefaultlibs and -nostdlib flags suppress all the runtime
> libraries that the driver puts on the link line. This feels wrong. If
> a user specifies "-fsanitize=<blah>" I think it is expected that the
> sanitizer library would be included on the link line.

In terms of user expectations, either (a) you pass -nodefaultlibs and
expect -fsanitize to still add the (non-default) libs it needs, or (b)
you pass -fsanitize but want to disable all of the libs with
-nodefaultlibs.

This patch implements (a), and I think that makes sense, since it means
that "clang -nodefaultlibs -fsanitize=... foo.c" is sufficient to build
a standalone thing that's still sanitized. The problem with (b) is that
a command like that would have to be invalid - you're required to also
link a particular sanitizer lib at that point, so you'd constantly need
to say "no default libs, except for the ones I need for this to even
compile".

In any case, what do similar flags that need to pull in runtimes to work
do, like PGO/coverage instrumentation? Whatever we end up doing should
be consistent across all of these.

>
> https://reviews.llvm.org/D24048
>
> Files:
>   lib/Driver/ToolChains.cpp
>   lib/Driver/ToolChains.h
>   lib/Driver/Tools.cpp
>
> Index: lib/Driver/Tools.cpp
> ===================================================================
> --- lib/Driver/Tools.cpp
> +++ lib/Driver/Tools.cpp
> @@ -8032,6 +8032,8 @@
>  
>      // Let the tool chain choose which runtime library to link.
>      getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs);
> +  } else {
> +    getMachOToolChain().AddLinkSanitizerLibArgs(Args, CmdArgs);
>    }
>  
>    if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
> Index: lib/Driver/ToolChains.h
> ===================================================================
> --- lib/Driver/ToolChains.h
> +++ lib/Driver/ToolChains.h
> @@ -282,6 +282,10 @@
>    virtual void AddLinkRuntimeLibArgs(const llvm::opt::ArgList &Args,
>                                       llvm::opt::ArgStringList &CmdArgs) const;
>  
> +  virtual void
> +  AddLinkSanitizerLibArgs(const llvm::opt::ArgList &Args,
> +                          llvm::opt::ArgStringList &CmdArgs) const {}
> +
>    virtual void addStartObjectFileArgs(const llvm::opt::ArgList &Args,
>                                        llvm::opt::ArgStringList &CmdArgs) const {
>    }
> @@ -571,6 +575,10 @@
>  
>    RuntimeLibType GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
>  
> +  void
> +  AddLinkSanitizerLibArgs(const llvm::opt::ArgList &Args,
> +                          llvm::opt::ArgStringList &CmdArgs) const override;
> +
>    void AddLinkRuntimeLibArgs(const llvm::opt::ArgList &Args,
>                               llvm::opt::ArgStringList &CmdArgs) const override;
>  
> Index: lib/Driver/ToolChains.cpp
> ===================================================================
> --- lib/Driver/ToolChains.cpp
> +++ lib/Driver/ToolChains.cpp
> @@ -425,22 +425,7 @@
>      return;
>    }
>  
> -  const SanitizerArgs &Sanitize = getSanitizerArgs();
> -  if (Sanitize.needsAsanRt())
> -    AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
> -  if (Sanitize.needsUbsanRt())
> -    AddLinkSanitizerLibArgs(Args, CmdArgs, "ubsan");
> -  if (Sanitize.needsTsanRt())
> -    AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
> -  if (Sanitize.needsStatsRt()) {
> -    StringRef OS = isTargetMacOS() ? "osx" : "iossim";
> -    AddLinkRuntimeLib(Args, CmdArgs,
> -                      (Twine("libclang_rt.stats_client_") + OS + ".a").str(),
> -                      /*AlwaysLink=*/true);
> -    AddLinkSanitizerLibArgs(Args, CmdArgs, "stats");
> -  }
> -  if (Sanitize.needsEsanRt())
> -    AddLinkSanitizerLibArgs(Args, CmdArgs, "esan");
> +  AddLinkSanitizerLibArgs(Args, CmdArgs);
>  
>    // Otherwise link libSystem, then the dynamic runtime library, and finally any
>    // target specific static runtime library.
> @@ -495,6 +480,26 @@
>    }
>  }
>  
> +void DarwinClang::AddLinkSanitizerLibArgs(
> +    const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CmdArgs) const {
> +  const SanitizerArgs &Sanitize = getSanitizerArgs();
> +  if (Sanitize.needsAsanRt())
> +    AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
> +  if (Sanitize.needsUbsanRt())
> +    AddLinkSanitizerLibArgs(Args, CmdArgs, "ubsan");
> +  if (Sanitize.needsTsanRt())
> +    AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
> +  if (Sanitize.needsStatsRt()) {
> +    StringRef OS = isTargetMacOS() ? "osx" : "iossim";
> +    AddLinkRuntimeLib(Args, CmdArgs,
> +                      (Twine("libclang_rt.stats_client_") + OS + ".a").str(),
> +                      /*AlwaysLink=*/true);
> +    AddLinkSanitizerLibArgs(Args, CmdArgs, "stats");
> +  }
> +  if (Sanitize.needsEsanRt())
> +    AddLinkSanitizerLibArgs(Args, CmdArgs, "esan");
> +}
> +
>  void Darwin::AddDeploymentTarget(DerivedArgList &Args) const {
>    const OptTable &Opts = getDriver().getOpts();
>