llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Akash Manna (akash-manna-sky) <details> <summary>Changes</summary> Fixes #<!-- -->191701 `GetFixedPointRank` switches on the kind of a `BuiltinType` and asserts if it gets anything else. The fixed point branch of `UsualArithmeticConversions` forwarded any other arithmetic operand into it, so `_Fract + _BitInt(31)` tripped the assertion since `_BitInt` is its own AST node. #<!-- -->199912 stopped that by checking for `_BitInt` before the branch, but the underlying assumption was still unguarded: an overflow behavior type such as `__ob_wrap int` is arithmetic, is not a `BuiltinType`, and is dispatched *after* the fixed point branch, so `1.0k + w` still crashed. The conditional operator had the same shape of bug, diagnosing a failed conversion only when a `_BitInt` was involved and otherwise handing a null type to `ImpCastExprToType`. The `_BitInt` special case is replaced by a `BuiltinType` check on both operands inside the fixed point branch itself. N1169 4.1.4 only defines conversions between fixed point types and the standard integer types, so that is the real precondition, and every operator now falls through to its normal `invalid operands to binary expression` error. `CheckConditionalOperands` diagnoses `incompatible operand types` for any null result from the usual arithmetic conversions rather than just the `_BitInt` case. --- Full diff: https://github.com/llvm/llvm-project/pull/221134.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+1) - (modified) clang/lib/Sema/SemaExpr.cpp (+10-9) - (added) clang/test/Sema/GH191701.c (+32) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 042d7112dbe7d..69566f10bb597 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -494,6 +494,7 @@ features cannot lower the translation-unit ABI level; - Fixed a crash when declaring a member template within a local class inside an OpenMP region. (#GH216052) - Fixed a bug where repeated #imports of modular headers in non-modular compilation were translated to #pragma clang module import. (#GH216924) - Fixed an assertion when `#pragma omp declare simd` or `#pragma omp declare variant` is followed by another OpenMP declarative directive containing a qualified identifier. (#GH217204) +- Fixed an assertion failure when a fixed point type was used in arithmetic with a `_BitInt` or overflow behavior type; the combination is now diagnosed as invalid operands. (#GH191701) #### Bug Fixes to Compiler Builtins diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c93efeb928c56..97eec9dc5b867 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -1757,10 +1757,6 @@ QualType Sema::UsualArithmeticConversions(ExprResult &LHS, ExprResult &RHS, // At this point, we have two different arithmetic types. - if ((LHSType->isFixedPointType() && RHSType->isBitIntType()) || - (LHSType->isBitIntType() && RHSType->isFixedPointType())) - return QualType(); - // Diagnose attempts to convert between __ibm128, __float128 and long double // where such conversions currently can't be handled. if (unsupportedTypeConversion(*this, LHSType, RHSType)) @@ -1781,8 +1777,14 @@ QualType Sema::UsualArithmeticConversions(ExprResult &LHS, ExprResult &RHS, return handleComplexIntConversion(*this, LHS, RHS, LHSType, RHSType, ACK == ArithConvKind::CompAssign); - if (LHSType->isFixedPointType() || RHSType->isFixedPointType()) + if (LHSType->isFixedPointType() || RHSType->isFixedPointType()) { + // N1169 4.1.4 only defines conversions between fixed point types and the + // standard integer types, so reject e.g. _BitInt or overflow behavior + // types. + if (!LHSType->getAs<BuiltinType>() || !RHSType->getAs<BuiltinType>()) + return QualType(); return handleFixedPointConversion(*this, LHSType, RHSType); + } if (LHSType->isOverflowBehaviorType() || RHSType->isOverflowBehaviorType()) return handleOverflowBehaviorTypeConversion( @@ -8983,10 +8985,9 @@ QualType Sema::CheckConditionalOperands(ExprResult &Cond, ExprResult &LHS, // If both operands have arithmetic type, do the usual arithmetic conversions // to find a common type: C99 6.5.15p3,5. if (LHSTy->isArithmeticType() && RHSTy->isArithmeticType()) { - // Disallow invalid arithmetic conversions, such as those between bit- - // precise integers types of different sizes, or between a bit-precise - // integer and another type. - if (ResTy.isNull() && (LHSTy->isBitIntType() || RHSTy->isBitIntType())) { + // Disallow invalid arithmetic conversions, such as those between a + // bit-precise integer and a fixed point type. + if (ResTy.isNull()) { Diag(QuestionLoc, diag::err_typecheck_cond_incompatible_operands) << LHSTy << RHSTy << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); diff --git a/clang/test/Sema/GH191701.c b/clang/test/Sema/GH191701.c new file mode 100644 index 0000000000000..446f2bc3bad40 --- /dev/null +++ b/clang/test/Sema/GH191701.c @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -ffixed-point -fexperimental-overflow-behavior-types %s + +// Fixed point types have no conversions with _BitInt or overflow behavior types. + +#define test(e, t) _Generic (e, default : 0, t : 1) + +void GH191701(_Fract ci) { + _BitInt(31) bi = 0; + _Static_assert(test(ci + bi, _Complex int), ""); // expected-error {{invalid operands to binary expression ('_Fract' and '_BitInt(31)')}} +} + +void bitint(int c, _Fract f, _Accum a, _Sat short _Fract sf, _BitInt(31) bi, + unsigned _BitInt(8) ubi) { + (void)(f + bi); // expected-error {{invalid operands to binary expression ('_Fract' and '_BitInt(31)')}} + (void)(bi + f); // expected-error {{invalid operands to binary expression ('_BitInt(31)' and '_Fract')}} + (void)(a - bi); // expected-error {{invalid operands to binary expression ('_Accum' and '_BitInt(31)')}} + (void)(bi * a); // expected-error {{invalid operands to binary expression ('_BitInt(31)' and '_Accum')}} + (void)(sf / ubi); // expected-error {{invalid operands to binary expression ('_Sat short _Fract' and 'unsigned _BitInt(8)')}} + (void)(f < bi); // expected-error {{invalid operands to binary expression ('_Fract' and '_BitInt(31)')}} + (void)(ubi == a); // expected-error {{invalid operands to binary expression ('unsigned _BitInt(8)' and '_Accum')}} + f += bi; // expected-error {{invalid operands to binary expression ('_Fract' and '_BitInt(31)')}} + bi -= a; // expected-error {{invalid operands to binary expression ('_BitInt(31)' and '_Accum')}} + (void)(c ? f : bi); // expected-error {{incompatible operand types ('_Fract' and '_BitInt(31)')}} +} + +void overflow_behavior(int c, _Fract f, _Accum a, int __ob_wrap w, + long __ob_trap t) { + (void)(f + w); // expected-error {{invalid operands to binary expression ('_Fract' and '__ob_wrap int')}} + (void)(t * a); // expected-error {{invalid operands to binary expression ('__ob_trap long' and '_Accum')}} + a -= w; // expected-error {{invalid operands to binary expression ('_Accum' and '__ob_wrap int')}} + (void)(c ? w : f); // expected-error {{incompatible operand types ('__ob_wrap int' and '_Fract')}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/221134 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
