From c0b4b454c3804aa1ffe8b500de87943c1f4099f7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 May 2024 12:17:34 +0200 Subject: [PATCH 1/2] interpret: make overflowing binops just normal binops --- compiler/rustc_const_eval/messages.ftl | 7 +- .../src/const_eval/dummy_machine.rs | 4 +- .../src/const_eval/machine.rs | 2 +- compiler/rustc_const_eval/src/errors.rs | 16 ++ .../src/interpret/discriminant.rs | 8 +- .../src/interpret/intrinsics.rs | 24 +- .../rustc_const_eval/src/interpret/machine.rs | 2 +- .../rustc_const_eval/src/interpret/operand.rs | 22 +- .../src/interpret/operator.rs | 237 ++++++------------ .../rustc_const_eval/src/interpret/step.rs | 14 +- .../src/interpret/terminator.rs | 2 +- .../rustc_middle/src/mir/interpret/error.rs | 19 +- compiler/rustc_middle/src/mir/tcx.rs | 14 +- .../src/dataflow_const_prop.rs | 31 ++- compiler/rustc_mir_transform/src/gvn.rs | 19 +- .../src/known_panics_lint.rs | 67 ++--- src/tools/miri/src/concurrency/data_race.rs | 11 +- src/tools/miri/src/intrinsics/atomic.rs | 8 +- src/tools/miri/src/intrinsics/mod.rs | 8 +- src/tools/miri/src/intrinsics/simd.rs | 36 +-- src/tools/miri/src/machine.rs | 2 +- src/tools/miri/src/operator.rs | 12 +- src/tools/miri/src/shims/x86/mod.rs | 53 ++-- .../tests/fail/intrinsics/simd-shl-too-far.rs | 2 +- .../fail/intrinsics/simd-shl-too-far.stderr | 4 +- .../tests/fail/intrinsics/simd-shr-too-far.rs | 2 +- .../fail/intrinsics/simd-shr-too-far.stderr | 4 +- .../tests/fail/intrinsics/unchecked_add1.rs | 2 +- .../fail/intrinsics/unchecked_add1.stderr | 4 +- .../tests/fail/intrinsics/unchecked_add2.rs | 2 +- .../fail/intrinsics/unchecked_add2.stderr | 4 +- .../tests/fail/intrinsics/unchecked_mul1.rs | 2 +- .../fail/intrinsics/unchecked_mul1.stderr | 4 +- .../tests/fail/intrinsics/unchecked_mul2.rs | 2 +- .../fail/intrinsics/unchecked_mul2.stderr | 4 +- .../tests/fail/intrinsics/unchecked_sub1.rs | 2 +- .../fail/intrinsics/unchecked_sub1.stderr | 4 +- .../tests/fail/intrinsics/unchecked_sub2.rs | 2 +- .../fail/intrinsics/unchecked_sub2.stderr | 4 +- tests/ui/consts/const-int-unchecked.stderr | 6 +- 40 files changed, 323 insertions(+), 349 deletions(-) diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index 20f0f27517ffe..2dbeb7d5e0ca4 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/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} diff --git a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs index 94c9f056b302e..530a05a1ed88c 100644 --- a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs +++ b/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. diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 836e548ae2b7f..315d4c15a9b17 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/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"); } diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 90d4f1168e4fd..e5ea4c3442eac 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/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); diff --git a/compiler/rustc_const_eval/src/interpret/discriminant.rs b/compiler/rustc_const_eval/src/interpret/discriminant.rs index 8ddc741de239a..f0f898fdd2882 100644 --- a/compiler/rustc_const_eval/src/interpret/discriminant.rs +++ b/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))) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index dce4d56f7e007..030ae942b8093 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -285,9 +285,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!( @@ -299,7 +300,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, @@ -309,7 +310,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 { @@ -515,9 +516,8 @@ 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}"), @@ -525,7 +525,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) } // `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( @@ -538,8 +539,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() { @@ -570,7 +572,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } } else { - val.to_scalar() + val }) } diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 2eaebc1924bc6..72a16dbe4d6a0 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/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!) diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index bad9732f48310..71d22e1ca8620 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/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. diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index 5f59e3d887e46..009f94538a3d3 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -1,78 +1,22 @@ +use either::Either; + use rustc_apfloat::{Float, FloatConvert}; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_middle::ty::layout::{LayoutOf, TyAndLayout}; -use rustc_middle::ty::{self, FloatTy, ScalarInt, Ty}; +use rustc_middle::ty::{self, FloatTy, ScalarInt}; use rustc_middle::{bug, span_bug}; use rustc_span::symbol::sym; -use rustc_target::abi::Abi; - -use super::{err_ub, throw_ub, throw_ub_custom, ImmTy, Immediate, InterpCx, Machine, PlaceTy}; -use crate::fluent_generated as fluent; - -impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { - /// Applies the binary operation `op` to the two operands and writes a tuple of the result - /// and a boolean signifying the potential overflow to the destination. - pub fn binop_with_overflow( - &mut self, - op: mir::BinOp, - left: &ImmTy<'tcx, M::Provenance>, - right: &ImmTy<'tcx, M::Provenance>, - dest: &PlaceTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx> { - let (val, overflowed) = self.overflowing_binary_op(op, left, right)?; - debug_assert_eq!( - Ty::new_tup(self.tcx.tcx, &[val.layout.ty, self.tcx.types.bool]), - dest.layout.ty, - "type mismatch for result of {op:?}", - ); - // Write the result to `dest`. - if let Abi::ScalarPair(..) = dest.layout.abi { - // We can use the optimized path and avoid `place_field` (which might do - // `force_allocation`). - let pair = Immediate::ScalarPair(val.to_scalar(), Scalar::from_bool(overflowed)); - self.write_immediate(pair, dest)?; - } else { - assert!(self.tcx.sess.opts.unstable_opts.randomize_layout); - // With randomized layout, `(int, bool)` might cease to be a `ScalarPair`, so we have to - // do a component-wise write here. This code path is slower than the above because - // `place_field` will have to `force_allocate` locals here. - let val_field = self.project_field(dest, 0)?; - self.write_scalar(val.to_scalar(), &val_field)?; - let overflowed_field = self.project_field(dest, 1)?; - self.write_scalar(Scalar::from_bool(overflowed), &overflowed_field)?; - } - Ok(()) - } - - /// Applies the binary operation `op` to the arguments and writes the result to the - /// destination. - pub fn binop_ignore_overflow( - &mut self, - op: mir::BinOp, - left: &ImmTy<'tcx, M::Provenance>, - right: &ImmTy<'tcx, M::Provenance>, - dest: &PlaceTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx> { - let val = self.wrapping_binary_op(op, left, right)?; - assert_eq!(val.layout.ty, dest.layout.ty, "type mismatch for result of {op:?}"); - self.write_immediate(*val, dest) - } -} +use super::{err_ub, throw_ub, ImmTy, InterpCx, Machine}; impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { - fn three_way_compare(&self, lhs: T, rhs: T) -> (ImmTy<'tcx, M::Provenance>, bool) { + fn three_way_compare(&self, lhs: T, rhs: T) -> ImmTy<'tcx, M::Provenance> { let res = Ord::cmp(&lhs, &rhs); - return (ImmTy::from_ordering(res, *self.tcx), false); + return ImmTy::from_ordering(res, *self.tcx); } - fn binary_char_op( - &self, - bin_op: mir::BinOp, - l: char, - r: char, - ) -> (ImmTy<'tcx, M::Provenance>, bool) { + fn binary_char_op(&self, bin_op: mir::BinOp, l: char, r: char) -> ImmTy<'tcx, M::Provenance> { use rustc_middle::mir::BinOp::*; if bin_op == Cmp { @@ -88,15 +32,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ge => l >= r, _ => span_bug!(self.cur_span(), "Invalid operation on char: {:?}", bin_op), }; - (ImmTy::from_bool(res, *self.tcx), false) + ImmTy::from_bool(res, *self.tcx) } - fn binary_bool_op( - &self, - bin_op: mir::BinOp, - l: bool, - r: bool, - ) -> (ImmTy<'tcx, M::Provenance>, bool) { + fn binary_bool_op(&self, bin_op: mir::BinOp, l: bool, r: bool) -> ImmTy<'tcx, M::Provenance> { use rustc_middle::mir::BinOp::*; let res = match bin_op { @@ -111,7 +50,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { BitXor => l ^ r, _ => span_bug!(self.cur_span(), "Invalid operation on bool: {:?}", bin_op), }; - (ImmTy::from_bool(res, *self.tcx), false) + ImmTy::from_bool(res, *self.tcx) } fn binary_float_op + Into>>( @@ -120,14 +59,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { layout: TyAndLayout<'tcx>, l: F, r: F, - ) -> (ImmTy<'tcx, M::Provenance>, bool) { + ) -> ImmTy<'tcx, M::Provenance> { use rustc_middle::mir::BinOp::*; // Performs appropriate non-deterministic adjustments of NaN results. let adjust_nan = |f: F| -> F { if f.is_nan() { M::generate_nan(self, &[l, r]) } else { f } }; - let val = match bin_op { + match bin_op { Eq => ImmTy::from_bool(l == r, *self.tcx), Ne => ImmTy::from_bool(l != r, *self.tcx), Lt => ImmTy::from_bool(l < r, *self.tcx), @@ -140,8 +79,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Div => ImmTy::from_scalar(adjust_nan((l / r).value).into(), layout), Rem => ImmTy::from_scalar(adjust_nan((l % r).value).into(), layout), _ => span_bug!(self.cur_span(), "invalid float op: `{:?}`", bin_op), - }; - (val, false) + } } fn binary_int_op( @@ -149,7 +87,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { bin_op: mir::BinOp, left: &ImmTy<'tcx, M::Provenance>, right: &ImmTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, (ImmTy<'tcx, M::Provenance>, bool)> { + ) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> { use rustc_middle::mir::BinOp::*; // This checks the size, so that we can just assert it below. @@ -169,25 +107,27 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ShrUnchecked => Some(sym::unchecked_shr), _ => None, }; + let with_overflow = bin_op.is_overflowing(); // Shift ops can have an RHS with a different numeric type. if matches!(bin_op, Shl | ShlUnchecked | Shr | ShrUnchecked) { let size = left.layout.size.bits(); - // The shift offset is implicitly masked to the type size. (This is the one MIR operator - // that does *not* directly map to a single LLVM operation.) Compute how much we - // actually shift and whether there was an overflow due to shifting too much. + // Compute the equivalent shift modulo `size` that is in the range `0..size`. (This is + // the one MIR operator that does *not* directly map to a single LLVM operation.) let (shift_amount, overflow) = if right.layout.abi.is_signed() { let shift_amount = r_signed(); let overflow = shift_amount < 0 || shift_amount >= i128::from(size); // Deliberately wrapping `as` casts: shift_amount *can* be negative, but the result // of the `as` will be equal modulo `size` (since it is a power of two). let masked_amount = (shift_amount as u128) % u128::from(size); - assert_eq!(overflow, shift_amount != (masked_amount as i128)); + assert_eq!(overflow, shift_amount != i128::try_from(masked_amount).unwrap()); (masked_amount, overflow) } else { let shift_amount = r_unsigned(); + let overflow = shift_amount >= u128::from(size); let masked_amount = shift_amount % u128::from(size); - (masked_amount, shift_amount != masked_amount) + assert_eq!(overflow, shift_amount != masked_amount); + (masked_amount, overflow) }; let shift_amount = u32::try_from(shift_amount).unwrap(); // we masked so this will always fit // Compute the shifted result. @@ -209,19 +149,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ScalarInt::truncate_from_uint(result, left.layout.size).0 }; - if overflow && let Some(intrinsic_name) = throw_ub_on_overflow { - throw_ub_custom!( - fluent::const_eval_overflow_shift, - val = if right.layout.abi.is_signed() { - r_signed().to_string() + if overflow && let Some(intrinsic) = throw_ub_on_overflow { + throw_ub!(ShiftOverflow { + intrinsic, + shift_amount: if right.layout.abi.is_signed() { + Either::Right(r_signed()) } else { - r_unsigned().to_string() - }, - name = intrinsic_name - ); + Either::Left(r_unsigned()) + } + }); } - return Ok((ImmTy::from_scalar_int(result, left.layout), overflow)); + return Ok(ImmTy::from_scalar_int(result, left.layout)); } // For the remaining ops, the types must be the same on both sides @@ -246,7 +185,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { _ => None, }; if let Some(op) = op { - return Ok((ImmTy::from_bool(op(&l_signed(), &r_signed()), *self.tcx), false)); + return Ok(ImmTy::from_bool(op(&l_signed(), &r_signed()), *self.tcx)); } if bin_op == Cmp { return Ok(self.three_way_compare(l_signed(), r_signed())); @@ -256,9 +195,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Rem if r.is_null() => throw_ub!(RemainderByZero), Div => Some(i128::overflowing_div), Rem => Some(i128::overflowing_rem), - Add | AddUnchecked => Some(i128::overflowing_add), - Sub | SubUnchecked => Some(i128::overflowing_sub), - Mul | MulUnchecked => Some(i128::overflowing_mul), + Add | AddUnchecked | AddWithOverflow => Some(i128::overflowing_add), + Sub | SubUnchecked | SubWithOverflow => Some(i128::overflowing_sub), + Mul | MulUnchecked | MulWithOverflow => Some(i128::overflowing_mul), _ => None, }; if let Some(op) = op { @@ -282,10 +221,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // If that truncation loses any information, we have an overflow. let (result, lossy) = ScalarInt::truncate_from_int(result, left.layout.size); let overflow = oflo || lossy; - if overflow && let Some(intrinsic_name) = throw_ub_on_overflow { - throw_ub_custom!(fluent::const_eval_overflow, name = intrinsic_name); + if overflow && let Some(intrinsic) = throw_ub_on_overflow { + throw_ub!(ArithOverflow { intrinsic }); } - return Ok((ImmTy::from_scalar_int(result, left.layout), overflow)); + let res = ImmTy::from_scalar_int(result, left.layout); + return Ok(if with_overflow { + let overflow = ImmTy::from_bool(overflow, *self.tcx); + ImmTy::from_pair(res, overflow, *self.tcx) + } else { + res + }); } } // From here on it's okay to treat everything as unsigned. @@ -296,7 +241,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(self.three_way_compare(l, r)); } - let val = match bin_op { + Ok(match bin_op { Eq => ImmTy::from_bool(l == r, *self.tcx), Ne => ImmTy::from_bool(l != r, *self.tcx), @@ -309,40 +254,42 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { BitAnd => ImmTy::from_uint(l & r, left.layout), BitXor => ImmTy::from_uint(l ^ r, left.layout), - Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Rem | Div => { + _ => { assert!(!left.layout.abi.is_signed()); let op: fn(u128, u128) -> (u128, bool) = match bin_op { - Add | AddUnchecked => u128::overflowing_add, - Sub | SubUnchecked => u128::overflowing_sub, - Mul | MulUnchecked => u128::overflowing_mul, + Add | AddUnchecked | AddWithOverflow => u128::overflowing_add, + Sub | SubUnchecked | SubWithOverflow => u128::overflowing_sub, + Mul | MulUnchecked | MulWithOverflow => u128::overflowing_mul, Div if r == 0 => throw_ub!(DivisionByZero), Rem if r == 0 => throw_ub!(RemainderByZero), Div => u128::overflowing_div, Rem => u128::overflowing_rem, - _ => bug!(), + _ => span_bug!( + self.cur_span(), + "invalid binary op {:?}: {:?}, {:?} (both {})", + bin_op, + left, + right, + right.layout.ty, + ), }; let (result, oflo) = op(l, r); // Truncate to target type. // If that truncation loses any information, we have an overflow. let (result, lossy) = ScalarInt::truncate_from_uint(result, left.layout.size); let overflow = oflo || lossy; - if overflow && let Some(intrinsic_name) = throw_ub_on_overflow { - throw_ub_custom!(fluent::const_eval_overflow, name = intrinsic_name); + if overflow && let Some(intrinsic) = throw_ub_on_overflow { + throw_ub!(ArithOverflow { intrinsic }); + } + let res = ImmTy::from_scalar_int(result, left.layout); + if with_overflow { + let overflow = ImmTy::from_bool(overflow, *self.tcx); + ImmTy::from_pair(res, overflow, *self.tcx) + } else { + res } - return Ok((ImmTy::from_scalar_int(result, left.layout), overflow)); } - - _ => span_bug!( - self.cur_span(), - "invalid binary op {:?}: {:?}, {:?} (both {})", - bin_op, - left, - right, - right.layout.ty, - ), - }; - - Ok((val, false)) + }) } fn binary_ptr_op( @@ -350,7 +297,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { bin_op: mir::BinOp, left: &ImmTy<'tcx, M::Provenance>, right: &ImmTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, (ImmTy<'tcx, M::Provenance>, bool)> { + ) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> { use rustc_middle::mir::BinOp::*; match bin_op { @@ -369,10 +316,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { offset_count.checked_mul(pointee_size).ok_or(err_ub!(PointerArithOverflow))?; let offset_ptr = self.ptr_offset_inbounds(ptr, offset_bytes)?; - Ok(( - ImmTy::from_scalar(Scalar::from_maybe_pointer(offset_ptr, self), left.layout), - false, - )) + Ok(ImmTy::from_scalar(Scalar::from_maybe_pointer(offset_ptr, self), left.layout)) } // Fall back to machine hook so Miri can support more pointer ops. @@ -381,12 +325,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } /// Returns the result of the specified operation, and whether it overflowed. - pub fn overflowing_binary_op( + pub fn binary_op( &self, bin_op: mir::BinOp, left: &ImmTy<'tcx, M::Provenance>, right: &ImmTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, (ImmTy<'tcx, M::Provenance>, bool)> { + ) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> { trace!( "Running binary op {:?}: {:?} ({}), {:?} ({})", bin_op, @@ -458,24 +402,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - #[inline] - pub fn wrapping_binary_op( - &self, - bin_op: mir::BinOp, - left: &ImmTy<'tcx, M::Provenance>, - right: &ImmTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> { - let (val, _overflow) = self.overflowing_binary_op(bin_op, left, right)?; - Ok(val) - } - /// Returns the result of the specified operation, whether it overflowed, and /// the result type. - pub fn overflowing_unary_op( + pub fn unary_op( &self, un_op: mir::UnOp, val: &ImmTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, (ImmTy<'tcx, M::Provenance>, bool)> { + ) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> { use rustc_middle::mir::UnOp::*; let layout = val.layout; @@ -489,7 +422,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Not => !val, _ => span_bug!(self.cur_span(), "Invalid bool op {:?}", un_op), }; - Ok((ImmTy::from_bool(res, *self.tcx), false)) + Ok(ImmTy::from_bool(res, *self.tcx)) } ty::Float(fty) => { // No NaN adjustment here, `-` is a bitwise operation! @@ -498,37 +431,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (Neg, FloatTy::F64) => Scalar::from_f64(-val.to_f64()?), _ => span_bug!(self.cur_span(), "Invalid float op {:?}", un_op), }; - Ok((ImmTy::from_scalar(res, layout), false)) + Ok(ImmTy::from_scalar(res, layout)) } _ => { assert!(layout.ty.is_integral()); let val = val.to_bits(layout.size)?; - let (res, overflow) = match un_op { - Not => (self.truncate(!val, layout), false), // bitwise negation, then truncate + let res = match un_op { + Not => self.truncate(!val, layout), // bitwise negation, then truncate Neg => { // arithmetic negation assert!(layout.abi.is_signed()); let val = self.sign_extend(val, layout) as i128; - let (res, overflow) = val.overflowing_neg(); + let res = val.wrapping_neg(); let res = res as u128; // Truncate to target type. - // If that truncation loses any information, we have an overflow. - let truncated = self.truncate(res, layout); - (truncated, overflow || self.sign_extend(truncated, layout) != res) + self.truncate(res, layout) } }; - Ok((ImmTy::from_uint(res, layout), overflow)) + Ok(ImmTy::from_uint(res, layout)) } } } - - #[inline] - pub fn wrapping_unary_op( - &self, - un_op: mir::UnOp, - val: &ImmTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> { - let (val, _overflow) = self.overflowing_unary_op(un_op, val)?; - Ok(val) - } } diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index cb72d55a9ba18..4d653bc47195b 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -167,19 +167,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let left = self.read_immediate(&self.eval_operand(left, layout)?)?; let layout = util::binop_right_homogeneous(bin_op).then_some(left.layout); let right = self.read_immediate(&self.eval_operand(right, layout)?)?; - if let Some(bin_op) = bin_op.overflowing_to_wrapping() { - self.binop_with_overflow(bin_op, &left, &right, &dest)?; - } else { - self.binop_ignore_overflow(bin_op, &left, &right, &dest)?; - } + let result = self.binary_op(bin_op, &left, &right)?; + assert_eq!(result.layout, dest.layout, "layout mismatch for result of {bin_op:?}"); + self.write_immediate(*result, &dest)?; } UnaryOp(un_op, ref operand) => { // The operand always has the same type as the result. let val = self.read_immediate(&self.eval_operand(operand, Some(dest.layout))?)?; - let val = self.wrapping_unary_op(un_op, &val)?; - assert_eq!(val.layout, dest.layout, "layout mismatch for result of {un_op:?}"); - self.write_immediate(*val, &dest)?; + let result = self.unary_op(un_op, &val)?; + assert_eq!(result.layout, dest.layout, "layout mismatch for result of {un_op:?}"); + self.write_immediate(*result, &dest)?; } Aggregate(box ref kind, ref operands) => { diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index b82c18578588b..cfe4a3d9cfc9d 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -97,7 +97,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { for (const_int, target) in targets.iter() { // Compare using MIR BinOp::Eq, to also support pointer values. // (Avoiding `self.binary_op` as that does some redundant layout computation.) - let res = self.wrapping_binary_op( + let res = self.binary_op( mir::BinOp::Eq, &discr, &ImmTy::from_uint(const_int, discr.layout), diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index 6e152cbcb6571..ed0e7c836c291 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -1,19 +1,22 @@ -use super::{AllocId, AllocRange, ConstAllocation, Pointer, Scalar}; +use std::borrow::Cow; +use std::{any::Any, backtrace::Backtrace, fmt}; -use crate::error; -use crate::mir::{ConstAlloc, ConstValue}; -use crate::ty::{self, layout, tls, Ty, TyCtxt, ValTree}; +use either::Either; use rustc_ast_ir::Mutability; use rustc_data_structures::sync::Lock; use rustc_errors::{DiagArgName, DiagArgValue, DiagMessage, ErrorGuaranteed, IntoDiagArg}; use rustc_macros::{HashStable, TyDecodable, TyEncodable}; use rustc_session::CtfeBacktrace; +use rustc_span::Symbol; use rustc_span::{def_id::DefId, Span, DUMMY_SP}; use rustc_target::abi::{call, Align, Size, VariantIdx, WrappingRange}; -use std::borrow::Cow; -use std::{any::Any, backtrace::Backtrace, fmt}; +use super::{AllocId, AllocRange, ConstAllocation, Pointer, Scalar}; + +use crate::error; +use crate::mir::{ConstAlloc, ConstValue}; +use crate::ty::{self, layout, tls, Ty, TyCtxt, ValTree}; #[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] pub enum ErrorHandled { @@ -310,6 +313,10 @@ pub enum UndefinedBehaviorInfo<'tcx> { RemainderOverflow, /// Overflowing inbounds pointer arithmetic. PointerArithOverflow, + /// Overflow in arithmetic that may not overflow. + ArithOverflow { intrinsic: Symbol }, + /// Shift by too much. + ShiftOverflow { intrinsic: Symbol, shift_amount: Either }, /// Invalid metadata in a wide pointer InvalidMeta(InvalidMetaKind), /// Reading a C string that does not end within its allocation. diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index e1ae2e0866675..56aba3c04edfb 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -297,9 +297,9 @@ impl BorrowKind { impl BinOp { pub fn to_hir_binop(self) -> hir::BinOpKind { match self { - BinOp::Add => hir::BinOpKind::Add, - BinOp::Sub => hir::BinOpKind::Sub, - BinOp::Mul => hir::BinOpKind::Mul, + BinOp::Add | BinOp::AddWithOverflow => hir::BinOpKind::Add, + BinOp::Sub | BinOp::SubWithOverflow => hir::BinOpKind::Sub, + BinOp::Mul | BinOp::MulWithOverflow => hir::BinOpKind::Mul, BinOp::Div => hir::BinOpKind::Div, BinOp::Rem => hir::BinOpKind::Rem, BinOp::BitXor => hir::BinOpKind::BitXor, @@ -314,9 +314,6 @@ impl BinOp { BinOp::Le => hir::BinOpKind::Le, BinOp::Ge => hir::BinOpKind::Ge, BinOp::Cmp - | BinOp::AddWithOverflow - | BinOp::SubWithOverflow - | BinOp::MulWithOverflow | BinOp::AddUnchecked | BinOp::SubUnchecked | BinOp::MulUnchecked @@ -338,6 +335,11 @@ impl BinOp { }) } + /// Returns whether this is a `FooWithOverflow` + pub fn is_overflowing(self) -> bool { + self.overflowing_to_wrapping().is_some() + } + /// If this is a `Foo`, return `Some(FooWithOverflow)`. pub fn wrapping_to_overflowing(self) -> Option { Some(match self { diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 3d24a56cdd7c6..53a016f01ecef 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -165,9 +165,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { } } } - Rvalue::BinaryOp(overflowing_op, box (left, right)) - if let Some(op) = overflowing_op.overflowing_to_wrapping() => - { + Rvalue::BinaryOp(op, box (left, right)) if op.is_overflowing() => { // Flood everything now, so we can use `insert_value_idx` directly later. state.flood(target.as_ref(), self.map()); @@ -177,7 +175,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { let overflow_target = self.map().apply(target, TrackElem::Field(1_u32.into())); if value_target.is_some() || overflow_target.is_some() { - let (val, overflow) = self.binary_op(state, op, left, right); + let (val, overflow) = self.binary_op(state, *op, left, right); if let Some(value_target) = value_target { // We have flooded `target` earlier. @@ -186,7 +184,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { if let Some(overflow_target) = overflow_target { let overflow = match overflow { FlatSet::Top => FlatSet::Top, - FlatSet::Elem(overflow) => FlatSet::Elem(Scalar::from_bool(overflow)), + FlatSet::Elem(overflow) => FlatSet::Elem(overflow), FlatSet::Bottom => FlatSet::Bottom, }; // We have flooded `target` earlier. @@ -266,15 +264,16 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> { FlatSet::Top => FlatSet::Top, } } - Rvalue::BinaryOp(op, box (left, right)) => { + Rvalue::BinaryOp(op, box (left, right)) if !op.is_overflowing() => { // Overflows must be ignored here. + // The overflowing operators are handled in `handle_assign`. let (val, _overflow) = self.binary_op(state, *op, left, right); val } Rvalue::UnaryOp(op, operand) => match self.eval_operand(operand, state) { FlatSet::Elem(value) => self .ecx - .wrapping_unary_op(*op, &value) + .unary_op(*op, &value) .map_or(FlatSet::Top, |val| self.wrap_immediate(*val)), FlatSet::Bottom => FlatSet::Bottom, FlatSet::Top => FlatSet::Top, @@ -439,7 +438,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { op: BinOp, left: &Operand<'tcx>, right: &Operand<'tcx>, - ) -> (FlatSet, FlatSet) { + ) -> (FlatSet, FlatSet) { let left = self.eval_operand(left, state); let right = self.eval_operand(right, state); @@ -447,9 +446,17 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { (FlatSet::Bottom, _) | (_, FlatSet::Bottom) => (FlatSet::Bottom, FlatSet::Bottom), // Both sides are known, do the actual computation. (FlatSet::Elem(left), FlatSet::Elem(right)) => { - match self.ecx.overflowing_binary_op(op, &left, &right) { - Ok((val, overflow)) => { - (FlatSet::Elem(val.to_scalar()), FlatSet::Elem(overflow)) + match self.ecx.binary_op(op, &left, &right) { + // Ideally this would return an Immediate, since it's sometimes + // a pair and sometimes not. But as a hack we always return a pair + // and just make the 2nd component `Bottom` when it does not exist. + Ok(val) => { + if matches!(val.layout.abi, Abi::ScalarPair(..)) { + let (val, overflow) = val.to_scalar_pair(); + (FlatSet::Elem(val), FlatSet::Elem(overflow)) + } else { + (FlatSet::Elem(val.to_scalar()), FlatSet::Bottom) + } } _ => (FlatSet::Top, FlatSet::Top), } @@ -475,7 +482,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> { (FlatSet::Elem(arg_scalar), FlatSet::Bottom) } BinOp::Mul if layout.ty.is_integral() && arg_value == 0 => { - (FlatSet::Elem(arg_scalar), FlatSet::Elem(false)) + (FlatSet::Elem(arg_scalar), FlatSet::Elem(Scalar::from_bool(false))) } _ => (FlatSet::Top, FlatSet::Top), } diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 1f3e407180b5f..9d2e7153eb569 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -223,7 +223,7 @@ enum Value<'tcx> { NullaryOp(NullOp<'tcx>, Ty<'tcx>), UnaryOp(UnOp, VnIndex), BinaryOp(BinOp, VnIndex, VnIndex), - CheckedBinaryOp(BinOp, VnIndex, VnIndex), + CheckedBinaryOp(BinOp, VnIndex, VnIndex), // FIXME get rid of this, work like MIR instead Cast { kind: CastKind, value: VnIndex, @@ -497,7 +497,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { UnaryOp(un_op, operand) => { let operand = self.evaluated[operand].as_ref()?; let operand = self.ecx.read_immediate(operand).ok()?; - let (val, _) = self.ecx.overflowing_unary_op(un_op, &operand).ok()?; + let val = self.ecx.unary_op(un_op, &operand).ok()?; val.into() } BinaryOp(bin_op, lhs, rhs) => { @@ -505,7 +505,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let lhs = self.ecx.read_immediate(lhs).ok()?; let rhs = self.evaluated[rhs].as_ref()?; let rhs = self.ecx.read_immediate(rhs).ok()?; - let (val, _) = self.ecx.overflowing_binary_op(bin_op, &lhs, &rhs).ok()?; + let val = self.ecx.binary_op(bin_op, &lhs, &rhs).ok()?; val.into() } CheckedBinaryOp(bin_op, lhs, rhs) => { @@ -513,14 +513,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { let lhs = self.ecx.read_immediate(lhs).ok()?; let rhs = self.evaluated[rhs].as_ref()?; let rhs = self.ecx.read_immediate(rhs).ok()?; - let (val, overflowed) = self.ecx.overflowing_binary_op(bin_op, &lhs, &rhs).ok()?; - let tuple = Ty::new_tup_from_iter( - self.tcx, - [val.layout.ty, self.tcx.types.bool].into_iter(), - ); - let tuple = self.ecx.layout_of(tuple).ok()?; - ImmTy::from_scalar_pair(val.to_scalar(), Scalar::from_bool(overflowed), tuple) - .into() + let val = self + .ecx + .binary_op(bin_op.wrapping_to_overflowing().unwrap(), &lhs, &rhs) + .ok()?; + val.into() } Cast { kind, value, from: _, to } => match kind { CastKind::IntToInt | CastKind::IntToFloat => { diff --git a/compiler/rustc_mir_transform/src/known_panics_lint.rs b/compiler/rustc_mir_transform/src/known_panics_lint.rs index 38fc37a3a3131..0fa5c1b912612 100644 --- a/compiler/rustc_mir_transform/src/known_panics_lint.rs +++ b/compiler/rustc_mir_transform/src/known_panics_lint.rs @@ -304,20 +304,25 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { fn check_unary_op(&mut self, op: UnOp, arg: &Operand<'tcx>, location: Location) -> Option<()> { let arg = self.eval_operand(arg)?; - if let (val, true) = self.use_ecx(|this| { - let val = this.ecx.read_immediate(&arg)?; - let (_res, overflow) = this.ecx.overflowing_unary_op(op, &val)?; - Ok((val, overflow)) - })? { - // `AssertKind` only has an `OverflowNeg` variant, so make sure that is - // appropriate to use. - assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow"); - self.report_assert_as_lint( - location, - AssertLintKind::ArithmeticOverflow, - AssertKind::OverflowNeg(val.to_const_int()), - ); - return None; + // The only operator that can overflow is `Neg`. + if op == UnOp::Neg && arg.layout.ty.is_integral() { + // Compute this as `0 - arg` so we can use `SubWithOverflow` to check for overflow. + let (arg, overflow) = self.use_ecx(|this| { + let arg = this.ecx.read_immediate(&arg)?; + let (_res, overflow) = this + .ecx + .binary_op(BinOp::SubWithOverflow, &ImmTy::from_int(0, arg.layout), &arg)? + .to_scalar_pair(); + Ok((arg, overflow.to_bool()?)) + })?; + if overflow { + self.report_assert_as_lint( + location, + AssertLintKind::ArithmeticOverflow, + AssertKind::OverflowNeg(arg.to_const_int()), + ); + return None; + } } Some(()) @@ -363,11 +368,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } } - if let (Some(l), Some(r)) = (l, r) { - // The remaining operators are handled through `overflowing_binary_op`. + // Div/Rem are handled via the assertions they trigger. + // But for Add/Sub/Mul, those assertions only exist in debug builds, and we want to + // lint in release builds as well, so we check on the operation instead. + // So normalize to the "overflowing" operator, and then ensure that it + // actually is an overflowing operator. + let op = op.wrapping_to_overflowing().unwrap_or(op); + // The remaining operators are handled through `wrapping_to_overflowing`. + if let (Some(l), Some(r)) = (l, r) + && l.layout.ty.is_integral() + && op.is_overflowing() + { if self.use_ecx(|this| { - let (_res, overflow) = this.ecx.overflowing_binary_op(op, &l, &r)?; - Ok(overflow) + let (_res, overflow) = this.ecx.binary_op(op, &l, &r)?.to_scalar_pair(); + overflow.to_bool() })? { self.report_assert_as_lint( location, @@ -399,8 +413,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } Rvalue::BinaryOp(op, box (left, right)) => { trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right); - let op = op.overflowing_to_wrapping().unwrap_or(*op); - self.check_binary_op(op, left, right, location)?; + self.check_binary_op(*op, left, right, location)?; } // Do not try creating references (#67862) @@ -547,17 +560,15 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let right = self.eval_operand(right)?; let right = self.use_ecx(|this| this.ecx.read_immediate(&right))?; - if let Some(bin_op) = bin_op.overflowing_to_wrapping() { - let (val, overflowed) = - self.use_ecx(|this| this.ecx.overflowing_binary_op(bin_op, &left, &right))?; - let overflowed = ImmTy::from_bool(overflowed, self.tcx); + let val = self.use_ecx(|this| this.ecx.binary_op(bin_op, &left, &right))?; + if matches!(val.layout.abi, Abi::ScalarPair(..)) { + // FIXME `Value` should properly support pairs in `Immediate`... but currently it does not. + let (val, overflow) = val.to_pair(&self.ecx); Value::Aggregate { variant: VariantIdx::ZERO, - fields: [Value::from(val), overflowed.into()].into_iter().collect(), + fields: [val.into(), overflow.into()].into_iter().collect(), } } else { - let val = - self.use_ecx(|this| this.ecx.wrapping_binary_op(bin_op, &left, &right))?; val.into() } } @@ -566,7 +577,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let operand = self.eval_operand(operand)?; let val = self.use_ecx(|this| this.ecx.read_immediate(&operand))?; - let val = self.use_ecx(|this| this.ecx.wrapping_unary_op(un_op, &val))?; + let val = self.use_ecx(|this| this.ecx.unary_op(un_op, &val))?; val.into() } diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index f2bec972b18b2..10835e0cb0b34 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -648,7 +648,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { place: &MPlaceTy<'tcx, Provenance>, rhs: &ImmTy<'tcx, Provenance>, op: mir::BinOp, - neg: bool, + not: bool, atomic: AtomicRwOrd, ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); @@ -656,9 +656,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { let old = this.allow_data_races_mut(|this| this.read_immediate(place))?; - // Atomics wrap around on overflow. - let val = this.wrapping_binary_op(op, &old, rhs)?; - let val = if neg { this.wrapping_unary_op(mir::UnOp::Not, &val)? } else { val }; + let val = this.binary_op(op, &old, rhs)?; + let val = if not { this.unary_op(mir::UnOp::Not, &val)? } else { val }; this.allow_data_races_mut(|this| this.write_immediate(*val, place))?; this.validate_atomic_rmw(place, atomic)?; @@ -700,7 +699,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { this.atomic_access_check(place, AtomicAccessType::Rmw)?; let old = this.allow_data_races_mut(|this| this.read_immediate(place))?; - let lt = this.wrapping_binary_op(mir::BinOp::Lt, &old, &rhs)?.to_scalar().to_bool()?; + let lt = this.binary_op(mir::BinOp::Lt, &old, &rhs)?.to_scalar().to_bool()?; #[rustfmt::skip] // rustfmt makes this unreadable let new_val = if min { @@ -744,7 +743,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { // Read as immediate for the sake of `binary_op()` let old = this.allow_data_races_mut(|this| this.read_immediate(place))?; // `binary_op` will bail if either of them is not a scalar. - let eq = this.wrapping_binary_op(mir::BinOp::Eq, &old, expect_old)?; + let eq = this.binary_op(mir::BinOp::Eq, &old, expect_old)?; // If the operation would succeed, but is "weak", fail some portion // of the time, based on `success_rate`. let success_rate = 1.0 - this.machine.cmpxchg_weak_failure_rate; diff --git a/src/tools/miri/src/intrinsics/atomic.rs b/src/tools/miri/src/intrinsics/atomic.rs index 501dbc1603e67..0daf59c943ced 100644 --- a/src/tools/miri/src/intrinsics/atomic.rs +++ b/src/tools/miri/src/intrinsics/atomic.rs @@ -4,8 +4,8 @@ use crate::*; use helpers::check_arg_count; pub enum AtomicOp { - /// The `bool` indicates whether the result of the operation should be negated - /// (must be a boolean-typed operation). + /// The `bool` indicates whether the result of the operation should be negated (`UnOp::Not`, + /// must be a boolean-typed operation). MirOp(mir::BinOp, bool), Max, Min, @@ -213,8 +213,8 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { this.write_immediate(*old, dest)?; // old value is returned Ok(()) } - AtomicOp::MirOp(op, neg) => { - let old = this.atomic_rmw_op_immediate(&place, &rhs, op, neg, atomic)?; + AtomicOp::MirOp(op, not) => { + let old = this.atomic_rmw_op_immediate(&place, &rhs, op, not, atomic)?; this.write_immediate(*old, dest)?; // old value is returned Ok(()) } diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs index 0a7927d062146..64163985a5204 100644 --- a/src/tools/miri/src/intrinsics/mod.rs +++ b/src/tools/miri/src/intrinsics/mod.rs @@ -365,8 +365,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "frem_algebraic" => mir::BinOp::Rem, _ => bug!(), }; - let res = this.wrapping_binary_op(op, &a, &b)?; - // `wrapping_binary_op` already called `generate_nan` if necessary. + let res = this.binary_op(op, &a, &b)?; + // `binary_op` already called `generate_nan` if necessary. this.write_immediate(*res, dest)?; } @@ -411,12 +411,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ), _ => {} } - let res = this.wrapping_binary_op(op, &a, &b)?; + let res = this.binary_op(op, &a, &b)?; if !float_finite(&res)? { throw_ub_format!("`{intrinsic_name}` intrinsic produced non-finite value as result"); } // This cannot be a NaN so we also don't have to apply any non-determinism. - // (Also, `wrapping_binary_op` already called `generate_nan` if needed.) + // (Also, `binary_op` already called `generate_nan` if needed.) this.write_immediate(*res, dest)?; } diff --git a/src/tools/miri/src/intrinsics/simd.rs b/src/tools/miri/src/intrinsics/simd.rs index 4cde364fbc47c..e8ef299c99edc 100644 --- a/src/tools/miri/src/intrinsics/simd.rs +++ b/src/tools/miri/src/intrinsics/simd.rs @@ -1,3 +1,5 @@ +use either::Either; + use rustc_apfloat::{Float, Round}; use rustc_middle::ty::layout::{HasParamEnv, LayoutOf}; use rustc_middle::{mir, ty, ty::FloatTy}; @@ -80,7 +82,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let val = match which { Op::MirOp(mir_op) => { // This already does NaN adjustments - this.wrapping_unary_op(mir_op, &op)?.to_scalar() + this.unary_op(mir_op, &op)?.to_scalar() } Op::Abs => { // Works for f32 and f64. @@ -215,8 +217,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "mul" => Op::MirOp(BinOp::Mul), "div" => Op::MirOp(BinOp::Div), "rem" => Op::MirOp(BinOp::Rem), - "shl" => Op::MirOp(BinOp::Shl), - "shr" => Op::MirOp(BinOp::Shr), + "shl" => Op::MirOp(BinOp::ShlUnchecked), + "shr" => Op::MirOp(BinOp::ShrUnchecked), "and" => Op::MirOp(BinOp::BitAnd), "or" => Op::MirOp(BinOp::BitOr), "xor" => Op::MirOp(BinOp::BitXor), @@ -241,15 +243,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let val = match which { Op::MirOp(mir_op) => { // This does NaN adjustments. - let (val, overflowed) = this.overflowing_binary_op(mir_op, &left, &right)?; - if matches!(mir_op, BinOp::Shl | BinOp::Shr) { - // Shifts have extra UB as SIMD operations that the MIR binop does not have. - // See . - if overflowed { - let r_val = right.to_scalar().to_bits(right.layout.size)?; - throw_ub_format!("overflowing shift by {r_val} in `simd_{intrinsic_name}` in SIMD lane {i}"); + let val = this.binary_op(mir_op, &left, &right).map_err(|err| { + match err.kind() { + InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ShiftOverflow { shift_amount, .. }) => { + // This resets the interpreter backtrace, but it's not worth avoiding that. + let shift_amount = match shift_amount { + Either::Left(v) => v.to_string(), + Either::Right(v) => v.to_string(), + }; + err_ub_format!("overflowing shift by {shift_amount} in `simd_{intrinsic_name}` in lane {i}").into() + } + _ => err } - } + })?; if matches!(mir_op, BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge) { // Special handling for boolean-returning operations assert_eq!(val.layout.ty, this.tcx.types.bool); @@ -368,11 +374,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let op = this.read_immediate(&this.project_index(&op, i)?)?; res = match which { Op::MirOp(mir_op) => { - this.wrapping_binary_op(mir_op, &res, &op)? + this.binary_op(mir_op, &res, &op)? } Op::MirOpBool(mir_op) => { let op = imm_from_bool(simd_element_to_bool(op)?); - this.wrapping_binary_op(mir_op, &res, &op)? + this.binary_op(mir_op, &res, &op)? } Op::MinMax(mmop) => { if matches!(res.layout.ty.kind(), ty::Float(_)) { @@ -383,7 +389,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { MinMax::Min => BinOp::Le, MinMax::Max => BinOp::Ge, }; - if this.wrapping_binary_op(mirop, &res, &op)?.to_scalar().to_bool()? { + if this.binary_op(mirop, &res, &op)?.to_scalar().to_bool()? { res } else { op @@ -412,7 +418,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let mut res = init; for i in 0..op_len { let op = this.read_immediate(&this.project_index(&op, i)?)?; - res = this.wrapping_binary_op(mir_op, &res, &op)?; + res = this.binary_op(mir_op, &res, &op)?; } this.write_immediate(*res, dest)?; } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index cbf02d701bcf5..0c6a2560b1332 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1025,7 +1025,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { bin_op: mir::BinOp, left: &ImmTy<'tcx, Provenance>, right: &ImmTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, (ImmTy<'tcx, Provenance>, bool)> { + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { ecx.binary_ptr_op(bin_op, left, right) } diff --git a/src/tools/miri/src/operator.rs b/src/tools/miri/src/operator.rs index d99be39177bcf..7a008266dbc89 100644 --- a/src/tools/miri/src/operator.rs +++ b/src/tools/miri/src/operator.rs @@ -14,7 +14,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { bin_op: mir::BinOp, left: &ImmTy<'tcx, Provenance>, right: &ImmTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, (ImmTy<'tcx, Provenance>, bool)> { + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { use rustc_middle::mir::BinOp::*; let this = self.eval_context_ref(); @@ -45,7 +45,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ge => left >= right, _ => bug!(), }; - (ImmTy::from_bool(res, *this.tcx), false) + ImmTy::from_bool(res, *this.tcx) } // Some more operations are possible with atomics. @@ -60,16 +60,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { right.to_scalar().to_target_usize(this)?, this.machine.layouts.usize, ); - let (result, overflowing) = this.overflowing_binary_op(bin_op, &left, &right)?; + let result = this.binary_op(bin_op, &left, &right)?; // Construct a new pointer with the provenance of `ptr` (the LHS). let result_ptr = Pointer::new( ptr.provenance, Size::from_bytes(result.to_scalar().to_target_usize(this)?), ); - ( - ImmTy::from_scalar(Scalar::from_maybe_pointer(result_ptr, this), left.layout), - overflowing, - ) + + ImmTy::from_scalar(Scalar::from_maybe_pointer(result_ptr, this), left.layout) } _ => span_bug!(this.cur_span(), "Invalid operator on pointers: {:?}", bin_op), diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 48b7222917b51..74c2b0c61af83 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -50,13 +50,16 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: let a = this.read_immediate(a)?; let b = this.read_immediate(b)?; - let (sum, overflow1) = this.overflowing_binary_op(mir::BinOp::Add, &a, &b)?; - let (sum, overflow2) = this.overflowing_binary_op( - mir::BinOp::Add, - &sum, - &ImmTy::from_uint(c_in, a.layout), - )?; - let c_out = overflow1 | overflow2; + let (sum, overflow1) = + this.binary_op(mir::BinOp::AddWithOverflow, &a, &b)?.to_pair(this); + let (sum, overflow2) = this + .binary_op( + mir::BinOp::AddWithOverflow, + &sum, + &ImmTy::from_uint(c_in, a.layout), + )? + .to_pair(this); + let c_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; this.write_scalar(Scalar::from_u8(c_out.into()), &this.project_field(dest, 0)?)?; this.write_immediate(*sum, &this.project_field(dest, 1)?)?; @@ -76,13 +79,11 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: let a = this.read_immediate(a)?; let b = this.read_immediate(b)?; - let (sub, overflow1) = this.overflowing_binary_op(mir::BinOp::Sub, &a, &b)?; - let (sub, overflow2) = this.overflowing_binary_op( - mir::BinOp::Sub, - &sub, - &ImmTy::from_uint(b_in, a.layout), - )?; - let b_out = overflow1 | overflow2; + let (sub, overflow1) = this.binary_op(mir::BinOp::SubWithOverflow, &a, &b)?.to_pair(this); + let (sub, overflow2) = this + .binary_op(mir::BinOp::SubWithOverflow, &sub, &ImmTy::from_uint(b_in, a.layout))? + .to_pair(this); + let b_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; this.write_scalar(Scalar::from_u8(b_out.into()), &this.project_field(dest, 0)?)?; this.write_immediate(*sub, &this.project_field(dest, 1)?)?; @@ -245,7 +246,7 @@ fn bin_op_float<'tcx, F: rustc_apfloat::Float>( ) -> InterpResult<'tcx, Scalar> { match which { FloatBinOp::Arith(which) => { - let res = this.wrapping_binary_op(which, left, right)?; + let res = this.binary_op(which, left, right)?; Ok(res.to_scalar()) } FloatBinOp::Cmp { gt, lt, eq, unord } => { @@ -744,12 +745,9 @@ fn int_abs<'tcx>( let op = this.read_immediate(&this.project_index(&op, i)?)?; let dest = this.project_index(&dest, i)?; - let lt_zero = this.wrapping_binary_op(mir::BinOp::Lt, &op, &zero)?; - let res = if lt_zero.to_scalar().to_bool()? { - this.wrapping_unary_op(mir::UnOp::Neg, &op)? - } else { - op - }; + let lt_zero = this.binary_op(mir::BinOp::Lt, &op, &zero)?; + let res = + if lt_zero.to_scalar().to_bool()? { this.unary_op(mir::UnOp::Neg, &op)? } else { op }; this.write_immediate(*res, &dest)?; } @@ -832,7 +830,7 @@ fn horizontal_bin_op<'tcx>( let res = if saturating { Immediate::from(this.saturating_arith(which, &lhs, &rhs)?) } else { - *this.wrapping_binary_op(which, &lhs, &rhs)? + *this.binary_op(which, &lhs, &rhs)? }; this.write_immediate(res, &this.project_index(&dest, j)?)?; @@ -884,8 +882,8 @@ fn conditional_dot_product<'tcx>( let left = this.read_immediate(&this.project_index(&left, j)?)?; let right = this.read_immediate(&this.project_index(&right, j)?)?; - let mul = this.wrapping_binary_op(mir::BinOp::Mul, &left, &right)?; - sum = this.wrapping_binary_op(mir::BinOp::Add, &sum, &mul)?; + let mul = this.binary_op(mir::BinOp::Mul, &left, &right)?; + sum = this.binary_op(mir::BinOp::Add, &sum, &mul)?; } } @@ -1276,11 +1274,8 @@ fn psign<'tcx>( let left = this.read_immediate(&this.project_index(&left, i)?)?; let right = this.read_scalar(&this.project_index(&right, i)?)?.to_int(dest.layout.size)?; - let res = this.wrapping_binary_op( - mir::BinOp::Mul, - &left, - &ImmTy::from_int(right.signum(), dest.layout), - )?; + let res = + this.binary_op(mir::BinOp::Mul, &left, &ImmTy::from_int(right.signum(), dest.layout))?; this.write_immediate(*res, &dest)?; } diff --git a/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.rs b/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.rs index 8a49c8403ae37..12aa7c10af4e4 100644 --- a/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.rs +++ b/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.rs @@ -10,6 +10,6 @@ fn main() { unsafe { let x = i32x2(1, 1); let y = i32x2(100, 0); - simd_shl(x, y); //~ERROR: overflowing shift by 100 in `simd_shl` in SIMD lane 0 + simd_shl(x, y); //~ERROR: overflowing shift by 100 in `simd_shl` in lane 0 } } diff --git a/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.stderr b/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.stderr index 3a4ec008b449a..475067db801e2 100644 --- a/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.stderr +++ b/src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflowing shift by 100 in `simd_shl` in SIMD lane 0 +error: Undefined Behavior: overflowing shift by 100 in `simd_shl` in lane 0 --> $DIR/simd-shl-too-far.rs:LL:CC | LL | simd_shl(x, y); - | ^^^^^^^^^^^^^^ overflowing shift by 100 in `simd_shl` in SIMD lane 0 + | ^^^^^^^^^^^^^^ overflowing shift by 100 in `simd_shl` in lane 0 | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.rs b/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.rs index 433998cbde665..ada7cf408c4ed 100644 --- a/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.rs +++ b/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.rs @@ -10,6 +10,6 @@ fn main() { unsafe { let x = i32x2(1, 1); let y = i32x2(20, 40); - simd_shr(x, y); //~ERROR: overflowing shift by 40 in `simd_shr` in SIMD lane 1 + simd_shr(x, y); //~ERROR: overflowing shift by 40 in `simd_shr` in lane 1 } } diff --git a/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.stderr b/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.stderr index 07636b8c0bbd3..0d6307837deb5 100644 --- a/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.stderr +++ b/src/tools/miri/tests/fail/intrinsics/simd-shr-too-far.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflowing shift by 40 in `simd_shr` in SIMD lane 1 +error: Undefined Behavior: overflowing shift by 40 in `simd_shr` in lane 1 --> $DIR/simd-shr-too-far.rs:LL:CC | LL | simd_shr(x, y); - | ^^^^^^^^^^^^^^ overflowing shift by 40 in `simd_shr` in SIMD lane 1 + | ^^^^^^^^^^^^^^ overflowing shift by 40 in `simd_shr` in lane 1 | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_add1.rs b/src/tools/miri/tests/fail/intrinsics/unchecked_add1.rs index 3f8b4e55151ec..0ed758da2a3c7 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_add1.rs +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_add1.rs @@ -1,4 +1,4 @@ fn main() { // MAX overflow - let _val = unsafe { 40000u16.unchecked_add(30000) }; //~ ERROR: overflow executing `unchecked_add` + let _val = unsafe { 40000u16.unchecked_add(30000) }; //~ ERROR: arithmetic overflow in `unchecked_add` } diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_add1.stderr b/src/tools/miri/tests/fail/intrinsics/unchecked_add1.stderr index 922de4226fa80..eae9ec7a44dcd 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_add1.stderr +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_add1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflow executing `unchecked_add` +error: Undefined Behavior: arithmetic overflow in `unchecked_add` --> $DIR/unchecked_add1.rs:LL:CC | LL | let _val = unsafe { 40000u16.unchecked_add(30000) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_add` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_add` | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_add2.rs b/src/tools/miri/tests/fail/intrinsics/unchecked_add2.rs index 3283dbf8e7ef3..3b495203eb41f 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_add2.rs +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_add2.rs @@ -1,4 +1,4 @@ fn main() { // MIN overflow - let _val = unsafe { (-30000i16).unchecked_add(-8000) }; //~ ERROR: overflow executing `unchecked_add` + let _val = unsafe { (-30000i16).unchecked_add(-8000) }; //~ ERROR: arithmetic overflow in `unchecked_add` } diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_add2.stderr b/src/tools/miri/tests/fail/intrinsics/unchecked_add2.stderr index 05456eaf19509..6a0dcfcd227e7 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_add2.stderr +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_add2.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflow executing `unchecked_add` +error: Undefined Behavior: arithmetic overflow in `unchecked_add` --> $DIR/unchecked_add2.rs:LL:CC | LL | let _val = unsafe { (-30000i16).unchecked_add(-8000) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_add` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_add` | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.rs b/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.rs index 2feed7759ec1c..3dc83a2dcef28 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.rs +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.rs @@ -1,4 +1,4 @@ fn main() { // MAX overflow - let _val = unsafe { 300u16.unchecked_mul(250u16) }; //~ ERROR: overflow executing `unchecked_mul` + let _val = unsafe { 300u16.unchecked_mul(250u16) }; //~ ERROR: arithmetic overflow in `unchecked_mul` } diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.stderr b/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.stderr index 533beaa65ff77..e37d9827c8cf6 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.stderr +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_mul1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflow executing `unchecked_mul` +error: Undefined Behavior: arithmetic overflow in `unchecked_mul` --> $DIR/unchecked_mul1.rs:LL:CC | LL | let _val = unsafe { 300u16.unchecked_mul(250u16) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_mul` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_mul` | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.rs b/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.rs index 42cd509404a55..49cd293737c71 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.rs +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.rs @@ -1,4 +1,4 @@ fn main() { // MIN overflow - let _val = unsafe { 1_000_000_000i32.unchecked_mul(-4) }; //~ ERROR: overflow executing `unchecked_mul` + let _val = unsafe { 1_000_000_000i32.unchecked_mul(-4) }; //~ ERROR: arithmetic overflow in `unchecked_mul` } diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.stderr b/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.stderr index 4c6bae6f0bf68..949077ce61d8c 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.stderr +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_mul2.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflow executing `unchecked_mul` +error: Undefined Behavior: arithmetic overflow in `unchecked_mul` --> $DIR/unchecked_mul2.rs:LL:CC | LL | let _val = unsafe { 1_000_000_000i32.unchecked_mul(-4) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_mul` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_mul` | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.rs b/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.rs index e5178bf4effaa..f24c97420948c 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.rs +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.rs @@ -1,4 +1,4 @@ fn main() { // MIN overflow - let _val = unsafe { 14u32.unchecked_sub(22) }; //~ ERROR: overflow executing `unchecked_sub` + let _val = unsafe { 14u32.unchecked_sub(22) }; //~ ERROR: arithmetic overflow in `unchecked_sub` } diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.stderr b/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.stderr index 72187e20e23c3..39bfd8a238404 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.stderr +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_sub1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflow executing `unchecked_sub` +error: Undefined Behavior: arithmetic overflow in `unchecked_sub` --> $DIR/unchecked_sub1.rs:LL:CC | LL | let _val = unsafe { 14u32.unchecked_sub(22) }; - | ^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_sub` + | ^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_sub` | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.rs b/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.rs index ac9fd1e5d0622..74b08b1dfb4e0 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.rs +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.rs @@ -1,4 +1,4 @@ fn main() { // MAX overflow - let _val = unsafe { 30000i16.unchecked_sub(-7000) }; //~ ERROR: overflow executing `unchecked_sub` + let _val = unsafe { 30000i16.unchecked_sub(-7000) }; //~ ERROR: arithmetic overflow in `unchecked_sub` } diff --git a/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.stderr b/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.stderr index ec28384f6336f..604eba99e0147 100644 --- a/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.stderr +++ b/src/tools/miri/tests/fail/intrinsics/unchecked_sub2.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: overflow executing `unchecked_sub` +error: Undefined Behavior: arithmetic overflow in `unchecked_sub` --> $DIR/unchecked_sub2.rs:LL:CC | LL | let _val = unsafe { 30000i16.unchecked_sub(-7000) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_sub` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_sub` | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/tests/ui/consts/const-int-unchecked.stderr b/tests/ui/consts/const-int-unchecked.stderr index 84b222972a13c..3130603231e1b 100644 --- a/tests/ui/consts/const-int-unchecked.stderr +++ b/tests/ui/consts/const-int-unchecked.stderr @@ -242,19 +242,19 @@ error[E0080]: evaluation of constant value failed --> $DIR/const-int-unchecked.rs:123:25 | LL | const _: u16 = unsafe { std::intrinsics::unchecked_add(40000u16, 30000) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_add` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_add` error[E0080]: evaluation of constant value failed --> $DIR/const-int-unchecked.rs:126:25 | LL | const _: u32 = unsafe { std::intrinsics::unchecked_sub(14u32, 22) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_sub` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_sub` error[E0080]: evaluation of constant value failed --> $DIR/const-int-unchecked.rs:129:25 | LL | const _: u16 = unsafe { std::intrinsics::unchecked_mul(300u16, 250u16) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_mul` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ arithmetic overflow in `unchecked_mul` error[E0080]: evaluation of constant value failed --> $DIR/const-int-unchecked.rs:132:25 From cb5319483e0691393bd3e09a808f734321a23d20 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 22 May 2024 09:00:07 +0200 Subject: [PATCH 2/2] clarify comment Co-authored-by: scottmcm --- compiler/rustc_const_eval/src/interpret/operator.rs | 4 +++- compiler/rustc_middle/src/mir/tcx.rs | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index 009f94538a3d3..6992856053a9b 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -324,7 +324,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - /// Returns the result of the specified operation, and whether it overflowed. + /// Returns the result of the specified operation. + /// + /// Whether this produces a scalar or a pair depends on the specific `bin_op`. pub fn binary_op( &self, bin_op: mir::BinOp, diff --git a/compiler/rustc_middle/src/mir/tcx.rs b/compiler/rustc_middle/src/mir/tcx.rs index 56aba3c04edfb..c191de45e70e1 100644 --- a/compiler/rustc_middle/src/mir/tcx.rs +++ b/compiler/rustc_middle/src/mir/tcx.rs @@ -297,6 +297,8 @@ impl BorrowKind { impl BinOp { pub fn to_hir_binop(self) -> hir::BinOpKind { match self { + // HIR `+`/`-`/`*` can map to either of these MIR BinOp, depending + // on whether overflow checks are enabled or not. BinOp::Add | BinOp::AddWithOverflow => hir::BinOpKind::Add, BinOp::Sub | BinOp::SubWithOverflow => hir::BinOpKind::Sub, BinOp::Mul | BinOp::MulWithOverflow => hir::BinOpKind::Mul, @@ -313,6 +315,7 @@ impl BinOp { BinOp::Gt => hir::BinOpKind::Gt, BinOp::Le => hir::BinOpKind::Le, BinOp::Ge => hir::BinOpKind::Ge, + // We don't have HIR syntax for these. BinOp::Cmp | BinOp::AddUnchecked | BinOp::SubUnchecked