Improve our handling of rtti/sanitize=vptr/sanitize=undefined

This patch removes the huge blob of code that is dealing with
rtti/exceptions/sanitizers and replaces it with:

A ToolChain function which, for a given set of Args, figures out if rtti
should be:
  - enabled
  - disabled implicitly
  - disabled explicitly

A change in the way SanitizerArgs figures out what sanitizers to enable
(or if it should error out, or warn);

And a check for exceptions/rtti interaction inside addExceptionArgs.

The RTTIMode algorithm is:
  - If -mkernel, -fapple-kext, or -fno-rtti are passed, rtti was disabled explicitly;
  - If -frtti was passed or we're not targetting the PS4, rtti is enabled;
  - If -fexceptions or -fcxx-exceptions was passed and we're targetting
    the PS4, rtti was enabled implicitly;
  - If we're targetting the PS4, rtti is disabled implicitly;
  - Otherwise, rtti is enabled;

Since the only flag needed to pass to -cc1 is -fno-rtti if we want to
disable it, there's no problem in saying rtti is enabled if we're
compiling C code, so we don't look at the input file type.

addExceptionArgs now looks at the RTTIMode and warns that rtti is being
enabled implicitly if targetting the PS4 and exceptions are on. It also
errors out if, targetting the PS4, -fno-rtti was passed, and exceptions
were turned on.

SanitizerArgs now errors out if rtti was disabled explicitly and the vptr
sanitizer was enabled implicitly, but just turns off vptr if rtti is
disabled but -fsanitize=undefined was passed.

Also fixed tests, removed duplicate name from addExceptionArgs comment,
and added one or two surrounding lines when running clang-format.
This changes test/Driver/fsanitize.c to make it not expect a warning when
passed -fsanitize=undefined -fno-rtti, but expect vptr to not be on.

Removed all users and definition of SanitizerArgs::sanitizesVptr().

Reviewers: samsonov

Subscribers: llvm-commits, samsonov, rsmith

Differential Revision: http://reviews.llvm.org/D7525

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@229801 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Driver/SanitizerArgs.h b/include/clang/Driver/SanitizerArgs.h
index c51fae3e6696eb9e5c418797efbf0f8a06ab1fbf..c2b1b4d26cda8695111fa4b2102e08ffeb721c39 100644 (file)
--- a/include/clang/Driver/SanitizerArgs.h
+++ b/include/clang/Driver/SanitizerArgs.h
@@ -49,7 +49,6 @@ class SanitizerArgs {
   bool needsUbsanRt() const;
   bool needsDfsanRt() const { return Sanitizers.has(SanitizerKind::DataFlow); }
 
-  bool sanitizesVptr() const { return Sanitizers.has(SanitizerKind::Vptr); }
   bool requiresPIE() const;
   bool needsUnwindTables() const;
   bool linkCXXRuntimes() const { return LinkCXXRuntimes; }

diff --git a/include/clang/Driver/ToolChain.h b/include/clang/Driver/ToolChain.h
index 3092e81b95b5b6aff1089cd4afe4f7723d86efa2..560df19e25cea6bce09d5ac9c19cba5e2d3c243a 100644 (file)
--- a/include/clang/Driver/ToolChain.h
+++ b/include/clang/Driver/ToolChain.h
@@ -53,10 +53,20 @@ public:
     RLT_Libgcc
   };
 
+  enum RTTIMode {
+    RM_EnabledExplicitly,
+    RM_EnabledImplicitly,
+    RM_DisabledExplicitly,
+    RM_DisabledImplicitly
+  };
+
 private:
   const Driver &D;
   const llvm::Triple Triple;
   const llvm::opt::ArgList &Args;
+  // We need to initialize CachedRTTIArg before CachedRTTIMode
+  const llvm::opt::Arg *const CachedRTTIArg;
+  const RTTIMode CachedRTTIMode;
 
   /// The list of toolchain specific path prefixes to search for
   /// files.
