Change how we deal with (implicit) -fno-rtti + -fsanitize=vptr

[filcab]

Summary

We were trying to get cc1 to disable the vptr sanitizer, but cc1 only
looks at -fsanitize=, not at -fno-sanitize=

I'm changing that behavior (which is only relied on by us, AFAICT) to:
If we're on an “implicit -fno-rtti” platform and the vptr sanitizer is
enabled, warn that -frtti is being turned on for this compilation, and
turn it on.

I also refactored common tests and added -check-prefix=(NO-)?RTTI to some
RUN lines.

[rsmith]

I don't think this is the right approach; the -fsanitize= setting deliberately doesn't affect whether RTTI is enabled, and I don't think we should change that. I think we should do the following:

• If -fsanitize=vptr is explicitly specified and RTTI is disabled, we should issue an error.
• If -fsanitize=vptr is implied by some sanitizer group (such as -fsanitize=undefined) and RTTI is disabled, we should not enable the vptr sanitizer (perhaps with a warning).

That presumably means handling this when we parse the sanitizer arguments, which might require us to reorder the processing of the arguments a bit.

[filcab]

In D7525#121009, @rsmith wrote:

I don't think this is the right approach; the -fsanitize= setting deliberately doesn't affect whether RTTI is enabled, and I don't think we should change that. I think we should do the following:

• If -fsanitize=vptr is explicitly specified and RTTI is disabled, we should issue an error.
• If -fsanitize=vptr is implied by some sanitizer group (such as -fsanitize=undefined) and RTTI is disabled, we should not enable the vptr sanitizer (perhaps with a warning).

That presumably means handling this when we parse the sanitizer arguments, which might require us to reorder the processing of the arguments a bit.

Thanks for the comment.

What would your preferred way of getting this to work, though?

• Have a (public) way to turn off specific sanitizers in SanitizerArgs, before the call to addArgs (and warn in the same place we had the warning)
• Have the constructor of SanitizerArgs (or addArgs) deal with it (doesn't seem very nice, especially since we'll have to repeat all this RTTI-related logic)
• Another way?

Thanks,

Filipe

[samsonov]

I agree with Richard here.

As for the implementation, I think we need to delete the code in Clang::ConstructJob and decide whether we should enable/disable -fsanitize=vptr when we parse sanitizer arguments. This means we should factor out the function, that would tell us, if RTTI is (a) enabled, (b) disabled explicitly by certain argument (e.g. -fno-rtti, -mkernel, ...), (c) disabled implicitly (e.g. if we're targeting PS4). Then this function can be used both in Clang::ConstructJob and in SanitizerArgs constructor. Probably it can be a method of ToolChain, though I'm not sure here.

[filcab]

This is getting a bit hard to reason about.

Here are the pieces:

• Default RTTI behavior for target (depends on other arguments like the type of input file)
• RTTI related options (explicit)
• Sanitizer options (explicit (vptr) or implicit (undefined))

SanitizerArgs _wants_ to hide details like “which sanitizers exist/are enabled”, which means it will have to sort out whether to warn/error or not by itself (it needs to know if there's an explicit RTTI-related argument) .

The RTTI-related code needs to know if C++ exceptions are on to decide if it implicitly turns on RTTI or not (this implies knowing not just the arguments, but also the input file).

To decide whether we need additional args for RTTI or not, and which ones to pass, we need to know the RTTI-related arguments passed and the input type (actually, we need IsCXX(Input)).

Easiest (kind of hacky) solution: Add enough arguments to getSanitizerArgs and to the SanitizerArgs constructor to allow it to decide whether to warn/error due to the vptr sanitizer (We'd parse the RTTI-related args, and use those results (enabled/disabledexplicitly/disabledimplicitly + an RTTI-related arg, if necessary) when calling getSanitizerArgs()).

Easy (kind of hacky) solution (with a major drawback): Make it easy to, via SanitizerArgs, add/remove sanitizers. It would be hard to know if the vptr sanitizer was enabled explicitly or via -fsanitize=undefined, with this solution (we'd have to re-parse everything, if we wanted to keep it working that way).

Non-hacky solutions: Sorry, couldn't think of one. Additional ones included stashing stuff in the ToolChain object… It doesn't seem like it's the proper thing to do.

[samsonov]

Note that currently types::IsCXX(InputType) is not used to determine
-fsanitize flag values at all - we just look at the explicit flags (-frtti,
-fno-rtti, -fapple-kext, -mkernel), and at the Triple (is it PS4?). Does it
make sense to leave it that way? There is a special case:
a) we're on PS4
b) we have a CXX input
c) we've provided -fexceptions, but haven't provided -frtti
In this case RTTI will still be implicitly enabled, but we would disable
vptr sanitizer earlier at this point.

I'd say it's fine to sacrifice this special case :) (for that matter, to
simplify things we could even make PS4 driver to issue a hard error if one
is building CXX code with explicit -fexceptions but no explicit -frtti).
That is, if we only look at Triple and input arguments, we can probably
have ToolChain::getRTTIMode() much like we have
ToolChain::getSanitizerArgs(), and use former in the latter.

