From 27b430bcb36e46cc5dcfd88ec6cec2639ad2467c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 16 Mar 2023 00:00:00 +0000 Subject: [PATCH] Tweak implementation of overflow checking assertions Extract and reuse logic controlling behaviour of overflow checking assertions instead of duplicating it three times. --- compiler/rustc_codegen_cranelift/src/base.rs | 15 ++++----------- compiler/rustc_codegen_ssa/src/mir/block.rs | 11 ++--------- .../rustc_const_eval/src/interpret/machine.rs | 4 ++-- .../src/interpret/terminator.rs | 8 ++------ compiler/rustc_middle/src/mir/mod.rs | 17 +++++++---------- compiler/rustc_middle/src/mir/syntax.rs | 3 +-- src/tools/miri/src/machine.rs | 2 +- 7 files changed, 19 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index d0af3729b23..1b8e9312e2f 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -346,17 +346,10 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { crate::abi::codegen_return(fx); } TerminatorKind::Assert { cond, expected, msg, target, cleanup: _ } => { - if !fx.tcx.sess.overflow_checks() { - let overflow_not_to_check = match msg { - AssertKind::OverflowNeg(..) => true, - AssertKind::Overflow(op, ..) => op.is_checkable(), - _ => false, - }; - if overflow_not_to_check { - let target = fx.get_block(*target); - fx.bcx.ins().jump(target, &[]); - continue; - } + if !fx.tcx.sess.overflow_checks() && msg.is_optional_overflow_check() { + let target = fx.get_block(*target); + fx.bcx.ins().jump(target, &[]); + continue; } let cond = codegen_operand(fx, cond).load_scalar(fx); diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 71c71d59b7a..f9aa2aecf65 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -563,15 +563,8 @@ fn codegen_assert_terminator( // with #[rustc_inherit_overflow_checks] and inlined from // another crate (mostly core::num generic/#[inline] fns), // while the current crate doesn't use overflow checks. - if !bx.cx().check_overflow() { - let overflow_not_to_check = match msg { - AssertKind::OverflowNeg(..) => true, - AssertKind::Overflow(op, ..) => op.is_checkable(), - _ => false, - }; - if overflow_not_to_check { - const_cond = Some(expected); - } + if !bx.cx().check_overflow() && msg.is_optional_overflow_check() { + const_cond = Some(expected); } // Don't codegen the panic block if success if known. diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 92fa59aec6e..c134d3a6b2f 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -155,7 +155,7 @@ fn enforce_abi(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { /// Whether Assert(OverflowNeg) and Assert(Overflow) MIR terminators should actually /// check for overflow. - fn ignore_checkable_overflow_assertions(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; + fn ignore_optional_overflow_checks(_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 @@ -474,7 +474,7 @@ fn use_addr_for_alignment_check(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool { } #[inline(always)] - fn ignore_checkable_overflow_assertions(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool { + fn ignore_optional_overflow_checks(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool { false } diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 685a5599cde..c2d1bc11c37 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -138,12 +138,8 @@ pub(super) fn eval_terminator( } Assert { ref cond, expected, ref msg, target, cleanup } => { - let ignored = M::ignore_checkable_overflow_assertions(self) - && match msg { - mir::AssertKind::OverflowNeg(..) => true, - mir::AssertKind::Overflow(op, ..) => op.is_checkable(), - _ => false, - }; + let ignored = + M::ignore_optional_overflow_checks(self) && msg.is_optional_overflow_check(); let cond_val = self.read_scalar(&self.eval_operand(cond, None)?)?.to_bool()?; if ignored || expected == cond_val { self.go_to_block(target); diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index b34651c3ea7..3b26eb8a8ed 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1274,6 +1274,13 @@ pub fn is_empty_unreachable(&self) -> bool { } impl AssertKind { + /// Returns true if this an overflow checking assertion controlled by -C overflow-checks. + pub fn is_optional_overflow_check(&self) -> bool { + use AssertKind::*; + use BinOp::*; + matches!(self, OverflowNeg(..) | Overflow(Add | Sub | Mul | Shl | Shr, ..)) + } + /// Getting a description does not require `O` to be printable, and does not /// require allocation. /// The caller is expected to handle `BoundsCheck` separately. @@ -1998,16 +2005,6 @@ pub fn describe_mutability(&self) -> &str { } } -impl BinOp { - /// The checkable operators are those whose overflow checking behavior is controlled by - /// -Coverflow-checks option. The remaining operators have either no overflow conditions (e.g., - /// BitAnd, BitOr, BitXor) or are always checked for overflow (e.g., Div, Rem). - pub fn is_checkable(self) -> bool { - use self::BinOp::*; - matches!(self, Add | Sub | Mul | Shl | Shr) - } -} - impl<'tcx> Debug for Rvalue<'tcx> { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { use self::Rvalue::*; diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index a28ecfa9bdc..b16b6616415 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -646,8 +646,7 @@ pub enum TerminatorKind<'tcx> { /// When overflow checking is disabled and this is run-time MIR (as opposed to compile-time MIR /// that is used for CTFE), the following variants of this terminator behave as `goto target`: /// - `OverflowNeg(..)`, - /// - `Overflow(op, ..)` if op is a "checkable" operation (add, sub, mul, shl, shr, but NOT - /// div or rem). + /// - `Overflow(op, ..)` if op is add, sub, mul, shl, shr, but NOT div or rem. Assert { cond: Operand<'tcx>, expected: bool, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 969c81f7e32..3a14704d9cc 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -822,7 +822,7 @@ fn enforce_abi(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { } #[inline(always)] - fn ignore_checkable_overflow_assertions(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { + fn ignore_optional_overflow_checks(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { !ecx.tcx.sess.overflow_checks() }