@@ -134,6 +144,12 @@ public:
 
   const SanitizerArgs& getSanitizerArgs() const;
 
+  // Returns the Arg * that explicitly turned on/off rtti, or nullptr.
+  const llvm::opt::Arg *getRTTIArg() const { return CachedRTTIArg; }
+
+  // Returns the RTTIMode for the toolchain with the current arguments.
+  RTTIMode getRTTIMode() const { return CachedRTTIMode; }
+
   // Tool access.
 
   /// TranslateArgs - Create a new derived argument list for any argument

diff --git a/lib/Driver/SanitizerArgs.cpp b/lib/Driver/SanitizerArgs.cpp
index 310f5e81f951e5548257189d61e8ca363dd59ec7..7bec38309514ce49369cd2fd0b6f1731e0fbb225 100644 (file)
--- a/lib/Driver/SanitizerArgs.cpp
+++ b/lib/Driver/SanitizerArgs.cpp
@@ -171,6 +171,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
                                 // Used to deduplicate diagnostics.
   unsigned Kinds = 0;
   unsigned NotSupported = getToolchainUnsupportedKinds(TC);
+  ToolChain::RTTIMode RTTIMode = TC.getRTTIMode();
+
   const Driver &D = TC.getDriver();
   for (ArgList::const_reverse_iterator I = Args.rbegin(), E = Args.rend();
        I != E; ++I) {
@@ -193,6 +195,28 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       }
       Add &= ~NotSupported;
 
+      // Test for -fno-rtti + explicit -fsanitizer=vptr before expanding groups
+      // so we don't error out if -fno-rtti and -fsanitize=undefined were
+      // passed.
+      if (Add & SanitizeKind::Vptr &&
+          (RTTIMode == ToolChain::RM_DisabledImplicitly ||
+           RTTIMode == ToolChain::RM_DisabledExplicitly)) {
+        if (RTTIMode == ToolChain::RM_DisabledImplicitly)
+          // Warn about not having rtti enabled if the vptr sanitizer is
+          // explicitly enabled
+          D.Diag(diag::warn_drv_disabling_vptr_no_rtti_default);
+        else {
+          const llvm::opt::Arg *NoRTTIArg = TC.getRTTIArg();
+          assert(NoRTTIArg &&
+                 "RTTI disabled explicitly but we have no argument!");
+          D.Diag(diag::err_drv_argument_not_allowed_with)
+              << "-fsanitize=vptr" << NoRTTIArg->getAsString(Args);
+        }
+
+        // Take out the Vptr sanitizer from the enabled sanitizers
+        AllRemove |= SanitizeKind::Vptr;
+      }
+
       Add = expandGroups(Add);
       // Group expansion may have enabled a sanitizer which is disabled later.
       Add &= ~AllRemove;
@@ -209,6 +233,15 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   }
   addAllOf(Sanitizers, Kinds);
 
+  // We disable the vptr sanitizer if it was enabled by group expansion but RTTI
+  // is disabled.
+  if (Sanitizers.has(SanitizerKind::Vptr) &&
+      (RTTIMode == ToolChain::RM_DisabledImplicitly ||
+       RTTIMode == ToolChain::RM_DisabledExplicitly)) {
+    Kinds &= ~SanitizeKind::Vptr;
+    Sanitizers.set(SanitizerKind::Vptr, 0);
+  }
+
   // Parse -f(no-)?sanitize-recover flags.
   unsigned RecoverableKinds = RecoverableByDefault;
   unsigned DiagnosedUnrecoverableKinds = 0;

diff --git a/lib/Driver/ToolChain.cpp b/lib/Driver/ToolChain.cpp
index 2bcfecf6bde61664988e3e285883be53525db0b5..5feeda4d3cbdb457d9be0b80eaa95ad38ba05e9b 100644 (file)
--- a/lib/Driver/ToolChain.cpp
+++ b/lib/Driver/ToolChain.cpp
@@ -26,14 +26,47 @@ using namespace clang::driver;
 using namespace clang;
 using namespace llvm::opt;
 
