Verifier now accepts multiple errors (fixes #387). (#452)

* Verifier now accepts multiple errors (fixes #387).

cranelift/src/wasm.rs

@@ -148,9 +148,9 @@ fn handle_module(
 
         let func_index = num_func_imports + def_index.index();
         if flag_check_translation {
-            context
+            if let Err(errors) = context.verify(fisa) {
-                .verify(fisa)
+                return Err(pretty_verifier_error(&context.func, fisa.isa, None, errors));
-                .map_err(|err| pretty_verifier_error(&context.func, fisa.isa, None, &err))?;
+            }
         } else {
             let compiled_size = context
                 .compile(isa)

lib/codegen/src/context.rs

@@ -30,7 +30,7 @@ use simple_gvn::do_simple_gvn;
 use std::vec::Vec;
 use timing;
 use unreachable_code::eliminate_unreachable_code;
-use verifier::{verify_context, verify_locations, VerifierResult};
+use verifier::{verify_context, verify_locations, VerifierErrors, VerifierResult};
 
 /// Persistent data structures and compilation pipeline.
 pub struct Context {
@@ -177,31 +177,43 @@ impl Context {
     ///
     /// Also check that the dominator tree and control flow graph are consistent with the function.
     pub fn verify<'a, FOI: Into<FlagsOrIsa<'a>>>(&self, fisa: FOI) -> VerifierResult<()> {
-        verify_context(&self.func, &self.cfg, &self.domtree, fisa)
+        let mut errors = VerifierErrors::default();
+        let _ = verify_context(&self.func, &self.cfg, &self.domtree, fisa, &mut errors);
+
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(errors)
+        }
     }
 
     /// Run the verifier only if the `enable_verifier` setting is true.
     pub fn verify_if<'a, FOI: Into<FlagsOrIsa<'a>>>(&self, fisa: FOI) -> CodegenResult<()> {
         let fisa = fisa.into();
         if fisa.flags.enable_verifier() {
-            self.verify(fisa).map_err(Into::into)
+            self.verify(fisa)?;
-        } else {
-            Ok(())
         }
+        Ok(())
     }
 
     /// Run the locations verifier on the function.
     pub fn verify_locations(&self, isa: &TargetIsa) -> VerifierResult<()> {
-        verify_locations(isa, &self.func, None)
+        let mut errors = VerifierErrors::default();
+        let _ = verify_locations(isa, &self.func, None, &mut errors);
+
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(errors)
+        }
     }
 
     /// Run the locations verifier only if the `enable_verifier` setting is true.
     pub fn verify_locations_if(&self, isa: &TargetIsa) -> CodegenResult<()> {
         if isa.flags().enable_verifier() {
-            self.verify_locations(isa).map_err(Into::into)
+            self.verify_locations(isa)?;
-        } else {
-            Ok(())
         }
+        Ok(())
     }
 
     /// Perform dead-code elimination on the function.

lib/codegen/src/dominator_tree.rs

@@ -673,7 +673,7 @@ mod test {
     use ir::types::*;
     use ir::{Function, InstBuilder, TrapCode};
     use settings;
-    use verifier::verify_context;
+    use verifier::{verify_context, VerifierErrors};
 
     #[test]
     fn empty() {
@@ -931,6 +931,10 @@ mod test {
         cfg.compute(cur.func);
 
         let flags = settings::Flags::new(settings::builder());
-        verify_context(cur.func, &cfg, &dt, &flags).unwrap();
+        let mut errors = VerifierErrors::default();
+
+        verify_context(cur.func, &cfg, &dt, &flags, &mut errors).unwrap();
+
+        assert!(errors.0.is_empty());
     }
 }

lib/codegen/src/print_errors.rs

@@ -9,7 +9,7 @@ use std::boxed::Box;
 use std::fmt;
 use std::fmt::Write;
 use std::string::{String, ToString};
-use verifier::VerifierError;
+use verifier::{VerifierError, VerifierErrors};
 use write::{decorate_function, FuncWriter, PlainWriter};
 
 /// Pretty-print a verifier error.
@@ -17,21 +17,26 @@ pub fn pretty_verifier_error<'a>(
     func: &ir::Function,
     isa: Option<&TargetIsa>,
     func_w: Option<Box<FuncWriter + 'a>>,
-    err: &VerifierError,
+    errors: VerifierErrors,
 ) -> String {
+    let mut errors = errors.0;
     let mut w = String::new();
 
-    match err.location {
+    // TODO: Use drain_filter here when it gets stabilized
-        ir::entities::AnyEntity::Inst(_) => {}
+    let mut i = 0;
-        _ => {
+
-            // Print the error, because the pretty_function_error below won't do it since it isn't
+    while i != errors.len() {
-            // tied to an instruction.
+        if let ir::entities::AnyEntity::Inst(_) = errors[i].location {
-            writeln!(w, "verifier error summary: {}\n", err.to_string()).unwrap();
+            let err = errors.remove(i);
+
+            writeln!(w, "Miscellaneous error: {}\n", err).unwrap()
+        } else {
+            i += 1;
         }
     }
 
     decorate_function(
-        &mut PrettyVerifierError(func_w.unwrap_or(Box::new(PlainWriter)), err),
+        &mut PrettyVerifierError(func_w.unwrap_or(Box::new(PlainWriter)), &mut errors),
         &mut w,
         func,
         isa,
@@ -39,7 +44,7 @@ pub fn pretty_verifier_error<'a>(
     w
 }
 
-struct PrettyVerifierError<'a>(Box<FuncWriter + 'a>, &'a VerifierError);
+struct PrettyVerifierError<'a>(Box<FuncWriter + 'a>, &'a mut Vec<VerifierError>);
 
 impl<'a> FuncWriter for PrettyVerifierError<'a> {
     fn write_instruction(
@@ -71,31 +76,35 @@ fn pretty_function_error(
     cur_inst: Inst,
     indent: usize,
     func_w: &mut FuncWriter,
-    err: &VerifierError,
+    errors: &mut Vec<VerifierError>,
 ) -> fmt::Result {
-    match err.location {
+    // TODO: Use drain_filter here when it gets stabilized
+    let mut i = 0;
+
+    while i != errors.len() {
+        match errors[i].location {
             ir::entities::AnyEntity::Inst(inst) if inst == cur_inst => {
+                let err = errors.remove(i);
+
                 func_w.write_instruction(w, func, isa, cur_inst, indent)?;
                 write!(w, "{1:0$}^", indent, "")?;
                 for _c in cur_inst.to_string().chars() {
                     write!(w, "~")?;
                 }
-            writeln!(w, " verifier {}\n", err.to_string())
+                writeln!(w, " verifier {}\n", err.to_string())?;
             }
-        _ => writeln!(
+            ir::entities::AnyEntity::Inst(_) => i += 1,
-            w,
+            _ => unreachable!(),
-            "{1:0$}{2}",
-            indent,
-            "",
-            func.dfg.display_inst(cur_inst, isa)
-        ),
         }
     }
 
+    Ok(())
+}
+
 /// Pretty-print a Cranelift error.
 pub fn pretty_error(func: &ir::Function, isa: Option<&TargetIsa>, err: CodegenError) -> String {
     if let CodegenError::Verifier(e) = err {
-        pretty_verifier_error(func, isa, None, &e)
+        pretty_verifier_error(func, isa, None, e)
     } else {
         err.to_string()
     }

lib/codegen/src/regalloc/context.rs

@@ -18,7 +18,7 @@ use regalloc::virtregs::VirtRegs;
 use result::CodegenResult;
 use timing;
 use topo_order::TopoOrder;
-use verifier::{verify_context, verify_cssa, verify_liveness, verify_locations};
+use verifier::{verify_context, verify_cssa, verify_liveness, verify_locations, VerifierErrors};
 
 /// Persistent memory allocations for register allocation.
 pub struct Context {
@@ -76,6 +76,8 @@ impl Context {
         let _tt = timing::regalloc();
         debug_assert!(domtree.is_valid());
 
+        let mut errors = VerifierErrors::default();
+
         // `Liveness` and `Coloring` are self-clearing.
         self.virtregs.clear();
 
@@ -87,7 +89,11 @@ impl Context {
         self.liveness.compute(isa, func, cfg);
 
         if isa.flags().enable_verifier() {
-            verify_liveness(isa, func, cfg, &self.liveness)?;
+            let ok = verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok();
+
+            if !ok {
+                return Err(errors.into());
+            }
         }
 
         // Pass: Coalesce and create Conventional SSA form.
@@ -101,9 +107,20 @@ impl Context {
         );
 
         if isa.flags().enable_verifier() {
-            verify_context(func, cfg, domtree, isa)?;
+            let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok()
-            verify_liveness(isa, func, cfg, &self.liveness)?;
+                && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok()
-            verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?;
+                && verify_cssa(
+                    func,
+                    cfg,
+                    domtree,
+                    &self.liveness,
+                    &self.virtregs,
+                    &mut errors,
+                ).is_ok();
+
+            if !ok {
+                return Err(errors.into());
+            }
         }
 
         // Pass: Spilling.
@@ -118,9 +135,20 @@ impl Context {
         );
 
         if isa.flags().enable_verifier() {
-            verify_context(func, cfg, domtree, isa)?;
+            let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok()
-            verify_liveness(isa, func, cfg, &self.liveness)?;
+                && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok()
-            verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?;
+                && verify_cssa(
+                    func,
+                    cfg,
+                    domtree,
+                    &self.liveness,
+                    &self.virtregs,
+                    &mut errors,
+                ).is_ok();
+
+            if !ok {
+                return Err(errors.into());
+            }
         }
 
         // Pass: Reload.
@@ -134,9 +162,20 @@ impl Context {
         );
 
         if isa.flags().enable_verifier() {
-            verify_context(func, cfg, domtree, isa)?;
+            let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok()
-            verify_liveness(isa, func, cfg, &self.liveness)?;
+                && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok()
-            verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?;
+                && verify_cssa(
+                    func,
+                    cfg,
+                    domtree,
+                    &self.liveness,
+                    &self.virtregs,
+                    &mut errors,
+                ).is_ok();
+
+            if !ok {
+                return Err(errors.into());
+            }
         }
 
         // Pass: Coloring.
@@ -144,11 +183,29 @@ impl Context {
             .run(isa, func, domtree, &mut self.liveness, &mut self.tracker);
 
         if isa.flags().enable_verifier() {
-            verify_context(func, cfg, domtree, isa)?;
+            let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok()
-            verify_liveness(isa, func, cfg, &self.liveness)?;
+                && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok()
-            verify_locations(isa, func, Some(&self.liveness))?;
+                && verify_locations(isa, func, Some(&self.liveness), &mut errors).is_ok()
-            verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?;
+                && verify_cssa(
+                    func,
+                    cfg,
+                    domtree,
+                    &self.liveness,
+                    &self.virtregs,
+                    &mut errors,
+                ).is_ok();
+
+            if !ok {
+                return Err(errors.into());
             }
+        }
+
+        // Even if we arrive here, (non-fatal) errors might have been reported, so we
+        // must make sure absolutely nothing is wrong
+        if errors.is_empty() {
             Ok(())
+        } else {
+            Err(errors.into())
+        }
     }
 }

lib/codegen/src/result.rs

@@ -1,18 +1,18 @@
 //! Result and error types representing the outcome of compiling a function.
 
-use verifier::VerifierError;
+use verifier::VerifierErrors;
 
 /// A compilation error.
 ///
 /// When Cranelift fails to compile a function, it will return one of these error codes.
 #[derive(Fail, Debug, PartialEq, Eq)]
 pub enum CodegenError {
-    /// An IR verifier error.
+    /// A list of IR verifier errors.
     ///
     /// This always represents a bug, either in the code that generated IR for Cranelift, or a bug
     /// in Cranelift itself.
-    #[fail(display = "Verifier error: {}", _0)]
+    #[fail(display = "Verifier errors:\n{}", _0)]
-    Verifier(#[cause] VerifierError),
+    Verifier(#[cause] VerifierErrors),
 
     /// An implementation limit was exceeded.
     ///
@@ -34,8 +34,8 @@ pub enum CodegenError {
 /// A convenient alias for a `Result` that uses `CodegenError` as the error type.
 pub type CodegenResult<T> = Result<T, CodegenError>;
 
-impl From<VerifierError> for CodegenError {
+impl From<VerifierErrors> for CodegenError {
-    fn from(e: VerifierError) -> Self {
+    fn from(e: VerifierErrors) -> Self {
         CodegenError::Verifier(e)
     }
 }

lib/codegen/src/verifier/cssa.rs

@@ -7,7 +7,7 @@ use ir::{ExpandedProgramPoint, Function};
 use regalloc::liveness::Liveness;
 use regalloc::virtregs::VirtRegs;
 use timing;
-use verifier::VerifierResult;
+use verifier::{VerifierErrors, VerifierStepResult};
 
 /// Verify conventional SSA form for `func`.
 ///
@@ -29,7 +29,8 @@ pub fn verify_cssa(
     domtree: &DominatorTree,
     liveness: &Liveness,
     virtregs: &VirtRegs,
-) -> VerifierResult<()> {
+    errors: &mut VerifierErrors,
+) -> VerifierStepResult<()> {
     let _tt = timing::verify_cssa();
 
     let mut preorder = DominatorTreePreorder::new();
@@ -43,8 +44,8 @@ pub fn verify_cssa(
         liveness,
         preorder,
     };
-    verifier.check_virtregs()?;
+    verifier.check_virtregs(errors)?;
-    verifier.check_cssa()?;
+    verifier.check_cssa(errors)?;
     Ok(())
 }
 
@@ -58,19 +59,19 @@ struct CssaVerifier<'a> {
 }
 
 impl<'a> CssaVerifier<'a> {
-    fn check_virtregs(&self) -> VerifierResult<()> {
+    fn check_virtregs(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         for vreg in self.virtregs.all_virtregs() {
             let values = self.virtregs.values(vreg);
 
             for (idx, &val) in values.iter().enumerate() {
                 if !self.func.dfg.value_is_valid(val) {
-                    return err!(val, "Invalid value in {}", vreg);
+                    return fatal!(errors, val, "Invalid value in {}", vreg);
                 }
                 if !self.func.dfg.value_is_attached(val) {
-                    return err!(val, "Detached value in {}", vreg);
+                    return fatal!(errors, val, "Detached value in {}", vreg);
                 }
                 if self.liveness.get(val).is_none() {
-                    return err!(val, "Value in {} has no live range", vreg);
+                    return fatal!(errors, val, "Value in {} has no live range", vreg);
                 };
 
                 // Check topological ordering with the previous values in the virtual register.
@@ -81,7 +82,8 @@ impl<'a> CssaVerifier<'a> {
                     let prev_ebb = self.func.layout.pp_ebb(prev_def);
 
                     if prev_def == def {
-                        return err!(
+                        return fatal!(
+                            errors,
                             val,
                             "Values {} and {} in {} = {} defined at the same program point",
                             prev_val,
@@ -95,7 +97,8 @@ impl<'a> CssaVerifier<'a> {
                     if self.preorder.dominates(def_ebb, prev_ebb)
                         && self.domtree.dominates(def, prev_def, &self.func.layout)
                     {
-                        return err!(
+                        return fatal!(
+                            errors,
                             val,
                             "Value in {} = {} def dominates previous {}",
                             vreg,
@@ -117,7 +120,8 @@ impl<'a> CssaVerifier<'a> {
                     {
                         let ctx = self.liveness.context(&self.func.layout);
                         if self.liveness[prev_val].overlaps_def(def, def_ebb, ctx) {
-                            return err!(
+                            return fatal!(
+                                errors,
                                 val,
                                 "Value def in {} = {} interferes with {}",
                                 vreg,
@@ -135,7 +139,7 @@ impl<'a> CssaVerifier<'a> {
         Ok(())
     }
 
-    fn check_cssa(&self) -> VerifierResult<()> {
+    fn check_cssa(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         for ebb in self.func.layout.ebbs() {
             let ebb_params = self.func.dfg.ebb_params(ebb);
             for BasicBlock { inst: pred, .. } in self.cfg.pred_iter(ebb) {
@@ -149,7 +153,8 @@ impl<'a> CssaVerifier<'a> {
 
                 for (&ebb_param, &pred_arg) in ebb_params.iter().zip(pred_args) {
                     if !self.virtregs.same_class(ebb_param, pred_arg) {
-                        return err!(
+                        return fatal!(
+                            errors,
                             pred,
                             "{} and {} must be in the same virtual register",
                             ebb_param,

lib/codegen/src/verifier/flags.rs

@@ -7,7 +7,7 @@ use ir::instructions::BranchInfo;
 use isa;
 use packed_option::PackedOption;
 use timing;
-use verifier::VerifierResult;
+use verifier::{VerifierErrors, VerifierStepResult};
 
 /// Verify that CPU flags are used correctly.
 ///
@@ -25,7 +25,8 @@ pub fn verify_flags(
     func: &ir::Function,
     cfg: &ControlFlowGraph,
     isa: Option<&isa::TargetIsa>,
-) -> VerifierResult<()> {
+    errors: &mut VerifierErrors,
+) -> VerifierStepResult<()> {
     let _tt = timing::verify_flags();
     let mut verifier = FlagsVerifier {
         func,
@@ -33,7 +34,7 @@ pub fn verify_flags(
         encinfo: isa.map(|isa| isa.encoding_info()),
         livein: EntityMap::new(),
     };
-    verifier.check()
+    verifier.check(errors)
 }
 
 struct FlagsVerifier<'a> {
@@ -46,7 +47,7 @@ struct FlagsVerifier<'a> {
 }
 
 impl<'a> FlagsVerifier<'a> {
-    fn check(&mut self) -> VerifierResult<()> {
+    fn check(&mut self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         // List of EBBs that need to be processed. EBBs may be re-added to this list when we detect
         // that one of their successor blocks needs a live-in flags value.
         let mut worklist = SparseSet::new();
@@ -55,7 +56,7 @@ impl<'a> FlagsVerifier<'a> {
         }
 
         while let Some(ebb) = worklist.pop() {
-            if let Some(value) = self.visit_ebb(ebb)? {
+            if let Some(value) = self.visit_ebb(ebb, errors)? {
                 // The EBB has live-in flags. Check if the value changed.
                 match self.livein[ebb].expand() {
                     // Revisit any predecessor blocks the first time we see a live-in for `ebb`.
@@ -66,7 +67,13 @@ impl<'a> FlagsVerifier<'a> {
                         }
                     }
                     Some(old) if old != value => {
-                        return err!(ebb, "conflicting live-in CPU flags: {} and {}", old, value);
+                        return fatal!(
+                            errors,
+                            ebb,
+                            "conflicting live-in CPU flags: {} and {}",
+                            old,
+                            value
+                        );
                     }
                     x => assert_eq!(x, Some(value)),
                 }
@@ -80,7 +87,11 @@ impl<'a> FlagsVerifier<'a> {
     }
 
     /// Check flags usage in `ebb` and return the live-in flags value, if any.
-    fn visit_ebb(&self, ebb: ir::Ebb) -> VerifierResult<Option<ir::Value>> {
+    fn visit_ebb(
+        &self,
+        ebb: ir::Ebb,
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<Option<ir::Value>> {
         // The single currently live flags value.
         let mut live_val = None;
 
@@ -93,7 +104,7 @@ impl<'a> FlagsVerifier<'a> {
                         // We've reached the def of `live_flags`, so it is no longer live above.
                         live_val = None;
                     } else if self.func.dfg.value_type(res).is_flags() {
-                        return err!(inst, "{} clobbers live CPU flags in {}", res, live);
+                        return fatal!(errors, inst, "{} clobbers live CPU flags in {}", res, live);
                     }
                 }
 
@@ -104,14 +115,14 @@ impl<'a> FlagsVerifier<'a> {
                     .and_then(|ei| ei.operand_constraints(self.func.encodings[inst]))
                     .map_or(false, |c| c.clobbers_flags) && live_val.is_some()
                 {
-                    return err!(inst, "encoding clobbers live CPU flags in {}", live);
+                    return fatal!(errors, inst, "encoding clobbers live CPU flags in {}", live);
                 }
             }
 
             // Now look for live ranges of CPU flags that end here.
             for &arg in self.func.dfg.inst_args(inst) {
                 if self.func.dfg.value_type(arg).is_flags() {
-                    merge(&mut live_val, arg, inst)?;
+                    merge(&mut live_val, arg, inst, errors)?;
                 }
             }
 
@@ -120,13 +131,13 @@ impl<'a> FlagsVerifier<'a> {
                 BranchInfo::NotABranch => {}
                 BranchInfo::SingleDest(dest, _) => {
                     if let Some(val) = self.livein[dest].expand() {
-                        merge(&mut live_val, val, inst)?;
+                        merge(&mut live_val, val, inst, errors)?;
                     }
                 }
                 BranchInfo::Table(jt) => {
                     for (_, dest) in self.func.jump_tables[jt].entries() {
                         if let Some(val) = self.livein[dest].expand() {
-                            merge(&mut live_val, val, inst)?;
+                            merge(&mut live_val, val, inst, errors)?;
                         }
                     }
                 }
@@ -139,10 +150,15 @@ impl<'a> FlagsVerifier<'a> {
 }
 
 // Merge live flags values, or return an error on conflicting values.
-fn merge(a: &mut Option<ir::Value>, b: ir::Value, inst: ir::Inst) -> VerifierResult<()> {
+fn merge(
+    a: &mut Option<ir::Value>,
+    b: ir::Value,
+    inst: ir::Inst,
+    errors: &mut VerifierErrors,
+) -> VerifierStepResult<()> {
     if let Some(va) = *a {
         if b != va {
-            return err!(inst, "conflicting live CPU flags: {} and {}", va, b);
+            return fatal!(errors, inst, "conflicting live CPU flags: {} and {}", va, b);
         }
     } else {
         *a = Some(b);

lib/codegen/src/verifier/liveness.rs

@@ -8,7 +8,7 @@ use regalloc::liveness::Liveness;
 use regalloc::liverange::LiveRange;
 use std::cmp::Ordering;
 use timing;
-use verifier::VerifierResult;
+use verifier::{VerifierErrors, VerifierStepResult};
 
 /// Verify liveness information for `func`.
 ///
@@ -27,7 +27,8 @@ pub fn verify_liveness(
     func: &Function,
     cfg: &ControlFlowGraph,
     liveness: &Liveness,
-) -> VerifierResult<()> {
+    errors: &mut VerifierErrors,
+) -> VerifierStepResult<()> {
     let _tt = timing::verify_liveness();
     let verifier = LivenessVerifier {
         isa,
@@ -35,8 +36,8 @@ pub fn verify_liveness(
         cfg,
         liveness,
     };
-    verifier.check_ebbs()?;
+    verifier.check_ebbs(errors)?;
-    verifier.check_insts()?;
+    verifier.check_insts(errors)?;
     Ok(())
 }
 
@@ -49,21 +50,21 @@ struct LivenessVerifier<'a> {
 
 impl<'a> LivenessVerifier<'a> {
     /// Check all EBB arguments.
-    fn check_ebbs(&self) -> VerifierResult<()> {
+    fn check_ebbs(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         for ebb in self.func.layout.ebbs() {
             for &val in self.func.dfg.ebb_params(ebb) {
                 let lr = match self.liveness.get(val) {
                     Some(lr) => lr,
-                    None => return err!(ebb, "EBB arg {} has no live range", val),
+                    None => return fatal!(errors, ebb, "EBB arg {} has no live range", val),
                 };
-                self.check_lr(ebb.into(), val, lr)?;
+                self.check_lr(ebb.into(), val, lr, errors)?;
             }
         }
         Ok(())
     }
 
     /// Check all instructions.
-    fn check_insts(&self) -> VerifierResult<()> {
+    fn check_insts(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         for ebb in self.func.layout.ebbs() {
             for inst in self.func.layout.ebb_insts(ebb) {
                 let encoding = self.func.encodings[inst];
@@ -72,14 +73,15 @@ impl<'a> LivenessVerifier<'a> {
                 for &val in self.func.dfg.inst_results(inst) {
                     let lr = match self.liveness.get(val) {
                         Some(lr) => lr,
-                        None => return err!(inst, "{} has no live range", val),
+                        None => return fatal!(errors, inst, "{} has no live range", val),
                     };
-                    self.check_lr(inst.into(), val, lr)?;
+                    self.check_lr(inst.into(), val, lr, errors)?;
 
                     if encoding.is_legal() {
                         // A legal instruction is not allowed to define ghost values.
                         if lr.affinity.is_unassigned() {
-                            return err!(
+                            return fatal!(
+                                errors,
                                 inst,
                                 "{} is a ghost value defined by a real [{}] instruction",
                                 val,
@@ -88,7 +90,8 @@ impl<'a> LivenessVerifier<'a> {
                         }
                     } else if !lr.affinity.is_unassigned() {
                         // A non-encoded instruction can only define ghost values.
-                        return err!(
+                        return fatal!(
+                            errors,
                             inst,
                             "{} is a real {} value defined by a ghost instruction",
                             val,
@@ -101,15 +104,16 @@ impl<'a> LivenessVerifier<'a> {
                 for &val in self.func.dfg.inst_args(inst) {
                     let lr = match self.liveness.get(val) {
                         Some(lr) => lr,
-                        None => return err!(inst, "{} has no live range", val),
+                        None => return fatal!(errors, inst, "{} has no live range", val),
                     };
                     if !self.live_at_use(lr, inst) {
-                        return err!(inst, "{} is not live at this use", val);
+                        return fatal!(errors, inst, "{} is not live at this use", val);
                     }
 
                     // A legal instruction is not allowed to depend on ghost values.
                     if encoding.is_legal() && lr.affinity.is_unassigned() {
-                        return err!(
+                        return fatal!(
+                            errors,
                             inst,
                             "{} is a ghost value used by a real [{}] instruction",
                             val,
@@ -141,7 +145,13 @@ impl<'a> LivenessVerifier<'a> {
     }
 
     /// Check the integrity of the live range `lr`.
-    fn check_lr(&self, def: ProgramPoint, val: Value, lr: &LiveRange) -> VerifierResult<()> {
+    fn check_lr(
+        &self,
+        def: ProgramPoint,
+        val: Value,
+        lr: &LiveRange,
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         let l = &self.func.layout;
 
         let loc: AnyEntity = match def.into() {
@@ -149,11 +159,17 @@ impl<'a> LivenessVerifier<'a> {
             ExpandedProgramPoint::Inst(i) => i.into(),
         };
         if lr.def() != def {
-            return err!(loc, "Wrong live range def ({}) for {}", lr.def(), val);
+            return fatal!(
+                errors,
+                loc,
+                "Wrong live range def ({}) for {}",
+                lr.def(),
+                val
+            );
         }
         if lr.is_dead() {
             if !lr.is_local() {
-                return err!(loc, "Dead live range {} should be local", val);
+                return fatal!(errors, loc, "Dead live range {} should be local", val);
             } else {
                 return Ok(());
             }
@@ -164,11 +180,17 @@ impl<'a> LivenessVerifier<'a> {
         };
         match lr.def_local_end().into() {
             ExpandedProgramPoint::Ebb(e) => {
-                return err!(loc, "Def local range for {} can't end at {}", val, e)
+                return fatal!(
+                    errors,
+                    loc,
+                    "Def local range for {} can't end at {}",
+                    val,
+                    e
+                )
             }
             ExpandedProgramPoint::Inst(i) => {
                 if self.func.layout.inst_ebb(i) != Some(def_ebb) {
-                    return err!(loc, "Def local end for {} in wrong ebb", val);
+                    return fatal!(errors, loc, "Def local end for {} in wrong ebb", val);
                 }
             }
         }
@@ -176,12 +198,19 @@ impl<'a> LivenessVerifier<'a> {
         // Now check the live-in intervals against the CFG.
         for (mut ebb, end) in lr.liveins(self.liveness.context(l)) {
             if !l.is_ebb_inserted(ebb) {
-                return err!(loc, "{} livein at {} which is not in the layout", val, ebb);
+                return fatal!(
+                    errors,
+                    loc,
+                    "{} livein at {} which is not in the layout",
+                    val,
+                    ebb
+                );
             }
             let end_ebb = match l.inst_ebb(end) {
                 Some(e) => e,
                 None => {
-                    return err!(
+                    return fatal!(
+                        errors,
                         loc,
                         "{} livein for {} ends at {} which is not in the layout",
                         val,
@@ -196,7 +225,8 @@ impl<'a> LivenessVerifier<'a> {
                 // If `val` is live-in at `ebb`, it must be live at all the predecessors.
                 for BasicBlock { inst: pred, .. } in self.cfg.pred_iter(ebb) {
                     if !self.live_at_use(lr, pred) {
-                        return err!(
+                        return fatal!(
+                            errors,
                             pred,
                             "{} is live in to {} but not live at predecessor",
                             val,
@@ -210,7 +240,15 @@ impl<'a> LivenessVerifier<'a> {
                 }
                 ebb = match l.next_ebb(ebb) {
                     Some(e) => e,
-                    None => return err!(loc, "end of {} livein ({}) never reached", val, end_ebb),
+                    None => {
+                        return fatal!(
+                            errors,
+                            loc,
+                            "end of {} livein ({}) never reached",
+                            val,
+                            end_ebb
+                        )
+                    }
                 };
             }
         }

lib/codegen/src/verifier/locations.rs

@@ -5,7 +5,7 @@ use isa;
 use regalloc::liveness::Liveness;
 use regalloc::RegDiversions;
 use timing;
-use verifier::VerifierResult;
+use verifier::{VerifierErrors, VerifierStepResult};
 
 /// Verify value locations for `func`.
 ///
@@ -22,7 +22,8 @@ pub fn verify_locations(
     isa: &isa::TargetIsa,
     func: &ir::Function,
     liveness: Option<&Liveness>,
-) -> VerifierResult<()> {
+    errors: &mut VerifierErrors,
+) -> VerifierStepResult<()> {
     let _tt = timing::verify_locations();
     let verifier = LocationVerifier {
         isa,
@@ -31,7 +32,7 @@ pub fn verify_locations(
         encinfo: isa.encoding_info(),
         liveness,
     };
-    verifier.check_constraints()?;
+    verifier.check_constraints(errors)?;
     Ok(())
 }
 
@@ -45,7 +46,7 @@ struct LocationVerifier<'a> {
 
 impl<'a> LocationVerifier<'a> {
     /// Check that the assigned value locations match the operand constraints of their uses.
-    fn check_constraints(&self) -> VerifierResult<()> {
+    fn check_constraints(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
         let dfg = &self.func.dfg;
         let mut divert = RegDiversions::new();
 
@@ -57,23 +58,23 @@ impl<'a> LocationVerifier<'a> {
                 let enc = self.func.encodings[inst];
 
                 if enc.is_legal() {
-                    self.check_enc_constraints(inst, enc, &divert)?
+                    self.check_enc_constraints(inst, enc, &divert, errors)?
                 } else {
-                    self.check_ghost_results(inst)?;
+                    self.check_ghost_results(inst, errors)?;
                 }
 
                 if let Some(sig) = dfg.call_signature(inst) {
-                    self.check_call_abi(inst, sig, &divert)?;
+                    self.check_call_abi(inst, sig, &divert, errors)?;
                 }
 
                 let opcode = dfg[inst].opcode();
                 if opcode.is_return() {
-                    self.check_return_abi(inst, &divert)?;
+                    self.check_return_abi(inst, &divert, errors)?;
                 } else if opcode.is_branch() && !divert.is_empty() {
-                    self.check_cfg_edges(inst, &divert)?;
+                    self.check_cfg_edges(inst, &divert, errors)?;
                 }
 
-                self.update_diversions(inst, &mut divert)?;
+                self.update_diversions(inst, &mut divert, errors)?;
             }
         }
 
@@ -86,7 +87,8 @@ impl<'a> LocationVerifier<'a> {
         inst: ir::Inst,
         enc: isa::Encoding,
         divert: &RegDiversions,
-    ) -> VerifierResult<()> {
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         let constraints = self
             .encinfo
             .operand_constraints(enc)
@@ -97,7 +99,8 @@ impl<'a> LocationVerifier<'a> {
         }
 
         // TODO: We could give a better error message here.
-        err!(
+        fatal!(
+            errors,
             inst,
             "{} constraints not satisfied",
             self.encinfo.display(enc)
@@ -106,13 +109,18 @@ impl<'a> LocationVerifier<'a> {
 
     /// Check that the result values produced by a ghost instruction are not assigned a value
     /// location.
-    fn check_ghost_results(&self, inst: ir::Inst) -> VerifierResult<()> {
+    fn check_ghost_results(
+        &self,
+        inst: ir::Inst,
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         let results = self.func.dfg.inst_results(inst);
 
         for &res in results {
             let loc = self.func.locations[res];
             if loc.is_assigned() {
-                return err!(
+                return fatal!(
+                    errors,
                     inst,
                     "ghost result {} value must not have a location ({}).",
                     res,
@@ -130,7 +138,8 @@ impl<'a> LocationVerifier<'a> {
         inst: ir::Inst,
         sig: ir::SigRef,
         divert: &RegDiversions,
-    ) -> VerifierResult<()> {
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         let sig = &self.func.dfg.signatures[sig];
         let varargs = self.func.dfg.inst_variable_args(inst);
         let results = self.func.dfg.inst_results(inst);
@@ -142,6 +151,7 @@ impl<'a> LocationVerifier<'a> {
                 abi,
                 divert.get(value, &self.func.locations),
                 ir::StackSlotKind::OutgoingArg,
+                errors,
             )?;
         }
 
@@ -152,6 +162,7 @@ impl<'a> LocationVerifier<'a> {
                 abi,
                 self.func.locations[value],
                 ir::StackSlotKind::OutgoingArg,
+                errors,
             )?;
         }
 
@@ -159,7 +170,12 @@ impl<'a> LocationVerifier<'a> {
     }
 
     /// Check the ABI argument locations for a return.
-    fn check_return_abi(&self, inst: ir::Inst, divert: &RegDiversions) -> VerifierResult<()> {
+    fn check_return_abi(
+        &self,
+        inst: ir::Inst,
+        divert: &RegDiversions,
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         let sig = &self.func.signature;
         let varargs = self.func.dfg.inst_variable_args(inst);
 
@@ -170,6 +186,7 @@ impl<'a> LocationVerifier<'a> {
                 abi,
                 divert.get(value, &self.func.locations),
                 ir::StackSlotKind::IncomingArg,
+                errors,
             )?;
         }
 
@@ -184,12 +201,14 @@ impl<'a> LocationVerifier<'a> {
         abi: &ir::AbiParam,
         loc: ir::ValueLoc,
         want_kind: ir::StackSlotKind,
-    ) -> VerifierResult<()> {
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         match abi.location {
             ir::ArgumentLoc::Unassigned => {}
             ir::ArgumentLoc::Reg(reg) => {
                 if loc != ir::ValueLoc::Reg(reg) {
-                    return err!(
+                    return fatal!(
+                        errors,
                         inst,
                         "ABI expects {} in {}, got {}",
                         value,
@@ -202,7 +221,8 @@ impl<'a> LocationVerifier<'a> {
                 if let ir::ValueLoc::Stack(ss) = loc {
                     let slot = &self.func.stack_slots[ss];
                     if slot.kind != want_kind {
-                        return err!(
+                        return fatal!(
+                            errors,
                             inst,
                             "call argument {} should be in a {} slot, but {} is {}",
                             value,
@@ -212,7 +232,8 @@ impl<'a> LocationVerifier<'a> {
                         );
                     }
                     if slot.offset.unwrap() != offset {
-                        return err!(
+                        return fatal!(
+                            errors,
                             inst,
                             "ABI expects {} at stack offset {}, but {} is at {}",
                             value,
@@ -222,7 +243,8 @@ impl<'a> LocationVerifier<'a> {
                         );
                     }
                 } else {
-                    return err!(
+                    return fatal!(
+                        errors,
                         inst,
                         "ABI expects {} at stack offset {}, got {}",
                         value,
@@ -237,7 +259,12 @@ impl<'a> LocationVerifier<'a> {
     }
 
     /// Update diversions to reflect the current instruction and check their consistency.
-    fn update_diversions(&self, inst: ir::Inst, divert: &mut RegDiversions) -> VerifierResult<()> {
+    fn update_diversions(
+        &self,
+        inst: ir::Inst,
+        divert: &mut RegDiversions,
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         let (arg, src) = match self.func.dfg[inst] {
             ir::InstructionData::RegMove { arg, src, .. }
             | ir::InstructionData::RegSpill { arg, src, .. } => (arg, ir::ValueLoc::Reg(src)),
@@ -247,14 +274,16 @@ impl<'a> LocationVerifier<'a> {
 
         if let Some(d) = divert.diversion(arg) {
             if d.to != src {
-                return err!(
+                return fatal!(
+                    errors,
                     inst,
                     "inconsistent with current diversion to {}",
                     d.to.display(&self.reginfo)
                 );
             }
         } else if self.func.locations[arg] != src {
-            return err!(
+            return fatal!(
+                errors,
                 inst,
                 "inconsistent with global location {}",
                 self.func.locations[arg].display(&self.reginfo)
@@ -268,7 +297,12 @@ impl<'a> LocationVerifier<'a> {
 
     /// We have active diversions before a branch. Make sure none of the diverted values are live
     /// on the outgoing CFG edges.
-    fn check_cfg_edges(&self, inst: ir::Inst, divert: &RegDiversions) -> VerifierResult<()> {
+    fn check_cfg_edges(
+        &self,
+        inst: ir::Inst,
+        divert: &RegDiversions,
+        errors: &mut VerifierErrors,
+    ) -> VerifierStepResult<()> {
         use ir::instructions::BranchInfo::*;
 
         // We can only check CFG edges if we have a liveness analysis.
@@ -287,7 +321,8 @@ impl<'a> LocationVerifier<'a> {
                 for d in divert.all() {
                     let lr = &liveness[d.value];
                     if lr.is_livein(ebb, liveness.context(&self.func.layout)) {
-                        return err!(
+                        return fatal!(
+                            errors,
                             inst,
                             "{} is diverted to {} and live in to {}",
                             d.value,
@@ -302,7 +337,8 @@ impl<'a> LocationVerifier<'a> {
                     let lr = &liveness[d.value];
                     for (_, ebb) in self.func.jump_tables[jt].entries() {
                         if lr.is_livein(ebb, liveness.context(&self.func.layout)) {
-                            return err!(
+                            return fatal!(
+                                errors,
                                 inst,
                                 "{} is diverted to {} and live in to {}",
                                 d.value,

lib/filetests/src/runone.rs

@@ -130,7 +130,7 @@ fn run_one_test<'a>(
     // Should we run the verifier before this test?
     if !context.verified && test.needs_verifier() {
         verify_function(&func, context.flags_or_isa())
-            .map_err(|e| pretty_verifier_error(&func, isa, None, &e))?;
+            .map_err(|errors| pretty_verifier_error(&func, isa, None, errors))?;
         context.verified = true;
     }
 

lib/filetests/src/test_verifier.rs

@@ -14,6 +14,7 @@ use cranelift_codegen::verify_function;
 use cranelift_reader::TestCommand;
 use match_directive::match_directive;
 use std::borrow::{Borrow, Cow};
+use std::fmt::Write;
 use subtest::{Context, SubTest, SubtestResult};
 
 struct TestVerifier;
@@ -41,37 +42,48 @@ impl SubTest for TestVerifier {
         let func = func.borrow();
 
         // Scan source annotations for "error:" directives.
-        let mut expected = None;
+        let mut expected = Vec::new();
+
         for comment in &context.details.comments {
             if let Some(tail) = match_directive(comment.text, "error:") {
-                // Currently, the verifier can only report one problem at a time.
+                expected.push((comment.entity, tail));
-                // Reject more than one `error:` directives.
-                if expected.is_some() {
-                    return Err("cannot handle multiple error: directives".to_string());
-                }
-                expected = Some((comment.entity, tail));
             }
         }
 
         match verify_function(func, context.flags_or_isa()) {
-            Ok(_) => match expected {
+            Ok(()) if expected.len() == 0 => Ok(()),
-                None => Ok(()),
+            Ok(()) => Err(format!("passed, but expected errors: {:?}", expected)),
-                Some((_, msg)) => Err(format!("passed, expected error: {}", msg)),
+
-            },
+            Err(ref errors) if expected.len() == 0 => {
-            Err(got) => match expected {
+                Err(format!("expected no error, but got:\n{}", errors))
-                None => Err(format!("verifier pass, got {}", got)),
+            }
-                Some((want_loc, want_msg)) if got.message.contains(want_msg) => {
+
-                    if want_loc == got.location {
+            Err(errors) => {
+                let mut errors = errors.0;
+                let mut msg = String::new();
+
+                // for each expected error, find a suitable match
+                for expect in expected {
+                    let pos = errors
+                        .iter()
+                        .position(|err| err.message.contains(expect.1) && err.location == expect.0);
+
+                    match pos {
+                        None => {
+                            write!(msg, "expected error {}", expect.0).unwrap();
+                        }
+                        Some(pos) => {
+                            errors.swap_remove(pos);
+                        }
+                    }
+                }
+
+                if msg.len() == 0 {
                     Ok(())
                 } else {
-                        Err(format!(
+                    Err(msg)
-                            "correct error reported on {}, but wanted {}",
+                }
-                            got.location, want_loc
+            }
-                        ))
-                    }
-                }
-                Some(_) => Err(format!("mismatching error: {}", got)),
-            },
         }
     }
 }

lib/frontend/src/frontend.rs

@@ -707,11 +707,9 @@ mod tests {
         }
 
         let flags = settings::Flags::new(settings::builder());
-        let res = verify_function(&func, &flags);
         // println!("{}", func.display(None));
-        match res {
+        if let Err(errors) = verify_function(&func, &flags) {
-            Ok(_) => {}
+            panic!("{}\n{}", func.display(None), errors)
-            Err(err) => panic!("{}{}", func.display(None), err),
         }
     }
 

lib/frontend/src/lib.rs

@@ -120,9 +120,8 @@
 //!     let flags = settings::Flags::new(settings::builder());
 //!     let res = verify_function(&func, &flags);
 //!     println!("{}", func.display(None));
-//!     match res {
+//!     if let Err(errors) = res {
-//!         Ok(_) => {}
+//!         panic!("{}", errors);
-//!         Err(err) => panic!("{}", err),
 //!     }
 //! }
 //! ```

lib/frontend/src/ssa.rs

@@ -1049,9 +1049,9 @@ mod tests {
         let flags = settings::Flags::new(settings::builder());
         match verify_function(&func, &flags) {
             Ok(()) => {}
-            Err(_err) => {
+            Err(_errors) => {
                 #[cfg(feature = "std")]
-                panic!(_err.message);
+                panic!(_errors);
                 #[cfg(not(feature = "std"))]
                 panic!("function failed to verify");
             }
@@ -1228,9 +1228,9 @@ mod tests {
         let flags = settings::Flags::new(settings::builder());
         match verify_function(&func, &flags) {
             Ok(()) => {}
-            Err(_err) => {
+            Err(_errors) => {
                 #[cfg(feature = "std")]
-                panic!(_err.message);
+                panic!(_errors);
                 #[cfg(not(feature = "std"))]
                 panic!("function failed to verify");
             }
@@ -1279,9 +1279,9 @@ mod tests {
         let flags = settings::Flags::new(settings::builder());
         match verify_function(&func, &flags) {
             Ok(()) => {}
-            Err(_err) => {
+            Err(_errors) => {
                 #[cfg(feature = "std")]
-                panic!(_err.message);
+                panic!(_errors);
                 #[cfg(not(feature = "std"))]
                 panic!("function failed to verify");
             }

lib/wasm/tests/wasm_testsuite.rs

@@ -77,7 +77,7 @@ fn handle_module(path: &Path, flags: &Flags) {
     translate_module(&data, &mut dummy_environ).unwrap();
     for func in dummy_environ.info.function_bodies.values() {
         verifier::verify_function(func, flags)
-            .map_err(|err| panic!(pretty_verifier_error(func, None, None, &err)))
+            .map_err(|errors| panic!(pretty_verifier_error(func, None, None, errors)))
             .unwrap();
     }
 }