[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
```


ed3d051782/clang/lib/Sema/SemaChecking.cpp (L9628-L9631)

The current behavior causes the `GetExprRange` to proceed with the throw
expression (`CO->getTrueExpr()`/`void` type)
This commit is contained in:
Oleksandr T. 2024-10-29 11:44:01 +02:00 committed by GitHub
parent 8d38fbf2f0
commit 05b6c2e4b9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 140 additions and 86 deletions

View File

@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^

View File

@ -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;

View File

@ -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}}
}
}