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 d017d1f829015..88533cdf9015a 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 3b267c1b1693d..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,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(); + 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 (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) { + 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 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..0151c794036ec --- /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}}