+static llvm::opt::Arg *GetRTTIArgument(const ArgList &Args) {
+  return Args.getLastArg(options::OPT_mkernel, options::OPT_fapple_kext,
+                         options::OPT_fno_rtti, options::OPT_frtti);
+}
+
+static ToolChain::RTTIMode CalculateRTTIMode(const ArgList &Args,
+                                             const llvm::Triple &Triple,
+                                             const Arg *CachedRTTIArg) {
+  // Explicit rtti/no-rtti args
+  if (CachedRTTIArg) {
+    if (CachedRTTIArg->getOption().matches(options::OPT_frtti))
+      return ToolChain::RM_EnabledExplicitly;
+    else
+      return ToolChain::RM_DisabledExplicitly;
+  }
+
+  // -frtti is default, except for the PS4 CPU.
+  if (!Triple.isPS4CPU())
+    return ToolChain::RM_EnabledImplicitly;
+
+  // On the PS4, turning on c++ exceptions turns on rtti.
+  // We're assuming that, if we see -fexceptions, rtti gets turned on.
+  Arg *Exceptions = Args.getLastArg(
+      options::OPT_fcxx_exceptions, options::OPT_fno_cxx_exceptions,
+      options::OPT_fexceptions, options::OPT_fno_exceptions);
+  if (Exceptions &&
+      (Exceptions->getOption().matches(options::OPT_fexceptions) ||
+       Exceptions->getOption().matches(options::OPT_fcxx_exceptions)))
+    return ToolChain::RM_EnabledImplicitly;
+
+  return ToolChain::RM_DisabledImplicitly;
+}
+
 ToolChain::ToolChain(const Driver &D, const llvm::Triple &T,
                      const ArgList &Args)