Does it all make sense?

[filcab]

Changed it to retain current behavior (never enable rtti just because of a sanitizer), but fix the failing case (we were assuming -cc1 would care about -fno-sanitize=).

Refactored the code so the rtti/no-rtti decision was made in one place, and the other code that cares just asks the ToolChain.

[samsonov]

include/clang/Driver/ToolChain.h

25 ↗

(On Diff #19935)

Why do you need this?

58 ↗

(On Diff #19935)

This value is never used.

85 ↗

(On Diff #19935)

looks like this should be either const, or mutable and calculated lazily.

lib/Driver/SanitizerArgs.cpp

204 ↗

(On Diff #19935)

RTTI can be disabled by a different argument.

206 ↗

(On Diff #19935)

Please don't: we generally try to emit as many errors as possible. Also, this loop iterates over arguments in the reverse order: if we have
clang -fno-rtti -fsanitize=foobarbaz -fsanitize=vptr

It would be weird to fail reporting unrecognized "foobarbaz".

227 ↗

(On Diff #19935)

I'm inclined to silently disable -fsanitize=vptr if it was enabled implicitly by -fsanitize=undefined.

lib/Driver/ToolChain.cpp

33 ↗

(On Diff #19935)

This doesn't seem right. You will return RM_DisabledExplicitly for "clang -fno-rtti -frtti a.cc" (which probably means you need a test for that case).

42 ↗

(On Diff #19935)

You need to parse these arguments only if you're on PS4. Otherwise you can return RM_Enabled straight away.

lib/Driver/Tools.cpp

1975 ↗

(On Diff #19935)

Looks like you can factor out function, that would take "Args" and return if exceptions are enabled explicitly, and if yes, which argument enables them.

1991 ↗

(On Diff #19935)

Wouldn't RM_EnabledExplicitly help here?

4204 ↗

(On Diff #19935)

(optional) Now that you have ToolChain, you probably don't need KernelOrKext.

[filcab]

I'll be uploading a new patch soon.

include/clang/Driver/ToolChain.h

25 ↗

(On Diff #19935)

Left-over from when I was overcomplicating.

58 ↗

(On Diff #19935)

Removed.

85 ↗

(On Diff #19935)

It's const, calculated on object construction.

lib/Driver/SanitizerArgs.cpp

204 ↗

(On Diff #19935)

I'm now checking which of the three possible arguments was used, and use that one.

206 ↗

(On Diff #19935)

I took the return out. I also added the Vptr sanitizer to AllRemove, since we don't want to warn about it, afterwards.

227 ↗

(On Diff #19935)

I moved this warning to only show up if we have -fsanitize=vptr and rtti is disabled implicitly, since that is the case we really need a heads-up.
If we used -fsanitize=undefined and rtti is disabled, then we silently disable vptr.

lib/Driver/ToolChain.cpp

33 ↗

(On Diff #19935)

I'm accounting for -frtti, now, and added a testcase for that example.

42 ↗

(On Diff #19935)

Done.

lib/Driver/Tools.cpp

1975 ↗

(On Diff #19935)

In the current state, I'd rather not.
To get a 100% correct answer to that question, we need an InputFile, and need to know if it's C++. We don't have that in the ToolChain, AFAICT.

The getRTTIMode function doesn't do an exact match. It assumes we “want rtti if in any mode with -fexceptions”, which is good enough for its use-case, because we don't need to pass anything to enable rtti, and -cc1 will do “what's right” if we're compiling a C program, pass -fexceptions, and don't pass any rtti-related argument.

addExceptionArgs() actually has to know about the input file and its type, to know if it should pass -fcxx-exceptions to the program. Which means it can't have a false positive when we used -fexceptions for a C file.

1991 ↗

(On Diff #19935)

Yes, of course. Added.

4204 ↗

(On Diff #19935)

KernelOrKext is there only to avoid calling for Args.hasArg(...) so many times. It's not absolutely necessary, but having access to ToolChain doesn't make a difference, since addExceptionArgs already had access to Args and still got that boolean passed.

I would tend to keep it, but don't feel strongly. Will remove if you want.

[filcab]

Updated with Alexey's comments.

[samsonov]

Looks better now.

include/clang/Driver/ToolChain.h

84 ↗

(On Diff #19957)

please move this field upper, right after D/Triple/Args

lib/Driver/SanitizerArgs.cpp

201 ↗

(On Diff #19957)

I think you can remove static_cast<unsigned> here and below.

209 ↗

(On Diff #19957)

Do you think you can cache argument that is believed to explicitly disable/enable RTTI next to ToolChain::CachedRTTIMode? Then you will be able to use it here and in addExceptionArgs() diagnostics.

238 ↗

(On Diff #19957)

Comment is now misleading.

lib/Driver/ToolChain.cpp

32 ↗

(On Diff #19957)

you can use

if (Arg *RTTIArg = Args.getLastArg(...)) {
  ...
}

here

53 ↗

(On Diff #19957)

We generally don't use else-after-return:

if (Exceptions && ...)
  return ToolChain::RM_EnabledImplicitly;
return ToolChain::RM_DisabledImplicitly;

lib/Driver/Tools.cpp

1937 ↗

(On Diff #19957)

Extra / ?

1975 ↗

(On Diff #19957)

now that you re-use this variable, it deserves a better name. ExceptionArg?

1984 ↗

(On Diff #19957)

You can hide this declaration under

if (Triple.isPS4CPU()) {..}

1990 ↗

(On Diff #19957)

What if RTTI was disabled by a different argument?

4041 ↗

(On Diff #19957)

Does anyone uses sanitizesVptr() now? If not, we'd better delete this method.

test/Driver/fsanitize.c

34 ↗

(On Diff #19957)

Please also add a test for silent disabling of vptr sanitizer if we pass "-fsanitize=undefined -fno-rtti"

[filcab]

Addressed the latest comments from Alexey

[samsonov]

LGTM. Feel free to submit this after addressing comments below. Thank you for working on this!

include/clang/Driver/ToolChain.h

68 ↗

(On Diff #20232)

Looks like it should be

const llvm::opt::Arg *const CachedRTTIArg;

lib/Driver/SanitizerArgs.cpp

217 ↗

(On Diff #20232)

Remove static_cast.

236 ↗

(On Diff #20232)

We disable the vptr sanitizer if it was enabled by group expansion, but RTTI is disabled.

241 ↗

(On Diff #20232)

Remove this comment.

lib/Driver/Tools.cpp

1995 ↗

(On Diff #20232)

add an assert that RTTIMode is not DisabledImplicitly?

cfe/trunk/include/clang/Driver/SanitizerArgs.h

public:
   bool needsMsanRt() const { return Sanitizers.has(SanitizerKind::Memory); }
   bool needsLsanRt() const {
     return Sanitizers.has(SanitizerKind::Leak) &&
            !Sanitizers.has(SanitizerKind::Address);
   }
   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; }
   void addArgs(const llvm::opt::ArgList &Args,
                llvm::opt::ArgStringList &CmdArgs) const;
 
  private:
   void clear();
   bool getDefaultBlacklist(const Driver &D, std::string &BLPath);
 };
 
 }  // namespace driver
 }  // namespace clang
 
 #endif

cfe/trunk/include/clang/Driver/ToolChain.h

enum CXXStdlibType {
     CST_Libstdcxx
   };
 
   enum RuntimeLibType {
     RLT_CompilerRT,
     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.
   path_list FilePaths;
 
   /// The list of toolchain specific path prefixes to search for
   /// programs.
   path_list ProgramPaths;
public:
 
   path_list &getProgramPaths() { return ProgramPaths; }
   const path_list &getProgramPaths() const { return ProgramPaths; }
 
   const MultilibSet &getMultilibs() const { return Multilibs; }
 
   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
   /// translations this ToolChain may wish to perform, or 0 if no tool chain
   /// specific translations are needed.
   ///
   /// \param BoundArch - The bound architecture name, or 0.
   virtual llvm::opt::DerivedArgList *

cfe/trunk/lib/Driver/SanitizerArgs.cpp

SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   clear();
   unsigned AllRemove = 0;  // During the loop below, the accumulated set of
                            // sanitizers disabled by the current sanitizer
                            // argument or any argument after it.
   unsigned DiagnosedKinds = 0;  // All Kinds we have diagnosed up to now.
                                 // 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) {
     const auto *Arg = *I;
     if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       Arg->claim();
       unsigned Add = parseArgValues(D, Arg, true);
 
       // Avoid diagnosing any sanitizer which is disabled later.
       Add &= ~AllRemove;
       // At this point we have not expanded groups, so any unsupported
       // sanitizers in Add are those which have been explicitly enabled.
       // Diagnose them.
       if (unsigned KindsToDiagnose = Add & NotSupported & ~DiagnosedKinds) {
         // Only diagnose the new kinds.
         std::string Desc = describeSanitizeArg(*I, KindsToDiagnose);
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << Desc << TC.getTriple().str();
         DiagnosedKinds |= KindsToDiagnose;
       }
       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;
       // Silently discard any unsupported sanitizers implicitly enabled through
       // group expansion.
       Add &= ~NotSupported;
 
       Kinds |= Add;
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       Arg->claim();
       unsigned Remove = parseArgValues(D, Arg, true);
       AllRemove |= expandGroups(Remove);
     }
   }
   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;
   for (const auto *Arg : Args) {
     if (Arg->getOption().matches(options::OPT_fsanitize_recover)) {
       // FIXME: Add deprecation notice, and then remove this flag.
       RecoverableKinds |= expandGroups(LegacyFsanitizeRecoverMask);
       Arg->claim();

cfe/trunk/lib/Driver/ToolChain.cpp

 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 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() {
 }
 
 const Driver &ToolChain::getDriver() const {
  return D;
 }

cfe/trunk/lib/Driver/Tools.cpp

static bool exceptionSettings(const ArgList &Args, const llvm::Triple &Triple) {
   if (Arg *A = Args.getLastArg(options::OPT_fexceptions,
                                options::OPT_fno_exceptions))
     if (A->getOption().matches(options::OPT_fexceptions))
       return true;
 
   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.
     Args.ClaimAllArgs(options::OPT_fexceptions);
     Args.ClaimAllArgs(options::OPT_fno_exceptions);
     Args.ClaimAllArgs(options::OPT_fobjc_exceptions);
     Args.ClaimAllArgs(options::OPT_fno_objc_exceptions);
     Args.ClaimAllArgs(options::OPT_fcxx_exceptions);
if (types::isObjC(InputType) &&
     CmdArgs.push_back("-fobjc-exceptions");
 
     EH |= shouldUseExceptionTablesForObjCExceptions(objcRuntime, Triple);
   }
 
   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;
     }
   }
 
   if (EH)
     CmdArgs.push_back("-fexceptions");
if (Args.hasFlag(options::OPT_fno_access_control,
     CmdArgs.push_back("-fno-access-control");
 
   // -felide-constructors is the default.
   if (Args.hasFlag(options::OPT_fno_elide_constructors,
                    options::OPT_felide_constructors,
                    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);
-        }
-      }
-    }
+  ToolChain::RTTIMode RTTIMode = getToolChain().getRTTIMode();
 
-    // -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");
-      }
-    }
-
-    if (!RTTIEnabled)
-      CmdArgs.push_back("-fno-rtti");
-  }
+  if (RTTIMode == ToolChain::RM_DisabledExplicitly ||
+      RTTIMode == ToolChain::RM_DisabledImplicitly)
 
   // -fshort-enums=0 is default for all architectures except Hexagon.
   if (Args.hasFlag(options::OPT_fshort_enums,
                    options::OPT_fno_short_enums,
                    getToolChain().getArch() ==
                    llvm::Triple::hexagon))
     CmdArgs.push_back("-fshort-enums");
 
if (ARC) {
       // FIXME: We should move this to a hard error.
       D.Diag(diag::warn_drv_objc_gc_unsupported)
         << GCArg->getAsString(Args);
     }
   }
 
   // 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())
     CmdArgs.push_back("-fsjlj-exceptions");
 
   // C++ "sane" operator new.
   if (!Args.hasFlag(options::OPT_fassume_sane_operator_new,
                     options::OPT_fno_assume_sane_operator_new))

cfe/trunk/test/Driver/fsanitize.c

 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP-ON-ERROR-UNDEF
 // CHECK-UNDEFINED-TRAP-ON-ERROR-UNDEF: '-fsanitize=undefined' not allowed with '-fsanitize-undefined-trap-on-error'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=vptr -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP-ON-ERROR-VPTR
 // 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'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address,memory -pie -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANA-SANM
 // CHECK-SANA-SANM: '-fsanitize=address' not allowed with '-fsanitize=memory'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=thread,memory -pie -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANT-SANM
 // CHECK-SANT-SANM: '-fsanitize=thread' not allowed with '-fsanitize=memory'

cfe/trunk/test/Driver/rtti-options.cpp

 // Check that we emit warnings/errors for different combinations of
 // exceptions, rtti, and vptr sanitizer flags when targetting the PS4.
 // 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
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-EXC-WARN %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fcxx-exceptions -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // In C++, -fexceptions implies -fcxx-exceptions
 // RUN: %clang -x c++ -### -c -target x86_64-scei-ps4 -fexceptions -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-EXC-ERROR %s