[compiler-rt] 279a4d0 - -fsanitize=function: support C

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Mon May 22 10:11:36 PDT 2023


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


        


More information about the llvm-commits mailing list