From ef33bff20de389be8896f99112f2c6179d79b23e Mon Sep 17 00:00:00 2001 From: Bruno De Fraine Date: Wed, 24 Sep 2025 11:31:04 +0200 Subject: [PATCH 1/3] [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(); + const auto *RFT = rtrans->getAs(); + 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(LFT); + const auto *RFPT = dyn_cast(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(ToFn); if (FromFPT && ToFPT) { - if (FromFPT->hasCFIUncheckedCallee() && !ToFPT->hasCFIUncheckedCallee()) { - QualType NewTy = Context.getFunctionType( - FromFPT->getReturnType(), FromFPT->getParamTypes(), - FromFPT->getExtProtoInfo().withCFIUncheckedCallee(false)); - FromFPT = cast(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(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(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 Date: Tue, 30 Sep 2025 13:52:23 +0200 Subject: [PATCH 2/3] 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(S.Context.getCanonicalType(ToType))) { - if (const auto *FromFn = - dyn_cast(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(); const auto *RFT = rtrans->getAs(); 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(LFT); const auto *RFPT = dyn_cast(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}} From 0c2bfc7777fd4c4df0170f2386ff89c7cff8b266 Mon Sep 17 00:00:00 2001 From: Bruno De Fraine Date: Tue, 7 Oct 2025 12:25:13 +0200 Subject: [PATCH 3/3] Update for reviewer comments - Fix typo in comment - Add release notes entry --- clang/docs/ReleaseNotes.rst | 5 +++++ clang/include/clang/Sema/Sema.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 70c82b090107a..45834abc44073 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -332,6 +332,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 503fab7190c40..88533cdf9015a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -10209,7 +10209,7 @@ class Sema final : public SemaBase { /// Determine whether the conversion from FromType to ToType is a valid /// conversion of ExtInfo/ExtProtoInfo on the nested function type. - /// More precisely, this method checks whether FromType can be tranformed + /// 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).