[clang] 41ce74e - [Clang][Sema] Add -Wincompatible-function-pointer-types-strict

[Sami Tolvanen via cfe-commits]

Author: Sami Tolvanen
Date: 2022-11-07T23:05:28Z
New Revision: 41ce74e6e983f523d44d3a80be5ae778c35df85a

URL: https://github.com/llvm/llvm-project/commit/41ce74e6e983f523d44d3a80be5ae778c35df85a
DIFF: https://github.com/llvm/llvm-project/commit/41ce74e6e983f523d44d3a80be5ae778c35df85a.diff

LOG: [Clang][Sema] Add -Wincompatible-function-pointer-types-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 help developers avoid
function pointer assignments that can lead to runtime CFI
failures. -Wincompatible-function-pointer-types doesn't warn about
enum to integer mismatches if the types are otherwise compatible, for
example, which isn't sufficient with CFI.

Add -Wincompatible-function-pointer-types-strict, which checks for a
stricter function type compatibility in assignments and helps warn about
assignments that can potentially lead to CFI failures.

Reviewed By: aaron.ballman, nickdesaulniers

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

Added: 
    clang/test/Sema/incompatible-function-pointer-types-strict.c

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaExpr.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 72b9a52c7cd0d..2ce5fd48ca13a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -366,11 +366,14 @@ 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.
+- Introduced ``-Wcast-function-type-strict`` and
+  ``-Wincompatible-function-pointer-types-strict`` to warn about function type
+  mismatches in casts and assignments that may result in runtime indirect call
+  `Control-Flow Integrity (CFI)
+  <https://clang.llvm.org/docs/ControlFlowIntegrity.html>`_ failures. The
+  ``-Wcast-function-type-strict`` diagnostic is grouped under
+  ``-Wcast-function-type`` as it identifies a more strict set of potentially
+  problematic function type casts.
 - Clang will now disambiguate NTTP types when printing diagnostic that contain NTTP types.
   Fixes `Issue 57562 <https://github.com/llvm/llvm-project/issues/57562>`_.
 - Better error recovery for pack expansion of expressions.

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1b1db765fa7a9..eea38a4cab8a3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8214,6 +8214,9 @@ def err_typecheck_convert_incompatible_function_pointer : Error<
 def ext_typecheck_convert_incompatible_function_pointer : ExtWarn<
   err_typecheck_convert_incompatible_function_pointer.Text>,
   InGroup<IncompatibleFunctionPointerTypes>, DefaultError;
+def warn_typecheck_convert_incompatible_function_pointer_strict : Warning<
+  err_typecheck_convert_incompatible_function_pointer.Text>,
+  InGroup<DiagGroup<"incompatible-function-pointer-types-strict">>, DefaultIgnore;
 def ext_typecheck_convert_discards_qualifiers : ExtWarn<
   "%select{%
diff {assigning to $ from $|assigning to 
diff erent types}0,1"
   "|%
diff {passing $ to parameter of type $|"

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e8c9cb966bae7..25d9d2e0c3baa 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12228,6 +12228,12 @@ class Sema final {
     /// extension.
     IncompatibleFunctionPointer,
 
+    /// IncompatibleFunctionPointerStrict - The assignment is between two
+    /// function pointer types that are not identical, but are compatible,
+    /// unless compiled with -fsanitize=cfi, in which case the type mismatch
+    /// may trip an indirect call runtime check.
+    IncompatibleFunctionPointerStrict,
+
     /// IncompatiblePointerSign - The assignment is between two pointers types
     /// which point to integers which have a 
diff erent sign, but are otherwise
     /// identical. This is a subset of the above, but broken out because it's by

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c40bcb083907b..1ba88ad6cc2a6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9240,7 +9240,8 @@ static bool IsInvalidCmseNSCallConversion(Sema &S, QualType FromType,
 // This circumvents the usual type rules specified in 6.2.7p1 & 6.7.5.[1-3].
 // FIXME: add a couple examples in this comment.
 static Sema::AssignConvertType
-checkPointerTypesForAssignment(Sema &S, QualType LHSType, QualType RHSType) {
+checkPointerTypesForAssignment(Sema &S, QualType LHSType, QualType RHSType,
+                               SourceLocation Loc) {
   assert(LHSType.isCanonical() && "LHS not canonicalized!");
   assert(RHSType.isCanonical() && "RHS not canonicalized!");
 
@@ -9309,6 +9310,13 @@ checkPointerTypesForAssignment(Sema &S, QualType LHSType, QualType RHSType) {
     return Sema::FunctionVoidPointer;
   }
 
+  if (!S.Diags.isIgnored(
+          diag::warn_typecheck_convert_incompatible_function_pointer_strict,
+          Loc) &&
+      RHSType->isFunctionPointerType() && LHSType->isFunctionPointerType() &&
+      !S.IsFunctionConversion(RHSType, LHSType, RHSType))
+    return Sema::IncompatibleFunctionPointerStrict;
+
   // C99 6.5.16.1p1 (constraint 3): both operands are pointers to qualified or
   // unqualified versions of compatible types, ...
   QualType ltrans = QualType(lhptee, 0), rtrans = QualType(rhptee, 0);
@@ -9660,7 +9668,8 @@ Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS,
         Kind = CK_NoOp;
       else
         Kind = CK_BitCast;
-      return checkPointerTypesForAssignment(*this, LHSType, RHSType);
+      return checkPointerTypesForAssignment(*this, LHSType, RHSType,
+                                            RHS.get()->getBeginLoc());
     }
 
     // int -> T*
@@ -16949,6 +16958,12 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
     ConvHints.tryToFixConversion(SrcExpr, SrcType, DstType, *this);
     MayHaveConvFixit = true;
     break;
+  case IncompatibleFunctionPointerStrict:
+    DiagKind =
+        diag::warn_typecheck_convert_incompatible_function_pointer_strict;
+    ConvHints.tryToFixConversion(SrcExpr, SrcType, DstType, *this);
+    MayHaveConvFixit = true;
+    break;
   case IncompatibleFunctionPointer:
     if (getLangOpts().CPlusPlus) {
       DiagKind = diag::err_typecheck_convert_incompatible_function_pointer;

diff  --git a/clang/test/Sema/incompatible-function-pointer-types-strict.c b/clang/test/Sema/incompatible-function-pointer-types-strict.c
new file mode 100644
index 0000000000000..647251de42030
--- /dev/null
+++ b/clang/test/Sema/incompatible-function-pointer-types-strict.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only %s -Wincompatible-function-pointer-types-strict -verify=soft,strict
+// RUN: %clang_cc1 -fsyntax-only %s -Werror=incompatible-function-pointer-types-strict -verify=hard,strict
+// RUN: %clang_cc1 -fsyntax-only %s -Wincompatible-function-pointer-types -verify=nonstrict
+// nonstrict-no-diagnostics
+
+enum E { A = -1, B };
+typedef enum E (*fn_a_t)(void);
+typedef void (*fn_b_t)(void);
+
+int a(void) { return 0; }
+void __attribute__((noreturn)) b(void) { while (1); }
+
+void fa(fn_a_t x) {} // strict-note {{passing argument to parameter 'x' here}}
+void fb(fn_b_t x) {}
+
+void baz(void) {
+  fa(&a); // soft-warning {{incompatible function pointer types passing 'int (*)(void)' to parameter of type 'fn_a_t' (aka 'enum E (*)(void)')}} \
+             hard-error {{incompatible function pointer types passing 'int (*)(void)' to parameter of type 'fn_a_t' (aka 'enum E (*)(void)')}}
+  fb(&b); // no-warning
+}