rust-lang · ajpaverd · Oct 28, 2019 · Oct 28, 2019 · Nov 18, 2019 · Jan 10, 2020
diff --git a/clang/docs/ClangCommandLineReference.rst b/clang/docs/ClangCommandLineReference.rst
@@ -126,7 +126,11 @@ Output path for the plist report
 
 .. option:: -cfguard
 
-Emit tables required for Windows Control Flow Guard.
+Emit tables and checks for Windows Control Flow Guard.
+
+.. option:: -cfguard-no-checks
+
+Emit tables required for Windows Control Flow Guard without checks.
 
 .. option:: -client\_name<arg>
 

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
@@ -2806,6 +2806,15 @@ def MSAllocator : InheritableAttr {
   let Documentation = [MSAllocatorDocs];
 }
 
+def CFGuard : InheritableAttr {
+  // Currently only the __declspec(guard(nocf)) modifier is supported. In future
+  // we might also want to support __declspec(guard(suppress)).
+  let Spellings = [Declspec<"guard">];
+  let Subjects = SubjectList<[Function]>;
+  let Args = [EnumArgument<"Guard", "GuardArg", ["nocf"], ["nocf"]>];
+  let Documentation = [CFGuardDocs];
+}
+
 def MSStruct : InheritableAttr {
   let Spellings = [GCC<"ms_struct">];
   let Subjects = SubjectList<[Record]>;

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
@@ -4184,6 +4184,26 @@ This attribute does not affect optimizations in any way, unlike GCC's
 }];
 }
 
