[PATCH] D140455: [Clang] Diagnose undefined behavior in a constant expression while evaluating a compound assignment with remainder as operand

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: aaron.ballman, erichkeane, tahonermann.
Herald added a project: All.
shafik requested review of this revision.

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.


https://reviews.llvm.org/D140455

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp


Index: clang/test/CXX/expr/expr.const/p2-0x.cpp
===================================================================
--- clang/test/CXX/expr/expr.const/p2-0x.cpp
+++ clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -298,6 +298,15 @@
     static_assert(isinf(f6), "");
     static_assert(isinf(f9), "");
   }
+
+#if __cplusplus >= 201703L
+namespace CompoundAssignment {
+int constexpr 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);
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2756,6 +2756,7 @@
 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);
@@ -2778,14 +2779,14 @@
       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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140455.484461.patch
Type: text/x-patch
Size: 2024 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221221/e18ab73e/attachment.bin>