From 907880edd917ea32524c7cc91b03495b6beb2ba3 Mon Sep 17 00:00:00 2001 From: Alexey Samsonov Date: Fri, 19 Jun 2015 19:57:46 +0000 Subject: [PATCH] [CFI] Require -flto instead of implying it. Summary: This is unfortunate, but would let us land http://reviews.llvm.org/D10467, that makes ToolChains responsible for computing the set of sanitizers they support. Unfortunately, Darwin ToolChains doesn't know about actual OS they target until ToolChain::TranslateArgs() is called. In particular, it means we won't be able to construct SanitizerArgs for these ToolChains before that. This change removes SanitizerArgs::needsLTO() method, so that now ToolChain::IsUsingLTO(), which is called very early, doesn't need SanitizerArgs to implement this method. Docs and test cases are updated accordingly. See https://llvm.org/bugs/show_bug.cgi?id=23539, which describes why we start all these. Test Plan: regression test suite Reviewers: pcc Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D10560 llvm-svn: 240170 --- clang/docs/ControlFlowIntegrity.rst | 4 ++-- clang/docs/UsersManual.rst | 10 +++++----- clang/include/clang/Driver/Driver.h | 2 +- clang/include/clang/Driver/SanitizerArgs.h | 1 - clang/lib/Driver/Driver.cpp | 12 +++--------- clang/lib/Driver/SanitizerArgs.cpp | 10 ++++++---- clang/lib/Driver/Tools.cpp | 9 ++++----- clang/test/Driver/fsanitize.c | 13 ++++++++----- 8 files changed, 29 insertions(+), 32 deletions(-) diff --git a/clang/docs/ControlFlowIntegrity.rst b/clang/docs/ControlFlowIntegrity.rst index 915385b..ce1c37b 100644 --- a/clang/docs/ControlFlowIntegrity.rst +++ b/clang/docs/ControlFlowIntegrity.rst @@ -20,8 +20,8 @@ program's control flow. These schemes have been optimized for performance, allowing developers to enable them in release builds. To enable Clang's available CFI schemes, use the flag ``-fsanitize=cfi``. -As currently implemented, CFI relies on link-time optimization (LTO); the CFI -schemes imply ``-flto``, and the linker used must support LTO, for example +As currently implemented, CFI relies on link-time optimization (LTO); so it is +required to specify ``-flto``, and the linker used must support LTO, for example via the `gold plugin`_. To allow the checks to be implemented efficiently, the program must be structured such that certain object files are compiled with CFI enabled, and are statically linked into the program. This may diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index dc99871..cd1b2b3 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -976,7 +976,7 @@ are listed below. - ``-fsanitize=dataflow``: :doc:`DataFlowSanitizer`, a general data flow analysis. - ``-fsanitize=cfi``: :doc:`control flow integrity ` - checks. Implies ``-flto``. + checks. Requires ``-flto``. - ``-fsanitize=safe-stack``: :doc:`safe stack ` protection against stack-based memory corruption errors. @@ -991,13 +991,13 @@ are listed below. - ``-fsanitize=cfi-cast-strict``: Enables :ref:`strict cast checks `. - ``-fsanitize=cfi-derived-cast``: Base-to-derived cast to the wrong - dynamic type. Implies ``-flto``. + dynamic type. Requires ``-flto``. - ``-fsanitize=cfi-unrelated-cast``: Cast from ``void*`` or another - unrelated type to the wrong dynamic type. Implies ``-flto``. + unrelated type to the wrong dynamic type. Requires ``-flto``. - ``-fsanitize=cfi-nvcall``: Non-virtual call via an object whose vptr is of - the wrong dynamic type. Implies ``-flto``. + the wrong dynamic type. Requires ``-flto``. - ``-fsanitize=cfi-vcall``: Virtual call via an object whose vptr is of the - wrong dynamic type. Implies ``-flto``. + wrong dynamic type. Requires ``-flto``. - ``-fsanitize=enum``: Load of a value of an enumerated type which is not in the range of representable values for that enumerated type. diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 0593ad0..d7bb1d2 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -402,7 +402,7 @@ public: /// handle this action. bool ShouldUseClangCompiler(const JobAction &JA) const; - bool IsUsingLTO(const ToolChain &TC, const llvm::opt::ArgList &Args) const; + bool IsUsingLTO(const llvm::opt::ArgList &Args) const; private: /// \brief Retrieves a ToolChain for a particular target triple. diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index aca43ec..ceba912 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -53,7 +53,6 @@ class SanitizerArgs { bool requiresPIE() const; bool needsUnwindTables() const; - bool needsLTO() const; bool linkCXXRuntimes() const { return LinkCXXRuntimes; } void addArgs(const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CmdArgs) const; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 1c6f4d1..0f3ebef 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1362,7 +1362,7 @@ Driver::ConstructPhaseAction(const ToolChain &TC, const ArgList &Args, types::TY_LLVM_BC); } case phases::Backend: { - if (IsUsingLTO(TC, Args)) { + if (IsUsingLTO(Args)) { types::ID Output = Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC; return llvm::make_unique(std::move(Input), Output); @@ -1383,14 +1383,8 @@ Driver::ConstructPhaseAction(const ToolChain &TC, const ArgList &Args, llvm_unreachable("invalid phase in ConstructPhaseAction"); } -bool Driver::IsUsingLTO(const ToolChain &TC, const ArgList &Args) const { - if (TC.getSanitizerArgs().needsLTO()) - return true; - - if (Args.hasFlag(options::OPT_flto, options::OPT_fno_lto, false)) - return true; - - return false; +bool Driver::IsUsingLTO(const ArgList &Args) const { + return Args.hasFlag(options::OPT_flto, options::OPT_fno_lto, false); } void Driver::BuildJobs(Compilation &C) const { diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 4d27a39..35041b5 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -188,10 +188,6 @@ bool SanitizerArgs::needsUnwindTables() const { return Sanitizers.Mask & NeedsUnwindTables; } -bool SanitizerArgs::needsLTO() const { - return Sanitizers.Mask & NeedsLTO; -} - void SanitizerArgs::clear() { Sanitizers.clear(); RecoverableSanitizers.clear(); @@ -301,6 +297,12 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, Kinds &= ~Vptr; } + // Check that LTO is enabled if we need it. + if ((Kinds & NeedsLTO) && !D.IsUsingLTO(Args)) { + D.Diag(diag::err_drv_argument_only_allowed_with) + << lastArgumentForMask(D, Args, Kinds & NeedsLTO) << "-flto"; + } + // Warn about incompatible groups of sanitizers. std::pair IncompatibleGroups[] = { std::make_pair(Address, Thread), std::make_pair(Address, Memory), diff --git a/clang/lib/Driver/Tools.cpp b/clang/lib/Driver/Tools.cpp index 424d2d3..2367914 100644 --- a/clang/lib/Driver/Tools.cpp +++ b/clang/lib/Driver/Tools.cpp @@ -6038,7 +6038,7 @@ void cloudabi::Link::ConstructJob(Compilation &C, const JobAction &JA, Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag); Args.AddAllArgs(CmdArgs, options::OPT_r); - if (D.IsUsingLTO(ToolChain, Args)) + if (D.IsUsingLTO(Args)) AddGoldPlugin(ToolChain, Args, CmdArgs); AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs); @@ -6189,8 +6189,7 @@ void darwin::Link::AddLinkArgs(Compilation &C, // If we are using LTO, then automatically create a temporary file path for // the linker to use, so that it's lifetime will extend past a possible // dsymutil step. - if (Version[0] >= 116 && D.IsUsingLTO(getToolChain(), Args) && - NeedsTempPath(Inputs)) { + if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) { const char *TmpPath = C.getArgs().MakeArgString( D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object))); C.addTempFile(TmpPath); @@ -7226,7 +7225,7 @@ void freebsd::Link::ConstructJob(Compilation &C, const JobAction &JA, Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag); Args.AddAllArgs(CmdArgs, options::OPT_r); - if (D.IsUsingLTO(getToolChain(), Args)) + if (D.IsUsingLTO(Args)) AddGoldPlugin(ToolChain, Args, CmdArgs); bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs); @@ -8086,7 +8085,7 @@ void gnutools::Link::ConstructJob(Compilation &C, const JobAction &JA, for (const auto &Path : Paths) CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + Path)); - if (D.IsUsingLTO(getToolChain(), Args)) + if (D.IsUsingLTO(Args)) AddGoldPlugin(ToolChain, Args, CmdArgs); if (Args.hasArg(options::OPT_Z_Xlinker__no_demangle)) diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c index 66e4925..cb5702d 100644 --- a/clang/test/Driver/fsanitize.c +++ b/clang/test/Driver/fsanitize.c @@ -202,17 +202,20 @@ // RUN: %clang -target x86_64-apple-darwin10 -fsanitize=function -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-UBSAN-DARWIN // CHECK-FSAN-UBSAN-DARWIN: unsupported option '-fsanitize=function' for target 'x86_64-apple-darwin10' -// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI -// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-derived-cast -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-DCAST -// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-unrelated-cast -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-UCAST -// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-nvcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NVCALL -// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-vcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-VCALL +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-derived-cast -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-DCAST +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-unrelated-cast -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-UCAST +// RUN: %clang -target x86_64-linux-gnu -flto -fsanitize=cfi-nvcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NVCALL +// RUN: %clang -target x86_64-linux-gnu -flto -fsanitize=cfi-vcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-VCALL // CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-unrelated-cast,cfi-nvcall,cfi-vcall // CHECK-CFI-DCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast // CHECK-CFI-UCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-unrelated-cast // CHECK-CFI-NVCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-nvcall // CHECK-CFI-VCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-vcall +// RUN: %clang -target x86_64-linux-gnu -flto -fsanitize=cfi-derived-cast -fno-lto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOLTO +// CHECK-CFI-NOLTO: '-fsanitize=cfi-derived-cast' only allowed with '-flto' + // RUN: %clang -target x86_64-linux-gnu -fsanitize-trap=address -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-TRAP // CHECK-ASAN-TRAP: error: unsupported argument 'address' to option '-fsanitize-trap' -- 2.7.4