Skip to content

Commit

Permalink
Rollup merge of #98888 - RalfJung:interpret-checked-bin, r=oli-obk
Browse files Browse the repository at this point in the history
interpret: fix CheckedBinOp behavior when overflow checking is disabled

Adjusts the interpreter to #98738.

r? `@oli-obk`
  • Loading branch information
matthiaskrgr authored Jul 5, 2022
2 parents 69195c0 + 2f6e996 commit cca43fe
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 4 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
sym::mul_with_overflow => BinOp::Mul,
_ => bug!(),
};
self.binop_with_overflow(bin_op, &lhs, &rhs, dest)?;
self.binop_with_overflow(
bin_op, /*force_overflow_checks*/ true, &lhs, &rhs, dest,
)?;
}
sym::saturating_add | sym::saturating_sub => {
let l = self.read_immediate(&args[0])?;
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ pub trait Machine<'mir, 'tcx>: Sized {
true
}

/// Whether CheckedBinOp MIR statements should actually check for overflow.
fn check_binop_checks_overflow(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;

/// Entry point for obtaining the MIR of anything that should get evaluated.
/// So not just functions and shims, but also const/static initializers, anonymous
/// constants, ...
Expand Down Expand Up @@ -468,6 +471,11 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
true
}

#[inline(always)]
fn check_binop_checks_overflow(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
true
}

#[inline(always)]
fn call_extra_fn(
_ecx: &mut InterpCx<$mir, $tcx, Self>,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ use super::{ImmTy, Immediate, InterpCx, Machine, PlaceTy};
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.
///
/// `force_overflow_checks` indicates whether overflow checks should be done even when
/// `tcx.sess.overflow_checks()` is `false`.
pub fn binop_with_overflow(
&mut self,
op: mir::BinOp,
force_overflow_checks: bool,
left: &ImmTy<'tcx, M::PointerTag>,
right: &ImmTy<'tcx, M::PointerTag>,
dest: &PlaceTy<'tcx, M::PointerTag>,
Expand All @@ -26,6 +30,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
"type mismatch for result of {:?}",
op,
);
// As per https://github.com/rust-lang/rust/pull/98738, we always return `false` in the 2nd
// component when overflow checking is disabled.
let overflowed =
overflowed && (force_overflow_checks || M::check_binop_checks_overflow(self));
// 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`).
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let left = self.read_immediate(&self.eval_operand(left, None)?)?;
let layout = binop_right_homogeneous(bin_op).then_some(left.layout);
let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
self.binop_with_overflow(bin_op, &left, &right, &dest)?;
self.binop_with_overflow(
bin_op, /*force_overflow_checks*/ false, &left, &right, &dest,
)?;
}

UnaryOp(un_op, ref operand) => {
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,8 +993,9 @@ pub enum Rvalue<'tcx> {

/// Same as `BinaryOp`, but yields `(T, bool)` with a `bool` indicating an error condition.
///
/// When overflow checking is disabled, the error condition is false. Otherwise, the error
/// condition is determined as described below.
/// When overflow checking is disabled and we are generating run-time code, the error condition
/// is false. Otherwise, and always during CTFE, the error condition is determined as described
/// below.
///
/// For addition, subtraction, and multiplication on integers the error condition is set when
/// the infinite precision result would be unequal to the actual result.
Expand Down

0 comments on commit cca43fe

Please sign in to comment.