-  : D(D), Triple(T), Args(Args) {
+    : D(D), Triple(T), Args(Args), CachedRTTIArg(GetRTTIArgument(Args)),
+      CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)) {
   if (Arg *A = Args.getLastArg(options::OPT_mthread_model))
     if (!isThreadModelSupported(A->getValue()))
       D.Diag(diag::err_drv_invalid_thread_model_for_target)
-          << A->getValue()
-          << A->getAsString(Args);
+          << A->getValue() << A->getAsString(Args);
 }
 
 ToolChain::~ToolChain() {

diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
index 6be4f51bace703b15516d551bdcf10c76bfb9183..f47a92b7c39bbc554837e4fddae32893e57545fb 100644 (file)
--- a/lib/Driver/Tools.cpp
+++ b/lib/Driver/Tools.cpp
@@ -1931,16 +1931,17 @@ static bool exceptionSettings(const ArgList &Args, const llvm::Triple &Triple) {
   return false;
 }
 
-/// addExceptionArgs - Adds exception related arguments to the driver command
-/// arguments. There's a master flag, -fexceptions and also language specific
-/// flags to enable/disable C++ and Objective-C exceptions.
-/// This makes it possible to for example disable C++ exceptions but enable
-/// Objective-C exceptions.
+/// Adds exception related arguments to the driver command arguments. There's a
+/// master flag, -fexceptions and also language specific flags to enable/disable
+/// C++ and Objective-C exceptions. This makes it possible to for example
+/// disable C++ exceptions but enable Objective-C exceptions.
 static void addExceptionArgs(const ArgList &Args, types::ID InputType,
-                             const llvm::Triple &Triple,
-                             bool KernelOrKext,
+                             const ToolChain &TC, bool KernelOrKext,
                              const ObjCRuntime &objcRuntime,
                              ArgStringList &CmdArgs) {
+  const Driver &D = TC.getDriver();
+  const llvm::Triple &Triple = TC.getTriple();
+
   if (KernelOrKext) {
     // -mkernel and -fapple-kext imply no exceptions, so claim exception related
     // arguments now to avoid warnings about unused arguments.
@@ -1970,15 +1971,30 @@ static void addExceptionArgs(const ArgList &Args, types::ID InputType,
   if (types::isCXX(InputType)) {
     bool CXXExceptionsEnabled =
         Triple.getArch() != llvm::Triple::xcore && !Triple.isPS4CPU();
-    if (Arg *A = Args.getLastArg(options::OPT_fcxx_exceptions,
-                                 options::OPT_fno_cxx_exceptions,
-                                 options::OPT_fexceptions,
-                                 options::OPT_fno_exceptions))
+    Arg *ExceptionArg = Args.getLastArg(
+        options::OPT_fcxx_exceptions, options::OPT_fno_cxx_exceptions,
+        options::OPT_fexceptions, options::OPT_fno_exceptions);
+    if (ExceptionArg)
       CXXExceptionsEnabled =
-          A->getOption().matches(options::OPT_fcxx_exceptions) ||
-          A->getOption().matches(options::OPT_fexceptions);
+          ExceptionArg->getOption().matches(options::OPT_fcxx_exceptions) ||
+          ExceptionArg->getOption().matches(options::OPT_fexceptions);
 
     if (CXXExceptionsEnabled) {
+      if (Triple.isPS4CPU()) {
+        ToolChain::RTTIMode RTTIMode = TC.getRTTIMode();
+        assert(ExceptionArg &&
+               "On the PS4 exceptions should only be enabled if passing "
+               "an argument");
+        if (RTTIMode == ToolChain::RM_DisabledExplicitly) {
+          const Arg *RTTIArg = TC.getRTTIArg();
+          assert(RTTIArg && "RTTI disabled explicitly but no RTTIArg!");
+          D.Diag(diag::err_drv_argument_not_allowed_with)
+              << RTTIArg->getAsString(Args) << ExceptionArg->getAsString(Args);
+        } else if (RTTIMode == ToolChain::RM_EnabledImplicitly)
+          D.Diag(diag::warn_drv_enabling_rtti_with_exceptions);
+      } else
+        assert(TC.getRTTIMode() != ToolChain::RM_DisabledImplicitly);
+
       CmdArgs.push_back("-fcxx-exceptions");
 
       EH = true;
@@ -4004,56 +4020,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                    false))
     CmdArgs.push_back("-fno-elide-constructors");
 
-  // -frtti is default, except for the PS4 CPU.
-  if (!Args.hasFlag(options::OPT_frtti, options::OPT_fno_rtti,
-                    !Triple.isPS4CPU()) ||
-      KernelOrKext) {
-    bool IsCXX = types::isCXX(InputType);
-    bool RTTIEnabled = false;
-    Arg *NoRTTIArg = Args.getLastArg(
-        options::OPT_mkernel, options::OPT_fapple_kext, options::OPT_fno_rtti);
-
-    // PS4 requires rtti when exceptions are enabled. If -fno-rtti was
-    // explicitly passed, error out. Otherwise enable rtti and emit a
-    // warning.
-    Arg *Exceptions = Args.getLastArg(
-        options::OPT_fcxx_exceptions, options::OPT_fno_cxx_exceptions,
-        options::OPT_fexceptions, options::OPT_fno_exceptions);
-    if (Triple.isPS4CPU() && Exceptions) {
-      bool CXXExceptions =
-          (IsCXX &&
-           Exceptions->getOption().matches(options::OPT_fexceptions)) ||
-          Exceptions->getOption().matches(options::OPT_fcxx_exceptions);
-      if (CXXExceptions) {
-        if (NoRTTIArg)
-          D.Diag(diag::err_drv_argument_not_allowed_with)
-              << NoRTTIArg->getAsString(Args) << Exceptions->getAsString(Args);
-        else {
-          RTTIEnabled = true;
-          D.Diag(diag::warn_drv_enabling_rtti_with_exceptions);
-        }
-      }
-    }
-
-    // -fno-rtti cannot usefully be combined with -fsanitize=vptr.
-    if (Sanitize.sanitizesVptr()) {
-      // If rtti was explicitly disabled and the vptr sanitizer is on, error
-      // out. Otherwise, warn that vptr will be disabled unless -frtti is
-      // passed.
-      if (NoRTTIArg) {
-        D.Diag(diag::err_drv_argument_not_allowed_with)
-            << "-fsanitize=vptr" << NoRTTIArg->getAsString(Args);
-      } else {
-        D.Diag(diag::warn_drv_disabling_vptr_no_rtti_default);
-        // All sanitizer switches have been pushed. This -fno-sanitize
-        // will override any -fsanitize={vptr,undefined} passed before it.
-        CmdArgs.push_back("-fno-sanitize=vptr");
-      }
-    }
+  ToolChain::RTTIMode RTTIMode = getToolChain().getRTTIMode();
 
-    if (!RTTIEnabled)
-      CmdArgs.push_back("-fno-rtti");
-  }
+  if (RTTIMode == ToolChain::RM_DisabledExplicitly ||
+      RTTIMode == ToolChain::RM_DisabledImplicitly)
+    CmdArgs.push_back("-fno-rtti");
 
   // -fshort-enums=0 is default for all architectures except Hexagon.
   if (Args.hasFlag(options::OPT_fshort_enums,
@@ -4231,7 +4202,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   // Handle GCC-style exception args.
   if (!C.getDriver().IsCLMode())
-    addExceptionArgs(Args, InputType, getToolChain().getTriple(), KernelOrKext,
+    addExceptionArgs(Args, InputType, getToolChain(), KernelOrKext,
                      objcRuntime, CmdArgs);
 
   if (getToolChain().UseSjLjExceptions())

diff --git a/test/Driver/fsanitize.c b/test/Driver/fsanitize.c
index a5eea446149f32e041cd8beaf759e22fc8ca5208..87ac07878eb1070fc0fc407a1a0fba3a0d8afc98 100644 (file)
--- a/test/Driver/fsanitize.c
+++ b/test/Driver/fsanitize.c
@@ -31,9 +31,11 @@
 // CHECK-UNDEFINED-TRAP-ON-ERROR-VPTR: '-fsanitize=vptr' not allowed with '-fsanitize-undefined-trap-on-error'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=vptr -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-NO-RTTI
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-NO-RTTI
 // CHECK-VPTR-NO-RTTI: '-fsanitize=vptr' not allowed with '-fno-rtti'
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-NO-RTTI
+// CHECK-UNDEFINED-NO-RTTI-NOT: vptr
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address,thread -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANA-SANT
 // CHECK-SANA-SANT: '-fsanitize=address' not allowed with '-fsanitize=thread'
 

diff --git a/test/Driver/rtti-options.cpp b/test/Driver/rtti-options.cpp
index 46a5aaa7acdf724439bc11406b0f8cfa1bcb2502..a14953ebfbd5843747c17f8aed3addbafd7d6504 100644 (file)
--- a/test/Driver/rtti-options.cpp
+++ b/test/Driver/rtti-options.cpp
@@ -3,19 +3,23 @@
 // No warnings/errors should be emitted for unknown, except if combining
 // the vptr sanitizer with -fno-rtti
 
+// RUN: %clang -### -c -fno-rtti -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-RTTI %s
+// RUN: %clang -### -c -frtti -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RTTI %s
+
 // -fsanitize=vptr
+// Make sure we only error/warn once, when trying to enable vptr and
+// undefined and have -fno-rtti
+// RUN: %clang -### -c -fsanitize=undefined -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR -check-prefix=CHECK-OK %s
+
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-WARN %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s
-// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=undefined %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-WARN %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=undefined -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
-// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=undefined -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=vptr %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=undefined %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=undefined -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
-// RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=undefined -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s
 
 // Exceptions + no/default rtti
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fcxx-exceptions -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-EXC-ERROR-CXX %s