[PATCH] D24091: [Driver] Warn on -nodefaultlibs and -fsanitize

[Justin Bogner via cfe-commits]

Chris Bieneman <beanz at apple.com> writes:
> beanz created this revision.
> beanz added reviewers: bogner, zaks.anna, bruno, filcab.
> beanz added a subscriber: cfe-commits.
> Herald added a subscriber: emaste.
>
> The FreeBSD and GNUTools drivers support -fsanitize arguments
> bypassing -nodefaultlibs. With https://reviews.llvm.org/D24048, Darwin
> will support that behavior as well.
>
> To make this a little less magical and behind the curtain this warning
> will fire when -nodefaultlibs is used with sanitizer arguments.

I don't think this makes any sense. If it's a legitimate use case to
pass these flags together, then anyone who does will have to manually
disable this warning - if it's invalid to pass the flags together, we
should just disallow it instead of warning. The warning either hurts
usability or doesn't do enough.

> https://reviews.llvm.org/D24091
>
> Files:
>   include/clang/Basic/DiagnosticDriverKinds.td
>   lib/Driver/Driver.cpp
>   test/Driver/nodefaultlib.c
>
> Index: test/Driver/nodefaultlib.c
> ===================================================================
> --- test/Driver/nodefaultlib.c
> +++ test/Driver/nodefaultlib.c
> @@ -8,3 +8,7 @@
>  // RUN: %clang -target i686-pc-linux-gnu -stdlib=libc++ -nodefaultlibs -lstdc++ -### %s 2>&1 | FileCheck -check-prefix=TEST2 %s
>  // TEST2-NOT: "-lc++"
>  // TEST2: "-lstdc++"
> +
> +// RUN: %clang -target i686-pc-linux-gnu -nodefaultlibs -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=TEST3 %s
> +// TEST3: warning: Passing -nodefaultlibs and -fsanitize may result in linking sanitizer runtimes.
> +// TEST3: libclang_rt.asan-i686.a
> Index: lib/Driver/Driver.cpp
> ===================================================================
> --- lib/Driver/Driver.cpp
> +++ lib/Driver/Driver.cpp
> @@ -1546,8 +1546,12 @@
>    Arg *FinalPhaseArg;
>    phases::ID FinalPhase = getFinalPhase(Args, &FinalPhaseArg);
>  
> -  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
> -    Diag(clang::diag::err_drv_emit_llvm_link);
> +  if (FinalPhase == phases::Link) {
> +    if(Args.hasArg(options::OPT_emit_llvm))
> +      Diag(clang::diag::err_drv_emit_llvm_link);
> +    if (Args.hasArg(options::OPT_nodefaultlibs) &&
> +        Args.hasArg(options::OPT_fsanitize_EQ))
> +      Diag(clang::diag::warn_drv_sanitizers_and_nodefaultlibs);
>    }
>  
>    // Reject -Z* at the top level, these options should never have been exposed
> Index: include/clang/Basic/DiagnosticDriverKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticDriverKinds.td
> +++ include/clang/Basic/DiagnosticDriverKinds.td
> @@ -271,4 +271,7 @@
>    InGroup<InvalidOrNonExistentDirectory>;
>  
>  def err_drv_unsupported_linker : Error<"unsupported value '%0' for -linker option">;
> +
> +def warn_drv_sanitizers_and_nodefaultlibs : Warning<
> +  "Passing -nodefaultlibs and -fsanitize may result in linking sanitizer runtimes.">;
>  }
>
>
> Index: test/Driver/nodefaultlib.c
> ===================================================================
> --- test/Driver/nodefaultlib.c
> +++ test/Driver/nodefaultlib.c
> @@ -8,3 +8,7 @@
>  // RUN: %clang -target i686-pc-linux-gnu -stdlib=libc++ -nodefaultlibs -lstdc++ -### %s 2>&1 | FileCheck -check-prefix=TEST2 %s
>  // TEST2-NOT: "-lc++"
>  // TEST2: "-lstdc++"
> +
> +// RUN: %clang -target i686-pc-linux-gnu -nodefaultlibs -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=TEST3 %s
> +// TEST3: warning: Passing -nodefaultlibs and -fsanitize may result in linking sanitizer runtimes.
> +// TEST3: libclang_rt.asan-i686.a
> Index: lib/Driver/Driver.cpp
> ===================================================================
> --- lib/Driver/Driver.cpp
> +++ lib/Driver/Driver.cpp
> @@ -1546,8 +1546,12 @@
>    Arg *FinalPhaseArg;
>    phases::ID FinalPhase = getFinalPhase(Args, &FinalPhaseArg);
>  
> -  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
> -    Diag(clang::diag::err_drv_emit_llvm_link);
> +  if (FinalPhase == phases::Link) {
> +    if(Args.hasArg(options::OPT_emit_llvm))
> +      Diag(clang::diag::err_drv_emit_llvm_link);
> +    if (Args.hasArg(options::OPT_nodefaultlibs) &&
> +        Args.hasArg(options::OPT_fsanitize_EQ))
> +      Diag(clang::diag::warn_drv_sanitizers_and_nodefaultlibs);
>    }
>  
>    // Reject -Z* at the top level, these options should never have been exposed
> Index: include/clang/Basic/DiagnosticDriverKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticDriverKinds.td
> +++ include/clang/Basic/DiagnosticDriverKinds.td
> @@ -271,4 +271,7 @@
>    InGroup<InvalidOrNonExistentDirectory>;
>  
>  def err_drv_unsupported_linker : Error<"unsupported value '%0' for -linker option">;
> +
> +def warn_drv_sanitizers_and_nodefaultlibs : Warning<
> +  "Passing -nodefaultlibs and -fsanitize may result in linking sanitizer runtimes.">;
>  }