Auto merge of rust-lang#125359 - RalfJung:interpret-overflowing-ops, r=oli-obk

interpret: make overflowing binops just normal binops

Follow-up to rust-lang#125173 (Cc `@scottmcm)`

Author: bors

compiler/rustc_const_eval/messages.ftl

@@ -246,11 +246,10 @@ const_eval_offset_from_unsigned_overflow =
 
 const_eval_operator_non_const =
     cannot call non-const operator in {const_eval_const_context}s
-const_eval_overflow =
-    overflow executing `{$name}`
-
+const_eval_overflow_arith =
+    arithmetic overflow in `{$intrinsic}`
 const_eval_overflow_shift =
-    overflowing shift by {$val} in `{$name}`
+    overflowing shift by {$shift_amount} in `{$intrinsic}`
 
 const_eval_panic =
     the evaluated program panicked at '{$msg}', {$file}:{$line}:{$col}

compiler/rustc_const_eval/src/const_eval/dummy_machine.rs

@@ -125,7 +125,7 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine {
         bin_op: BinOp,
         left: &interpret::ImmTy<'tcx, Self::Provenance>,
         right: &interpret::ImmTy<'tcx, Self::Provenance>,
-    ) -> interpret::InterpResult<'tcx, (ImmTy<'tcx, Self::Provenance>, bool)> {
+    ) -> interpret::InterpResult<'tcx, ImmTy<'tcx, Self::Provenance>> {
         use rustc_middle::mir::BinOp::*;
         Ok(match bin_op {
             Eq | Ne | Lt | Le | Gt | Ge => {
@@ -154,7 +154,7 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine {
                     Ge => left >= right,
                     _ => bug!(),
                 };
-                (ImmTy::from_bool(res, *ecx.tcx), false)
+                ImmTy::from_bool(res, *ecx.tcx)
             }
 
             // Some more operations are possible with atomics.

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -589,7 +589,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         _bin_op: mir::BinOp,
         _left: &ImmTy<'tcx>,
         _right: &ImmTy<'tcx>,
-    ) -> InterpResult<'tcx, (ImmTy<'tcx>, bool)> {
+    ) -> InterpResult<'tcx, ImmTy<'tcx>> {
         throw_unsup_format!("pointer arithmetic or comparison is not supported at compile-time");
     }
 

compiler/rustc_const_eval/src/errors.rs

@@ -1,5 +1,6 @@
 use std::borrow::Cow;
 
+use either::Either;
 use rustc_errors::{
     codes::*, Diag, DiagArgValue, DiagCtxt, DiagMessage, Diagnostic, EmissionGuarantee, Level,
 };
@@ -481,6 +482,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             DivisionOverflow => const_eval_division_overflow,
             RemainderOverflow => const_eval_remainder_overflow,
             PointerArithOverflow => const_eval_pointer_arithmetic_overflow,
+            ArithOverflow { .. } => const_eval_overflow_arith,
+            ShiftOverflow { .. } => const_eval_overflow_shift,
             InvalidMeta(InvalidMetaKind::SliceTooBig) => const_eval_invalid_meta_slice,
             InvalidMeta(InvalidMetaKind::TooBig) => const_eval_invalid_meta,
             UnterminatedCString(_) => const_eval_unterminated_c_string,
@@ -539,6 +542,19 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
             | UninhabitedEnumVariantWritten(_)
             | UninhabitedEnumVariantRead(_) => {}
 
+            ArithOverflow { intrinsic } => {
+                diag.arg("intrinsic", intrinsic);
+            }
+            ShiftOverflow { intrinsic, shift_amount } => {
+                diag.arg("intrinsic", intrinsic);
+                diag.arg(
+                    "shift_amount",
+                    match shift_amount {
+                        Either::Left(v) => v.to_string(),
+                        Either::Right(v) => v.to_string(),
+                    },
+                );
+            }
             BoundsCheckFailed { len, index } => {
                 diag.arg("len", len);
                 diag.arg("index", index);

compiler/rustc_const_eval/src/interpret/discriminant.rs

@@ -172,7 +172,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         let tag_val = ImmTy::from_uint(tag_bits, tag_layout);
                         let niche_start_val = ImmTy::from_uint(niche_start, tag_layout);
                         let variant_index_relative_val =
-                            self.wrapping_binary_op(mir::BinOp::Sub, &tag_val, &niche_start_val)?;
+                            self.binary_op(mir::BinOp::Sub, &tag_val, &niche_start_val)?;
                         let variant_index_relative =
                             variant_index_relative_val.to_scalar().assert_bits(tag_val.layout.size);
                         // Check if this is in the range that indicates an actual discriminant.
@@ -292,11 +292,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let variant_index_relative_val =
                     ImmTy::from_uint(variant_index_relative, tag_layout);
                 let tag = self
-                    .wrapping_binary_op(
-                        mir::BinOp::Add,
-                        &variant_index_relative_val,
-                        &niche_start_val,
-                    )?
+                    .binary_op(mir::BinOp::Add, &variant_index_relative_val, &niche_start_val)?
                     .to_scalar()
                     .assert_int();
                 Ok(Some((tag, tag_field)))

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -286,9 +286,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     let (val, overflowed) = {
                         let a_offset = ImmTy::from_uint(a_offset, usize_layout);
                         let b_offset = ImmTy::from_uint(b_offset, usize_layout);
-                        self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?
+                        self.binary_op(BinOp::SubWithOverflow, &a_offset, &b_offset)?
+                            .to_scalar_pair()
                     };
-                    if overflowed {
+                    if overflowed.to_bool()? {
                         // a < b
                         if intrinsic_name == sym::ptr_offset_from_unsigned {
                             throw_ub_custom!(
@@ -300,7 +301,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         // The signed form of the intrinsic allows this. If we interpret the
                         // difference as isize, we'll get the proper signed difference. If that
                         // seems *positive*, they were more than isize::MAX apart.
-                        let dist = val.to_scalar().to_target_isize(self)?;
+                        let dist = val.to_target_isize(self)?;
                         if dist >= 0 {
                             throw_ub_custom!(
                                 fluent::const_eval_offset_from_underflow,
@@ -310,7 +311,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         dist
                     } else {
                         // b >= a
-                        let dist = val.to_scalar().to_target_isize(self)?;
+                        let dist = val.to_target_isize(self)?;
                         // If converting to isize produced a *negative* result, we had an overflow
                         // because they were more than isize::MAX apart.
                         if dist < 0 {
@@ -516,17 +517,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // Performs an exact division, resulting in undefined behavior where
         // `x % y != 0` or `y == 0` or `x == T::MIN && y == -1`.
         // First, check x % y != 0 (or if that computation overflows).
-        let (res, overflow) = self.overflowing_binary_op(BinOp::Rem, a, b)?;
-        assert!(!overflow); // All overflow is UB, so this should never return on overflow.
-        if res.to_scalar().assert_bits(a.layout.size) != 0 {
+        let rem = self.binary_op(BinOp::Rem, a, b)?;
+        if rem.to_scalar().assert_bits(a.layout.size) != 0 {
             throw_ub_custom!(
                 fluent::const_eval_exact_div_has_remainder,
                 a = format!("{a}"),
                 b = format!("{b}")
             )
         }
         // `Rem` says this is all right, so we can let `Div` do its job.
-        self.binop_ignore_overflow(BinOp::Div, a, b, &dest.clone().into())
+        let res = self.binary_op(BinOp::Div, a, b)?;
+        self.write_immediate(*res, dest)
     }
 
     pub fn saturating_arith(
@@ -539,8 +540,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         assert!(matches!(l.layout.ty.kind(), ty::Int(..) | ty::Uint(..)));
         assert!(matches!(mir_op, BinOp::Add | BinOp::Sub));
 
-        let (val, overflowed) = self.overflowing_binary_op(mir_op, l, r)?;
-        Ok(if overflowed {
+        let (val, overflowed) =
+            self.binary_op(mir_op.wrapping_to_overflowing().unwrap(), l, r)?.to_scalar_pair();
+        Ok(if overflowed.to_bool()? {
             let size = l.layout.size;
             let num_bits = size.bits();
             if l.layout.abi.is_signed() {
@@ -571,7 +573,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 }
             }
         } else {
-            val.to_scalar()
+            val
         })
     }
 

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -252,7 +252,7 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
         bin_op: mir::BinOp,
         left: &ImmTy<'tcx, Self::Provenance>,
         right: &ImmTy<'tcx, Self::Provenance>,
-    ) -> InterpResult<'tcx, (ImmTy<'tcx, Self::Provenance>, bool)>;
+    ) -> InterpResult<'tcx, ImmTy<'tcx, Self::Provenance>>;
 
     /// Generate the NaN returned by a float operation, given the list of inputs.
     /// (This is all inputs, not just NaN inputs!)

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -7,7 +7,7 @@ use either::{Either, Left, Right};
 
 use rustc_hir::def::Namespace;
 use rustc_middle::mir::interpret::ScalarSizeMismatch;
-use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
+use rustc_middle::ty::layout::{HasParamEnv, HasTyCtxt, LayoutOf, TyAndLayout};
 use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter};
 use rustc_middle::ty::{ConstInt, ScalarInt, Ty, TyCtxt};
 use rustc_middle::{bug, span_bug};
@@ -249,6 +249,15 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
         Self::from_scalar(Scalar::from_i8(c as i8), layout)
     }
 
+    pub fn from_pair(a: Self, b: Self, tcx: TyCtxt<'tcx>) -> Self {
+        let layout = tcx
+            .layout_of(
+                ty::ParamEnv::reveal_all().and(Ty::new_tup(tcx, &[a.layout.ty, b.layout.ty])),
+            )
+            .unwrap();
+        Self::from_scalar_pair(a.to_scalar(), b.to_scalar(), layout)
+    }
+
     /// Return the immediate as a `ScalarInt`. Ensures that it has the size that the layout of the
     /// immediate indicates.
     #[inline]
@@ -270,6 +279,17 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
         ConstInt::new(int, self.layout.ty.is_signed(), self.layout.ty.is_ptr_sized_integral())
     }
 
+    #[inline]
+    #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
+    pub fn to_pair(self, cx: &(impl HasTyCtxt<'tcx> + HasParamEnv<'tcx>)) -> (Self, Self) {
+        let layout = self.layout;
+        let (val0, val1) = self.to_scalar_pair();
+        (
+            ImmTy::from_scalar(val0, layout.field(cx, 0)),
+            ImmTy::from_scalar(val1, layout.field(cx, 1)),
+        )
+    }
+
     /// Compute the "sub-immediate" that is located within the `base` at the given offset with the
     /// given layout.
     // Not called `offset` to avoid confusion with the trait method.