[PATCH] Don't run diagnostic for sanitizer arguments several times

Alexey Samsonov samsonov at google.com
Tue Aug 6 06:47:22 PDT 2013


Hi rsmith,

I've noticed that clang driver may print several copies
of the same warning/error message relevant to sanitizer arguments,
as they are parsed multiple times. This patch removes one copy
of these diagnostics. However, we still have two copies of error messages
if we combine compilation and linking in one invocation (as sanitizer
flags are relevant for both jobs). I wonder if we can and should actually
do something here.

Also, is there a reason for writing a bunch of SanitizerArgs code in the header
instead of .cpp file?

http://llvm-reviews.chandlerc.com/D1300

Files:
  test/Driver/fsanitize.c
  lib/Driver/SanitizerArgs.h
  lib/Driver/Tools.cpp
  lib/Driver/ToolChains.cpp

Index: test/Driver/fsanitize.c
===================================================================
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -142,3 +142,7 @@
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN
 // CHECK-MSAN: "-fno-assume-sane-operator-new"
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=zzz -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DIAG1
+// CHECK-DIAG1: unsupported argument 'zzz' to option 'fsanitize='
+// CHECK-DIAG1-NOT: unsupported argument 'zzz' to option 'fsanitize='
Index: lib/Driver/SanitizerArgs.h
===================================================================
--- lib/Driver/SanitizerArgs.h
+++ lib/Driver/SanitizerArgs.h
@@ -52,7 +52,8 @@
   SanitizerArgs() : Kind(0), BlacklistFile(""), MsanTrackOrigins(false),
                     AsanZeroBaseShadow(false), UbsanTrapOnError(false) {}
   /// Parses the sanitizer arguments from an argument list.
-  SanitizerArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
+  SanitizerArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
+                bool DiagnoseErrors = true);
 
   bool needsAsanRt() const { return Kind & NeedsAsanRt; }
   bool needsTsanRt() const { return Kind & NeedsTsanRt; }
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -1616,15 +1616,16 @@
     }
 }
 
