Add a new Assert terminator to MIR for bounds & arithmetic checks.

Author: eddyb

src/librustc/mir/repr.rs

@@ -10,7 +10,7 @@
 
 use graphviz::IntoCow;
 use middle::const_val::ConstVal;
-use rustc_const_math::{ConstUsize, ConstInt};
+use rustc_const_math::{ConstUsize, ConstInt, ConstMathErr};
 use hir::def_id::DefId;
 use ty::subst::Substs;
 use ty::{self, AdtDef, ClosureSubsts, FnOutput, Region, Ty};
@@ -354,6 +354,16 @@ pub enum TerminatorKind<'tcx> {
         /// Cleanups to be done if the call unwinds.
         cleanup: Option<BasicBlock>
     },
+
+    /// Jump to the target if the condition has the expected value,
+    /// otherwise panic with a message and a cleanup target.
+    Assert {
+        cond: Operand<'tcx>,
+        expected: bool,
+        msg: AssertMessage<'tcx>,
+        target: BasicBlock,
+        cleanup: Option<BasicBlock>
+    }
 }
 
 impl<'tcx> Terminator<'tcx> {
@@ -389,6 +399,8 @@ impl<'tcx> TerminatorKind<'tcx> {
             Drop { ref target, unwind: None, .. } => {
                 slice::ref_slice(target).into_cow()
             }
+            Assert { target, cleanup: Some(unwind), .. } => vec![target, unwind].into_cow(),
+            Assert { ref target, .. } => slice::ref_slice(target).into_cow(),
         }
     }
 
@@ -413,6 +425,8 @@ impl<'tcx> TerminatorKind<'tcx> {
             Drop { ref mut target, unwind: None, .. } => {
                 vec![target]
             }
+            Assert { ref mut target, cleanup: Some(ref mut unwind), .. } => vec![target, unwind],
+            Assert { ref mut target, .. } => vec![target]
         }
     }
 }
@@ -495,6 +509,26 @@ impl<'tcx> TerminatorKind<'tcx> {
                 }
                 write!(fmt, ")")
             }
+            Assert { ref cond, expected, ref msg, .. } => {
+                write!(fmt, "assert(")?;
+                if !expected {
+                    write!(fmt, "!")?;
+                }
+                write!(fmt, "{:?}, ", cond)?;
+
+                match *msg {
+                    AssertMessage::BoundsCheck { ref len, ref index } => {
+                        write!(fmt, "{:?}, {:?}, {:?}",
+                               "index out of bounds: the len is {} but the index is {}",
+                               len, index)?;
+                    }
+                    AssertMessage::Math(ref err) => {
+                        write!(fmt, "{:?}", err.description())?;
+                    }
+                }
+
+                write!(fmt, ")")
+            }
         }
     }
 
@@ -532,10 +566,21 @@ impl<'tcx> TerminatorKind<'tcx> {
             Drop { unwind: Some(_), .. } => {
                 vec!["return".into_cow(), "unwind".into_cow()]
             }
+            Assert { cleanup: None, .. } => vec!["".into()],
+            Assert { .. } =>
+                vec!["success".into_cow(), "unwind".into_cow()]
         }
     }
 }
 
+#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
+pub enum AssertMessage<'tcx> {
+    BoundsCheck {
+        len: Operand<'tcx>,
+        index: Operand<'tcx>
+    },
+    Math(ConstMathErr)
+}
 
 ///////////////////////////////////////////////////////////////////////////
 // Statements

src/librustc/mir/visit.rs

@@ -127,6 +127,11 @@ macro_rules! make_mir_visitor {
                 self.super_terminator_kind(block, kind);
             }
 
+            fn visit_assert_message(&mut self,
+                                    msg: & $($mutability)* AssertMessage<'tcx>) {
+                self.super_assert_message(msg);
+            }
+
             fn visit_rvalue(&mut self,
                             rvalue: & $($mutability)* Rvalue<'tcx>) {
                 self.super_rvalue(rvalue);
@@ -426,6 +431,31 @@ macro_rules! make_mir_visitor {
                         }
                         cleanup.map(|t| self.visit_branch(block, t));
                     }
