[clang] a03aef4 - [Clang] [Sema] Fix incomplete C mode incompatible ExtInfo/ExtProtoInfo conversion diagnostic (#160477)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 9 08:51:02 PDT 2025
Author: Bruno De Fraine
Date: 2025-10-09T17:30:25+02:00
New Revision: a03aef4faedb06ac0a70846dc4deeffd0ee4bff0
URL: https://github.com/llvm/llvm-project/commit/a03aef4faedb06ac0a70846dc4deeffd0ee4bff0
DIFF: https://github.com/llvm/llvm-project/commit/a03aef4faedb06ac0a70846dc4deeffd0ee4bff0.diff
LOG: [Clang] [Sema] Fix incomplete C mode incompatible ExtInfo/ExtProtoInfo conversion diagnostic (#160477)
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", ARM
CMSE attributes, ARM SME attributes, and function effect attributes,
worked around the issue by introducing either holes or additional checks
which complicates the logic.
Here, we solve this longstanding issue by using IsFunctionConversion 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 introduced for checking newer attributes are removed.
Fixes #41465
Fixes #85415
Added:
clang/test/Sema/incompatible-function-pointer-types-extinfo.c
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9a0d69c6c1b0e..ca7e1330d0a76 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -357,6 +357,11 @@ Improvements to Clang's diagnostics
properly being rejected when used at compile-time. It was not implemented
and caused assertion failures before (#GH158471).
+- Closed a loophole in the diagnosis of function pointer conversions changing
+ extended function type information in C mode (#GH41465). Function conversions
+ that were previously incorrectly accepted in case of other irrelevant
+ conditions are now consistently diagnosed, identical to C++ mode.
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 265462a86e405..37598f8530c09 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 transformed
+ /// 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 4d3c7d611f370..4230ea702e128 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,18 +9169,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)
+ // 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 (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) {
+ 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. Transform ltrans instead.
+ FunctionProtoType::ExtProtoInfo EPI;
+ EPI.ExtInfo = LFT->getExtInfo();
+ ltrans = S.Context.getFunctionType(RFPT->getReturnType(),
+ RFPT->getParamTypes(), EPI);
+ } 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))
- return AssignConvertType::IncompatibleFunctionPointer;
- if (S.IsInvalidSMECallConversion(rtrans, ltrans))
- return AssignConvertType::IncompatibleFunctionPointer;
return ConvTy;
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 8d32ef6707381..8339bb19a1edc 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1892,14 +1892,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;
@@ -1958,25 +1951,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;
}
}
@@ -2007,11 +1989,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..b4de356b154fc
--- /dev/null
+++ b/clang/test/Sema/incompatible-function-pointer-types-extinfo.c
@@ -0,0 +1,56 @@
+// 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
diff erence in C.
+//
+// Test case adapted from issue #41465, with suggestions from PR #160477.
+
+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 converting between prototype/no-prototype
+
+void p1(void);
+void i1(int );
+void n1( );
+void p2(void) __attribute__((noreturn));
+void i2(int ) __attribute__((noreturn));
+void n2( ) __attribute__((noreturn));
+
+void (*t1)() = p1;
+void (*t2)() = i1; // proto-error {{incompatible function pointer types}}
+void (*t3)() = n1;
+void (*t4)() __attribute__((noreturn)) = p1; // expected-error {{incompatible function pointer types}}
+void (*t5)() __attribute__((noreturn)) = i1; // expected-error {{incompatible function pointer types}}
+void (*t6)() __attribute__((noreturn)) = n1; // expected-error {{incompatible function pointer types}}
+
+void (*t7)() = p2;
+void (*t8)() = i2; // proto-error {{incompatible function pointer types}}
+void (*t9)() = n2;
+void (*tA)() __attribute__((noreturn)) = p2;
+void (*tB)() __attribute__((noreturn)) = i2; // proto-error {{incompatible function pointer types}}
+void (*tC)() __attribute__((noreturn)) = n2;
More information about the cfe-commits
mailing list