-SanitizerArgs::SanitizerArgs(const ToolChain &TC, const ArgList &Args)
+SanitizerArgs::SanitizerArgs(const ToolChain &TC, const ArgList &Args,
+                             bool DiagnoseErrors)
     : Kind(0), BlacklistFile(""), MsanTrackOrigins(false),
       AsanZeroBaseShadow(false) {
   unsigned AllKinds = 0;  // All kinds of sanitizers that were turned on
                           // at least once (possibly, disabled further).
   const Driver &D = TC.getDriver();
   for (ArgList::const_iterator I = Args.begin(), E = Args.end(); I != E; ++I) {
     unsigned Add, Remove;
-    if (!parse(D, Args, *I, Add, Remove, true))
+    if (!parse(D, Args, *I, Add, Remove, DiagnoseErrors))
       continue;
     (*I)->claim();
     Kind |= Add;
@@ -1637,16 +1638,17 @@
     Args.hasFlag(options::OPT_fsanitize_undefined_trap_on_error,
                  options::OPT_fno_sanitize_undefined_trap_on_error, false);
 
-  if (Args.hasArg(options::OPT_fcatch_undefined_behavior) &&
+  if (DiagnoseErrors &&
+      Args.hasArg(options::OPT_fcatch_undefined_behavior) &&
       !Args.hasFlag(options::OPT_fsanitize_undefined_trap_on_error,
                     options::OPT_fno_sanitize_undefined_trap_on_error, true)) {
     D.Diag(diag::err_drv_argument_not_allowed_with)
       << "-fcatch-undefined-behavior"
       << "-fno-sanitize-undefined-trap-on-error";
   }
 
   // Warn about undefined sanitizer options that require runtime support.
-  if (UbsanTrapOnError && notAllowedWithTrap()) {
+  if (DiagnoseErrors && UbsanTrapOnError && notAllowedWithTrap()) {
     if (Args.hasArg(options::OPT_fcatch_undefined_behavior))
       D.Diag(diag::err_drv_argument_not_allowed_with)
         << lastArgumentForKind(D, Args, NotAllowedWithTrap)
@@ -1664,35 +1666,37 @@
   bool NeedsTsan = needsTsanRt();
   bool NeedsMsan = needsMsanRt();
   bool NeedsLsan = needsLeakDetection();
-  if (NeedsAsan && NeedsTsan)
-    D.Diag(diag::err_drv_argument_not_allowed_with)
-      << lastArgumentForKind(D, Args, NeedsAsanRt)
-      << lastArgumentForKind(D, Args, NeedsTsanRt);
-  if (NeedsAsan && NeedsMsan)
-    D.Diag(diag::err_drv_argument_not_allowed_with)
-      << lastArgumentForKind(D, Args, NeedsAsanRt)
-      << lastArgumentForKind(D, Args, NeedsMsanRt);
-  if (NeedsTsan && NeedsMsan)
-    D.Diag(diag::err_drv_argument_not_allowed_with)
-      << lastArgumentForKind(D, Args, NeedsTsanRt)
-      << lastArgumentForKind(D, Args, NeedsMsanRt);
-  if (NeedsLsan && NeedsTsan)
-    D.Diag(diag::err_drv_argument_not_allowed_with)
-      << lastArgumentForKind(D, Args, NeedsLeakDetection)
-      << lastArgumentForKind(D, Args, NeedsTsanRt);
-  if (NeedsLsan && NeedsMsan)
-    D.Diag(diag::err_drv_argument_not_allowed_with)
-      << lastArgumentForKind(D, Args, NeedsLeakDetection)
-      << lastArgumentForKind(D, Args, NeedsMsanRt);
+  if (DiagnoseErrors) {
+    if (NeedsAsan && NeedsTsan)
+      D.Diag(diag::err_drv_argument_not_allowed_with)
+        << lastArgumentForKind(D, Args, NeedsAsanRt)
+        << lastArgumentForKind(D, Args, NeedsTsanRt);
+    if (NeedsAsan && NeedsMsan)
+      D.Diag(diag::err_drv_argument_not_allowed_with)
+        << lastArgumentForKind(D, Args, NeedsAsanRt)
+        << lastArgumentForKind(D, Args, NeedsMsanRt);
+    if (NeedsTsan && NeedsMsan)
+      D.Diag(diag::err_drv_argument_not_allowed_with)
+        << lastArgumentForKind(D, Args, NeedsTsanRt)
+        << lastArgumentForKind(D, Args, NeedsMsanRt);
+    if (NeedsLsan && NeedsTsan)
+      D.Diag(diag::err_drv_argument_not_allowed_with)
+        << lastArgumentForKind(D, Args, NeedsLeakDetection)
+        << lastArgumentForKind(D, Args, NeedsTsanRt);
+    if (NeedsLsan && NeedsMsan)
+      D.Diag(diag::err_drv_argument_not_allowed_with)
+        << lastArgumentForKind(D, Args, NeedsLeakDetection)
+        << lastArgumentForKind(D, Args, NeedsMsanRt);
+  }
   // FIXME: Currenly -fsanitize=leak is silently ignored in the presence of
   // -fsanitize=address. Perhaps it should print an error, or perhaps
   // -f(-no)sanitize=leak should change whether leak detection is enabled by
   // default in ASan?
 
   // If -fsanitize contains extra features of ASan, it should also
   // explicitly contain -fsanitize=address (probably, turned off later in the
   // command line).
-  if ((Kind & AddressFull) != 0 && (AllKinds & Address) == 0)
+  if (DiagnoseErrors && (Kind & AddressFull) != 0 && (AllKinds & Address) == 0)
     D.Diag(diag::warn_drv_unused_sanitizer)
      << lastArgumentForKind(D, Args, AddressFull)
      << "-fsanitize=address";
@@ -1704,7 +1708,7 @@
       std::string BLPath = BLArg->getValue();
       if (llvm::sys::fs::exists(BLPath))
         BlacklistFile = BLPath;
-      else
+      else if (DiagnoseErrors)
         D.Diag(diag::err_drv_no_such_file) << BLPath;
     }
   } else {
@@ -1732,7 +1736,7 @@
                      options::OPT_fno_sanitize_address_zero_base_shadow,
                      ZeroBaseShadowDefault);
     // Zero-base shadow is a requirement on Android.
-    if (IsAndroid && !AsanZeroBaseShadow) {
+    if (DiagnoseErrors && IsAndroid && !AsanZeroBaseShadow) {
       D.Diag(diag::err_drv_argument_not_allowed_with)
           << "-fno-sanitize-address-zero-base-shadow"
           << lastArgumentForKind(D, Args, Address);
Index: lib/Driver/ToolChains.cpp
===================================================================
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -2354,7 +2354,7 @@
   addPathIfExists(SysRoot + "/lib", Paths);
   addPathIfExists(SysRoot + "/usr/lib", Paths);
 
-  IsPIEDefault = SanitizerArgs(*this, Args).hasZeroBaseShadow();
+  IsPIEDefault = SanitizerArgs(*this, Args, false).hasZeroBaseShadow();
 }
 
 bool Linux::HasNativeLLVMSupport() const {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1300.1.patch
Type: text/x-patch
Size: 7220 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130806/bf5b5996/attachment.bin>


More information about the cfe-commits mailing list