+
+                    TerminatorKind::Assert { ref $($mutability)* cond,
+                                             expected: _,
+                                             ref $($mutability)* msg,
+                                             target,
+                                             cleanup } => {
+                        self.visit_operand(cond);
+                        self.visit_assert_message(msg);
+                        self.visit_branch(block, target);
+                        cleanup.map(|t| self.visit_branch(block, t));
+                    }
+                }
+            }
+
+            fn super_assert_message(&mut self,
+                                    msg: & $($mutability)* AssertMessage<'tcx>) {
+                match *msg {
+                    AssertMessage::BoundsCheck {
+                        ref $($mutability)* len,
+                        ref $($mutability)* index
+                    } => {
+                        self.visit_operand(len);
+                        self.visit_operand(index);
+                    }
+                    AssertMessage::Math(_) => {}
                 }
             }
 

src/librustc_borrowck/borrowck/mir/dataflow/mod.rs

@@ -450,13 +450,14 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D>
             repr::TerminatorKind::Return |
             repr::TerminatorKind::Resume => {}
             repr::TerminatorKind::Goto { ref target } |
+            repr::TerminatorKind::Assert { ref target, cleanup: None, .. } |
             repr::TerminatorKind::Drop { ref target, location: _, unwind: None } |
-
             repr::TerminatorKind::DropAndReplace {
                 ref target, value: _, location: _, unwind: None
             } => {
                 self.propagate_bits_into_entry_set_for(in_out, changed, target);
             }
+            repr::TerminatorKind::Assert { ref target, cleanup: Some(ref unwind), .. } |
             repr::TerminatorKind::Drop { ref target, location: _, unwind: Some(ref unwind) } |
             repr::TerminatorKind::DropAndReplace {
                 ref target, value: _, location: _, unwind: Some(ref unwind)

src/librustc_borrowck/borrowck/mir/gather_moves.rs

@@ -663,6 +663,22 @@ fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> MoveD
                 bb_ctxt.on_operand(SK::If, cond, source);
             }
 
+            TerminatorKind::Assert {
+                ref cond, expected: _,
+                ref msg, target: _, cleanup: _
+            } => {
+                // The `cond` is always of (copyable) type `bool`,
+                // so there will never be anything to move.
+                let _ = cond;
+                match *msg {
+                    AssertMessage:: BoundsCheck { ref len, ref index } => {
+                        // Same for the usize length and index in bounds-checking.
+                        let _ = (len, index);
+                    }
+                    AssertMessage::Math(_) => {}
+                }
+            }
+
             TerminatorKind::SwitchInt { switch_ty: _, values: _, targets: _, ref discr } |
             TerminatorKind::Switch { adt_def: _, targets: _, ref discr } => {
                 // The `discr` is not consumed; that is instead

src/librustc_const_math/err.rs

@@ -10,7 +10,7 @@
 
 use syntax::ast;
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, RustcEncodable, RustcDecodable)]
 pub enum ConstMathErr {
     NotInRange,
     CmpBetweenUnequalTypes,
@@ -25,7 +25,7 @@ pub enum ConstMathErr {
 }
 pub use self::ConstMathErr::*;
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, RustcEncodable, RustcDecodable)]
 pub enum Op {
     Add,
     Sub,

src/librustc_mir/build/block.rs

@@ -12,7 +12,6 @@ use build::{BlockAnd, BlockAndExtension, Builder};
 use hair::*;
 use rustc::mir::repr::*;
 use rustc::hir;
-use syntax::codemap::Span;
 
 impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     pub fn ast_block(&mut self,
@@ -82,22 +81,4 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             block.unit()
         })
     }
-
-    // Helper method for generating a conditional branch
-    // Returns (TrueBlock, FalseBlock)
-    pub fn build_cond_br(&mut self, block: BasicBlock, span: Span,
-                         cond: Operand<'tcx>) -> (BasicBlock, BasicBlock) {
-        let scope_id = self.innermost_scope_id();
-
-        let then_block = self.cfg.start_new_block();
-        let else_block = self.cfg.start_new_block();
-
-        self.cfg.terminate(block, scope_id, span,
-                           TerminatorKind::If {
-                               cond: cond,
-                               targets: (then_block, else_block)
-                           });
-
-        (then_block, else_block)
-    }
 }

src/librustc_mir/build/expr/as_lvalue.rs

@@ -66,15 +66,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                                                            idx.clone(),
                                                            Operand::Consume(len.clone())));
 
