Skip to content

Commit

Permalink
[Clang] Diagnose undefined behavior in a constant expression while ev…
Browse files Browse the repository at this point in the history
…aluating a compound assignment with remainder as operand

Currently we don't diagnose overflow in a constant expression for the case of
compound assignment with remainder as a operand.

In handleIntIntBinOp the arguments LHS and Result can be the same source but in
the check for remainder in this function we assigned to Result before checking
for overflow. In all the other operations the check is done before Result is
assigned to.

Differential Revision: https://reviews.llvm.org/D140455
  • Loading branch information
shafik committed Jan 12, 2023
1 parent ed00101 commit a013839
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,8 @@ Improvements to Clang's diagnostics
``#pragma clang __debug sloc_usage`` can also be used to request this report.
- Clang no longer permits the keyword 'bool' in a concept declaration as a
concepts-ts compatibility extension.
- Clang now diagnoses overflow undefined behavior in a constant expression while
evaluating a compound assignment with remainder as operand.

Non-comprehensive list of changes in this release
-------------------------------------------------
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2751,6 +2751,7 @@ static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
BinaryOperatorKind Opcode, APSInt RHS,
APSInt &Result) {
bool HandleOverflowResult = true;
switch (Opcode) {
default:
Info.FFDiag(E);
Expand All @@ -2773,14 +2774,14 @@ static bool handleIntIntBinOp(EvalInfo &Info, const Expr *E, const APSInt &LHS,
Info.FFDiag(E, diag::note_expr_divide_by_zero);
return false;
}
Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
// Check for overflow case: INT_MIN / -1 or INT_MIN % -1. APSInt supports
// this operation and gives the two's complement result.
if (RHS.isNegative() && RHS.isAllOnes() && LHS.isSigned() &&
LHS.isMinSignedValue())
return HandleOverflow(Info, E, -LHS.extend(LHS.getBitWidth() + 1),
E->getType());
return true;
HandleOverflowResult = HandleOverflow(
Info, E, -LHS.extend(LHS.getBitWidth() + 1), E->getType());
Result = (Opcode == BO_Rem ? LHS % RHS : LHS / RHS);
return HandleOverflowResult;
case BO_Shl: {
if (Info.getLangOpts().OpenCL)
// OpenCL 6.3j: shift values are effectively % word size of LHS.
Expand Down
9 changes: 9 additions & 0 deletions clang/test/CXX/expr/expr.const/p2-0x.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,15 @@ constexpr float negpi = -pi; // expect no error on unary operator
static_assert(isinf(f6), "");
static_assert(isinf(f9), "");
}

#if __cplusplus >= 201703L
namespace CompoundAssignment {
constexpr int rem() { // expected-error {{constexpr function never produces a constant expression}}
int x = ~__INT_MAX__;
return x%=-1; // cxx20-note {{value 2147483648 is outside the range of representable values of type 'int'}}
}
}
#endif
}

// - a lambda-expression (5.1.2);
Expand Down

0 comments on commit a013839

Please sign in to comment.