Skip to content

Commit

Permalink
Auto merge of rust-lang#125359 - RalfJung:interpret-overflowing-ops, …
Browse files Browse the repository at this point in the history
…r=<try>

interpret: make overflowing binops just normal binops

Follow-up to rust-lang#125173 (Cc `@scottmcm)`
  • Loading branch information
bors committed May 22, 2024
2 parents b54dd08 + cb53194 commit d373d09
Show file tree
Hide file tree
Showing 40 changed files with 329 additions and 350 deletions.
7 changes: 3 additions & 4 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/dummy_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;

use either::Either;
use rustc_errors::{
codes::*, Diag, DiagArgValue, DiagCtxt, DiagMessage, Diagnostic, EmissionGuarantee, Level,
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_const_eval/src/interpret/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)))
Expand Down
24 changes: 13 additions & 11 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -515,17 +516,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(
Expand All @@ -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() {
Expand Down Expand Up @@ -570,7 +572,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}
} else {
val.to_scalar()
val
})
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand Down
22 changes: 21 additions & 1 deletion compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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]
Expand All @@ -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.
Expand Down
Loading

0 comments on commit d373d09

Please sign in to comment.