-                let (success, failure) = (this.cfg.start_new_block(), this.cfg.start_new_block());
-                this.cfg.terminate(block,
-                                   scope_id,
-                                   expr_span,
-                                   TerminatorKind::If {
-                                       cond: Operand::Consume(lt),
-                                       targets: (success, failure),
-                                   });
-                this.panic_bounds_check(failure, idx.clone(), Operand::Consume(len), expr_span);
+                let msg = AssertMessage::BoundsCheck {
+                    len: Operand::Consume(len),
+                    index: idx.clone()
+                };
+                let success = this.assert(block, Operand::Consume(lt), true,
+                                          msg, expr_span);
                 success.and(slice.index(idx))
             }
             ExprKind::SelfRef => {

src/librustc_mir/build/expr/as_rvalue.rs

@@ -12,6 +12,7 @@
 
 use std;
 
+use rustc_const_math::{ConstMathErr, Op};
 use rustc_data_structures::fnv::FnvHashMap;
 
 use build::{BlockAnd, BlockAndExtension, Builder};
@@ -88,10 +89,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     this.cfg.push_assign(block, scope_id, expr_span, &is_min,
                                          Rvalue::BinaryOp(BinOp::Eq, arg.clone(), minval));
 
-                    let (of_block, ok_block) = this.build_cond_br(block, expr_span,
-                                                                  Operand::Consume(is_min));
-                    this.panic(of_block, "attempted to negate with overflow", expr_span);
-                    block = ok_block;
+                    let err = ConstMathErr::Overflow(Op::Neg);
+                    block = this.assert(block, Operand::Consume(is_min), false,
+                                        AssertMessage::Math(err), expr_span);
                 }
                 block.and(Rvalue::UnaryOp(op, arg))
             }
@@ -261,27 +261,32 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             let val = result_value.clone().field(val_fld, ty);
             let of = result_value.field(of_fld, bool_ty);
 
-            let msg = if op == BinOp::Shl || op == BinOp::Shr {
-                "shift operation overflowed"
-            } else {
-                "arithmetic operation overflowed"
-            };
+            let err = ConstMathErr::Overflow(match op {
+                BinOp::Add => Op::Add,
+                BinOp::Sub => Op::Sub,
+                BinOp::Mul => Op::Mul,
+                BinOp::Shl => Op::Shl,
+                BinOp::Shr => Op::Shr,
+                _ => {
+                    bug!("MIR build_binary_op: {:?} is not checkable", op)
+                }
+            });
 
-            let (of_block, ok_block) = self.build_cond_br(block, span, Operand::Consume(of));
-            self.panic(of_block, msg, span);
+            block = self.assert(block, Operand::Consume(of), false,
+                                AssertMessage::Math(err), span);
 
-            ok_block.and(Rvalue::Use(Operand::Consume(val)))
+            block.and(Rvalue::Use(Operand::Consume(val)))
         } else {
             if ty.is_integral() && (op == BinOp::Div || op == BinOp::Rem) {
                 // Checking division and remainder is more complex, since we 1. always check
                 // and 2. there are two possible failure cases, divide-by-zero and overflow.
 
-                let (zero_msg, overflow_msg) = if op == BinOp::Div {
-                    ("attempted to divide by zero",
-                     "attempted to divide with overflow")
+                let (zero_err, overflow_err) = if op == BinOp::Div {
+                    (ConstMathErr::DivisionByZero,
+                     ConstMathErr::Overflow(Op::Div))
                 } else {
-                    ("attempted remainder with a divisor of zero",
-                     "attempted remainder with overflow")
+                    (ConstMathErr::RemainderByZero,
+                     ConstMathErr::Overflow(Op::Rem))
                 };
 
                 // Check for / 0
@@ -290,11 +295,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 self.cfg.push_assign(block, scope_id, span, &is_zero,
                                      Rvalue::BinaryOp(BinOp::Eq, rhs.clone(), zero));
 
-                let (zero_block, ok_block) = self.build_cond_br(block, span,
-                                                                Operand::Consume(is_zero));
-                self.panic(zero_block, zero_msg, span);
-
-                block = ok_block;
+                block = self.assert(block, Operand::Consume(is_zero), false,
+                                    AssertMessage::Math(zero_err), span);
 
                 // We only need to check for the overflow in one case:
                 // MIN / -1, and only for signed values.
@@ -318,11 +320,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     self.cfg.push_assign(block, scope_id, span, &of,
                                          Rvalue::BinaryOp(BinOp::BitAnd, is_neg_1, is_min));
 
-                    let (of_block, ok_block) = self.build_cond_br(block, span,
-                                                                  Operand::Consume(of));
-                    self.panic(of_block, overflow_msg, span);
-
-                    block = ok_block;
+                    block = self.assert(block, Operand::Consume(of), false,
+                                        AssertMessage::Math(overflow_err), span);
                 }
             }