diff --git a/cranelift/src/wasm.rs b/cranelift/src/wasm.rs index d448f75389..34fcb26ad0 100644 --- a/cranelift/src/wasm.rs +++ b/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 - .verify(fisa) - .map_err(|err| pretty_verifier_error(&context.func, fisa.isa, None, &err))?; + if let Err(errors) = context.verify(fisa) { + return Err(pretty_verifier_error(&context.func, fisa.isa, None, errors)); + } } else { let compiled_size = context .compile(isa) diff --git a/lib/codegen/src/context.rs b/lib/codegen/src/context.rs index afb2b3ee55..e5930cd575 100644 --- a/lib/codegen/src/context.rs +++ b/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>>(&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>>(&self, fisa: FOI) -> CodegenResult<()> { let fisa = fisa.into(); if fisa.flags.enable_verifier() { - self.verify(fisa).map_err(Into::into) - } else { - Ok(()) + self.verify(fisa)?; } + 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) - } else { - Ok(()) + self.verify_locations(isa)?; } + Ok(()) } /// Perform dead-code elimination on the function. diff --git a/lib/codegen/src/dominator_tree.rs b/lib/codegen/src/dominator_tree.rs index bd75a16a9f..a246f671bc 100644 --- a/lib/codegen/src/dominator_tree.rs +++ b/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()); } } diff --git a/lib/codegen/src/print_errors.rs b/lib/codegen/src/print_errors.rs index f85f5ccfc4..03ba2457a1 100644 --- a/lib/codegen/src/print_errors.rs +++ b/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>, - err: &VerifierError, + errors: VerifierErrors, ) -> String { + let mut errors = errors.0; let mut w = String::new(); - match err.location { - ir::entities::AnyEntity::Inst(_) => {} - _ => { - // Print the error, because the pretty_function_error below won't do it since it isn't - // tied to an instruction. - writeln!(w, "verifier error summary: {}\n", err.to_string()).unwrap(); + // TODO: Use drain_filter here when it gets stabilized + let mut i = 0; + + while i != errors.len() { + if let ir::entities::AnyEntity::Inst(_) = errors[i].location { + 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, &'a VerifierError); +struct PrettyVerifierError<'a>(Box, &'a mut Vec); 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, ) -> fmt::Result { - match err.location { - ir::entities::AnyEntity::Inst(inst) if inst == cur_inst => { - 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, "~")?; + // 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()) + ir::entities::AnyEntity::Inst(_) => i += 1, + _ => unreachable!(), } - _ => writeln!( - w, - "{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() } diff --git a/lib/codegen/src/regalloc/context.rs b/lib/codegen/src/regalloc/context.rs index 349fa212cb..08e658bd4c 100644 --- a/lib/codegen/src/regalloc/context.rs +++ b/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)?; - verify_liveness(isa, func, cfg, &self.liveness)?; - verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; + let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok() + && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok() + && 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)?; - verify_liveness(isa, func, cfg, &self.liveness)?; - verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; + let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok() + && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok() + && 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)?; - verify_liveness(isa, func, cfg, &self.liveness)?; - verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; + let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok() + && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok() + && 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)?; - verify_liveness(isa, func, cfg, &self.liveness)?; - verify_locations(isa, func, Some(&self.liveness))?; - verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; + let ok = verify_context(func, cfg, domtree, isa, &mut errors).is_ok() + && verify_liveness(isa, func, cfg, &self.liveness, &mut errors).is_ok() + && verify_locations(isa, func, Some(&self.liveness), &mut errors).is_ok() + && 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()) } - Ok(()) } } diff --git a/lib/codegen/src/result.rs b/lib/codegen/src/result.rs index 0956b4f4d2..dace896049 100644 --- a/lib/codegen/src/result.rs +++ b/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)] - Verifier(#[cause] VerifierError), + #[fail(display = "Verifier errors:\n{}", _0)] + 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 = Result; -impl From for CodegenError { - fn from(e: VerifierError) -> Self { +impl From for CodegenError { + fn from(e: VerifierErrors) -> Self { CodegenError::Verifier(e) } } diff --git a/lib/codegen/src/verifier/cssa.rs b/lib/codegen/src/verifier/cssa.rs index c2d745c4fb..3162bfb5e9 100644 --- a/lib/codegen/src/verifier/cssa.rs +++ b/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_cssa()?; + verifier.check_virtregs(errors)?; + 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, diff --git a/lib/codegen/src/verifier/flags.rs b/lib/codegen/src/verifier/flags.rs index 3895cd63a0..0d16d17ef5 100644 --- a/lib/codegen/src/verifier/flags.rs +++ b/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> { + fn visit_ebb( + &self, + ebb: ir::Ebb, + errors: &mut VerifierErrors, + ) -> VerifierStepResult> { // 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, b: ir::Value, inst: ir::Inst) -> VerifierResult<()> { +fn merge( + a: &mut Option, + 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); diff --git a/lib/codegen/src/verifier/liveness.rs b/lib/codegen/src/verifier/liveness.rs index f884feea79..e5fae0585f 100644 --- a/lib/codegen/src/verifier/liveness.rs +++ 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_insts()?; + verifier.check_ebbs(errors)?; + 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 + ) + } }; } } diff --git a/lib/codegen/src/verifier/locations.rs b/lib/codegen/src/verifier/locations.rs index cc393bb46f..73997d0a92 100644 --- a/lib/codegen/src/verifier/locations.rs +++ b/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, diff --git a/lib/codegen/src/verifier/mod.rs b/lib/codegen/src/verifier/mod.rs index 2888d5336d..66ad44fa11 100644 --- a/lib/codegen/src/verifier/mod.rs +++ b/lib/codegen/src/verifier/mod.rs @@ -82,23 +82,91 @@ pub use self::cssa::verify_cssa; pub use self::liveness::verify_liveness; pub use self::locations::verify_locations; -// Create an `Err` variant of `VerifierResult` from a location and `format!` arguments. -macro_rules! err { - ( $loc:expr, $msg:expr ) => { - Err(::verifier::VerifierError { +/// Report an error. +/// +/// The first argument must be a `&mut VerifierErrors` reference, and the following +/// argument defines the location of the error and must implement `Into`. +/// Finally, subsequent arguments will be formatted using `format!()` and set +/// as the error message. +macro_rules! report { + ( $errors: expr, $loc: expr, $msg: tt ) => { + $errors.0.push(::verifier::VerifierError { location: $loc.into(), message: String::from($msg), }) }; - ( $loc:expr, $fmt:expr, $( $arg:expr ),+ ) => { - Err(::verifier::VerifierError { + ( $errors: expr, $loc: expr, $fmt: tt, $( $arg: expr ),+ ) => { + $errors.0.push(::verifier::VerifierError { location: $loc.into(), message: format!( $fmt, $( $arg ),+ ), }) }; } +/// Diagnose a fatal error, and return `Err`. +macro_rules! fatal { + ( $( $arg: expr ),+ ) => ({ + report!( $( $arg ),+ ); + Err(()) + }); +} + +/// Diagnose a non-fatal error, and return `Ok`. +macro_rules! nonfatal { + ( $( $arg: expr ),+ ) => ({ + report!( $( $arg ),+ ); + Ok(()) + }); +} + +/// Shorthand syntax for calling functions of the form +/// `verify_foo(a, b, &mut VerifierErrors) -> VerifierStepResult` +/// as if they had the form `verify_foo(a, b) -> VerifierResult`. +/// +/// This syntax also ensures that no errors whatsoever were reported, +/// even if they were not fatal. +/// +/// # Example +/// ```rust,ignore +/// verify!(verify_context, func, cfg, domtree, fisa) +/// +/// // ... is equivalent to... +/// +/// let mut errors = VerifierErrors::new(); +/// let result = verify_context(func, cfg, domtree, fisa, &mut errors); +/// +/// if errors.is_empty() { +/// Ok(result.unwrap()) +/// } else { +/// Err(errors) +/// } +/// ``` +#[macro_export] +macro_rules! verify { + ( $verifier: expr; $fun: ident $(, $arg: expr )* ) => ({ + let mut errors = $crate::verifier::VerifierErrors::default(); + let result = $verifier.$fun( $( $arg, )* &mut errors); + + if errors.is_empty() { + Ok(result.unwrap()) + } else { + Err(errors) + } + }); + + ( $fun: path, $(, $arg: expr )* ) => ({ + let mut errors = $crate::verifier::VerifierErrors::default(); + let result = $fun( $( $arg, )* &mut errors); + + if errors.is_empty() { + Ok(result.unwrap()) + } else { + Err(errors) + } + }); +} + mod cssa; mod flags; mod liveness; @@ -109,7 +177,7 @@ mod locations; pub struct VerifierError { /// The entity causing the verifier error. pub location: AnyEntity, - /// Error message. + /// The error message. pub message: String, } @@ -119,8 +187,71 @@ impl Display for VerifierError { } } -/// Verifier result. -pub type VerifierResult = Result; +/// Result of a step in the verification process. +/// +/// Functions that return `VerifierStepResult<()>` should also take a +/// mutable reference to `VerifierErrors` as argument in order to report +/// errors. +/// +/// Here, `Ok` represents a step that **did not lead to a fatal error**, +/// meaning that the verification process may continue. However, other (non-fatal) +/// errors might have been reported through the previously mentioned `VerifierErrors` +/// argument. +pub type VerifierStepResult = Result; + +/// Result of a verification operation. +/// +/// Unlike `VerifierStepResult<()>` which may be `Ok` while still having reported +/// errors, this type always returns `Err` if an error (fatal or not) was reported. +/// +/// Typically, this error will be constructed by using `verify!` on a function +/// that returns `VerifierStepResult`. +pub type VerifierResult = Result; + +/// List of verifier errors. +#[derive(Fail, Debug, Default, PartialEq, Eq)] +pub struct VerifierErrors(pub Vec); + +impl VerifierErrors { + /// Return a new `VerifierErrors` struct. + #[inline] + pub fn new() -> Self { + VerifierErrors(Vec::new()) + } + + /// Return whether no errors were reported. + #[inline] + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Return whether one or more errors were reported. + #[inline] + pub fn has_error(&self) -> bool { + !self.0.is_empty() + } +} + +impl From> for VerifierErrors { + fn from(v: Vec) -> Self { + VerifierErrors(v) + } +} + +impl Into> for VerifierErrors { + fn into(self) -> Vec { + self.0 + } +} + +impl Display for VerifierErrors { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + for err in &self.0 { + writeln!(f, "- {}", err)?; + } + Ok(()) + } +} /// Verify `func`. pub fn verify_function<'a, FOI: Into>>( @@ -128,7 +259,7 @@ pub fn verify_function<'a, FOI: Into>>( fisa: FOI, ) -> VerifierResult<()> { let _tt = timing::verifier(); - Verifier::new(func, fisa.into()).run() + verify!(Verifier::new(func, fisa.into()); run) } /// Verify `func` after checking the integrity of associated context data structures `cfg` and @@ -138,16 +269,17 @@ pub fn verify_context<'a, FOI: Into>>( cfg: &ControlFlowGraph, domtree: &DominatorTree, fisa: FOI, -) -> VerifierResult<()> { + errors: &mut VerifierErrors, +) -> VerifierStepResult<()> { let _tt = timing::verifier(); let verifier = Verifier::new(func, fisa.into()); if cfg.is_valid() { - verifier.cfg_integrity(cfg)?; + verifier.cfg_integrity(cfg, errors)?; } if domtree.is_valid() { - verifier.domtree_integrity(domtree)?; + verifier.domtree_integrity(domtree, errors)?; } - verifier.run() + verifier.run(errors) } struct Verifier<'a> { @@ -174,7 +306,7 @@ impl<'a> Verifier<'a> { // Check for: // - cycles in the global value declarations. // - use of 'vmctx' when no special parameter declares it. - fn verify_global_values(&self) -> VerifierResult<()> { + fn verify_global_values(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { let mut seen = SparseSet::new(); for gv in self.func.global_values.keys() { @@ -184,7 +316,7 @@ impl<'a> Verifier<'a> { let mut cur = gv; while let ir::GlobalValueData::Deref { base, .. } = self.func.global_values[cur] { if seen.insert(base).is_some() { - return err!(gv, "deref cycle: {}", DisplayList(seen.as_slice())); + return fatal!(errors, gv, "deref cycle: {}", DisplayList(seen.as_slice())); } cur = base; @@ -196,7 +328,7 @@ impl<'a> Verifier<'a> { .special_param(ir::ArgumentPurpose::VMContext) .is_none() { - return err!(cur, "undeclared vmctx reference {}", cur); + return fatal!(errors, cur, "undeclared vmctx reference {}", cur); } } } @@ -204,26 +336,36 @@ impl<'a> Verifier<'a> { Ok(()) } - fn ebb_integrity(&self, ebb: Ebb, inst: Inst) -> VerifierResult<()> { + fn ebb_integrity( + &self, + ebb: Ebb, + inst: Inst, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let is_terminator = self.func.dfg[inst].opcode().is_terminator(); let is_last_inst = self.func.layout.last_inst(ebb) == Some(inst); if is_terminator && !is_last_inst { // Terminating instructions only occur at the end of blocks. - return err!( + return fatal!( + errors, inst, "a terminator instruction was encountered before the end of {}", ebb ); } if is_last_inst && !is_terminator { - return err!(ebb, "block does not end in a terminator instruction"); + return fatal!( + errors, + ebb, + "block does not end in a terminator instruction" + ); } // Instructions belong to the correct ebb. let inst_ebb = self.func.layout.inst_ebb(inst); if inst_ebb != Some(ebb) { - return err!(inst, "should belong to {} not {:?}", ebb, inst_ebb); + return fatal!(errors, inst, "should belong to {} not {:?}", ebb, inst_ebb); } // Parameters belong to the correct ebb. @@ -231,11 +373,11 @@ impl<'a> Verifier<'a> { match self.func.dfg.value_def(arg) { ValueDef::Param(arg_ebb, _) => { if ebb != arg_ebb { - return err!(arg, "does not belong to {}", ebb); + return fatal!(errors, arg, "does not belong to {}", ebb); } } _ => { - return err!(arg, "expected an argument, found a result"); + return fatal!(errors, arg, "expected an argument, found a result"); } } } @@ -243,13 +385,21 @@ impl<'a> Verifier<'a> { Ok(()) } - fn instruction_integrity(&self, inst: Inst) -> VerifierResult<()> { + fn instruction_integrity( + &self, + inst: Inst, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let inst_data = &self.func.dfg[inst]; let dfg = &self.func.dfg; // The instruction format matches the opcode if inst_data.opcode().format() != InstructionFormat::from(inst_data) { - return err!(inst, "instruction opcode doesn't match instruction format"); + return fatal!( + errors, + inst, + "instruction opcode doesn't match instruction format" + ); } let fixed_results = inst_data.opcode().constraints().fixed_results(); @@ -262,7 +412,8 @@ impl<'a> Verifier<'a> { // All result values for multi-valued instructions are created let got_results = dfg.inst_results(inst).len(); if got_results != total_results { - return err!( + return fatal!( + errors, inst, "expected {} result values, found {}", total_results, @@ -270,29 +421,39 @@ impl<'a> Verifier<'a> { ); } - self.verify_entity_references(inst) + self.verify_entity_references(inst, errors) } - fn verify_entity_references(&self, inst: Inst) -> VerifierResult<()> { + fn verify_entity_references( + &self, + inst: Inst, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { use ir::instructions::InstructionData::*; for &arg in self.func.dfg.inst_args(inst) { - self.verify_inst_arg(inst, arg)?; + self.verify_inst_arg(inst, arg, errors)?; // All used values must be attached to something. let original = self.func.dfg.resolve_aliases(arg); if !self.func.dfg.value_is_attached(original) { - return err!(inst, "argument {} -> {} is not attached", arg, original); + return fatal!( + errors, + inst, + "argument {} -> {} is not attached", + arg, + original + ); } } for &res in self.func.dfg.inst_results(inst) { - self.verify_inst_result(inst, res)?; + self.verify_inst_result(inst, res, errors)?; } match self.func.dfg[inst] { MultiAry { ref args, .. } => { - self.verify_value_list(inst, args)?; + self.verify_value_list(inst, args, errors)?; } Jump { destination, @@ -319,50 +480,50 @@ impl<'a> Verifier<'a> { ref args, .. } => { - self.verify_ebb(inst, destination)?; - self.verify_value_list(inst, args)?; + self.verify_ebb(inst, destination, errors)?; + self.verify_value_list(inst, args, errors)?; } BranchTable { table, .. } => { - self.verify_jump_table(inst, table)?; + self.verify_jump_table(inst, table, errors)?; } Call { func_ref, ref args, .. } => { - self.verify_func_ref(inst, func_ref)?; - self.verify_value_list(inst, args)?; + self.verify_func_ref(inst, func_ref, errors)?; + self.verify_value_list(inst, args, errors)?; } CallIndirect { sig_ref, ref args, .. } => { - self.verify_sig_ref(inst, sig_ref)?; - self.verify_value_list(inst, args)?; + self.verify_sig_ref(inst, sig_ref, errors)?; + self.verify_value_list(inst, args, errors)?; } FuncAddr { func_ref, .. } => { - self.verify_func_ref(inst, func_ref)?; + self.verify_func_ref(inst, func_ref, errors)?; } StackLoad { stack_slot, .. } | StackStore { stack_slot, .. } => { - self.verify_stack_slot(inst, stack_slot)?; + self.verify_stack_slot(inst, stack_slot, errors)?; } UnaryGlobalValue { global_value, .. } => { - self.verify_global_value(inst, global_value)?; + self.verify_global_value(inst, global_value, errors)?; } HeapAddr { heap, .. } => { - self.verify_heap(inst, heap)?; + self.verify_heap(inst, heap, errors)?; } TableAddr { table, .. } => { - self.verify_table(inst, table)?; + self.verify_table(inst, table, errors)?; } RegSpill { dst, .. } => { - self.verify_stack_slot(inst, dst)?; + self.verify_stack_slot(inst, dst, errors)?; } RegFill { src, .. } => { - self.verify_stack_slot(inst, src)?; + self.verify_stack_slot(inst, src, errors)?; } LoadComplex { ref args, .. } => { - self.verify_value_list(inst, args)?; + self.verify_value_list(inst, args, errors)?; } StoreComplex { ref args, .. } => { - self.verify_value_list(inst, args)?; + self.verify_value_list(inst, args, errors)?; } // Exhaustive list so we can't forget to add new formats @@ -396,93 +557,148 @@ impl<'a> Verifier<'a> { Ok(()) } - fn verify_ebb(&self, inst: Inst, e: Ebb) -> VerifierResult<()> { + fn verify_ebb( + &self, + inst: Inst, + e: Ebb, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.dfg.ebb_is_valid(e) || !self.func.layout.is_ebb_inserted(e) { - return err!(inst, "invalid ebb reference {}", e); + return fatal!(errors, inst, "invalid ebb reference {}", e); } if let Some(entry_block) = self.func.layout.entry_block() { if e == entry_block { - return err!(inst, "invalid reference to entry ebb {}", e); + return fatal!(errors, inst, "invalid reference to entry ebb {}", e); } } Ok(()) } - fn verify_sig_ref(&self, inst: Inst, s: SigRef) -> VerifierResult<()> { + fn verify_sig_ref( + &self, + inst: Inst, + s: SigRef, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.dfg.signatures.is_valid(s) { - err!(inst, "invalid signature reference {}", s) + fatal!(errors, inst, "invalid signature reference {}", s) } else { Ok(()) } } - fn verify_func_ref(&self, inst: Inst, f: FuncRef) -> VerifierResult<()> { + fn verify_func_ref( + &self, + inst: Inst, + f: FuncRef, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.dfg.ext_funcs.is_valid(f) { - err!(inst, "invalid function reference {}", f) + fatal!(errors, inst, "invalid function reference {}", f) } else { Ok(()) } } - fn verify_stack_slot(&self, inst: Inst, ss: StackSlot) -> VerifierResult<()> { + fn verify_stack_slot( + &self, + inst: Inst, + ss: StackSlot, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.stack_slots.is_valid(ss) { - err!(inst, "invalid stack slot {}", ss) + fatal!(errors, inst, "invalid stack slot {}", ss) } else { Ok(()) } } - fn verify_global_value(&self, inst: Inst, gv: GlobalValue) -> VerifierResult<()> { + fn verify_global_value( + &self, + inst: Inst, + gv: GlobalValue, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.global_values.is_valid(gv) { - err!(inst, "invalid global value {}", gv) + fatal!(errors, inst, "invalid global value {}", gv) } else { Ok(()) } } - fn verify_heap(&self, inst: Inst, heap: ir::Heap) -> VerifierResult<()> { + fn verify_heap( + &self, + inst: Inst, + heap: ir::Heap, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.heaps.is_valid(heap) { - err!(inst, "invalid heap {}", heap) + fatal!(errors, inst, "invalid heap {}", heap) } else { Ok(()) } } - fn verify_table(&self, inst: Inst, table: ir::Table) -> VerifierResult<()> { + fn verify_table( + &self, + inst: Inst, + table: ir::Table, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.tables.is_valid(table) { - err!(inst, "invalid table {}", table) + fatal!(errors, inst, "invalid table {}", table) } else { Ok(()) } } - fn verify_value_list(&self, inst: Inst, l: &ValueList) -> VerifierResult<()> { + fn verify_value_list( + &self, + inst: Inst, + l: &ValueList, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !l.is_valid(&self.func.dfg.value_lists) { - err!(inst, "invalid value list reference {:?}", l) + fatal!(errors, inst, "invalid value list reference {:?}", l) } else { Ok(()) } } - fn verify_jump_table(&self, inst: Inst, j: JumpTable) -> VerifierResult<()> { + fn verify_jump_table( + &self, + inst: Inst, + j: JumpTable, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if !self.func.jump_tables.is_valid(j) { - err!(inst, "invalid jump table reference {}", j) + fatal!(errors, inst, "invalid jump table reference {}", j) } else { Ok(()) } } - fn verify_value(&self, loc_inst: Inst, v: Value) -> VerifierResult<()> { + fn verify_value( + &self, + loc_inst: Inst, + v: Value, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let dfg = &self.func.dfg; if !dfg.value_is_valid(v) { - err!(loc_inst, "invalid value reference {}", v) + fatal!(errors, loc_inst, "invalid value reference {}", v) } else { Ok(()) } } - fn verify_inst_arg(&self, loc_inst: Inst, v: Value) -> VerifierResult<()> { - self.verify_value(loc_inst, v)?; + fn verify_inst_arg( + &self, + loc_inst: Inst, + v: Value, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { + self.verify_value(loc_inst, v, errors)?; let dfg = &self.func.dfg; let loc_ebb = self.func.layout.pp_ebb(loc_inst); @@ -493,7 +709,8 @@ impl<'a> Verifier<'a> { ValueDef::Result(def_inst, _) => { // Value is defined by an instruction that exists. if !dfg.inst_is_valid(def_inst) { - return err!( + return fatal!( + errors, loc_inst, "{} is defined by invalid instruction {}", v, @@ -502,7 +719,8 @@ impl<'a> Verifier<'a> { } // Defining instruction is inserted in an EBB. if self.func.layout.inst_ebb(def_inst) == None { - return err!( + return fatal!( + errors, loc_inst, "{} is defined by {} which has no EBB", v, @@ -515,10 +733,16 @@ impl<'a> Verifier<'a> { .expected_domtree .dominates(def_inst, loc_inst, &self.func.layout) { - return err!(loc_inst, "uses value from non-dominating {}", def_inst); + return fatal!( + errors, + loc_inst, + "uses value from non-dominating {}", + def_inst + ); } if def_inst == loc_inst { - return err!( + return fatal!( + errors, loc_inst, "uses value from itself {}, {}", def_inst, @@ -530,11 +754,12 @@ impl<'a> Verifier<'a> { ValueDef::Param(ebb, _) => { // Value is defined by an existing EBB. if !dfg.ebb_is_valid(ebb) { - return err!(loc_inst, "{} is defined by invalid EBB {}", v, ebb); + return fatal!(errors, loc_inst, "{} is defined by invalid EBB {}", v, ebb); } // Defining EBB is inserted in the layout if !self.func.layout.is_ebb_inserted(ebb) { - return err!( + return fatal!( + errors, loc_inst, "{} is defined by {} which is not in the layout", v, @@ -547,20 +772,31 @@ impl<'a> Verifier<'a> { .expected_domtree .dominates(ebb, loc_inst, &self.func.layout) { - return err!(loc_inst, "uses value arg from non-dominating {}", ebb); + return fatal!( + errors, + loc_inst, + "uses value arg from non-dominating {}", + ebb + ); } } } Ok(()) } - fn verify_inst_result(&self, loc_inst: Inst, v: Value) -> VerifierResult<()> { - self.verify_value(loc_inst, v)?; + fn verify_inst_result( + &self, + loc_inst: Inst, + v: Value, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { + self.verify_value(loc_inst, v, errors)?; match self.func.dfg.value_def(v) { ValueDef::Result(def_inst, _) => { if def_inst != loc_inst { - err!( + fatal!( + errors, loc_inst, "instruction result {} is not defined by the instruction", v @@ -569,7 +805,8 @@ impl<'a> Verifier<'a> { Ok(()) } } - ValueDef::Param(_, _) => err!( + ValueDef::Param(_, _) => fatal!( + errors, loc_inst, "instruction result {} is not defined by the instruction", v @@ -577,7 +814,11 @@ impl<'a> Verifier<'a> { } } - fn domtree_integrity(&self, domtree: &DominatorTree) -> VerifierResult<()> { + fn domtree_integrity( + &self, + domtree: &DominatorTree, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { // We consider two `DominatorTree`s to be equal if they return the same immediate // dominator for each EBB. Therefore the current domtree is valid if it matches the freshly // computed one. @@ -585,7 +826,8 @@ impl<'a> Verifier<'a> { let expected = self.expected_domtree.idom(ebb); let got = domtree.idom(ebb); if got != expected { - return err!( + return fatal!( + errors, ebb, "invalid domtree, expected idom({}) = {:?}, got {:?}", ebb, @@ -596,7 +838,8 @@ impl<'a> Verifier<'a> { } // We also verify if the postorder defined by `DominatorTree` is sane if domtree.cfg_postorder().len() != self.expected_domtree.cfg_postorder().len() { - return err!( + return fatal!( + errors, AnyEntity::Function, "incorrect number of Ebbs in postorder traversal" ); @@ -608,7 +851,8 @@ impl<'a> Verifier<'a> { .enumerate() { if test_ebb != true_ebb { - return err!( + return fatal!( + errors, test_ebb, "invalid domtree, postorder ebb number {} should be {}, got {}", index, @@ -623,7 +867,8 @@ impl<'a> Verifier<'a> { .expected_domtree .rpo_cmp(prev_ebb, next_ebb, &self.func.layout) != Ordering::Greater { - return err!( + return fatal!( + errors, next_ebb, "invalid domtree, rpo_cmp does not says {} is greater than {}", prev_ebb, @@ -634,13 +879,14 @@ impl<'a> Verifier<'a> { Ok(()) } - fn typecheck_entry_block_params(&self) -> VerifierResult<()> { + fn typecheck_entry_block_params(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { if let Some(ebb) = self.func.layout.entry_block() { let expected_types = &self.func.signature.params; let ebb_param_count = self.func.dfg.num_ebb_params(ebb); if ebb_param_count != expected_types.len() { - return err!( + return fatal!( + errors, ebb, "entry block parameters ({}) must match function signature ({})", ebb_param_count, @@ -651,7 +897,8 @@ impl<'a> Verifier<'a> { for (i, &arg) in self.func.dfg.ebb_params(ebb).iter().enumerate() { let arg_type = self.func.dfg.value_type(arg); if arg_type != expected_types[i].value_type { - return err!( + return fatal!( + errors, ebb, "entry block parameter {} expected to have type {}, got {}", i, @@ -664,7 +911,7 @@ impl<'a> Verifier<'a> { Ok(()) } - fn typecheck(&self, inst: Inst) -> VerifierResult<()> { + fn typecheck(&self, inst: Inst, errors: &mut VerifierErrors) -> VerifierStepResult<()> { let inst_data = &self.func.dfg[inst]; let constraints = inst_data.opcode().constraints(); @@ -673,7 +920,12 @@ impl<'a> Verifier<'a> { let ctrl_type = self.func.dfg.ctrl_typevar(inst); if !value_typeset.contains(ctrl_type) { - return err!(inst, "has an invalid controlling type {}", ctrl_type); + return fatal!( + errors, + inst, + "has an invalid controlling type {}", + ctrl_type + ); } ctrl_type @@ -683,23 +935,29 @@ impl<'a> Verifier<'a> { types::VOID }; - self.typecheck_results(inst, ctrl_type)?; - self.typecheck_fixed_args(inst, ctrl_type)?; - self.typecheck_variable_args(inst)?; - self.typecheck_return(inst)?; - self.typecheck_special(inst, ctrl_type)?; + self.typecheck_results(inst, ctrl_type, errors)?; + self.typecheck_fixed_args(inst, ctrl_type, errors)?; + self.typecheck_variable_args(inst, errors)?; + self.typecheck_return(inst, errors)?; + self.typecheck_special(inst, ctrl_type, errors)?; Ok(()) } - fn typecheck_results(&self, inst: Inst, ctrl_type: Type) -> VerifierResult<()> { + fn typecheck_results( + &self, + inst: Inst, + ctrl_type: Type, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let mut i = 0; for &result in self.func.dfg.inst_results(inst) { let result_type = self.func.dfg.value_type(result); let expected_type = self.func.dfg.compute_result_type(inst, i, ctrl_type); if let Some(expected_type) = expected_type { if result_type != expected_type { - return err!( + return fatal!( + errors, inst, "expected result {} ({}) to have type {}, found {}", i, @@ -709,19 +967,24 @@ impl<'a> Verifier<'a> { ); } } else { - return err!(inst, "has more result values than expected"); + return fatal!(errors, inst, "has more result values than expected"); } i += 1; } // There aren't any more result types left. if self.func.dfg.compute_result_type(inst, i, ctrl_type) != None { - return err!(inst, "has fewer result values than expected"); + return fatal!(errors, inst, "has fewer result values than expected"); } Ok(()) } - fn typecheck_fixed_args(&self, inst: Inst, ctrl_type: Type) -> VerifierResult<()> { + fn typecheck_fixed_args( + &self, + inst: Inst, + ctrl_type: Type, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let constraints = self.func.dfg[inst].opcode().constraints(); for (i, &arg) in self.func.dfg.inst_fixed_args(inst).iter().enumerate() { @@ -729,7 +992,8 @@ impl<'a> Verifier<'a> { match constraints.value_argument_constraint(i, ctrl_type) { ResolvedConstraint::Bound(expected_type) => { if arg_type != expected_type { - return err!( + return fatal!( + errors, inst, "arg {} ({}) has type {}, expected {}", i, @@ -741,7 +1005,8 @@ impl<'a> Verifier<'a> { } ResolvedConstraint::Free(type_set) => { if !type_set.contains(arg_type) { - return err!( + return fatal!( + errors, inst, "arg {} ({}) with type {} failed to satisfy type set {:?}", i, @@ -756,7 +1021,11 @@ impl<'a> Verifier<'a> { Ok(()) } - fn typecheck_variable_args(&self, inst: Inst) -> VerifierResult<()> { + fn typecheck_variable_args( + &self, + inst: Inst, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { match self.func.dfg.analyze_branch(inst) { BranchInfo::SingleDest(ebb, _) => { let iter = self @@ -765,13 +1034,14 @@ impl<'a> Verifier<'a> { .ebb_params(ebb) .iter() .map(|&v| self.func.dfg.value_type(v)); - self.typecheck_variable_args_iterator(inst, iter)?; + self.typecheck_variable_args_iterator(inst, iter, errors)?; } BranchInfo::Table(table) => { for (_, ebb) in self.func.jump_tables[table].entries() { let arg_count = self.func.dfg.num_ebb_params(ebb); if arg_count != 0 { - return err!( + return fatal!( + errors, inst, "takes no arguments, but had target {} with {} arguments", ebb, @@ -790,16 +1060,16 @@ impl<'a> Verifier<'a> { .params .iter() .map(|a| a.value_type); - self.typecheck_variable_args_iterator(inst, arg_types)?; - self.check_outgoing_args(inst, sig_ref)?; + self.typecheck_variable_args_iterator(inst, arg_types, errors)?; + self.check_outgoing_args(inst, sig_ref, errors)?; } CallInfo::Indirect(sig_ref, _) => { let arg_types = self.func.dfg.signatures[sig_ref] .params .iter() .map(|a| a.value_type); - self.typecheck_variable_args_iterator(inst, arg_types)?; - self.check_outgoing_args(inst, sig_ref)?; + self.typecheck_variable_args_iterator(inst, arg_types, errors)?; + self.check_outgoing_args(inst, sig_ref, errors)?; } CallInfo::NotACall => {} } @@ -810,7 +1080,8 @@ impl<'a> Verifier<'a> { &self, inst: Inst, iter: I, - ) -> VerifierResult<()> { + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let variable_args = self.func.dfg.inst_variable_args(inst); let mut i = 0; @@ -823,7 +1094,8 @@ impl<'a> Verifier<'a> { let arg = variable_args[i]; let arg_type = self.func.dfg.value_type(arg); if expected_type != arg_type { - return err!( + return fatal!( + errors, inst, "arg {} ({}) has type {}, expected {}", i, @@ -835,7 +1107,8 @@ impl<'a> Verifier<'a> { i += 1; } if i != variable_args.len() { - return err!( + return fatal!( + errors, inst, "mismatched argument count for `{}`: got {}, expected {}", self.func.dfg.display_inst(inst, None), @@ -850,7 +1123,12 @@ impl<'a> Verifier<'a> { /// /// When a signature has been legalized, all values passed as outgoing arguments on the stack /// must be assigned to a matching `OutgoingArg` stack slot. - fn check_outgoing_args(&self, inst: Inst, sig_ref: SigRef) -> VerifierResult<()> { + fn check_outgoing_args( + &self, + inst: Inst, + sig_ref: SigRef, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let sig = &self.func.dfg.signatures[sig_ref]; // Before legalization, there's nothing to check. @@ -867,10 +1145,11 @@ impl<'a> Verifier<'a> { let arg_loc = self.func.locations[arg]; if let ValueLoc::Stack(ss) = arg_loc { // Argument value is assigned to a stack slot as expected. - self.verify_stack_slot(inst, ss)?; + self.verify_stack_slot(inst, ss, errors)?; let slot = &self.func.stack_slots[ss]; if slot.kind != StackSlotKind::OutgoingArg { - return err!( + return fatal!( + errors, inst, "Outgoing stack argument {} in wrong stack slot: {} = {}", arg, @@ -879,7 +1158,8 @@ impl<'a> Verifier<'a> { ); } if slot.offset != Some(offset) { - return err!( + return fatal!( + errors, inst, "Outgoing stack argument {} should have offset {}: {} = {}", arg, @@ -889,7 +1169,8 @@ impl<'a> Verifier<'a> { ); } if slot.size != abi.value_type.bytes() { - return err!( + return fatal!( + errors, inst, "Outgoing stack argument {} wrong size for {}: {} = {}", arg, @@ -900,7 +1181,8 @@ impl<'a> Verifier<'a> { } } else { let reginfo = self.isa.map(|i| i.register_info()); - return err!( + return fatal!( + errors, inst, "Outgoing stack argument {} in wrong location: {}", arg, @@ -912,17 +1194,22 @@ impl<'a> Verifier<'a> { Ok(()) } - fn typecheck_return(&self, inst: Inst) -> VerifierResult<()> { + fn typecheck_return(&self, inst: Inst, errors: &mut VerifierErrors) -> VerifierStepResult<()> { if self.func.dfg[inst].opcode().is_return() { let args = self.func.dfg.inst_variable_args(inst); let expected_types = &self.func.signature.returns; if args.len() != expected_types.len() { - return err!(inst, "arguments of return must match function signature"); + return fatal!( + errors, + inst, + "arguments of return must match function signature" + ); } for (i, (&arg, &expected_type)) in args.iter().zip(expected_types).enumerate() { let arg_type = self.func.dfg.value_type(arg); if arg_type != expected_type.value_type { - return err!( + return fatal!( + errors, inst, "arg {} ({}) has type {}, must match function signature of {}", i, @@ -938,13 +1225,19 @@ impl<'a> Verifier<'a> { // Check special-purpose type constraints that can't be expressed in the normal opcode // constraints. - fn typecheck_special(&self, inst: Inst, ctrl_type: Type) -> VerifierResult<()> { + fn typecheck_special( + &self, + inst: Inst, + ctrl_type: Type, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { if let ir::InstructionData::Unary { opcode, arg } = self.func.dfg[inst] { let arg_type = self.func.dfg.value_type(arg); match opcode { Opcode::Bextend | Opcode::Uextend | Opcode::Sextend | Opcode::Fpromote => { if arg_type.lane_count() != ctrl_type.lane_count() { - return err!( + return fatal!( + errors, inst, "input {} and output {} must have same number of lanes", arg_type, @@ -952,7 +1245,8 @@ impl<'a> Verifier<'a> { ); } if arg_type.lane_bits() >= ctrl_type.lane_bits() { - return err!( + return fatal!( + errors, inst, "input {} must be smaller than output {}", arg_type, @@ -962,7 +1256,8 @@ impl<'a> Verifier<'a> { } Opcode::Breduce | Opcode::Ireduce | Opcode::Fdemote => { if arg_type.lane_count() != ctrl_type.lane_count() { - return err!( + return fatal!( + errors, inst, "input {} and output {} must have same number of lanes", arg_type, @@ -970,7 +1265,8 @@ impl<'a> Verifier<'a> { ); } if arg_type.lane_bits() <= ctrl_type.lane_bits() { - return err!( + return fatal!( + errors, inst, "input {} must be larger than output {}", arg_type, @@ -984,7 +1280,11 @@ impl<'a> Verifier<'a> { Ok(()) } - fn cfg_integrity(&self, cfg: &ControlFlowGraph) -> VerifierResult<()> { + fn cfg_integrity( + &self, + cfg: &ControlFlowGraph, + errors: &mut VerifierErrors, + ) -> VerifierStepResult<()> { let mut expected_succs = BTreeSet::::new(); let mut got_succs = BTreeSet::::new(); let mut expected_preds = BTreeSet::::new(); @@ -996,7 +1296,8 @@ impl<'a> Verifier<'a> { let missing_succs: Vec = expected_succs.difference(&got_succs).cloned().collect(); if !missing_succs.is_empty() { - return err!( + return fatal!( + errors, ebb, "cfg lacked the following successor(s) {:?}", missing_succs @@ -1005,7 +1306,12 @@ impl<'a> Verifier<'a> { let excess_succs: Vec = got_succs.difference(&expected_succs).cloned().collect(); if !excess_succs.is_empty() { - return err!(ebb, "cfg had unexpected successor(s) {:?}", excess_succs); + return fatal!( + errors, + ebb, + "cfg had unexpected successor(s) {:?}", + excess_succs + ); } expected_preds.extend( @@ -1017,7 +1323,8 @@ impl<'a> Verifier<'a> { let missing_preds: Vec = expected_preds.difference(&got_preds).cloned().collect(); if !missing_preds.is_empty() { - return err!( + return fatal!( + errors, ebb, "cfg lacked the following predecessor(s) {:?}", missing_preds @@ -1026,7 +1333,12 @@ impl<'a> Verifier<'a> { let excess_preds: Vec = got_preds.difference(&expected_preds).cloned().collect(); if !excess_preds.is_empty() { - return err!(ebb, "cfg had unexpected predecessor(s) {:?}", excess_preds); + return fatal!( + errors, + ebb, + "cfg had unexpected predecessor(s) {:?}", + excess_preds + ); } expected_succs.clear(); @@ -1039,7 +1351,7 @@ impl<'a> Verifier<'a> { /// If the verifier has been set up with an ISA, make sure that the recorded encoding for the /// instruction (if any) matches how the ISA would encode it. - fn verify_encoding(&self, inst: Inst) -> VerifierResult<()> { + fn verify_encoding(&self, inst: Inst, errors: &mut VerifierErrors) -> VerifierStepResult<()> { // When the encodings table is empty, we don't require any instructions to be encoded. // // Once some instructions are encoded, we require all side-effecting instructions to have a @@ -1063,7 +1375,8 @@ impl<'a> Verifier<'a> { ).peekable(); if encodings.peek().is_none() { - return err!( + return fatal!( + errors, inst, "Instruction failed to re-encode {}", isa.encoding_info().display(encoding) @@ -1090,7 +1403,8 @@ impl<'a> Verifier<'a> { .unwrap(); } - return err!( + return fatal!( + errors, inst, "encoding {} should be {}{}", isa.encoding_info().display(encoding), @@ -1132,14 +1446,15 @@ impl<'a> Verifier<'a> { // Provide the ISA default encoding as a hint. match self.func.encode(inst, isa) { Ok(enc) => { - return err!( + return fatal!( + errors, inst, "{} must have an encoding (e.g., {})", text, isa.encoding_info().display(enc) ) } - Err(_) => return err!(inst, "{} must have an encoding", text), + Err(_) => return fatal!(errors, inst, "{} must have an encoding", text), } } @@ -1148,35 +1463,39 @@ impl<'a> Verifier<'a> { /// Verify the `return_at_end` property which requires that there are no internal return /// instructions. - fn verify_return_at_end(&self) -> VerifierResult<()> { + fn verify_return_at_end(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { for ebb in self.func.layout.ebbs() { let inst = self.func.layout.last_inst(ebb).unwrap(); if self.func.dfg[inst].opcode().is_return() && Some(ebb) != self.func.layout.last_ebb() { - return err!(inst, "Internal return not allowed with return_at_end=1"); + return fatal!( + errors, + inst, + "Internal return not allowed with return_at_end=1" + ); } } Ok(()) } - pub fn run(&self) -> VerifierResult<()> { - self.verify_global_values()?; - self.typecheck_entry_block_params()?; + pub fn run(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { + self.verify_global_values(errors)?; + self.typecheck_entry_block_params(errors)?; for ebb in self.func.layout.ebbs() { for inst in self.func.layout.ebb_insts(ebb) { - self.ebb_integrity(ebb, inst)?; - self.instruction_integrity(inst)?; - self.typecheck(inst)?; - self.verify_encoding(inst)?; + self.ebb_integrity(ebb, inst, errors)?; + self.instruction_integrity(inst, errors)?; + self.typecheck(inst, errors)?; + self.verify_encoding(inst, errors)?; } } if self.flags.return_at_end() { - self.verify_return_at_end()?; + self.verify_return_at_end(errors)?; } - verify_flags(self.func, &self.expected_cfg, self.isa)?; + verify_flags(self.func, &self.expected_cfg, self.isa, errors)?; Ok(()) } @@ -1184,7 +1503,7 @@ impl<'a> Verifier<'a> { #[cfg(test)] mod tests { - use super::{Verifier, VerifierError}; + use super::{Verifier, VerifierError, VerifierErrors}; use entity::EntityList; use ir::instructions::{InstructionData, Opcode}; use ir::Function; @@ -1192,9 +1511,9 @@ mod tests { macro_rules! assert_err_with_msg { ($e:expr, $msg:expr) => { - match $e { - Ok(_) => panic!("Expected an error"), - Err(VerifierError { message, .. }) => { + match $e.0.get(0) { + None => panic!("Expected an error"), + Some(&VerifierError { ref message, .. }) => { if !message.contains($msg) { #[cfg(feature = "std")] panic!(format!( @@ -1214,7 +1533,10 @@ mod tests { let func = Function::new(); let flags = &settings::Flags::new(settings::builder()); let verifier = Verifier::new(&func, flags.into()); - assert_eq!(verifier.run(), Ok(())); + let mut errors = VerifierErrors::default(); + + assert_eq!(verifier.run(&mut errors), Ok(())); + assert!(errors.0.is_empty()); } #[test] @@ -1237,6 +1559,10 @@ mod tests { ); let flags = &settings::Flags::new(settings::builder()); let verifier = Verifier::new(&func, flags.into()); - assert_err_with_msg!(verifier.run(), "instruction format"); + let mut errors = VerifierErrors::default(); + + let _ = verifier.run(&mut errors); + + assert_err_with_msg!(errors, "instruction format"); } } diff --git a/lib/filetests/src/runone.rs b/lib/filetests/src/runone.rs index b98f5f8cfb..b365b4a96b 100644 --- a/lib/filetests/src/runone.rs +++ b/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; } diff --git a/lib/filetests/src/test_verifier.rs b/lib/filetests/src/test_verifier.rs index 422deaf25b..9c356d6733 100644 --- a/lib/filetests/src/test_verifier.rs +++ b/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. - // Reject more than one `error:` directives. - if expected.is_some() { - return Err("cannot handle multiple error: directives".to_string()); - } - expected = Some((comment.entity, tail)); + expected.push((comment.entity, tail)); } } match verify_function(func, context.flags_or_isa()) { - Ok(_) => match expected { - None => Ok(()), - Some((_, msg)) => Err(format!("passed, expected error: {}", msg)), - }, - Err(got) => match expected { - None => Err(format!("verifier pass, got {}", got)), - Some((want_loc, want_msg)) if got.message.contains(want_msg) => { - if want_loc == got.location { - Ok(()) - } else { - Err(format!( - "correct error reported on {}, but wanted {}", - got.location, want_loc - )) + Ok(()) if expected.len() == 0 => Ok(()), + Ok(()) => Err(format!("passed, but expected errors: {:?}", expected)), + + Err(ref errors) if expected.len() == 0 => { + Err(format!("expected no error, but got:\n{}", errors)) + } + + 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); + } } } - Some(_) => Err(format!("mismatching error: {}", got)), - }, + + if msg.len() == 0 { + Ok(()) + } else { + Err(msg) + } + } } } } diff --git a/lib/frontend/src/frontend.rs b/lib/frontend/src/frontend.rs index 16934282a8..5888828ba8 100644 --- a/lib/frontend/src/frontend.rs +++ b/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 { - Ok(_) => {} - Err(err) => panic!("{}{}", func.display(None), err), + if let Err(errors) = verify_function(&func, &flags) { + panic!("{}\n{}", func.display(None), errors) } } diff --git a/lib/frontend/src/lib.rs b/lib/frontend/src/lib.rs index 7fca5ae122..6830d23857 100644 --- a/lib/frontend/src/lib.rs +++ b/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 { -//! Ok(_) => {} -//! Err(err) => panic!("{}", err), +//! if let Err(errors) = res { +//! panic!("{}", errors); //! } //! } //! ``` diff --git a/lib/frontend/src/ssa.rs b/lib/frontend/src/ssa.rs index f5a3c4bd8a..5e6c4fbf9b 100644 --- a/lib/frontend/src/ssa.rs +++ b/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"); } diff --git a/lib/wasm/tests/wasm_testsuite.rs b/lib/wasm/tests/wasm_testsuite.rs index 166b3af076..06d7da4e7b 100644 --- a/lib/wasm/tests/wasm_testsuite.rs +++ b/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(); } }