[Clang][Sema] Add -Wcast-function-type-strict

Clang supports indirect call Control-Flow Integrity (CFI) sanitizers
(e.g. -fsanitize=cfi-icall), which enforce an exact type match between
a function pointer and the target function. Unfortunately, Clang
doesn't provide diagnostics that would help developers avoid function
type casts that lead to runtime CFI failures. -Wcast-function-type,
while helpful, only warns about ABI incompatibility, which isn't
sufficient with CFI.

Add -Wcast-function-type-strict, which checks for a strict type
compatibility in function type casts and helps warn about casts that
can potentially lead to CFI failures.

Reviewed By: nickdesaulniers, aaron.ballman

Differential Revision: https://reviews.llvm.org/D134831

Author: samitolvanen

clang/docs/ReleaseNotes.rst

@@ -328,6 +328,11 @@ Improvements to Clang's diagnostics
 - Clang now correctly points to the problematic parameter for the ``-Wnonnull``
   warning. This fixes
   `Issue 58273 <https://github.com/llvm/llvm-project/issues/58273>`_.
+- Introduced ``-Wcast-function-type-strict`` to warn about function type mismatches
+  in casts that may result in runtime indirect call `Control-Flow Integrity (CFI)
+  <https://clang.llvm.org/docs/ControlFlowIntegrity.html>`_ failures. This diagnostic
+  is grouped under ``-Wcast-function-type`` as it identifies a more strict set of
+  potentially problematic function type casts.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

clang/include/clang/Basic/DiagnosticGroups.td

@@ -538,7 +538,8 @@ def PrivateExtern : DiagGroup<"private-extern">;
 def SelTypeCast : DiagGroup<"cast-of-sel-type">;
 def FunctionDefInObjCContainer : DiagGroup<"function-def-in-objc-container">;
 def BadFunctionCast : DiagGroup<"bad-function-cast">;
-def CastFunctionType : DiagGroup<"cast-function-type">;
+def CastFunctionTypeStrict : DiagGroup<"cast-function-type-strict">;
+def CastFunctionType : DiagGroup<"cast-function-type", [CastFunctionTypeStrict]>;
 def ObjCPropertyImpl : DiagGroup<"objc-property-implementation">;
 def ObjCPropertyNoAttribute : DiagGroup<"objc-property-no-attribute">;
 def ObjCPropertyAssignOnObjectType : DiagGroup<"objc-property-assign-on-object-type">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8694,6 +8694,8 @@ def warn_bad_function_cast : Warning<
 def warn_cast_function_type : Warning<
   "cast %diff{from $ to $ |}0,1converts to incompatible function type">,
   InGroup<CastFunctionType>, DefaultIgnore;
+def warn_cast_function_type_strict : Warning<warn_cast_function_type.Text>,
+  InGroup<CastFunctionTypeStrict>, DefaultIgnore;
 def err_cast_pointer_to_non_pointer_int : Error<
   "pointer cannot be cast to type %0">;
 def err_nullptr_cast : Error<

clang/lib/Sema/SemaCast.cpp

@@ -1059,11 +1059,19 @@ static bool argTypeIsABIEquivalent(QualType SrcType, QualType DestType,
   return Context.hasSameUnqualifiedType(SrcType, DestType);
 }
 
-static bool checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
-                                  QualType DestType) {
-  if (Self.Diags.isIgnored(diag::warn_cast_function_type,
-                           SrcExpr.get()->getExprLoc()))
-    return true;
+static unsigned int checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
+                                          QualType DestType) {
+  unsigned int DiagID = 0;
+  const unsigned int DiagList[] = {diag::warn_cast_function_type_strict,
+                                   diag::warn_cast_function_type};
+  for (auto ID : DiagList) {
+    if (!Self.Diags.isIgnored(ID, SrcExpr.get()->getExprLoc())) {
+      DiagID = ID;
+      break;
+    }
+  }
+  if (!DiagID)
+    return 0;
 
   QualType SrcType = SrcExpr.get()->getType();
   const FunctionType *SrcFTy = nullptr;
@@ -1078,10 +1086,17 @@ static bool checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
     SrcFTy = SrcType->castAs<FunctionType>();
     DstFTy = DestType.getNonReferenceType()->castAs<FunctionType>();
   } else {
-    return true;
+    return 0;
   }
   assert(SrcFTy && DstFTy);
 
