[PATCH] D148827: -fsanitize=function: support C

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: Sanitizers, efriedma, pcc, peter.smith, sberg, samitolvanen.
Herald added a subscriber: Enna1.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added a subscriber: cfe-commits.

With D148785 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148827

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/ubsan-function.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c


Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c
===================================================================
--- /dev/null
+++ 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
+}
Index: clang/test/CodeGen/ubsan-function.c
===================================================================
--- /dev/null
+++ 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(); }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -572,10 +572,11 @@
 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 @@
 
   // 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);
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5349,8 +5349,9 @@
 
   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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148827.515418.patch
Type: text/x-patch
Size: 3289 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230420/a187ebe0/attachment-0001.bin>