llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: firstmoonlight <details> <summary>Changes</summary> Fix an assertion failure in Sema::ActOnFriendTypeDecl when parsing an invalid friend type declaration that incorrectly includes a storage-class specifier (e.g., 'static', 'extern', 'register'). Root cause: If the type specifier is marked as invalid, DeclSpec::Finish returns early. However, even when the type specifier is invalid, some other checks can still be performed instead of skipping everything. This change allows necessary checks to proceed, preventing the assertion in ActOnFriendTypeDecl and enabling proper error diagnostics. Fixes: https://github.com/llvm/llvm-project/issues/186569 --- Patch is 27.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/190597.diff 2 Files Affected: - (modified) clang/lib/Sema/DeclSpec.cpp (+259-259) - (modified) clang/test/CXX/class/class.friend/p6.cpp (+1) ``````````diff diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp index 479a959e0aadc..bc3fbe43c7a94 100644 --- a/clang/lib/Sema/DeclSpec.cpp +++ b/clang/lib/Sema/DeclSpec.cpp @@ -1162,286 +1162,286 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) { // Check the type specifier components first. No checking for an invalid // type. - if (TypeSpecType == TST_error) - return; - - // If decltype(auto) is used, no other type specifiers are permitted. - if (TypeSpecType == TST_decltype_auto && - (getTypeSpecWidth() != TypeSpecifierWidth::Unspecified || - TypeSpecComplex != TSC_unspecified || - getTypeSpecSign() != TypeSpecifierSign::Unspecified || - TypeAltiVecVector || TypeAltiVecPixel || TypeAltiVecBool || - TypeQualifiers)) { - const unsigned NumLocs = 9; - SourceLocation ExtraLocs[NumLocs] = { - TSWRange.getBegin(), TSCLoc, TSSLoc, - AltiVecLoc, TQ_constLoc, TQ_restrictLoc, - TQ_volatileLoc, TQ_atomicLoc, TQ_unalignedLoc}; - FixItHint Hints[NumLocs]; - SourceLocation FirstLoc; - for (unsigned I = 0; I != NumLocs; ++I) { - if (ExtraLocs[I].isValid()) { - if (FirstLoc.isInvalid() || - S.getSourceManager().isBeforeInTranslationUnit(ExtraLocs[I], - FirstLoc)) - FirstLoc = ExtraLocs[I]; - Hints[I] = FixItHint::CreateRemoval(ExtraLocs[I]); + if (TypeSpecType != TST_error) { + // If decltype(auto) is used, no other type specifiers are permitted. + if (TypeSpecType == TST_decltype_auto && + (getTypeSpecWidth() != TypeSpecifierWidth::Unspecified || + TypeSpecComplex != TSC_unspecified || + getTypeSpecSign() != TypeSpecifierSign::Unspecified || + TypeAltiVecVector || TypeAltiVecPixel || TypeAltiVecBool || + TypeQualifiers)) { + const unsigned NumLocs = 9; + SourceLocation ExtraLocs[NumLocs] = { + TSWRange.getBegin(), TSCLoc, TSSLoc, + AltiVecLoc, TQ_constLoc, TQ_restrictLoc, + TQ_volatileLoc, TQ_atomicLoc, TQ_unalignedLoc}; + FixItHint Hints[NumLocs]; + SourceLocation FirstLoc; + for (unsigned I = 0; I != NumLocs; ++I) { + if (ExtraLocs[I].isValid()) { + if (FirstLoc.isInvalid() || + S.getSourceManager().isBeforeInTranslationUnit(ExtraLocs[I], + FirstLoc)) + FirstLoc = ExtraLocs[I]; + Hints[I] = FixItHint::CreateRemoval(ExtraLocs[I]); + } } + TypeSpecWidth = static_cast<unsigned>(TypeSpecifierWidth::Unspecified); + TypeSpecComplex = TSC_unspecified; + TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unspecified); + TypeAltiVecVector = TypeAltiVecPixel = TypeAltiVecBool = false; + TypeQualifiers = 0; + S.Diag(TSTLoc, diag::err_decltype_auto_cannot_be_combined) + << Hints[0] << Hints[1] << Hints[2] << Hints[3] + << Hints[4] << Hints[5] << Hints[6] << Hints[7]; } - TypeSpecWidth = static_cast<unsigned>(TypeSpecifierWidth::Unspecified); - TypeSpecComplex = TSC_unspecified; - TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unspecified); - TypeAltiVecVector = TypeAltiVecPixel = TypeAltiVecBool = false; - TypeQualifiers = 0; - S.Diag(TSTLoc, diag::err_decltype_auto_cannot_be_combined) - << Hints[0] << Hints[1] << Hints[2] << Hints[3] - << Hints[4] << Hints[5] << Hints[6] << Hints[7]; - } - // Validate and finalize AltiVec vector declspec. - if (TypeAltiVecVector) { - // No vector long long without VSX (or ZVector). - if ((getTypeSpecWidth() == TypeSpecifierWidth::LongLong) && - !S.Context.getTargetInfo().hasFeature("vsx") && - !S.getLangOpts().ZVector) - S.Diag(TSWRange.getBegin(), diag::err_invalid_vector_long_long_decl_spec); - - // No vector __int128 prior to Power8 (or ZVector). - if ((TypeSpecType == TST_int128) && - !S.Context.getTargetInfo().hasFeature("power8-vector") && - !S.getLangOpts().ZVector) - S.Diag(TSTLoc, diag::err_invalid_vector_int128_decl_spec); - - // Complex vector types are not supported. - if (TypeSpecComplex != TSC_unspecified) - S.Diag(TSCLoc, diag::err_invalid_vector_complex_decl_spec); - else if (TypeAltiVecBool) { - // Sign specifiers are not allowed with vector bool. (PIM 2.1) - if (getTypeSpecSign() != TypeSpecifierSign::Unspecified) { - S.Diag(TSSLoc, diag::err_invalid_vector_bool_decl_spec) - << getSpecifierName(getTypeSpecSign()); - } - // Only char/int are valid with vector bool prior to Power10. - // Power10 adds instructions that produce vector bool data - // for quadwords as well so allow vector bool __int128. - if (((TypeSpecType != TST_unspecified) && (TypeSpecType != TST_char) && - (TypeSpecType != TST_int) && (TypeSpecType != TST_int128)) || - TypeAltiVecPixel) { - S.Diag(TSTLoc, diag::err_invalid_vector_bool_decl_spec) - << (TypeAltiVecPixel ? "__pixel" : - getSpecifierName((TST)TypeSpecType, Policy)); - } - // vector bool __int128 requires Power10 (or ZVector). + // Validate and finalize AltiVec vector declspec. + if (TypeAltiVecVector) { + // No vector long long without VSX (or ZVector). + if ((getTypeSpecWidth() == TypeSpecifierWidth::LongLong) && + !S.Context.getTargetInfo().hasFeature("vsx") && + !S.getLangOpts().ZVector) + S.Diag(TSWRange.getBegin(), diag::err_invalid_vector_long_long_decl_spec); + + // No vector __int128 prior to Power8 (or ZVector). if ((TypeSpecType == TST_int128) && - (!S.Context.getTargetInfo().hasFeature("power10-vector") && - !S.getLangOpts().ZVector)) - S.Diag(TSTLoc, diag::err_invalid_vector_bool_int128_decl_spec); - - // Only 'short' and 'long long' are valid with vector bool. (PIM 2.1) - if ((getTypeSpecWidth() != TypeSpecifierWidth::Unspecified) && - (getTypeSpecWidth() != TypeSpecifierWidth::Short) && - (getTypeSpecWidth() != TypeSpecifierWidth::LongLong)) - S.Diag(TSWRange.getBegin(), diag::err_invalid_vector_bool_decl_spec) - << getSpecifierName(getTypeSpecWidth()); - - // Elements of vector bool are interpreted as unsigned. (PIM 2.1) - if ((TypeSpecType == TST_char) || (TypeSpecType == TST_int) || - (TypeSpecType == TST_int128) || - (getTypeSpecWidth() != TypeSpecifierWidth::Unspecified)) + !S.Context.getTargetInfo().hasFeature("power8-vector") && + !S.getLangOpts().ZVector) + S.Diag(TSTLoc, diag::err_invalid_vector_int128_decl_spec); + + // Complex vector types are not supported. + if (TypeSpecComplex != TSC_unspecified) + S.Diag(TSCLoc, diag::err_invalid_vector_complex_decl_spec); + else if (TypeAltiVecBool) { + // Sign specifiers are not allowed with vector bool. (PIM 2.1) + if (getTypeSpecSign() != TypeSpecifierSign::Unspecified) { + S.Diag(TSSLoc, diag::err_invalid_vector_bool_decl_spec) + << getSpecifierName(getTypeSpecSign()); + } + // Only char/int are valid with vector bool prior to Power10. + // Power10 adds instructions that produce vector bool data + // for quadwords as well so allow vector bool __int128. + if (((TypeSpecType != TST_unspecified) && (TypeSpecType != TST_char) && + (TypeSpecType != TST_int) && (TypeSpecType != TST_int128)) || + TypeAltiVecPixel) { + S.Diag(TSTLoc, diag::err_invalid_vector_bool_decl_spec) + << (TypeAltiVecPixel ? "__pixel" : + getSpecifierName((TST)TypeSpecType, Policy)); + } + // vector bool __int128 requires Power10 (or ZVector). + if ((TypeSpecType == TST_int128) && + (!S.Context.getTargetInfo().hasFeature("power10-vector") && + !S.getLangOpts().ZVector)) + S.Diag(TSTLoc, diag::err_invalid_vector_bool_int128_decl_spec); + + // Only 'short' and 'long long' are valid with vector bool. (PIM 2.1) + if ((getTypeSpecWidth() != TypeSpecifierWidth::Unspecified) && + (getTypeSpecWidth() != TypeSpecifierWidth::Short) && + (getTypeSpecWidth() != TypeSpecifierWidth::LongLong)) + S.Diag(TSWRange.getBegin(), diag::err_invalid_vector_bool_decl_spec) + << getSpecifierName(getTypeSpecWidth()); + + // Elements of vector bool are interpreted as unsigned. (PIM 2.1) + if ((TypeSpecType == TST_char) || (TypeSpecType == TST_int) || + (TypeSpecType == TST_int128) || + (getTypeSpecWidth() != TypeSpecifierWidth::Unspecified)) + TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unsigned); + } else if (TypeSpecType == TST_double) { + // vector long double and vector long long double are never allowed. + // vector double is OK for Power7 and later, and ZVector. + if (getTypeSpecWidth() == TypeSpecifierWidth::Long || + getTypeSpecWidth() == TypeSpecifierWidth::LongLong) + S.Diag(TSWRange.getBegin(), + diag::err_invalid_vector_long_double_decl_spec); + else if (!S.Context.getTargetInfo().hasFeature("vsx") && + !S.getLangOpts().ZVector) + S.Diag(TSTLoc, diag::err_invalid_vector_double_decl_spec); + } else if (TypeSpecType == TST_float) { + // vector float is unsupported for ZVector unless we have the + // vector-enhancements facility 1 (ISA revision 12). + if (S.getLangOpts().ZVector && + !S.Context.getTargetInfo().hasFeature("arch12")) + S.Diag(TSTLoc, diag::err_invalid_vector_float_decl_spec); + } else if (getTypeSpecWidth() == TypeSpecifierWidth::Long) { + // Vector long is unsupported for ZVector, or without VSX, and deprecated + // for AltiVec. + // It has also been historically deprecated on AIX (as an alias for + // "vector int" in both 32-bit and 64-bit modes). It was then made + // unsupported in the Clang-based XL compiler since the deprecated type + // has a number of conflicting semantics and continuing to support it + // is a disservice to users. + if (S.getLangOpts().ZVector || + !S.Context.getTargetInfo().hasFeature("vsx") || + S.Context.getTargetInfo().getTriple().isOSAIX()) + S.Diag(TSWRange.getBegin(), diag::err_invalid_vector_long_decl_spec); + else + S.Diag(TSWRange.getBegin(), + diag::warn_vector_long_decl_spec_combination) + << getSpecifierName((TST)TypeSpecType, Policy); + } + + if (TypeAltiVecPixel) { + //TODO: perform validation + TypeSpecType = TST_int; TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unsigned); - } else if (TypeSpecType == TST_double) { - // vector long double and vector long long double are never allowed. - // vector double is OK for Power7 and later, and ZVector. - if (getTypeSpecWidth() == TypeSpecifierWidth::Long || - getTypeSpecWidth() == TypeSpecifierWidth::LongLong) - S.Diag(TSWRange.getBegin(), - diag::err_invalid_vector_long_double_decl_spec); - else if (!S.Context.getTargetInfo().hasFeature("vsx") && - !S.getLangOpts().ZVector) - S.Diag(TSTLoc, diag::err_invalid_vector_double_decl_spec); - } else if (TypeSpecType == TST_float) { - // vector float is unsupported for ZVector unless we have the - // vector-enhancements facility 1 (ISA revision 12). - if (S.getLangOpts().ZVector && - !S.Context.getTargetInfo().hasFeature("arch12")) - S.Diag(TSTLoc, diag::err_invalid_vector_float_decl_spec); - } else if (getTypeSpecWidth() == TypeSpecifierWidth::Long) { - // Vector long is unsupported for ZVector, or without VSX, and deprecated - // for AltiVec. - // It has also been historically deprecated on AIX (as an alias for - // "vector int" in both 32-bit and 64-bit modes). It was then made - // unsupported in the Clang-based XL compiler since the deprecated type - // has a number of conflicting semantics and continuing to support it - // is a disservice to users. - if (S.getLangOpts().ZVector || - !S.Context.getTargetInfo().hasFeature("vsx") || - S.Context.getTargetInfo().getTriple().isOSAIX()) - S.Diag(TSWRange.getBegin(), diag::err_invalid_vector_long_decl_spec); - else - S.Diag(TSWRange.getBegin(), - diag::warn_vector_long_decl_spec_combination) - << getSpecifierName((TST)TypeSpecType, Policy); + TypeSpecWidth = static_cast<unsigned>(TypeSpecifierWidth::Short); + TypeSpecOwned = false; + } } - if (TypeAltiVecPixel) { - //TODO: perform validation - TypeSpecType = TST_int; - TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unsigned); - TypeSpecWidth = static_cast<unsigned>(TypeSpecifierWidth::Short); - TypeSpecOwned = false; + bool IsFixedPointType = + TypeSpecType == TST_accum || TypeSpecType == TST_fract; + + // signed/unsigned are only valid with int/char/wchar_t/_Accum. + if (getTypeSpecSign() != TypeSpecifierSign::Unspecified) { + if (TypeSpecType == TST_unspecified) + TypeSpecType = TST_int; // unsigned -> unsigned int, signed -> signed int. + else if (TypeSpecType != TST_int && TypeSpecType != TST_int128 && + TypeSpecType != TST_char && TypeSpecType != TST_wchar && + !IsFixedPointType && TypeSpecType != TST_bitint) { + S.Diag(TSSLoc, diag::err_invalid_sign_spec) + << getSpecifierName((TST)TypeSpecType, Policy); + // signed double -> double. + TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unspecified); + } } - } - bool IsFixedPointType = - TypeSpecType == TST_accum || TypeSpecType == TST_fract; - - // signed/unsigned are only valid with int/char/wchar_t/_Accum. - if (getTypeSpecSign() != TypeSpecifierSign::Unspecified) { - if (TypeSpecType == TST_unspecified) - TypeSpecType = TST_int; // unsigned -> unsigned int, signed -> signed int. - else if (TypeSpecType != TST_int && TypeSpecType != TST_int128 && - TypeSpecType != TST_char && TypeSpecType != TST_wchar && - !IsFixedPointType && TypeSpecType != TST_bitint) { - S.Diag(TSSLoc, diag::err_invalid_sign_spec) - << getSpecifierName((TST)TypeSpecType, Policy); - // signed double -> double. - TypeSpecSign = static_cast<unsigned>(TypeSpecifierSign::Unspecified); + // Validate the width of the type. + switch (getTypeSpecWidth()) { + case TypeSpecifierWidth::Unspecified: + break; + case TypeSpecifierWidth::Short: // short int + case TypeSpecifierWidth::LongLong: // long long int + if (TypeSpecType == TST_unspecified) + TypeSpecType = TST_int; // short -> short int, long long -> long long int. + else if (!(TypeSpecType == TST_int || + (IsFixedPointType && + getTypeSpecWidth() != TypeSpecifierWidth::LongLong))) { + S.Diag(TSWRange.getBegin(), diag::err_invalid_width_spec) + << (int)TypeSpecWidth << getSpecifierName((TST)TypeSpecType, Policy); + TypeSpecType = TST_int; + TypeSpecSat = false; + TypeSpecOwned = false; + } + break; + case TypeSpecifierWidth::Long: // long double, long int + if (TypeSpecType == TST_unspecified) + TypeSpecType = TST_int; // long -> long int. + else if (TypeSpecType != TST_int && TypeSpecType != TST_double && + !IsFixedPointType) { + S.Diag(TSWRange.getBegin(), diag::err_invalid_width_spec) + << (int)TypeSpecWidth << getSpecifierName((TST)TypeSpecType, Policy); + TypeSpecType = TST_int; + TypeSpecSat = false; + TypeSpecOwned = false; + } + break; } - } - // Validate the width of the type. - switch (getTypeSpecWidth()) { - case TypeSpecifierWidth::Unspecified: - break; - case TypeSpecifierWidth::Short: // short int - case TypeSpecifierWidth::LongLong: // long long int - if (TypeSpecType == TST_unspecified) - TypeSpecType = TST_int; // short -> short int, long long -> long long int. - else if (!(TypeSpecType == TST_int || - (IsFixedPointType && - getTypeSpecWidth() != TypeSpecifierWidth::LongLong))) { - S.Diag(TSWRange.getBegin(), diag::err_invalid_width_spec) - << (int)TypeSpecWidth << getSpecifierName((TST)TypeSpecType, Policy); - TypeSpecType = TST_int; - TypeSpecSat = false; - TypeSpecOwned = false; - } - break; - case TypeSpecifierWidth::Long: // long double, long int - if (TypeSpecType == TST_unspecified) - TypeSpecType = TST_int; // long -> long int. - else if (TypeSpecType != TST_int && TypeSpecType != TST_double && - !IsFixedPointType) { - S.Diag(TSWRange.getBegin(), diag::err_invalid_width_spec) - << (int)TypeSpecWidth << getSpecifierName((TST)TypeSpecType, Policy); - TypeSpecType = TST_int; - TypeSpecSat = false; - TypeSpecOwned = false; + // TODO: if the implementation does not implement _Complex, disallow their + // use. Need information about the backend. + if (TypeSpecComplex != TSC_unspecified) { + if (TypeSpecType == TST_unspecified) { + S.Diag(TSCLoc, diag::ext_plain_complex) + << FixItHint::CreateInsertion( + S.getLocForEndOfToken(getTypeSpecComplexLoc()), + " double"); + TypeSpecType = TST_double; // _Complex -> _Complex double. + } else if (TypeSpecType == TST_int || TypeSpecType == TST_char) { + // Note that this intentionally doesn't include _Complex _Bool. + if (!S.getLangOpts().CPlusPlus) + S.Diag(TSTLoc, diag::ext_integer_complex); + } else if (TypeSpecType != TST_float && TypeSpecType != TST_double && + TypeSpecType != TST_float128 && TypeSpecType != TST_float16 && + TypeSpecType != TST_ibm128) { + // FIXME: __fp16? + S.Diag(TSCLoc, diag::err_invalid_complex_spec) + << getSpecifierName((TST)TypeSpecType, Policy); + TypeSpecComplex = TSC_unspecified; + } } - break; - } - // TODO: if the implementation does not implement _Complex, disallow their - // use. Need information about the backend. - if (TypeSpecComplex != TSC_unspecified) { - if (TypeSpecType == TST_unspecified) { - S.Diag(TSCLoc, diag::ext_plain_complex) - << FixItHint::CreateInsertion( - S.getLocForEndOfToken(getTypeSpecComplexLoc()), - " double"); - TypeSpecType = TST_double; // _Complex -> _Complex double. - } else if (TypeSpecType == TST_int || TypeSpecType == TST_char) { - // Note that this intentionally doesn't include _Complex _Bool. - if (!S.getLangOpts().CPlusPlus) - S.Diag(TSTLoc, diag::ext_integer_complex); - } else if (TypeSpecType != TST_float && TypeSpecType != TST_double && - TypeSpecType != TST_float128 && TypeSpecType != TST_float16 && - TypeSpecType != TST_ibm128) { - // FIXME: __fp16? - S.Diag(TSCLoc, diag::err_invalid_complex_spec) - << getSpecifierName((TST)TypeSpecType, Policy); - TypeSpecComplex = TSC_unspecified; + // C11 6.7.1/3, C++11 [dcl.stc]p1, GNU TLS: __thread, thread_local and + // _Thread_local can only appear with the 'static' and 'extern' storage class + // specifiers. We also allow __private_extern__ as an extension. + if (ThreadStorageClassSpec != TSCS_unspecified) { + switch (StorageClassSpec) { + case SCS_unspecified: + case SCS_extern: + case SCS_private_extern: + case SCS_static: + break; + default: + if (S.getSourceManager().isBeforeInTranslationUnit( + getThreadStorageClassSpecLoc(), getStorageClassSpecLoc())) + S.Diag(getStorageClassSpecLoc(), + diag::err_invali... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/190597 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
