diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/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"]>]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/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 diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp --- a/clang/lib/CodeGen/CGCall.cpp +++ b/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(CurFuncDecl)) { + if (FD->hasAttr() && + FD->getAttr()->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(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. diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -836,12 +836,6 @@ if (D && D->hasAttr()) Fn->addFnAttr("cfi-canonical-jump-table"); - if (D && D->hasAttr()) { - // Add the "guard_nocf" attribute to the function. - if (D->getAttr()->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(D)) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/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; } diff --git a/clang/test/CodeGen/guard_nocf.c b/clang/test/CodeGen/guard_nocf.c --- a/clang/test/CodeGen/guard_nocf.c +++ b/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"{{.*}} } diff --git a/clang/test/CodeGenCXX/guard_nocf.cpp b/clang/test/CodeGenCXX/guard_nocf.cpp --- a/clang/test/CodeGenCXX/guard_nocf.cpp +++ b/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"{{.*}} } diff --git a/clang/test/Sema/attr-guard_nocf.c b/clang/test/Sema/attr-guard_nocf.c new file mode 100644 --- /dev/null +++ b/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'}} +} diff --git a/clang/test/Sema/attr-guard_nocf.cpp b/clang/test/Sema/attr-guard_nocf.cpp new file mode 100644 --- /dev/null +++ b/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'}} +} diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst --- a/llvm/docs/LangRef.rst +++ b/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: diff --git a/llvm/lib/Transforms/CFGuard/CFGuard.cpp b/llvm/lib/Transforms/CFGuard/CFGuard.cpp --- a/llvm/lib/Transforms/CFGuard/CFGuard.cpp +++ b/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 IndirectCalls; @@ -267,17 +267,15 @@ for (BasicBlock &BB : F.getBasicBlockList()) { for (Instruction &I : BB.getInstList()) { auto *CB = dyn_cast(&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; } diff --git a/llvm/test/CodeGen/AArch64/cfguard-checks.ll b/llvm/test/CodeGen/AArch64/cfguard-checks.ll --- a/llvm/test/CodeGen/AArch64/cfguard-checks.ll +++ b/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 diff --git a/llvm/test/CodeGen/ARM/cfguard-checks.ll b/llvm/test/CodeGen/ARM/cfguard-checks.ll --- a/llvm/test/CodeGen/ARM/cfguard-checks.ll +++ b/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 diff --git a/llvm/test/CodeGen/X86/cfguard-checks.ll b/llvm/test/CodeGen/X86/cfguard-checks.ll --- a/llvm/test/CodeGen/X86/cfguard-checks.ll +++ b/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