ajpaverd updated this revision to Diff 236572.
ajpaverd marked 5 inline comments as done.
ajpaverd added a comment.
Make guard_nocf an attribute of individual call instructions
Instead of using "guard_nocf" as a function attribute, make this an attribute
on individual call instructions for which Control Flow Guard checks should not
be added. This allows the attribute (or lack thereof) to be propagated with the
call instructions if they are inlined into another function.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72167/new/
https://reviews.llvm.org/D72167
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/CodeGen/guard_nocf.c
clang/test/CodeGenCXX/guard_nocf.cpp
clang/test/Sema/attr-guard_nocf.c
clang/test/Sema/attr-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,13 +8,13 @@
declare i32 @target_func()
-; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
-define i32 @func_guard_nocf() #0 {
+; Test that Control Flow Guard checks are not added on calls with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() {
entry:
%func_ptr = alloca i32 ()*, align 8
store i32 ()* @target_func, i32 ()** %func_ptr, align 8
%0 = load i32 ()*, i32 ()** %func_ptr, align 8
- %1 = call i32 %0()
+ %1 = call i32 %0() #0
ret i32 %1
; X32-LABEL: func_guard_nocf
Index: llvm/test/CodeGen/ARM/cfguard-checks.ll
===================================================================
--- llvm/test/CodeGen/ARM/cfguard-checks.ll
+++ llvm/test/CodeGen/ARM/cfguard-checks.ll
@@ -7,13 +7,13 @@
declare i32 @target_func()
-; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+; Test that Control Flow Guard checks are not added on calls 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
%0 = load i32 ()*, i32 ()** %func_ptr, align 8
- %1 = call arm_aapcs_vfpcc i32 %0()
+ %1 = call arm_aapcs_vfpcc i32 %0() #1
ret i32 %1
; CHECK-LABEL: func_guard_nocf
@@ -22,11 +22,12 @@
; CHECK-NOT: __guard_check_icall_fptr
; CHECK: blx r0
}
-attributes #0 = { "guard_nocf" "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #0 = { "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #1 = { "guard_nocf" }
; Test that Control Flow Guard checks are added even at -O0.
-define i32 @func_optnone_cf() #1 {
+define i32 @func_optnone_cf() #2 {
entry:
%func_ptr = alloca i32 ()*, align 8
store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -47,11 +48,11 @@
; CHECK: blx r1
; CHECK-NEXT: blx r4
}
-attributes #1 = { noinline optnone "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #2 = { noinline optnone "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
; Test that Control Flow Guard checks are correctly added in optimized code (common case).
-define i32 @func_cf() #2 {
+define i32 @func_cf() #0 {
entry:
%func_ptr = alloca i32 ()*, align 8
store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -70,11 +71,10 @@
; CHECK: blx r1
; CHECK-NEXT: blx r4
}
-attributes #2 = { "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
; Test that Control Flow Guard checks are correctly added on invoke instructions.
-define i32 @func_cf_invoke() #2 personality i8* bitcast (void ()* @h to i8*) {
+define i32 @func_cf_invoke() #0 personality i8* bitcast (void ()* @h to i8*) {
entry:
%0 = alloca i32, align 4
%func_ptr = alloca i32 ()*, align 8
@@ -112,7 +112,7 @@
%struct._SETJMP_FLOAT128 = type { [2 x i64] }
@buf1 = internal global [16 x %struct._SETJMP_FLOAT128] zeroinitializer, align 16
-define i32 @func_cf_setjmp() #2 {
+define i32 @func_cf_setjmp() #0 {
%1 = alloca i32, align 4
%2 = alloca i32, align 4
store i32 0, i32* %1, align 4
Index: llvm/test/CodeGen/AArch64/cfguard-checks.ll
===================================================================
--- llvm/test/CodeGen/AArch64/cfguard-checks.ll
+++ llvm/test/CodeGen/AArch64/cfguard-checks.ll
@@ -7,13 +7,13 @@
declare i32 @target_func()
-; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
-define i32 @func_guard_nocf() #0 {
+; Test that Control Flow Guard checks are not added on calls with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() {
entry:
%func_ptr = alloca i32 ()*, align 8
store i32 ()* @target_func, i32 ()** %func_ptr, align 8
%0 = load i32 ()*, i32 ()** %func_ptr, align 8
- %1 = call i32 %0()
+ %1 = call i32 %0() #0
ret i32 %1
; CHECK-LABEL: func_guard_nocf
Index: llvm/lib/Transforms/CFGuard/CFGuard.cpp
===================================================================
--- llvm/lib/Transforms/CFGuard/CFGuard.cpp
+++ llvm/lib/Transforms/CFGuard/CFGuard.cpp
@@ -254,8 +254,8 @@
bool CFGuard::runOnFunction(Function &F) {
- // Skip modules and functions for which CFGuard checks have been disabled.
- if (cfguard_module_flag != 2 || F.hasFnAttribute("guard_nocf"))
+ // Skip modules for which CFGuard checks have been disabled.
+ if (cfguard_module_flag != 2)
return false;
SmallVector<CallBase *, 8> IndirectCalls;
@@ -267,17 +267,15 @@
for (BasicBlock &BB : F.getBasicBlockList()) {
for (Instruction &I : BB.getInstList()) {
auto *CB = dyn_cast<CallBase>(&I);
- if (CB && CB->isIndirectCall()) {
+ if (CB && CB->isIndirectCall() && !CB->hasFnAttr("guard_nocf")) {
IndirectCalls.push_back(CB);
CFGuardCounter++;
}
}
}
- // If no checks are needed, return early and add this attribute to indicate
- // that subsequent CFGuard passes can skip this function.
+ // If no checks are needed, return early.
if (IndirectCalls.empty()) {
- F.addFnAttr("guard_nocf");
return false;
}
Index: llvm/docs/LangRef.rst
===================================================================
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -1840,9 +1840,6 @@
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/Sema/attr-guard_nocf.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/attr-guard_nocf.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only %s
+
+// Function definition.
+__declspec(guard(nocf)) void testGuardNoCF() { // no warning
+}
+
+// Can not be used on variable, parameter, or function pointer declarations.
+int __declspec(guard(nocf)) i; // expected-warning {{'guard' attribute only applies to functions}}
+void testGuardNoCFFuncParam(double __declspec(guard(nocf)) i) {} // expected-warning {{'guard' attribute only applies to functions}}
+__declspec(guard(nocf)) typedef void (*FuncPtrWithGuardNoCF)(void); // expected-warning {{'guard' attribute only applies to functions}}
+
+// 'guard' Attribute requries an argument.
+__declspec(guard) void testGuardNoCFParams() { // expected-error {{'guard' attribute takes one argument}}
+}
+
+// 'guard' Attribute requries an identifier as argument.
+__declspec(guard(1)) void testGuardNoCFParamType() { // expected-error {{'guard' attribute requires an identifier}}
+}
+
+// 'guard' Attribute only takes a single argument.
+__declspec(guard(nocf, nocf)) void testGuardNoCFTooManyParams() { // expected-error {{use of undeclared identifier 'nocf'}}
+}
+
+// 'guard' Attribute argument must be a supported identifier.
+__declspec(guard(cf)) void testGuardNoCFInvalidParam() { // expected-warning {{'guard' attribute argument not supported: 'cf'}}
+}
Index: clang/test/Sema/attr-guard_nocf.c
===================================================================
--- /dev/null
+++ clang/test/Sema/attr-guard_nocf.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s
+
+// Function definition.
+__declspec(guard(nocf)) void testGuardNoCF() { // no warning
+}
+
+// Can not be used on variable, parameter, or function pointer declarations.
+int __declspec(guard(nocf)) i; // expected-warning {{'guard' attribute only applies to functions}}
+void testGuardNoCFFuncParam(double __declspec(guard(nocf)) i) {} // expected-warning {{'guard' attribute only applies to functions}}
+__declspec(guard(nocf)) typedef void (*FuncPtrWithGuardNoCF)(void); // expected-warning {{'guard' attribute only applies to functions}}
+
+// 'guard' Attribute requries an argument.
+__declspec(guard) void testGuardNoCFParams() { // expected-error {{'guard' attribute takes one argument}}
+}
+
+// 'guard' Attribute requries an identifier as argument.
+__declspec(guard(1)) void testGuardNoCFParamType() { // expected-error {{'guard' attribute requires an identifier}}
+}
+
+// 'guard' Attribute only takes a single argument.
+__declspec(guard(nocf, nocf)) void testGuardNoCFTooManyParams() { // expected-error {{use of undeclared identifier 'nocf'}}
+}
+
+// 'guard' Attribute argument must be a supported identifier.
+__declspec(guard(cf)) void testGuardNoCFInvalidParam() { // expected-warning {{'guard' attribute argument not supported: 'cf'}}
+}
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===================================================================
--- clang/test/CodeGenCXX/guard_nocf.cpp
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -1,14 +1,84 @@
-// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
-// The "guard_nocf" attribute should be added to this function
-__declspec(guard(nocf)) void Func_guard_nocf() {
+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]+]]
-// The "guard_nocf" attribute should not be added to this function.
-void Func_guard_cf() {
+// 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: @{{.*}}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
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
Index: clang/test/CodeGen/guard_nocf.c
===================================================================
--- clang/test/CodeGen/guard_nocf.c
+++ clang/test/CodeGen/guard_nocf.c
@@ -1,14 +1,53 @@
-// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - %s | FileCheck %s
-// The "guard_nocf" attribute should be added to this function
-__declspec(guard(nocf)) void Func_guard_nocf() {
+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]+]]
-// The "guard_nocf" attribute should not be added to this function.
-void Func_guard_cf() {
+// 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: @{{.*}}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
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6629,11 +6629,11 @@
}
static void handleCFGuardAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- // The guard attribute takes a single argument.
+ // The guard attribute takes a single identifier argument.
if (!AL.isArgIdent(0)) {
- S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
- << AL << 0 << AANT_ArgumentIdentifier;
+ S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+ << AL << AANT_ArgumentIdentifier;
return;
}
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -836,12 +836,6 @@
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/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4415,6 +4415,19 @@
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 FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
+ if (FD->hasAttr<CFGuardAttr>() &&
+ FD->getAttr<CFGuardAttr>()->getGuard() == CFGuardAttr::GuardArg::nocf) {
+ if (!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));
@@ -4433,8 +4446,7 @@
// For more details, see the comment before the definition of
// IPVK_IndirectCallTarget in InstrProfData.inc.
if (!CI->getCalledFunction())
- PGO.valueProfile(Builder, llvm::IPVK_IndirectCallTarget,
- CI, CalleePtr);
+ PGO.valueProfile(Builder, llvm::IPVK_IndirectCallTarget, CI, CalleePtr);
// In ObjC ARC mode with no ObjC ARC exception safety, tell the ARC
// optimizer it can aggressively ignore unwind edges.
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4565,8 +4565,8 @@
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
+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
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2908,6 +2908,8 @@
}
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"]>];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
