Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak implementation of overflow checking assertions #109234

Merged
merged 1 commit into from
Mar 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
11 changes: 2 additions & 9 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,15 +563,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// 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.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ pub trait Machine<'mir, 'tcx>: Sized {

/// 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
Expand Down Expand Up @@ -474,7 +474,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
}

#[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
}

Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

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);
Expand Down
17 changes: 7 additions & 10 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,13 @@ impl<'tcx> BasicBlockData<'tcx> {
}

impl<O> AssertKind<O> {
/// 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.
Expand Down Expand Up @@ -1998,16 +2005,6 @@ impl BorrowKind {
}
}

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::*;
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
}

#[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()
}

Expand Down