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

[shafik]

Summary

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.

[aaron.ballman]

LGTM aside from some style nits. Can you also add a release note for the fix?

clang/lib/AST/ExprConstant.cpp

2754–2755

2772–2785

clang/test/CXX/expr/expr.const/p2-0x.cpp

304

[shafik]

• Move HandleOverflowResult closer to where it is used.
• move int after constexpr in function rem
• Add release note

[shafik]

• Moving back HandleOverflowResult to before the switch statement to avoid "jump bypasses variable initialization" error.

clang/docs/ReleaseNotes.rst

- ``-Winteger-overflow`` will diagnose overflow in more cases. This fixes
   `Issue 58944 <https://github.com/llvm/llvm-project/issues/58944>`_.
 - Clang has an internal limit of 2GB of preprocessed source code per
   compilation, including source reachable through imported AST files such as
   PCH or modules. When Clang hits this limit, it now produces notes mentioning
   which header and source files are consuming large amounts of this space.
   ``#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
 -------------------------------------------------
 - It's now possible to set the crash diagnostics directory through
   the environment variable ``CLANG_CRASH_DIAGNOSTICS_DIR``.
   The ``-fcrash-diagnostics-dir`` flag takes precedence.
 - When using header modules, inclusion of a private header and violations of
   the `use-declaration rules

clang/lib/AST/ExprConstant.cpp

static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
   }
   return true;
 }
 
 /// Perform the given binary integer operation.
 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);
     return false;
   case BO_Mul:
     return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() * 2,
                                 std::multiplies<APSInt>(), Result);
   case BO_Add:
     return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
                                 std::plus<APSInt>(), Result);
   case BO_Sub:
     return CheckedIntArithmetic(Info, E, LHS, RHS, LHS.getBitWidth() + 1,
                                 std::minus<APSInt>(), Result);
   case BO_And: Result = LHS & RHS; return true;
   case BO_Xor: Result = LHS ^ RHS; return true;
   case BO_Or:  Result = LHS | RHS; return true;
   case BO_Div:
   case BO_Rem:
     if (RHS == 0) {
       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.
       RHS &= APSInt(llvm::APInt(RHS.getBitWidth(),
                     static_cast<uint64_t>(LHS.getBitWidth() - 1)),
                     RHS.isUnsigned());
     else if (RHS.isSigned() && RHS.isNegative()) {
       // During constant-folding, a negative shift is an opposite shift. Such
       // a shift is not a constant expression.

clang/test/CXX/expr/expr.const/p2-0x.cpp

#pragma float_control(pop)
     static_assert(!isinf(f1), "");
     static_assert(isinf(f2), "");
     static_assert(!isinf(f3), "");
     static_assert(isinf(f4), "");
     static_assert(isinf(f5), "");
     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);
 struct Lambda {
   int n : []{ return 1; }(); // cxx11-error {{constant expression}} cxx11-error {{integral constant expression}} cxx11-note {{non-literal type}}
 };
 
 // - an lvalue-to-rvalue conversion (4.1) unless it is applied to