samitolvanen created this revision.
Herald added a project: All.
samitolvanen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136790
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/incompatible-function-pointer-types-strict.c
Index: clang/test/Sema/incompatible-function-pointer-types-strict.c
===================================================================
--- /dev/null
+++ 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
+}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9240,7 +9240,8 @@
// 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 @@
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 @@
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 @@
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;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12188,6 +12188,12 @@
/// 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 different sign, but are otherwise
/// identical. This is a subset of the above, but broken out because it's by
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8212,6 +8212,9 @@
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 different types}0,1"
"|%diff{passing $ to parameter of type $|"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
