From: Andrew Paverd Date: Fri, 10 Jan 2020 11:08:18 +0000 (+0000) Subject: Add support for __declspec(guard(nocf)) X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=bdd88b7ed3956534a0a71b1ea2bc88c69d48f9b7;p=platform%2Fupstream%2Fllvm.git Add support for __declspec(guard(nocf)) Summary: 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. Reviewers: rnk, dmajor, pcc, hans, aaron.ballman Reviewed By: aaron.ballman Subscribers: aaron.ballman, tomrittervg, hiraditya, cfe-commits, llvm-commits Tags: #clang, #llvm Differential Revision: https://reviews.llvm.org/D72167 --- diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c992d64..d1c42e8 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2907,6 +2907,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 index 70bf251..6692c38 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -4558,6 +4558,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 ` +}]; +} + def HIPPinnedShadowDocs : Documentation { let Category = DocCatType; let Content = [{ diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index b49b194..e4803fd 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4415,6 +4415,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(CurFuncDecl)) { + if (const auto *A = FD->getAttr()) { + 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(CallingConv)); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 7f1da40..142c6f1 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6626,6 +6626,25 @@ static void handleHandleAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(Attr::Create(S.Context, Argument, 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 //===----------------------------------------------------------------------===// @@ -7254,6 +7273,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/guard_nocf.c b/clang/test/CodeGen/guard_nocf.c new file mode 100644 index 0000000..2fe5573 --- /dev/null +++ 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 new file mode 100644 index 0000000..3dc5c50 --- /dev/null +++ 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"{{.*}} } diff --git a/clang/test/Sema/attr-guard_nocf.c b/clang/test/Sema/attr-guard_nocf.c new file mode 100644 index 0000000..a91640e --- /dev/null +++ b/clang/test/Sema/attr-guard_nocf.c @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s +// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %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/lib/Transforms/CFGuard/CFGuard.cpp b/llvm/lib/Transforms/CFGuard/CFGuard.cpp index 3eca006..7c5e90c 100644 --- a/llvm/lib/Transforms/CFGuard/CFGuard.cpp +++ b/llvm/lib/Transforms/CFGuard/CFGuard.cpp @@ -254,8 +254,8 @@ bool CFGuard::doInitialization(Module &M) { 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)) + // Skip modules for which CFGuard checks have been disabled. + if (cfguard_module_flag != 2) return false; SmallVector IndirectCalls; @@ -267,17 +267,15 @@ bool CFGuard::runOnFunction(Function &F) { 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(Attribute::NoCfCheck); return false; } diff --git a/llvm/test/CodeGen/AArch64/cfguard-checks.ll b/llvm/test/CodeGen/AArch64/cfguard-checks.ll index 627741c..5ebe1dd 100644 --- a/llvm/test/CodeGen/AArch64/cfguard-checks.ll +++ b/llvm/test/CodeGen/AArch64/cfguard-checks.ll @@ -7,22 +7,22 @@ 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 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_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. diff --git a/llvm/test/CodeGen/ARM/cfguard-checks.ll b/llvm/test/CodeGen/ARM/cfguard-checks.ll index c75afc6..3fab04e 100644 --- a/llvm/test/CodeGen/ARM/cfguard-checks.ll +++ b/llvm/test/CodeGen/ARM/cfguard-checks.ll @@ -7,26 +7,27 @@ 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 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_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 = { "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 @@ entry: ; 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 @@ entry: ; 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 @@ declare void @h() %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 index 6e8d714..8da0668 100644 --- a/llvm/test/CodeGen/X86/cfguard-checks.ll +++ b/llvm/test/CodeGen/X86/cfguard-checks.ll @@ -8,26 +8,26 @@ 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 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_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.