From: Filipe Cabecinhas Date: Thu, 19 Feb 2015 01:04:49 +0000 (+0000) Subject: Improve our handling of rtti/sanitize=vptr/sanitize=undefined X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bdc1288ac803c9d9eb73d28aee0716c60337fb65;p=clang 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 c51fae3e66..c2b1b4d26c 100644 --- 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 3092e81b95..560df19e25 100644 --- 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 310f5e81f9..7bec383095 100644 --- 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 2bcfecf6bd..5feeda4d3c 100644 --- 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 6be4f51bac..f47a92b7c3 100644 --- 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 a5eea44614..87ac07878e 100644 --- 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 46a5aaa7ac..a14953ebfb 100644 --- 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