From b5fda64b49e5bf889344360ae459658680519b2b Mon Sep 17 00:00:00 2001 From: Angus Holder Date: Wed, 29 Mar 2017 21:14:42 +0100 Subject: [PATCH] Type checking and Dominator Tree integrity checks in Verifier (#66) * Verify that a recomputed dominator tree is identical to the existing one. * The verifier now typechecks instruction results and arguments. * The verifier now typechecks instruction results and arguments. * The verifier now typechecks instruction results and arguments. * Added `inst_{fixed,variable}_args` accessor functions. * Improved error messages in verifier. * Type check return statements against the function signature. --- lib/cretonne/meta/gen_instr.py | 4 - lib/cretonne/src/ir/dfg.rs | 28 ++- lib/cretonne/src/ir/instructions.rs | 1 - lib/cretonne/src/legalizer/boundary.rs | 6 +- lib/cretonne/src/verifier.rs | 245 ++++++++++++++++++++++++- 5 files changed, 269 insertions(+), 15 deletions(-) diff --git a/lib/cretonne/meta/gen_instr.py b/lib/cretonne/meta/gen_instr.py index f3c36a716f..0b473e60f3 100644 --- a/lib/cretonne/meta/gen_instr.py +++ b/lib/cretonne/meta/gen_instr.py @@ -210,10 +210,6 @@ def gen_instruction_data_impl(fmt): fmt.doc_comment( """ Get the value arguments to this instruction. - - This is returned as two `Value` slices. The first one - represents the fixed arguments, the second any variable - arguments. """) gen_arguments_method(fmt, False) fmt.doc_comment( diff --git a/lib/cretonne/src/ir/dfg.rs b/lib/cretonne/src/ir/dfg.rs index 24727362b8..f7d214182b 100644 --- a/lib/cretonne/src/ir/dfg.rs +++ b/lib/cretonne/src/ir/dfg.rs @@ -344,16 +344,40 @@ impl DataFlowGraph { DisplayInst(self, inst) } - /// Get the value arguments on `inst` as a slice. + /// Get all value arguments on `inst` as a slice. pub fn inst_args(&self, inst: Inst) -> &[Value] { self.insts[inst].arguments(&self.value_lists) } - /// Get the value arguments on `inst` as a mutable slice. + /// Get all value arguments on `inst` as a mutable slice. pub fn inst_args_mut(&mut self, inst: Inst) -> &mut [Value] { self.insts[inst].arguments_mut(&mut self.value_lists) } + /// Get the fixed value arguments on `inst` as a slice. + pub fn inst_fixed_args(&self, inst: Inst) -> &[Value] { + let fixed_args = self[inst].opcode().constraints().fixed_value_arguments(); + &self.inst_args(inst)[..fixed_args] + } + + /// Get the fixed value arguments on `inst` as a mutable slice. + pub fn inst_fixed_args_mut(&mut self, inst: Inst) -> &mut [Value] { + let fixed_args = self[inst].opcode().constraints().fixed_value_arguments(); + &mut self.inst_args_mut(inst)[..fixed_args] + } + + /// Get the variable value arguments on `inst` as a slice. + pub fn inst_variable_args(&self, inst: Inst) -> &[Value] { + let fixed_args = self[inst].opcode().constraints().fixed_value_arguments(); + &self.inst_args(inst)[fixed_args..] + } + + /// Get the variable value arguments on `inst` as a mutable slice. + pub fn inst_variable_args_mut(&mut self, inst: Inst) -> &mut [Value] { + let fixed_args = self[inst].opcode().constraints().fixed_value_arguments(); + &mut self.inst_args_mut(inst)[fixed_args..] + } + /// Create result values for an instruction that produces multiple results. /// /// Instructions that produce 0 or 1 result values only need to be created with `make_inst`. If diff --git a/lib/cretonne/src/ir/instructions.rs b/lib/cretonne/src/ir/instructions.rs index fe76791211..f5fd0c0a98 100644 --- a/lib/cretonne/src/ir/instructions.rs +++ b/lib/cretonne/src/ir/instructions.rs @@ -398,7 +398,6 @@ pub struct OpcodeConstraints { /// Offset into `OPERAND_CONSTRAINT` table of the descriptors for this opcode. The first /// `fixed_results()` entries describe the result constraints, then follows constraints for the /// fixed `Value` input operands. (`fixed_value_arguments()` of them). - /// format. constraint_offset: u16, } diff --git a/lib/cretonne/src/legalizer/boundary.rs b/lib/cretonne/src/legalizer/boundary.rs index 8ec8ffeb9c..2e3766057d 100644 --- a/lib/cretonne/src/legalizer/boundary.rs +++ b/lib/cretonne/src/legalizer/boundary.rs @@ -346,12 +346,8 @@ fn check_call_signature(dfg: &DataFlowGraph, inst: Inst) -> Result<(), SigRef> { /// Check if the arguments of the return `inst` match the signature. fn check_return_signature(dfg: &DataFlowGraph, inst: Inst, sig: &Signature) -> bool { - let fixed_values = dfg[inst].opcode().constraints().fixed_value_arguments(); check_arg_types(dfg, - dfg.inst_args(inst) - .iter() - .skip(fixed_values) - .cloned(), + dfg.inst_variable_args(inst).iter().cloned(), &sig.return_types) } diff --git a/lib/cretonne/src/verifier.rs b/lib/cretonne/src/verifier.rs index d3de786b6e..433b4bc84d 100644 --- a/lib/cretonne/src/verifier.rs +++ b/lib/cretonne/src/verifier.rs @@ -26,7 +26,6 @@ //! //! - All predecessors in the CFG must be branches to the EBB. //! - All branches to an EBB must be present in the CFG. -//! TODO: //! - A recomputed dominator tree is identical to the existing one. //! //! Type checking @@ -42,6 +41,7 @@ //! - All return instructions must have return value operands matching the current //! function signature. //! +//! TODO: //! Ad hoc checking //! //! - Stack slot loads and stores must be in-bounds. @@ -56,8 +56,9 @@ use dominator_tree::DominatorTree; use flowgraph::ControlFlowGraph; use ir::entities::AnyEntity; -use ir::instructions::{InstructionFormat, BranchInfo}; -use ir::{types, Function, ValueDef, Ebb, Inst, SigRef, FuncRef, ValueList, JumpTable, Value}; +use ir::instructions::{InstructionFormat, BranchInfo, ResolvedConstraint, CallInfo}; +use ir::{types, Function, ValueDef, Ebb, Inst, SigRef, FuncRef, ValueList, JumpTable, Value, Type}; +use Context; use std::fmt::{self, Display, Formatter}; use std::result; @@ -101,6 +102,13 @@ pub fn verify_function(func: &Function) -> Result<()> { Verifier::new(func).run() } +/// Verify `ctx`. +pub fn verify_context(ctx: &Context) -> Result<()> { + let verifier = Verifier::new(&ctx.func); + verifier.domtree_integrity(&ctx.domtree)?; + verifier.run() +} + struct Verifier<'a> { func: &'a Function, cfg: ControlFlowGraph, @@ -377,11 +385,242 @@ impl<'a> Verifier<'a> { Ok(()) } + fn domtree_integrity(&self, domtree: &DominatorTree) -> Result<()> { + // 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. + for ebb in self.func.layout.ebbs() { + let expected = domtree.idom(ebb); + let got = self.domtree.idom(ebb); + if got != expected { + return err!(ebb, + "invalid domtree, expected idom({}) = {:?}, got {:?}", + ebb, + expected, + got); + } + } + Ok(()) + } + + fn typecheck_entry_block_arguments(&self) -> Result<()> { + if let Some(ebb) = self.func.layout.entry_block() { + let expected_types = &self.func.signature.argument_types; + let ebb_arg_count = self.func.dfg.num_ebb_args(ebb); + + if ebb_arg_count != expected_types.len() { + return err!(ebb, "entry block arguments must match function signature"); + } + + for (i, arg) in self.func + .dfg + .ebb_args(ebb) + .enumerate() { + let arg_type = self.func.dfg.value_type(arg); + if arg_type != expected_types[i].value_type { + return err!(ebb, + "entry block argument {} expected to have type {}, got {}", + i, + expected_types[i], + arg_type); + } + } + } + Ok(()) + } + + fn typecheck(&self, inst: Inst) -> Result<()> { + let inst_data = &self.func.dfg[inst]; + let constraints = inst_data.opcode().constraints(); + + let ctrl_type = if let Some(value_typeset) = constraints.ctrl_typeset() { + // For polymorphic opcodes, determine the controlling type variable first. + let ctrl_type = inst_data.ctrl_typevar(&self.func.dfg); + + if !value_typeset.contains(ctrl_type) { + return err!(inst, "has an invalid controlling type {}", ctrl_type); + } + + ctrl_type + } else { + // Non-polymorphic instructions don't check the controlling type variable, so `Option` + // is unnecessary and we can just make it `VOID`. + types::VOID + }; + + self.typecheck_results(inst, ctrl_type)?; + self.typecheck_fixed_args(inst, ctrl_type)?; + self.typecheck_variable_args(inst)?; + self.typecheck_return(inst)?; + + Ok(()) + } + + fn typecheck_results(&self, inst: Inst, ctrl_type: Type) -> Result<()> { + 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!(inst, + "expected result {} ({}) to have type {}, found {}", + i, + result, + expected_type, + result_type); + } + } else { + return err!(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"); + } + Ok(()) + } + + fn typecheck_fixed_args(&self, inst: Inst, ctrl_type: Type) -> Result<()> { + let constraints = self.func.dfg[inst].opcode().constraints(); + + for (i, &arg) in self.func + .dfg + .inst_fixed_args(inst) + .iter() + .enumerate() { + let arg_type = self.func.dfg.value_type(arg); + match constraints.value_argument_constraint(i, ctrl_type) { + ResolvedConstraint::Bound(expected_type) => { + if arg_type != expected_type { + return err!(inst, + "arg {} ({}) has type {}, expected {}", + i, + arg, + arg_type, + expected_type); + } + } + ResolvedConstraint::Free(type_set) => { + if !type_set.contains(arg_type) { + return err!(inst, + "arg {} ({}) with type {} failed to satisfy type set {:?}", + i, + arg, + arg_type, + type_set); + } + } + } + } + Ok(()) + } + + fn typecheck_variable_args(&self, inst: Inst) -> Result<()> { + match self.func.dfg[inst].analyze_branch(&self.func.dfg.value_lists) { + BranchInfo::SingleDest(ebb, _) => { + let iter = self.func + .dfg + .ebb_args(ebb) + .map(|v| self.func.dfg.value_type(v)); + self.typecheck_variable_args_iterator(inst, iter)?; + } + BranchInfo::Table(table) => { + for (_, ebb) in self.func.jump_tables[table].entries() { + let arg_count = self.func.dfg.num_ebb_args(ebb); + if arg_count != 0 { + return err!(inst, + "takes no arguments, but had target {} with {} arguments", + ebb, + arg_count); + } + } + } + BranchInfo::NotABranch => {} + } + + match self.func.dfg[inst].analyze_call(&self.func.dfg.value_lists) { + CallInfo::Direct(func_ref, _) => { + let sig_ref = self.func.dfg.ext_funcs[func_ref].signature; + let arg_types = + self.func.dfg.signatures[sig_ref].argument_types.iter().map(|a| a.value_type); + self.typecheck_variable_args_iterator(inst, arg_types)?; + } + CallInfo::Indirect(sig_ref, _) => { + let arg_types = + self.func.dfg.signatures[sig_ref].argument_types.iter().map(|a| a.value_type); + self.typecheck_variable_args_iterator(inst, arg_types)?; + } + CallInfo::NotACall => {} + } + Ok(()) + } + + fn typecheck_variable_args_iterator>(&self, + inst: Inst, + iter: I) + -> Result<()> { + let variable_args = self.func.dfg.inst_variable_args(inst); + let mut i = 0; + + for expected_type in iter { + if i >= variable_args.len() { + // Result count mismatch handled below, we want the full argument count first though + i += 1; + continue; + } + let arg = variable_args[i]; + let arg_type = self.func.dfg.value_type(arg); + if expected_type != arg_type { + return err!(inst, + "arg {} ({}) has type {}, expected {}", + i, + variable_args[i], + arg_type, + expected_type); + } + i += 1; + } + if i != variable_args.len() { + return err!(inst, + "mismatched argument count, got {}, expected {}", + variable_args.len(), + i); + } + Ok(()) + } + + fn typecheck_return(&self, inst: Inst) -> Result<()> { + if self.func.dfg[inst].opcode().is_return() { + let args = self.func.dfg.inst_variable_args(inst); + let expected_types = &self.func.signature.return_types; + if args.len() != expected_types.len() { + return err!(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!(inst, + "arg {} ({}) has type {}, must match function signature of {}", + i, + arg, + arg_type, + expected_type); + } + } + } + Ok(()) + } + pub fn run(&self) -> Result<()> { + self.typecheck_entry_block_arguments()?; 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.cfg_integrity(ebb)?; }