+  if (Self.Context.hasSameType(SrcFTy, DstFTy))
+    return 0;
+
+  // For strict checks, ensure we have an exact match.
+  if (DiagID == diag::warn_cast_function_type_strict)
+    return DiagID;
+
   auto IsVoidVoid = [](const FunctionType *T) {
     if (!T->getReturnType()->isVoidType())
       return false;
@@ -1092,16 +1107,16 @@ static bool checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
 
   // Skip if either function type is void(*)(void)
   if (IsVoidVoid(SrcFTy) || IsVoidVoid(DstFTy))
-    return true;
+    return 0;
 
   // Check return type.
   if (!argTypeIsABIEquivalent(SrcFTy->getReturnType(), DstFTy->getReturnType(),
                               Self.Context))
-    return false;
+    return DiagID;
 
   // Check if either has unspecified number of parameters
   if (SrcFTy->isFunctionNoProtoType() || DstFTy->isFunctionNoProtoType())
-    return true;
+    return 0;
 
   // Check parameter types.
 
@@ -1114,19 +1129,19 @@ static bool checkCastFunctionType(Sema &Self, const ExprResult &SrcExpr,
   unsigned DstNumParams = DstFPTy->getNumParams();
   if (NumParams > DstNumParams) {
     if (!DstFPTy->isVariadic())
-      return false;
+      return DiagID;
     NumParams = DstNumParams;
   } else if (NumParams < DstNumParams) {
     if (!SrcFPTy->isVariadic())
-      return false;
+      return DiagID;
   }
 
   for (unsigned i = 0; i < NumParams; ++i)
     if (!argTypeIsABIEquivalent(SrcFPTy->getParamType(i),
                                 DstFPTy->getParamType(i), Self.Context))
-      return false;
+      return DiagID;
 
-  return true;
+  return 0;
 }
 
 /// CheckReinterpretCast - Check that a reinterpret_cast\<DestType\>(SrcExpr) is
@@ -1167,8 +1182,8 @@ void CastOperation::CheckReinterpretCast() {
       checkObjCConversion(Sema::CCK_OtherCast);
     DiagnoseReinterpretUpDownCast(Self, SrcExpr.get(), DestType, OpRange);
 
-    if (!checkCastFunctionType(Self, SrcExpr, DestType))
-      Self.Diag(OpRange.getBegin(), diag::warn_cast_function_type)
+    if (unsigned DiagID = checkCastFunctionType(Self, SrcExpr, DestType))
+      Self.Diag(OpRange.getBegin(), DiagID)
           << SrcExpr.get()->getType() << DestType << OpRange;
   } else {
     SrcExpr = ExprError();
@@ -2797,8 +2812,8 @@ void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
     if (Kind == CK_BitCast)
       checkCastAlign();
 
-    if (!checkCastFunctionType(Self, SrcExpr, DestType))
-      Self.Diag(OpRange.getBegin(), diag::warn_cast_function_type)
+    if (unsigned DiagID = checkCastFunctionType(Self, SrcExpr, DestType))
+      Self.Diag(OpRange.getBegin(), DiagID)
           << SrcExpr.get()->getType() << DestType << OpRange;
 
   } else {
@@ -3156,9 +3171,8 @@ void CastOperation::CheckCStyleCast() {
     }
   }
 
-  if (!checkCastFunctionType(Self, SrcExpr, DestType))
-    Self.Diag(OpRange.getBegin(), diag::warn_cast_function_type)
-        << SrcType << DestType << OpRange;
+  if (unsigned DiagID = checkCastFunctionType(Self, SrcExpr, DestType))
+    Self.Diag(OpRange.getBegin(), DiagID) << SrcType << DestType << OpRange;
 
   if (isa<PointerType>(SrcType) && isa<PointerType>(DestType)) {
     QualType SrcTy = cast<PointerType>(SrcType)->getPointeeType();

clang/test/Sema/warn-cast-function-type-strict.c

@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type-strict -verify
+
+
+int t(int array[static 12]);
+int u(int i);
+const int v(int i);
+int x(long);
+
+typedef int (f1)(long);
+typedef int (f2)(void*);
+typedef int (f3)();
+typedef void (f4)();
+typedef void (f5)(void);
+typedef int (f6)(long, int);
+typedef int (f7)(long,...);
+typedef int (f8)(int *);
+typedef int (f9)(const int);
+typedef int (f10)(int);
+
+f1 *a;
+f2 *b;
+f3 *c;
+f4 *d;
+f5 *e;
+f6 *f;
+f7 *g;
+f8 *h;
+f9 *i;
+f10 *j;
+
+void foo(void) {
+  a = (f1 *)x;
+  b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
+  c = (f3 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f3 *' (aka 'int (*)()') converts to incompatible function type}} */
+  d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)()') converts to incompatible function type}} */
+  e = (f5 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f5 *' (aka 'void (*)(void)') converts to incompatible function type}} */
+  f = (f6 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
+  g = (f7 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f7 *' (aka 'int (*)(long, ...)') converts to incompatible function type}} */
+  h = (f8 *)t;
+  i = (f9 *)u;
+  j = (f10 *)v; /* expected-warning {{cast from 'const int (*)(int)' to 'f10 *' (aka 'int (*)(int)') converts to incompatible function type}} */
+}

clang/test/Sema/warn-cast-function-type.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -x c %s -fsyntax-only -Wcast-function-type -triple x86_64-- -verify
+// RUN: %clang_cc1 %s -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -verify
 
 int x(long);
 

clang/test/SemaCXX/warn-cast-function-type-strict.cpp

@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wcast-function-type -verify
+// RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wcast-function-type-strict -verify
+
+int x(long);
+
+typedef int (f1)(long);
+typedef int (f2)(void*);
+typedef int (f3)(...);
+typedef void (f4)(...);
+typedef void (f5)(void);
+typedef int (f6)(long, int);
+typedef int (f7)(long,...);
+typedef int (&f8)(long, int);
+
+f1 *a;
+f2 *b;
+f3 *c;
+f4 *d;
+f5 *e;
+f6 *f;
+f7 *g;
+
+struct S
+{
+  void foo (int*);
+  void bar (int);
+};
+
+typedef void (S::*mf)(int);
+
+void foo() {
+  a = (f1 *)x;
+  b = (f2 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
+  b = reinterpret_cast<f2 *>(x); // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
+  c = (f3 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f3 *' (aka 'int (*)(...)') converts to incompatible function type}}
+  d = (f4 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)(...)') converts to incompatible function type}}
+  e = (f5 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f5 *' (aka 'void (*)()') converts to incompatible function type}}
+  f = (f6 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}}
+  g = (f7 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f7 *' (aka 'int (*)(long, ...)') converts to incompatible function type}}
+
+  mf p1 = (mf)&S::foo; // expected-warning {{cast from 'void (S::*)(int *)' to 'mf' (aka 'void (S::*)(int)') converts to incompatible function type}}
+
+  f8 f2 = (f8)x; // expected-warning {{cast from 'int (long)' to 'f8' (aka 'int (&)(long, int)') converts to incompatible function type}}
+  (void)f2;
+
+  int (^y)(long);
+  f = (f6 *)y; // expected-warning {{cast from 'int (^)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}}
+}

clang/test/SemaCXX/warn-cast-function-type.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -x c++ %s -fblocks -fsyntax-only -Wcast-function-type -triple x86_64-- -verify
+// RUN: %clang_cc1 %s -fblocks -fsyntax-only -Wcast-function-type -Wno-cast-function-type-strict -verify
 
 int x(long);
 
@@ -29,19 +29,19 @@ typedef void (S::*mf)(int);
 
 void foo() {
   a = (f1 *)x;
-  b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
-  b = reinterpret_cast<f2 *>(x); /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
+  b = (f2 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
+  b = reinterpret_cast<f2 *>(x); // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
   c = (f3 *)x;
-  d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)(...)') converts to incompatible function type}} */
+  d = (f4 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)(...)') converts to incompatible function type}}
   e = (f5 *)x;
-  f = (f6 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
+  f = (f6 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}}
   g = (f7 *)x;
 
-  mf p1 = (mf)&S::foo; /* expected-warning {{cast from 'void (S::*)(int *)' to 'mf' (aka 'void (S::*)(int)') converts to incompatible function type}} */
+  mf p1 = (mf)&S::foo; // expected-warning {{cast from 'void (S::*)(int *)' to 'mf' (aka 'void (S::*)(int)') converts to incompatible function type}}
 
-  f8 f2 = (f8)x; /* expected-warning {{cast from 'int (long)' to 'f8' (aka 'int (&)(long, int)') converts to incompatible function type}} */
+  f8 f2 = (f8)x; // expected-warning {{cast from 'int (long)' to 'f8' (aka 'int (&)(long, int)') converts to incompatible function type}}
   (void)f2;
 
   int (^y)(long);
-  f = (f6 *)y; /* expected-warning {{cast from 'int (^)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}} */
+  f = (f6 *)y; // expected-warning {{cast from 'int (^)(long)' to 'f6 *' (aka 'int (*)(long, int)') converts to incompatible function type}}
 }