+def CFGuardDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Code can indicate CFG checks are not wanted with the ``__declspec(guard(nocf))``
+attribute. This directs the compiler to not insert any CFG checks for the entire
+function. This approach is typically used only sparingly in specific situations 
+where the programmer has manually inserted "CFG-equivalent" protection. The 
+programmer knows that they are calling through some read-only function table 
+whose address is obtained through read-only memory references and for which the 
+index is masked to the function table limit. This approach may also be applied 
+to small wrapper functions that are not inlined and that do nothing more than 
+make a call through a function pointer. Since incorrect usage of this directive 
+can compromise the security of CFG, the programmer must be very careful using 
+the directive. Typically, this usage is limited to very small functions that 
+only call one function.
+
+`Control Flow Guard documentation <https://docs.microsoft.com/en-us/windows/win32/secbp/pe-metadata>`
+}];
+}
+
 def HIPPinnedShadowDocs : Documentation {
   let Category = DocCatType;
   let Content = [{

diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
@@ -37,6 +37,7 @@ CODEGENOPT(AssumeSaneOperatorNew , 1, 1) ///< implicit __attribute__((malloc)) o
 CODEGENOPT(Autolink          , 1, 1) ///< -fno-autolink
 CODEGENOPT(ObjCAutoRefCountExceptions , 1, 0) ///< Whether ARC should be EH-safe.
 CODEGENOPT(Backchain         , 1, 0) ///< -mbackchain
+CODEGENOPT(ControlFlowGuardNoChecks  , 1, 0) ///< -cfguard-no-checks
 CODEGENOPT(ControlFlowGuard  , 1, 0) ///< -cfguard
 CODEGENOPT(CoverageExtraChecksum, 1, 0) ///< Whether we need a second checksum for functions in GCNO files.
 CODEGENOPT(CoverageNoFunctionNamesInData, 1, 0) ///< Do not include function names in GCDA files.

diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td
@@ -399,6 +399,10 @@ def msign_return_address_key_EQ : Joined<["-"], "msign-return-address-key=">,
     Values<"a_key,b_key">;
 def mbranch_target_enforce : Flag<["-"], "mbranch-target-enforce">;
 def fno_dllexport_inlines : Flag<["-"], "fno-dllexport-inlines">;
+def cfguard_no_checks : Flag<["-"], "cfguard-no-checks">,
+    HelpText<"Emit Windows Control Flow Guard tables only (no checks)">;
+def cfguard : Flag<["-"], "cfguard">,
+    HelpText<"Emit Windows Control Flow Guard tables and checks">;
 
 //===----------------------------------------------------------------------===//
 // Dependency Output Options

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -497,8 +497,6 @@ def bind__at__load : Flag<["-"], "bind_at_load">;
 def bundle__loader : Separate<["-"], "bundle_loader">;
 def bundle : Flag<["-"], "bundle">;
 def b : JoinedOrSeparate<["-"], "b">, Flags<[Unsupported]>;
-def cfguard : Flag<["-"], "cfguard">, Flags<[CC1Option]>,
-  HelpText<"Emit tables required for Windows Control Flow Guard.">;
 def cl_opt_disable : Flag<["-"], "cl-opt-disable">, Group<opencl_Group>, Flags<[CC1Option]>,
   HelpText<"OpenCL only. This option disables all optimizations. By default optimizations are enabled.">;
 def cl_strict_aliasing : Flag<["-"], "cl-strict-aliasing">, Group<opencl_Group>, Flags<[CC1Option]>,

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
@@ -4346,6 +4346,17 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   if (callOrInvoke)
     *callOrInvoke = CI;
 
+  // If this is within a function that has the guard(nocf) attribute and is an
+  // indirect call, add the "guard_nocf" attribute to this call to indicate that
+  // Control Flow Guard checks should not be added, even if the call is inlined.
+  if (const auto *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
+    if (const auto *A = FD->getAttr<CFGuardAttr>()) {
+      if (A->getGuard() == CFGuardAttr::GuardArg::nocf && !CI->getCalledFunction())
+        Attrs = Attrs.addAttribute(
+            getLLVMContext(), llvm::AttributeList::FunctionIndex, "guard_nocf");
+    }
+  }
+
   // Apply the attributes and calling convention.
   CI->setAttributes(Attrs);
   CI->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -487,8 +487,11 @@ void CodeGenModule::Release() {
     getModule().addModuleFlag(llvm::Module::Warning, "CodeViewGHash", 1);
   }
   if (CodeGenOpts.ControlFlowGuard) {
-    // We want function ID tables for Control Flow Guard.
-    getModule().addModuleFlag(llvm::Module::Warning, "cfguardtable", 1);
+    // Function ID tables and checks for Control Flow Guard (cfguard=2).
+    getModule().addModuleFlag(llvm::Module::Warning, "cfguard", 2);
+  } else if (CodeGenOpts.ControlFlowGuardNoChecks) {
+    // Function ID tables for Control Flow Guard (cfguard=1).
+    getModule().addModuleFlag(llvm::Module::Warning, "cfguard", 1);
   }
   if (CodeGenOpts.OptimizationLevel > 0 && CodeGenOpts.StrictVTablePointers) {
     // We don't support LTO with 2 with different StrictVTablePointers

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5916,26 +5916,19 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
   }
 
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
-    SmallVector<StringRef, 1> SplitArgs;
-    StringRef(A->getValue()).split(SplitArgs, ",");
-    bool Instrument = false;
-    bool NoChecks = false;
-    for (StringRef Arg : SplitArgs) {
-      if (Arg.equals_lower("cf"))
-        Instrument = true;
-      else if (Arg.equals_lower("cf-"))
-        Instrument = false;
-      else if (Arg.equals_lower("nochecks"))
-        NoChecks = true;
-      else if (Arg.equals_lower("nochecks-"))
-        NoChecks = false;
-      else
-        D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << Arg;
-    }
-    // Currently there's no support emitting CFG instrumentation; the flag only
-    // emits the table of address-taken functions.
-    if (Instrument || NoChecks)
+    StringRef GuardArgs = A->getValue();
+    // The only valid options are "cf", "cf,nochecks", and "cf-".
+    if (GuardArgs.equals_lower("cf")) {
+      // Emit CFG instrumentation and the table of address-taken functions.
       CmdArgs.push_back("-cfguard");
+    } else if (GuardArgs.equals_lower("cf,nochecks")) {
+      // Emit only the table of address-taken functions.
+      CmdArgs.push_back("-cfguard-no-checks");
+    } else if (GuardArgs.equals_lower("cf-")) {
+      // Do nothing, but we might want to emit a security warning in future.
+    } else {
+      D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
+    }
   }
 }
 

diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -417,6 +417,17 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
+  // Control Flow Guard checks
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+    StringRef GuardArgs = A->getValue();
+    if (GuardArgs.equals_lower("cf") || GuardArgs.equals_lower("cf,nochecks")) {
+      // MSVC doesn't yet support the "nochecks" modifier.
+      CmdArgs.push_back("-guard:cf");
+    } else if (GuardArgs.equals_lower("cf-")) {
+      CmdArgs.push_back("-guard:cf-");
+    }
+  }
+
   if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
                    options::OPT_fno_openmp, false)) {
     CmdArgs.push_back("-nodefaultlib:vcomp.lib");
@@ -674,6 +685,17 @@ std::unique_ptr<Command> visualstudio::Compiler::GetCommand(
                           : "/Zc:threadSafeInit-");
   }
 
+  // Control Flow Guard checks
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+    StringRef GuardArgs = A->getValue();
+    if (GuardArgs.equals_lower("cf") || GuardArgs.equals_lower("cf,nochecks")) {
+      // MSVC doesn't yet support the "nochecks" modifier.
+      CmdArgs.push_back("/guard:cf");
+    } else if (GuardArgs.equals_lower("cf-")) {
+      CmdArgs.push_back("/guard:cf-");
+    }
+  }
+
   // Pass through all unknown arguments so that the fallback command can see
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);

diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -944,6 +944,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.MainFileName = Args.getLastArgValue(OPT_main_file_name);
   Opts.VerifyModule = !Args.hasArg(OPT_disable_llvm_verifier);
 
+  Opts.ControlFlowGuardNoChecks = Args.hasArg(OPT_cfguard_no_checks);
   Opts.ControlFlowGuard = Args.hasArg(OPT_cfguard);
 
   Opts.DisableGCov = Args.hasArg(OPT_test_coverage);

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6593,6 +6593,25 @@ static void handleMSAllocatorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   handleSimpleAttribute<MSAllocatorAttr>(S, D, AL);
 }
 
