From 6f01ff61b3d9d722031fdb39747283c911b97049 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Jul 2022 08:57:10 -0400 Subject: [PATCH 1/2] interpret: fix CheckedBinOp behavior when overflow checking is disabled --- compiler/rustc_const_eval/src/interpret/intrinsics.rs | 4 +++- compiler/rustc_const_eval/src/interpret/operator.rs | 8 ++++++++ compiler/rustc_const_eval/src/interpret/step.rs | 4 +++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index e51c51cf45e..6744aace849 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -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])?; diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index dec7fa8c4aa..454243ddca6 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -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>, @@ -26,6 +30,10 @@ 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 || self.tcx.sess.overflow_checks()); + // 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`). diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 98f69456e49..2ee7ed57ab5 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -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) => { From 2f6e99666218a2f4c0e6b958d710aac445ff85c0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Jul 2022 19:24:41 -0400 Subject: [PATCH 2/2] always check overflow in CheckedBinOp in CTFE --- compiler/rustc_const_eval/src/interpret/machine.rs | 8 ++++++++ compiler/rustc_const_eval/src/interpret/operator.rs | 3 ++- compiler/rustc_middle/src/mir/syntax.rs | 5 +++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index c18ac84171d..34644f4eb37 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -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, ... @@ -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>, diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index 454243ddca6..942bdb36645 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -32,7 +32,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ); // 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 || self.tcx.sess.overflow_checks()); + 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 diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 3b7eb820df8..2f4c0ae96b3 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -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.