[clang] [clang][Sema] Fix incomplete C mode incompatible ExtInfo/ExtProtoInfo conversion diagnostic (PR #160477)
Bruno De Fraine via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 30 05:39:34 PDT 2025
https://github.com/brunodf-snps updated https://github.com/llvm/llvm-project/pull/160477
>From ef33bff20de389be8896f99112f2c6179d79b23e Mon Sep 17 00:00:00 2001
From: Bruno De Fraine <brunodf at synopsys.com>
Date: Wed, 24 Sep 2025 11:31:04 +0200
Subject: [PATCH 1/2] [clang][Sema] Fix incomplete C mode incompatible
ExtInfo/ExtProtoInfo conversion diagnostic
To raise -Wincompatible-function-pointer-type for extended function type
information conversions in C mode, checkPointerTypesForAssignment
contained an invocation of IsFunctionConversion that is quite dubious:
it verifies the conversion from LHS to RHS, while we are converting from
RHS to LHS. Presumably, this was done because result and argument types
don't have to match exactly in C mode (compatible types is sufficient),
while IsFunctionConversion checks that an exact match in function types
can be obtained by transforming extended function type information. As a
result of the dubious invocation, the diagnostic for incompatible
extended function type changes was incomplete and would illogically
disappear when other changes to the function type were involved (this
was already raised in 2019 but left unsolved). Newer additions of
extended function type information such as "cfi_unchecked_callee" and
function effect attributes, worked around the issue by introducing more
complex checking logic that could open other holes.
Here, we solve this longstanding issue by using IsFunctionConverions to
verify the extended function type information conversion in the correct
direction (from RHS to LHS), after first removing other differences in
the function type (argument and result type mismatch, prototype vs. no
prototype) to allow IsFunctionConversion to obtain an exact match. The
complications for checking "cfi_unchecked_callee" and function effect
attributes are removed.
Fixes #160474
---
clang/include/clang/Sema/Sema.h | 15 +++----
clang/lib/Sema/SemaExpr.cpp | 43 ++++++++++++++++---
clang/lib/Sema/SemaOverload.cpp | 32 +++-----------
...ompatible-function-pointer-types-extinfo.c | 39 +++++++++++++++++
4 files changed, 87 insertions(+), 42 deletions(-)
create mode 100644 clang/test/Sema/incompatible-function-pointer-types-extinfo.c
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d017d1f829015..503fab7190c40 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10208,15 +10208,12 @@ class Sema final : public SemaBase {
bool CStyle, bool &ObjCLifetimeConversion);
/// Determine whether the conversion from FromType to ToType is a valid
- /// conversion that strips "noexcept" or "noreturn" or "cfi_unchecked_callee"
- /// off the nested function type. This also checks if "cfi_unchecked_callee"
- /// was added to the function type. If "cfi_unchecked_callee" is added and
- /// `AddingCFIUncheckedCallee` is provided, it will be set to true. The same
- /// thing applies for `DiscardingCFIUncheckedCallee` if the attribute is
- /// discarded.
- bool IsFunctionConversion(QualType FromType, QualType ToType,
- bool *DiscardingCFIUncheckedCallee = nullptr,
- bool *AddingCFIUncheckedCallee = nullptr) const;
+ /// conversion of ExtInfo/ExtProtoInfo on the nested function type.
+ /// More precisely, this method checks whether FromType can be tranformed
+ /// into an exact match for ToType, by transforming its extended function
+ /// type information in legal manner (e.g. by strictly stripping "noreturn"
+ /// or "noexcept", or by stripping "noescape" for arguments).
+ bool IsFunctionConversion(QualType FromType, QualType ToType) const;
/// Same as `IsFunctionConversion`, but if this would return true, it sets
/// `ResultTy` to `ToType`.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3b267c1b1693d..6cb55659581d2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9187,12 +9187,43 @@ static AssignConvertType checkPointerTypesForAssignment(Sema &S,
return AssignConvertType::IncompatibleFunctionPointer;
return AssignConvertType::IncompatiblePointer;
}
- bool DiscardingCFIUncheckedCallee, AddingCFIUncheckedCallee;
- if (!S.getLangOpts().CPlusPlus &&
- S.IsFunctionConversion(ltrans, rtrans, &DiscardingCFIUncheckedCallee,
- &AddingCFIUncheckedCallee)) {
- // Allow conversions between CFIUncheckedCallee-ness.
- if (!DiscardingCFIUncheckedCallee && !AddingCFIUncheckedCallee)
+ const auto *LFT = ltrans->getAs<FunctionType>();
+ const auto *RFT = rtrans->getAs<FunctionType>();
+ if (!S.getLangOpts().CPlusPlus && LFT && RFT) {
+ // The invocation of IsFunctionConversion below will try to transform rtrans
+ // to obtain an exact match for ltrans. This should not fail because of
+ // mismatches in result type and parameter types, they were already checked
+ // by typesAreCompatible above. So we will recreate rtrans using the result
+ // type and parameter types from ltrans, but keeping its
+ // ExtInfo/ExtProtoInfo.
+ const auto *LFPT = dyn_cast<FunctionProtoType>(LFT);
+ const auto *RFPT = dyn_cast<FunctionProtoType>(RFT);
+ if (LFPT && RFPT) {
+ rtrans = S.Context.getFunctionType(LFPT->getReturnType(),
+ LFPT->getParamTypes(),
+ RFPT->getExtProtoInfo());
+ } else if (LFPT) {
+ FunctionProtoType::ExtProtoInfo EPI;
+ EPI.ExtInfo = RFT->getExtInfo();
+ rtrans = S.Context.getFunctionType(LFPT->getReturnType(),
+ LFPT->getParamTypes(), EPI);
+ } else if (RFPT) {
+ // In this case, we want to retain rtrans as a FunctionProtoType, to keep
+ // all of its ExtProtoInfo. To enable an exact match, we recreate ltrans
+ // as a FunctionProtoType, using its own info but filling in the parameter
+ // types from rtrans.
+ FunctionProtoType::ExtProtoInfo EPI;
+ EPI.ExtInfo = LFT->getExtInfo();
+ ltrans = S.Context.getFunctionType(LFT->getReturnType(),
+ RFPT->getParamTypes(), EPI);
+ rtrans = S.Context.getFunctionType(
+ LFT->getReturnType(), RFPT->getParamTypes(), RFPT->getExtProtoInfo());
+ } else {
+ rtrans = S.Context.getFunctionNoProtoType(LFT->getReturnType(),
+ RFT->getExtInfo());
+ }
+ if (!S.Context.hasSameUnqualifiedType(rtrans, ltrans) &&
+ !S.IsFunctionConversion(rtrans, ltrans))
return AssignConvertType::IncompatibleFunctionPointer;
}
if (IsInvalidCmseNSCallConversion(S, ltrans, rtrans))
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index ea5c4265d736d..84b4b83e573e6 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1891,14 +1891,7 @@ bool Sema::TryFunctionConversion(QualType FromType, QualType ToType,
return Changed;
}
-bool Sema::IsFunctionConversion(QualType FromType, QualType ToType,
- bool *DiscardingCFIUncheckedCallee,
- bool *AddingCFIUncheckedCallee) const {
- if (DiscardingCFIUncheckedCallee)
- *DiscardingCFIUncheckedCallee = false;
- if (AddingCFIUncheckedCallee)
- *AddingCFIUncheckedCallee = false;
-
+bool Sema::IsFunctionConversion(QualType FromType, QualType ToType) const {
if (Context.hasSameUnqualifiedType(FromType, ToType))
return false;
@@ -1957,25 +1950,14 @@ bool Sema::IsFunctionConversion(QualType FromType, QualType ToType,
const auto *ToFPT = dyn_cast<FunctionProtoType>(ToFn);
if (FromFPT && ToFPT) {
- if (FromFPT->hasCFIUncheckedCallee() && !ToFPT->hasCFIUncheckedCallee()) {
- QualType NewTy = Context.getFunctionType(
- FromFPT->getReturnType(), FromFPT->getParamTypes(),
- FromFPT->getExtProtoInfo().withCFIUncheckedCallee(false));
- FromFPT = cast<FunctionProtoType>(NewTy.getTypePtr());
- FromFn = FromFPT;
- Changed = true;
- if (DiscardingCFIUncheckedCallee)
- *DiscardingCFIUncheckedCallee = true;
- } else if (!FromFPT->hasCFIUncheckedCallee() &&
- ToFPT->hasCFIUncheckedCallee()) {
+ if (FromFPT->hasCFIUncheckedCallee() != ToFPT->hasCFIUncheckedCallee()) {
QualType NewTy = Context.getFunctionType(
FromFPT->getReturnType(), FromFPT->getParamTypes(),
- FromFPT->getExtProtoInfo().withCFIUncheckedCallee(true));
+ FromFPT->getExtProtoInfo().withCFIUncheckedCallee(
+ ToFPT->hasCFIUncheckedCallee()));
FromFPT = cast<FunctionProtoType>(NewTy.getTypePtr());
FromFn = FromFPT;
Changed = true;
- if (AddingCFIUncheckedCallee)
- *AddingCFIUncheckedCallee = true;
}
}
@@ -2006,11 +1988,7 @@ bool Sema::IsFunctionConversion(QualType FromType, QualType ToType,
Changed = true;
}
- // For C, when called from checkPointerTypesForAssignment,
- // we need to not alter FromFn, or else even an innocuous cast
- // like dropping effects will fail. In C++ however we do want to
- // alter FromFn (because of the way PerformImplicitConversion works).
- if (Context.hasAnyFunctionEffects() && getLangOpts().CPlusPlus) {
+ if (Context.hasAnyFunctionEffects()) {
FromFPT = cast<FunctionProtoType>(FromFn); // in case FromFn changed above
// Transparently add/drop effects; here we are concerned with
diff --git a/clang/test/Sema/incompatible-function-pointer-types-extinfo.c b/clang/test/Sema/incompatible-function-pointer-types-extinfo.c
new file mode 100644
index 0000000000000..d82313df172f0
--- /dev/null
+++ b/clang/test/Sema/incompatible-function-pointer-types-extinfo.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only %s -std=c99 -verify=expected
+// RUN: %clang_cc1 -fsyntax-only %s -std=c11 -verify=expected
+// RUN: %clang_cc1 -fsyntax-only %s -std=c23 -verify=expected,proto
+
+// This verifies that -Wincompatible-function-pointer-type diagnostics for
+// extended function type information are consistent, also in case of other
+// allowed funcion type difference in C.
+//
+// Test case adapted from issue #160474.
+
+enum E { A = -1, B };
+
+// Case 1: assignment adding noreturn
+
+int f1 (int);
+int (*fp1a)(int) __attribute__((noreturn)) = &f1; // expected-error {{incompatible function pointer types}}
+enum E (*fp1b)(int) __attribute__((noreturn)) = &f1; // expected-error {{incompatible function pointer types}}
+int (*fp1c)() __attribute__((noreturn)) = &f1; // expected-error {{incompatible function pointer types}}
+
+// Case 2: assignment adding noescape on arg
+
+int f2 (int* ) __attribute__((noreturn));
+int (*fp2a)(int* __attribute__((noescape))) __attribute__((noreturn)) = &f2; // expected-error {{incompatible function pointer types}}
+int (*fp2b)(int* __attribute__((noescape))) = &f2; // expected-error {{incompatible function pointer types}}
+
+// Case 3: assignment adding cfi_unchecked_callee
+
+int f3 (int* );
+int (*fp3a)(int* ) __attribute__((noreturn )) = &f3; // expected-error {{incompatible function pointer types}}
+int (*fp3b)(int* __attribute__((noescape))) = &f3; // expected-error {{incompatible function pointer types}}
+int (*fp3c)(int* ) __attribute__((noreturn,cfi_unchecked_callee)) = &f3; // expected-error {{incompatible function pointer types}}
+int (*fp3d)(int* __attribute__((noescape))) __attribute__(( cfi_unchecked_callee)) = &f3; // expected-error {{incompatible function pointer types}}
+int (*fp3e)(int* __attribute__((noescape))) __attribute__((noreturn,cfi_unchecked_callee)) = &f3; // expected-error {{incompatible function pointer types}}
+
+// Case 4: assignment to function with no prototype
+
+int f4 (int);
+int (*fp4a)(int) = &f4;
+int (*fp4b)() = &f4; // proto-error {{incompatible function pointer types}}
>From f9dcba220758e9bf21cc90297ecfb77b3abd64b5 Mon Sep 17 00:00:00 2001
From: Bruno De Fraine <brunodf at synopsys.com>
Date: Tue, 30 Sep 2025 13:52:23 +0200
Subject: [PATCH 2/2] SemaExpr: further tweaks to
checkPointerTypesForAssignment
- Don't transform both ltrans and rtrans when one transform could
suffice.
- Further simplify unneeded code checks that were added to work around
the invalid invocation.
- Fix formatting in test case.
---
clang/lib/Sema/SemaExpr.cpp | 38 ++++---------------
...ompatible-function-pointer-types-extinfo.c | 2 +-
2 files changed, 8 insertions(+), 32 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6cb55659581d2..225b3d2ff1a42 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9014,24 +9014,6 @@ bool Sema::IsInvalidSMECallConversion(QualType FromType, QualType ToType) {
return FromAttributes != ToAttributes;
}
-// Check if we have a conversion between incompatible cmse function pointer
-// types, that is, a conversion between a function pointer with the
-// cmse_nonsecure_call attribute and one without.
-static bool IsInvalidCmseNSCallConversion(Sema &S, QualType FromType,
- QualType ToType) {
- if (const auto *ToFn =
- dyn_cast<FunctionType>(S.Context.getCanonicalType(ToType))) {
- if (const auto *FromFn =
- dyn_cast<FunctionType>(S.Context.getCanonicalType(FromType))) {
- FunctionType::ExtInfo ToEInfo = ToFn->getExtInfo();
- FunctionType::ExtInfo FromEInfo = FromFn->getExtInfo();
-
- return ToEInfo.getCmseNSCall() != FromEInfo.getCmseNSCall();
- }
- }
- return false;
-}
-
// checkPointerTypesForAssignment - This is a very tricky routine (despite
// being closely modeled after the C99 spec:-). The odd characteristic of this
// routine is it effectively iqnores the qualifiers on the top level pointee.
@@ -9187,15 +9169,17 @@ static AssignConvertType checkPointerTypesForAssignment(Sema &S,
return AssignConvertType::IncompatibleFunctionPointer;
return AssignConvertType::IncompatiblePointer;
}
+ // Note: in C++, typesAreCompatible(ltrans, rtrans) will have guaranteed
+ // hasSameType, so we can skip further checks.
const auto *LFT = ltrans->getAs<FunctionType>();
const auto *RFT = rtrans->getAs<FunctionType>();
if (!S.getLangOpts().CPlusPlus && LFT && RFT) {
// The invocation of IsFunctionConversion below will try to transform rtrans
// to obtain an exact match for ltrans. This should not fail because of
// mismatches in result type and parameter types, they were already checked
- // by typesAreCompatible above. So we will recreate rtrans using the result
- // type and parameter types from ltrans, but keeping its
- // ExtInfo/ExtProtoInfo.
+ // by typesAreCompatible above. So we will recreate rtrans (or where
+ // appropriate ltrans) using the result type and parameter types from ltrans
+ // (respectively rtrans), but keeping its ExtInfo/ExtProtoInfo.
const auto *LFPT = dyn_cast<FunctionProtoType>(LFT);
const auto *RFPT = dyn_cast<FunctionProtoType>(RFT);
if (LFPT && RFPT) {
@@ -9209,15 +9193,11 @@ static AssignConvertType checkPointerTypesForAssignment(Sema &S,
LFPT->getParamTypes(), EPI);
} else if (RFPT) {
// In this case, we want to retain rtrans as a FunctionProtoType, to keep
- // all of its ExtProtoInfo. To enable an exact match, we recreate ltrans
- // as a FunctionProtoType, using its own info but filling in the parameter
- // types from rtrans.
+ // all of its ExtProtoInfo. Transform ltrans instead.
FunctionProtoType::ExtProtoInfo EPI;
EPI.ExtInfo = LFT->getExtInfo();
- ltrans = S.Context.getFunctionType(LFT->getReturnType(),
+ ltrans = S.Context.getFunctionType(RFPT->getReturnType(),
RFPT->getParamTypes(), EPI);
- rtrans = S.Context.getFunctionType(
- LFT->getReturnType(), RFPT->getParamTypes(), RFPT->getExtProtoInfo());
} else {
rtrans = S.Context.getFunctionNoProtoType(LFT->getReturnType(),
RFT->getExtInfo());
@@ -9226,10 +9206,6 @@ static AssignConvertType checkPointerTypesForAssignment(Sema &S,
!S.IsFunctionConversion(rtrans, ltrans))
return AssignConvertType::IncompatibleFunctionPointer;
}
- if (IsInvalidCmseNSCallConversion(S, ltrans, rtrans))
- return AssignConvertType::IncompatibleFunctionPointer;
- if (S.IsInvalidSMECallConversion(rtrans, ltrans))
- return AssignConvertType::IncompatibleFunctionPointer;
return ConvTy;
}
diff --git a/clang/test/Sema/incompatible-function-pointer-types-extinfo.c b/clang/test/Sema/incompatible-function-pointer-types-extinfo.c
index d82313df172f0..0151c794036ec 100644
--- a/clang/test/Sema/incompatible-function-pointer-types-extinfo.c
+++ b/clang/test/Sema/incompatible-function-pointer-types-extinfo.c
@@ -34,6 +34,6 @@ int (*fp3e)(int* __attribute__((noescape))) __attribute__((noreturn,cfi_unchecke
// Case 4: assignment to function with no prototype
-int f4 (int);
+int f4 (int);
int (*fp4a)(int) = &f4;
int (*fp4b)() = &f4; // proto-error {{incompatible function pointer types}}
More information about the cfe-commits
mailing list