+static void handleCFGuardAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // The guard attribute takes a single identifier argument.
+
+  if (!AL.isArgIdent(0)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+        << AL << AANT_ArgumentIdentifier;
+    return;
+  }
+
+  CFGuardAttr::GuardArg Arg;
+  IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
+  if (!CFGuardAttr::ConvertStrToGuardArg(II->getName(), Arg)) {
+    S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << II;
+    return;
+  }
+
+  D->addAttr(::new (S.Context) CFGuardAttr(S.Context, AL, Arg));
+}
+
 //===----------------------------------------------------------------------===//
 // Top Level Sema Entry Points
 //===----------------------------------------------------------------------===//
@@ -7196,6 +7215,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case ParsedAttr::AT_AbiTag:
     handleAbiTagAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_CFGuard:
+    handleCFGuardAttr(S, D, AL);
+    break;
 
   // Thread safety attributes:
   case ParsedAttr::AT_AssertExclusiveLock:

diff --git a/clang/test/CodeGen/cfguardtable.c b/clang/test/CodeGen/cfguardtable.c
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s
-
-void f() {}
-
-// Check that the cfguardtable metadata flag gets set on the module.
-// CHECK: !"cfguardtable", i32 1}
+// RUN: %clang_cc1 -cfguard-no-checks -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARDNOCHECKS
+// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARD
+
+void f() {}
+
+// Check that the cfguard metadata flag gets correctly set on the module.
+// CFGUARDNOCHECKS: !"cfguard", i32 1}
+// CFGUARD: !"cfguard", i32 2}
diff --git a/clang/test/CodeGen/guard_nocf.c b/clang/test/CodeGen/guard_nocf.c
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - %s | FileCheck %s
+
+void target_func();
+void (*func_ptr)() = &target_func;
+
+// The "guard_nocf" attribute must be added.
+__declspec(guard(nocf)) void nocf0() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// The "guard_nocf" attribute must *not* be added.
+void cf0() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: cf0
+// CHECK: call{{.*}}[[CF:#[0-9]+]]
+
+// If the modifier is present on either the function declaration or definition,
+// the "guard_nocf" attribute must be added.
+__declspec(guard(nocf)) void nocf1();
+void nocf1() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf1
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+void nocf2();
+__declspec(guard(nocf)) void nocf2() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf2
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// When inlining a function, the "guard_nocf" attribute on indirect calls must
+// be preserved.
+void nocf3() {
+  nocf0();
+}
+// CHECK-LABEL: nocf3
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// When inlining into a function marked as __declspec(guard(nocf)), the
+// "guard_nocf" attribute must *not* be added to the inlined calls.
+__declspec(guard(nocf)) void cf1() {
+  cf0();
+}
+// CHECK-LABEL: cf1
+// CHECK: call{{.*}}[[CF:#[0-9]+]]
+
+// CHECK: attributes [[NOCF]] = { {{.*}}"guard_nocf"{{.*}} }
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
diff --git a/clang/test/CodeGenCXX/guard_nocf.cpp b/clang/test/CodeGenCXX/guard_nocf.cpp
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+
+void target_func();
+void (*func_ptr)() = &target_func;
+
+// The "guard_nocf" attribute must be added.
+__declspec(guard(nocf)) void nocf0() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// The "guard_nocf" attribute must *not* be added.
+void cf0() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: cf0
+// CHECK: call{{.*}}[[CF:#[0-9]+]]
+
+// If the modifier is present on either the function declaration or definition,
+// the "guard_nocf" attribute must be added.
+__declspec(guard(nocf)) void nocf1();
+void nocf1() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf1
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+void nocf2();
+__declspec(guard(nocf)) void nocf2() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf2
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// When inlining a function, the "guard_nocf" attribute on indirect calls must
+// be preserved.
+void nocf3() {
+  nocf0();
+}
+// CHECK-LABEL: nocf3
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// When inlining into a function marked as __declspec(guard(nocf)), the
+// "guard_nocf" attribute must *not* be added to the inlined calls.
+__declspec(guard(nocf)) void cf1() {
+  cf0();
+}
+// CHECK-LABEL: cf1
+// CHECK: call{{.*}}[[CF:#[0-9]+]]
+
+// When the __declspec(guard(nocf)) modifier is present on an override function,
+// the "guard_nocf" attribute must be added.
+struct Base {
+  virtual void nocf4();
+};
+
+struct Derived : Base {
+  __declspec(guard(nocf)) void nocf4() override {
+    (*func_ptr)();
+  }
+};
+Derived d;
+// CHECK-LABEL: nocf4
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// When the modifier is not present on an override function, the "guard_nocf"
+// attribute must *not* be added, even if the modifier is present on the virtual
+// function.
+struct Base1 {
+  __declspec(guard(nocf)) virtual void cf2();
+};
+
+struct Derived1 : Base1 {
+  void cf2() override {
+    (*func_ptr)();
+  }
+};
+Derived1 d1;
+// CHECK-LABEL: cf2
+// CHECK: call{{.*}}[[CF:#[0-9]+]]
+
+// CHECK: attributes [[NOCF]] = { {{.*}}"guard_nocf"{{.*}} }
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }