Author: Fangrui Song Date: 2023-05-22T10:11:30-07:00 New Revision: 279a4d0d67c874e80c171666822f2fabdd6fa926
URL: https://github.com/llvm/llvm-project/commit/279a4d0d67c874e80c171666822f2fabdd6fa926 DIFF: https://github.com/llvm/llvm-project/commit/279a4d0d67c874e80c171666822f2fabdd6fa926.diff LOG: -fsanitize=function: support C With D148785, -fsanitize=function no longer uses C++ RTTI objects and therefore can support C. The rationale for reporting errors is C11 6.5.2.2p9: > If the function is defined with a type that is not compatible with the type > (of the expression) pointed to by the expression that denotes the called > function, the behavior is undefined. The mangled types approach we use does not exactly match the C type compatibility (see `f(callee1)` below). This is probably fine as the rules are unlikely leveraged in practice. In addition, the call is warned by -Wincompatible-function-pointer-types-strict. ``` void callee0(int (*a)[]) {} void callee1(int (*a)[1]) {} void f(void (*fp)(int (*)[])) { fp(0); } int main() { int a[1]; f(callee0); f(callee1); // compatible but flagged by -fsanitize=function, -fsanitize=kcfi, and -Wincompatible-function-pointer-types-strict } ``` Skip indirect call sites of a function type without a prototype to avoid deal with C11 6.5.2.2p6. -fsanitize=kcfi skips such calls as well. Reviewed By: #sanitizers, vitalybuka Differential Revision: https://reviews.llvm.org/D148827 Added: clang/test/CodeGen/ubsan-function.c compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c Modified: clang/docs/ControlFlowIntegrity.rst clang/docs/UndefinedBehaviorSanitizer.rst clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.cpp compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py Removed: ################################################################################ diff --git a/clang/docs/ControlFlowIntegrity.rst b/clang/docs/ControlFlowIntegrity.rst index f375199f40617..7de805e2154db 100644 --- a/clang/docs/ControlFlowIntegrity.rst +++ b/clang/docs/ControlFlowIntegrity.rst @@ -314,10 +314,8 @@ to find bugs in local development builds, whereas ``-fsanitize=cfi-icall`` is a security hardening mechanism designed to be deployed in release builds. ``-fsanitize=function`` has a higher space and time overhead due to a more -complex type check at indirect call sites, as well as a need for run-time -type information (RTTI), which may make it unsuitable for deployment. Because -of the need for RTTI, ``-fsanitize=function`` can only be used with C++ -programs, whereas ``-fsanitize=cfi-icall`` can protect both C and C++ programs. +complex type check at indirect call sites, which may make it unsuitable for +deployment. On the other hand, ``-fsanitize=function`` conforms more closely with the C++ standard and user expectations around interaction with shared libraries; diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst index ff02ce8ad892d..bfcdeba0632e5 100644 --- a/clang/docs/UndefinedBehaviorSanitizer.rst +++ b/clang/docs/UndefinedBehaviorSanitizer.rst @@ -100,7 +100,7 @@ Available checks are: by Clang (and by ISO/IEC/IEEE 60559 / IEEE 754) as producing either an infinity or NaN value, so is not included in ``-fsanitize=undefined``. - ``-fsanitize=function``: Indirect call of a function through a - function pointer of the wrong type (C++ only). + function pointer of the wrong type. - ``-fsanitize=implicit-unsigned-integer-truncation``, ``-fsanitize=implicit-signed-integer-truncation``: Implicit conversion from integer of larger bit width to smaller bit width, if that results in data diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index a915849a9822d..736f3e8bf2d16 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5349,8 +5349,9 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee CGCallee Callee = OrigCallee; - if (getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function) && - (!TargetDecl || !isa<FunctionDecl>(TargetDecl))) { + if (SanOpts.has(SanitizerKind::Function) && + (!TargetDecl || !isa<FunctionDecl>(TargetDecl)) && + !isa<FunctionNoProtoType>(PointeeType)) { if (llvm::Constant *PrefixSig = CGM.getTargetCodeGenInfo().getUBSanFunctionSignature(CGM)) { SanitizerScope SanScope(this); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index e52c4831087f7..a0a6299039db3 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -572,10 +572,11 @@ llvm::ConstantInt * CodeGenFunction::getUBSanFunctionTypeHash(QualType Ty) const { // Remove any (C++17) exception specifications, to allow calling e.g. a // noexcept function through a non-noexcept pointer. - auto ProtoTy = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None); + if (!isa<FunctionNoProtoType>(Ty)) + Ty = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None); std::string Mangled; llvm::raw_string_ostream Out(Mangled); - CGM.getCXXABI().getMangleContext().mangleTypeName(ProtoTy, Out, false); + CGM.getCXXABI().getMangleContext().mangleTypeName(Ty, Out, false); return llvm::ConstantInt::get(CGM.Int32Ty, static_cast<uint32_t>(llvm::xxHash64(Mangled))); } @@ -945,7 +946,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy, // If we are checking function types, emit a function type signature as // prologue data. - if (FD && getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function)) { + if (FD && SanOpts.has(SanitizerKind::Function)) { if (llvm::Constant *PrologueSig = getPrologueSignature(CGM, FD)) { llvm::LLVMContext &Ctx = Fn->getContext(); llvm::MDBuilder MDB(Ctx); diff --git a/clang/test/CodeGen/ubsan-function.c b/clang/test/CodeGen/ubsan-function.c new file mode 100644 index 0000000000000..c3113757b468a --- /dev/null +++ b/clang/test/CodeGen/ubsan-function.c @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -emit-llvm -triple x86_64 -std=c17 -fsanitize=function %s -o - | FileCheck %s + +// CHECK-LABEL: define{{.*}} @call_no_prototype( +// CHECK-NOT: __ubsan_handle_function_type_mismatch +void call_no_prototype(void (*f)()) { f(); } + +// CHECK-LABEL: define{{.*}} @call_prototype( +// CHECK: __ubsan_handle_function_type_mismatch +void call_prototype(void (*f)(void)) { f(); } diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c new file mode 100644 index 0000000000000..4e92f96e4100b --- /dev/null +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c @@ -0,0 +1,14 @@ +// RUN: %clang -g -fsanitize=function %s -o %t +// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --implicit-check-not='runtime error:' + +void f(void (*fp)(int (*)[])) { fp(0); } + +void callee0(int (*a)[]) {} +void callee1(int (*a)[1]) {} + +int main() { + int a[1]; + f(callee0); + // CHECK: runtime error: call to function callee1 through pointer to incorrect function type 'void (*)(int (*)[])' + f(callee1); // compatible type in C, but flagged +} diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp index 645ec3c8c6d76..2f7db994364ca 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp @@ -1,6 +1,3 @@ -// Work around "Cannot represent a diff erence across sections" -// UNSUPPORTED: target=powerpc64-{{.*}} - // RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique // RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC -shared -o %t-so.so // RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py index 8d6451c5f2f0a..b5524bf1fd180 100644 --- a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py @@ -1,5 +1,8 @@ if config.host_os not in ['Darwin', 'FreeBSD', 'Linux', 'NetBSD']: config.unsupported = True +# Work around "Cannot represent a diff erence across sections" +if config.target_arch == 'powerpc64': + config.unsupported = True # Work around "library ... not found: needed by main executable" in qemu. if config.android and config.target_arch not in ['x86', 'x86_64']: config.unsupported = True _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
