[clang] 05b6c2e - [Clang] fix range calculation for conditionals with throw expressions (#112081)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 29 02:44:05 PDT 2024
Author: Oleksandr T.
Date: 2024-10-29T10:44:01+01:00
New Revision: 05b6c2e4b933e7a3606899c72067c92b6077287b
URL: https://github.com/llvm/llvm-project/commit/05b6c2e4b933e7a3606899c72067c92b6077287b
DIFF: https://github.com/llvm/llvm-project/commit/05b6c2e4b933e7a3606899c72067c92b6077287b.diff
LOG: [Clang] fix range calculation for conditionals with throw expressions (#112081)
Fixes #111854
---
The issue arises when `GetExprRange` encounters a `ConditionalOperator`
with a `CXXThrowExpr`
```md
ConditionalOperator 0x1108658e0 'int'
|-CXXBoolLiteralExpr 0x110865878 '_Bool' true
|-CXXThrowExpr 0x1108658a8 'void'
| `-IntegerLiteral 0x110865888 'int' 0
`-IntegerLiteral 0x1108658c0 'int' 0
```
https://github.com/llvm/llvm-project/blob/ed3d05178274890fb804f43ae1bcdfd33b5fd8f0/clang/lib/Sema/SemaChecking.cpp#L9628-L9631
The current behavior causes the `GetExprRange` to proceed with the throw
expression (`CO->getTrueExpr()`/`void` type)
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/conditional-expr.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 424f02ef08d70e..a39ffc8366dda4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -571,7 +571,8 @@ Bug Fixes to C++ Support
- Clang incorrectly considered a class with an anonymous union member to not be
const-default-constructible even if a union member has a default member initializer.
(#GH95854).
-- Fixed an assertion failure when evaluating an invalid expression in an array initializer (#GH112140)
+- Fixed an assertion failure when evaluating an invalid expression in an array initializer. (#GH112140)
+- Fixed an assertion failure in range calculations for conditional throw expressions. (#GH111854)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index d027e4c6dfdb4d..3308b898a5b68f 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -9592,16 +9592,23 @@ static QualType GetExprType(const Expr *E) {
return Ty;
}
-/// Pseudo-evaluate the given integer expression, estimating the
-/// range of values it might take.
+/// Attempts to estimate an approximate range for the given integer expression.
+/// Returns a range if successful, otherwise it returns \c std::nullopt if a
+/// reliable estimation cannot be determined.
///
/// \param MaxWidth The width to which the value will be truncated.
-/// \param Approximate If \c true, return a likely range for the result: in
-/// particular, assume that arithmetic on narrower types doesn't leave
-/// those types. If \c false, return a range including all possible
-/// result values.
-static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
- bool InConstantContext, bool Approximate) {
+/// \param InConstantContext If \c true, interpret the expression within a
+/// constant context.
+/// \param Approximate If \c true, provide a likely range of values by assuming
+/// that arithmetic on narrower types remains within those types.
+/// If \c false, return a range that includes all possible values
+/// resulting from the expression.
+/// \returns A range of values that the expression might take, or
+/// std::nullopt if a reliable estimation cannot be determined.
+static std::optional<IntRange> TryGetExprRange(ASTContext &C, const Expr *E,
+ unsigned MaxWidth,
+ bool InConstantContext,
+ bool Approximate) {
E = E->IgnoreParens();
// Try a full evaluation first.
@@ -9614,8 +9621,8 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
// being of the new, wider type.
if (const auto *CE = dyn_cast<ImplicitCastExpr>(E)) {
if (CE->getCastKind() == CK_NoOp || CE->getCastKind() == CK_LValueToRValue)
- return GetExprRange(C, CE->getSubExpr(), MaxWidth, InConstantContext,
- Approximate);
+ return TryGetExprRange(C, CE->getSubExpr(), MaxWidth, InConstantContext,
+ Approximate);
IntRange OutputTypeRange = IntRange::forValueOfType(C, GetExprType(CE));
@@ -9626,40 +9633,52 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
if (!isIntegerCast)
return OutputTypeRange;
- IntRange SubRange = GetExprRange(C, CE->getSubExpr(),
- std::min(MaxWidth, OutputTypeRange.Width),
- InConstantContext, Approximate);
+ std::optional<IntRange> SubRange = TryGetExprRange(
+ C, CE->getSubExpr(), std::min(MaxWidth, OutputTypeRange.Width),
+ InConstantContext, Approximate);
+ if (!SubRange)
+ return std::nullopt;
// Bail out if the subexpr's range is as wide as the cast type.
- if (SubRange.Width >= OutputTypeRange.Width)
+ if (SubRange->Width >= OutputTypeRange.Width)
return OutputTypeRange;
// Otherwise, we take the smaller width, and we're non-negative if
// either the output type or the subexpr is.
- return IntRange(SubRange.Width,
- SubRange.NonNegative || OutputTypeRange.NonNegative);
+ return IntRange(SubRange->Width,
+ SubRange->NonNegative || OutputTypeRange.NonNegative);
}
if (const auto *CO = dyn_cast<ConditionalOperator>(E)) {
// If we can fold the condition, just take that operand.
bool CondResult;
if (CO->getCond()->EvaluateAsBooleanCondition(CondResult, C))
- return GetExprRange(C,
- CondResult ? CO->getTrueExpr() : CO->getFalseExpr(),
- MaxWidth, InConstantContext, Approximate);
+ return TryGetExprRange(
+ C, CondResult ? CO->getTrueExpr() : CO->getFalseExpr(), MaxWidth,
+ InConstantContext, Approximate);
// Otherwise, conservatively merge.
- // GetExprRange requires an integer expression, but a throw expression
+ // TryGetExprRange requires an integer expression, but a throw expression
// results in a void type.
- Expr *E = CO->getTrueExpr();
- IntRange L = E->getType()->isVoidType()
- ? IntRange{0, true}
- : GetExprRange(C, E, MaxWidth, InConstantContext, Approximate);
- E = CO->getFalseExpr();
- IntRange R = E->getType()->isVoidType()
- ? IntRange{0, true}
- : GetExprRange(C, E, MaxWidth, InConstantContext, Approximate);
- return IntRange::join(L, R);
+ Expr *TrueExpr = CO->getTrueExpr();
+ if (TrueExpr->getType()->isVoidType())
+ return std::nullopt;
+
+ std::optional<IntRange> L =
+ TryGetExprRange(C, TrueExpr, MaxWidth, InConstantContext, Approximate);
+ if (!L)
+ return std::nullopt;
+
+ Expr *FalseExpr = CO->getFalseExpr();
+ if (FalseExpr->getType()->isVoidType())
+ return std::nullopt;
+
+ std::optional<IntRange> R =
+ TryGetExprRange(C, FalseExpr, MaxWidth, InConstantContext, Approximate);
+ if (!R)
+ return std::nullopt;
+
+ return IntRange::join(*L, *R);
}
if (const auto *BO = dyn_cast<BinaryOperator>(E)) {
@@ -9696,8 +9715,8 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
// been coerced to the LHS type.
case BO_Assign:
// TODO: bitfields?
- return GetExprRange(C, BO->getRHS(), MaxWidth, InConstantContext,
- Approximate);
+ return TryGetExprRange(C, BO->getRHS(), MaxWidth, InConstantContext,
+ Approximate);
// Operations with opaque sources are black-listed.
case BO_PtrMemD:
@@ -9729,18 +9748,20 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
// Right shift by a constant can narrow its left argument.
case BO_Shr:
case BO_ShrAssign: {
- IntRange L = GetExprRange(C, BO->getLHS(), MaxWidth, InConstantContext,
- Approximate);
+ std::optional<IntRange> L = TryGetExprRange(
+ C, BO->getLHS(), MaxWidth, InConstantContext, Approximate);
+ if (!L)
+ return std::nullopt;
// If the shift amount is a positive constant, drop the width by
// that much.
if (std::optional<llvm::APSInt> shift =
BO->getRHS()->getIntegerConstantExpr(C)) {
if (shift->isNonNegative()) {
- if (shift->uge(L.Width))
- L.Width = (L.NonNegative ? 0 : 1);
+ if (shift->uge(L->Width))
+ L->Width = (L->NonNegative ? 0 : 1);
else
- L.Width -= shift->getZExtValue();
+ L->Width -= shift->getZExtValue();
}
}
@@ -9749,8 +9770,8 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
// Comma acts as its right operand.
case BO_Comma:
- return GetExprRange(C, BO->getRHS(), MaxWidth, InConstantContext,
- Approximate);
+ return TryGetExprRange(C, BO->getRHS(), MaxWidth, InConstantContext,
+ Approximate);
case BO_Add:
if (!Approximate)
@@ -9774,26 +9795,31 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
case BO_Div: {
// Don't 'pre-truncate' the operands.
unsigned opWidth = C.getIntWidth(GetExprType(E));
- IntRange L = GetExprRange(C, BO->getLHS(), opWidth, InConstantContext,
- Approximate);
+ std::optional<IntRange> L = TryGetExprRange(
+ C, BO->getLHS(), opWidth, InConstantContext, Approximate);
+ if (!L)
+ return std::nullopt;
// If the divisor is constant, use that.
if (std::optional<llvm::APSInt> divisor =
BO->getRHS()->getIntegerConstantExpr(C)) {
unsigned log2 = divisor->logBase2(); // floor(log_2(divisor))
- if (log2 >= L.Width)
- L.Width = (L.NonNegative ? 0 : 1);
+ if (log2 >= L->Width)
+ L->Width = (L->NonNegative ? 0 : 1);
else
- L.Width = std::min(L.Width - log2, MaxWidth);
+ L->Width = std::min(L->Width - log2, MaxWidth);
return L;
}
// Otherwise, just use the LHS's width.
// FIXME: This is wrong if the LHS could be its minimal value and the RHS
// could be -1.
- IntRange R = GetExprRange(C, BO->getRHS(), opWidth, InConstantContext,
- Approximate);
- return IntRange(L.Width, L.NonNegative && R.NonNegative);
+ std::optional<IntRange> R = TryGetExprRange(
+ C, BO->getRHS(), opWidth, InConstantContext, Approximate);
+ if (!R)
+ return std::nullopt;
+
+ return IntRange(L->Width, L->NonNegative && R->NonNegative);
}
case BO_Rem:
@@ -9810,11 +9836,17 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
// performed the computation.
QualType T = GetExprType(E);
unsigned opWidth = C.getIntWidth(T);
- IntRange L =
- GetExprRange(C, BO->getLHS(), opWidth, InConstantContext, Approximate);
- IntRange R =
- GetExprRange(C, BO->getRHS(), opWidth, InConstantContext, Approximate);
- IntRange C = Combine(L, R);
+ std::optional<IntRange> L = TryGetExprRange(C, BO->getLHS(), opWidth,
+ InConstantContext, Approximate);
+ if (!L)
+ return std::nullopt;
+
+ std::optional<IntRange> R = TryGetExprRange(C, BO->getRHS(), opWidth,
+ InConstantContext, Approximate);
+ if (!R)
+ return std::nullopt;
+
+ IntRange C = Combine(*L, *R);
C.NonNegative |= T->isUnsignedIntegerOrEnumerationType();
C.Width = std::min(C.Width, MaxWidth);
return C;
@@ -9832,26 +9864,30 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth,
return IntRange::forValueOfType(C, GetExprType(E));
default:
- return GetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
- Approximate);
+ return TryGetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
+ Approximate);
}
}
if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E))
- return GetExprRange(C, OVE->getSourceExpr(), MaxWidth, InConstantContext,
- Approximate);
+ return TryGetExprRange(C, OVE->getSourceExpr(), MaxWidth, InConstantContext,
+ Approximate);
if (const auto *BitField = E->getSourceBitField())
return IntRange(BitField->getBitWidthValue(C),
BitField->getType()->isUnsignedIntegerOrEnumerationType());
+ if (GetExprType(E)->isVoidType())
+ return std::nullopt;
+
return IntRange::forValueOfType(C, GetExprType(E));
}
-static IntRange GetExprRange(ASTContext &C, const Expr *E,
- bool InConstantContext, bool Approximate) {
- return GetExprRange(C, E, C.getIntWidth(GetExprType(E)), InConstantContext,
- Approximate);
+static std::optional<IntRange> TryGetExprRange(ASTContext &C, const Expr *E,
+ bool InConstantContext,
+ bool Approximate) {
+ return TryGetExprRange(C, E, C.getIntWidth(GetExprType(E)), InConstantContext,
+ Approximate);
}
/// Checks whether the given value, which currently has the given
@@ -10096,8 +10132,10 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
return false;
- IntRange OtherValueRange = GetExprRange(
+ std::optional<IntRange> OtherValueRange = TryGetExprRange(
S.Context, Other, S.isConstantEvaluatedContext(), /*Approximate=*/false);
+ if (!OtherValueRange)
+ return false;
QualType OtherT = Other->getType();
if (const auto *AT = OtherT->getAs<AtomicType>())
@@ -10115,11 +10153,11 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
bool OtherIsBooleanDespiteType =
!OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue();
if (OtherIsBooleanDespiteType || IsObjCSignedCharBool)
- OtherTypeRange = OtherValueRange = IntRange::forBoolType();
+ OtherTypeRange = *OtherValueRange = IntRange::forBoolType();
// Check if all values in the range of possible values of this expression
// lead to the same comparison outcome.
- PromotedRange OtherPromotedValueRange(OtherValueRange, Value.getBitWidth(),
+ PromotedRange OtherPromotedValueRange(*OtherValueRange, Value.getBitWidth(),
Value.isUnsigned());
auto Cmp = OtherPromotedValueRange.compare(Value);
auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant);
@@ -10143,7 +10181,7 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
// Don't warn if the non-constant operand actually always evaluates to the
// same value.
- if (!TautologicalTypeCompare && OtherValueRange.Width == 0)
+ if (!TautologicalTypeCompare && OtherValueRange->Width == 0)
return false;
// Suppress the diagnostic for an in-range comparison if the constant comes
@@ -10182,7 +10220,7 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
if (!TautologicalTypeCompare) {
S.Diag(E->getOperatorLoc(), diag::warn_tautological_compare_value_range)
- << RhsConstant << OtherValueRange.Width << OtherValueRange.NonNegative
+ << RhsConstant << OtherValueRange->Width << OtherValueRange->NonNegative
<< E->getOpcodeStr() << OS.str() << *Result
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
return true;
@@ -10312,9 +10350,11 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
}
// Otherwise, calculate the effective range of the signed operand.
- IntRange signedRange =
- GetExprRange(S.Context, signedOperand, S.isConstantEvaluatedContext(),
- /*Approximate=*/true);
+ std::optional<IntRange> signedRange =
+ TryGetExprRange(S.Context, signedOperand, S.isConstantEvaluatedContext(),
+ /*Approximate=*/true);
+ if (!signedRange)
+ return;
// Go ahead and analyze implicit conversions in the operands. Note
// that we skip the implicit conversions on both sides.
@@ -10322,7 +10362,7 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
AnalyzeImplicitConversions(S, RHS, E->getOperatorLoc());
// If the signed range is non-negative, -Wsign-compare won't fire.
- if (signedRange.NonNegative)
+ if (signedRange->NonNegative)
return;
// For (in)equality comparisons, if the unsigned operand is a
@@ -10331,15 +10371,17 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
// change the result of the comparison.
if (E->isEqualityOp()) {
unsigned comparisonWidth = S.Context.getIntWidth(T);
- IntRange unsignedRange =
- GetExprRange(S.Context, unsignedOperand, S.isConstantEvaluatedContext(),
- /*Approximate=*/true);
+ std::optional<IntRange> unsignedRange = TryGetExprRange(
+ S.Context, unsignedOperand, S.isConstantEvaluatedContext(),
+ /*Approximate=*/true);
+ if (!unsignedRange)
+ return;
// We should never be unable to prove that the unsigned operand is
// non-negative.
- assert(unsignedRange.NonNegative && "unsigned range includes negative?");
+ assert(unsignedRange->NonNegative && "unsigned range includes negative?");
- if (unsignedRange.Width < comparisonWidth)
+ if (unsignedRange->Width < comparisonWidth)
return;
}
@@ -11146,10 +11188,12 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
if (SourceBT && TargetBT && SourceBT->isIntegerType() &&
TargetBT->isFloatingType() && !IsListInit) {
// Determine the number of precision bits in the source integer type.
- IntRange SourceRange =
- GetExprRange(Context, E, isConstantEvaluatedContext(),
- /*Approximate=*/true);
- unsigned int SourcePrecision = SourceRange.Width;
+ std::optional<IntRange> SourceRange =
+ TryGetExprRange(Context, E, isConstantEvaluatedContext(),
+ /*Approximate=*/true);
+ if (!SourceRange)
+ return;
+ unsigned int SourcePrecision = SourceRange->Width;
// Determine the number of precision bits in the
// target floating point type.
@@ -11212,14 +11256,16 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
E, Diag(CC, diag::warn_impcast_int_to_objc_signed_char_bool)
<< E->getType());
}
+ std::optional<IntRange> LikelySourceRange = TryGetExprRange(
+ Context, E, isConstantEvaluatedContext(), /*Approximate=*/true);
+ if (!LikelySourceRange)
+ return;
IntRange SourceTypeRange =
IntRange::forTargetOfCanonicalType(Context, Source);
- IntRange LikelySourceRange = GetExprRange(
- Context, E, isConstantEvaluatedContext(), /*Approximate=*/true);
IntRange TargetRange = IntRange::forTargetOfCanonicalType(Context, Target);
- if (LikelySourceRange.Width > TargetRange.Width) {
+ if (LikelySourceRange->Width > TargetRange.Width) {
// If the source is a constant, use a default-on diagnostic.
// TODO: this should happen for bitfield stores, too.
Expr::EvalResult Result;
@@ -11266,8 +11312,8 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
}
}
- if (TargetRange.Width == LikelySourceRange.Width &&
- !TargetRange.NonNegative && LikelySourceRange.NonNegative &&
+ if (TargetRange.Width == LikelySourceRange->Width &&
+ !TargetRange.NonNegative && LikelySourceRange->NonNegative &&
Source->isSignedIntegerType()) {
// Warn when doing a signed to signed conversion, warn if the positive
// source value is exactly the width of the target type, which will
@@ -11293,9 +11339,9 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
}
if ((!isa<EnumType>(Target) || !isa<EnumType>(Source)) &&
- ((TargetRange.NonNegative && !LikelySourceRange.NonNegative) ||
- (!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
- LikelySourceRange.Width == TargetRange.Width))) {
+ ((TargetRange.NonNegative && !LikelySourceRange->NonNegative) ||
+ (!TargetRange.NonNegative && LikelySourceRange->NonNegative &&
+ LikelySourceRange->Width == TargetRange.Width))) {
if (SourceMgr.isInSystemMacro(CC))
return;
diff --git a/clang/test/SemaCXX/conditional-expr.cpp b/clang/test/SemaCXX/conditional-expr.cpp
index 01effaa189322b..8f17555fd806ff 100644
--- a/clang/test/SemaCXX/conditional-expr.cpp
+++ b/clang/test/SemaCXX/conditional-expr.cpp
@@ -429,3 +429,10 @@ void g() {
long e = a = b ? throw 0 : throw 1;
}
} // namespace PR46484
+
+namespace GH111854 {
+void f() {
+ (true ? throw 0 : 0) <= 0; // expected-warning {{relational comparison result unused}}
+ (false ? 0 : throw 0) <= 0; // expected-warning {{relational comparison result unused}}
+}
+}
More information about the cfe-commits
mailing list