ajpaverd created this revision.
ajpaverd added reviewers: rnk, dmajor, pcc, hans.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Avoid using the `nocf_check` attribute with Control Flow Guard. Instead, use a 
new `"guard_nocf"` function attribute to indicate that checks should not be 
added on indirect calls within that function. Add support for 
`__declspec(guard(nocf))` following the same syntax as MSVC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72167

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  llvm/docs/LangRef.rst
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll

Index: llvm/test/CodeGen/X86/cfguard-checks.ll
===================================================================
--- llvm/test/CodeGen/X86/cfguard-checks.ll
+++ llvm/test/CodeGen/X86/cfguard-checks.ll
@@ -8,8 +8,8 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -17,17 +17,17 @@
   %1 = call i32 %0()
   ret i32 %1
 
-  ; X32-LABEL: func_nocf_checks
+  ; X32-LABEL: func_guard_nocf
   ; X32: 	     movl  $_target_func, %eax
   ; X32-NOT: __guard_check_icall_fptr
 	; X32: 	     calll *%eax
 
-  ; X64-LABEL: func_nocf_checks
+  ; X64-LABEL: func_guard_nocf
   ; X64:       leaq	target_func(%rip), %rax
   ; X64-NOT: __guard_dispatch_icall_fptr
   ; X64:       callq	*%rax
 }
-attributes #0 = { nocf_check }
+attributes #0 = { "guard_nocf" }
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/test/CodeGen/ARM/cfguard-checks.ll
===================================================================
--- llvm/test/CodeGen/ARM/cfguard-checks.ll
+++ llvm/test/CodeGen/ARM/cfguard-checks.ll
@@ -7,8 +7,8 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -16,13 +16,13 @@
   %1 = call arm_aapcs_vfpcc i32 %0()
   ret i32 %1
 
-  ; CHECK-LABEL: func_nocf_checks
+  ; CHECK-LABEL: func_guard_nocf
   ; CHECK:       movw r0, :lower16:target_func
 	; CHECK:       movt r0, :upper16:target_func
   ; CHECK-NOT:   __guard_check_icall_fptr
 	; CHECK:       blx r0
 }
-attributes #0 = { nocf_check "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #0 = { "guard_nocf" "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/test/CodeGen/AArch64/cfguard-checks.ll
===================================================================
--- llvm/test/CodeGen/AArch64/cfguard-checks.ll
+++ llvm/test/CodeGen/AArch64/cfguard-checks.ll
@@ -7,8 +7,8 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -16,13 +16,13 @@
   %1 = call i32 %0()
   ret i32 %1
 
-  ; CHECK-LABEL: func_nocf_checks
+  ; CHECK-LABEL: func_guard_nocf
   ; CHECK:       adrp x8, target_func
 	; CHECK:       add x8, x8, target_func
   ; CHECK-NOT:   __guard_check_icall_fptr
 	; CHECK:       blr x8
 }
-attributes #0 = { nocf_check }
+attributes #0 = { "guard_nocf" }
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/lib/Transforms/CFGuard/CFGuard.cpp
===================================================================
--- llvm/lib/Transforms/CFGuard/CFGuard.cpp
+++ llvm/lib/Transforms/CFGuard/CFGuard.cpp
@@ -255,7 +255,7 @@
 bool CFGuard::runOnFunction(Function &F) {
 
   // Skip modules and functions for which CFGuard checks have been disabled.
-  if (cfguard_module_flag != 2 || F.hasFnAttribute(Attribute::NoCfCheck))
+  if (cfguard_module_flag != 2 || F.hasFnAttribute("guard_nocf"))
     return false;
 
   SmallVector<CallBase *, 8> IndirectCalls;
@@ -277,7 +277,7 @@
   // If no checks are needed, return early and add this attribute to indicate
   // that subsequent CFGuard passes can skip this function.
   if (IndirectCalls.empty()) {
-    F.addFnAttr(Attribute::NoCfCheck);
+    F.addFnAttr("guard_nocf");
     return false;
   }
 
Index: llvm/docs/LangRef.rst
===================================================================
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -1840,6 +1840,9 @@
     the function. The instrumentation checks that the return address for the
     function has not changed between the function prolog and eiplog. It is
     currently x86_64-specific.
+``"guard_nocf"``
+    This attribute indicates that no Control Flow Guard checks will be added
+    within this function.
 
 .. _glattrs:
 
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O0 -o - %s | FileCheck %s
+
+// The "guard_nocf" attribute should be added to this function
+__declspec(guard(nocf)) void Func_guard_nocf() {
+}
+
+// The "guard_nocf" attribute should not be added to this function.
+void Func_guard_cf() {
+}
+
+// CHECK: @{{.*}}Func_guard_nocf{{.*}}[[NOCF:#[0-9]+]]
+// CHECK: @{{.*}}Func_guard_cf{{.*}}[[CF:#[0-9]+]]
+// CHECK: attributes [[NOCF]] = { {{.*}}"guard_nocf"{{.*}} }
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
\ No newline at end of file
Index: clang/test/CodeGen/guard_nocf.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/guard_nocf.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O0 -o - %s | FileCheck %s
+
+// The "guard_nocf" attribute should be added to this function
+__declspec(guard(nocf)) void Func_guard_nocf() {
+}
+
+// The "guard_nocf" attribute should not be added to this function.
+void Func_guard_cf() {
+}
+
+// CHECK: @{{.*}}Func_guard_nocf{{.*}}[[NOCF:#[0-9]+]]
+// CHECK: @{{.*}}Func_guard_cf{{.*}}[[CF:#[0-9]+]]
+// CHECK: attributes [[NOCF]] = { {{.*}}"guard_nocf"{{.*}} }
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6628,6 +6628,25 @@
   D->addAttr(Attr::Create(S.Context, Argument, AL));
 }
 
+static void handleCFGuardAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // The guard attribute takes a single argument.
+
+  if (!AL.isArgIdent(0)) {
+    S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+        << AL << 0 << 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
 //===----------------------------------------------------------------------===//
@@ -7256,6 +7275,9 @@
   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:
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -836,6 +836,12 @@
   if (D && D->hasAttr<CFICanonicalJumpTableAttr>())
     Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr<CFGuardAttr>()) {
+    // Add the "guard_nocf" attribute to the function.
+    if (D->getAttr<CFGuardAttr>()->getGuard() == CFGuardAttr::GuardArg::nocf)
+      Fn->addFnAttr("guard_nocf");
+  }
+
   if (getLangOpts().OpenCL) {
     // Add metadata for a kernel function.
     if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4558,6 +4558,27 @@
 }];
 }
 
+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 = [{
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2907,6 +2907,13 @@
   let Documentation = [MSAllocatorDocs];
 }
 
+def CFGuard : InheritableAttr {
+  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]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to