From c44dcfec2ba45b372839809a50d265dd78b1bdd7 Mon Sep 17 00:00:00 2001 From: Lachlan Sneff Date: Thu, 10 Jan 2019 11:26:52 -0500 Subject: [PATCH] call_indirect checks signature structural equality instead of nominal equality --- lib/clif-backend/src/codegen.rs | 64 +++++++++++++++++---------------- lib/runtime/examples/test.rs | 36 ++----------------- lib/runtime/src/backing.rs | 4 --- lib/runtime/src/sig_registry.rs | 39 ++++++++++---------- lib/runtime/src/vm.rs | 11 +----- 5 files changed, 56 insertions(+), 98 deletions(-) diff --git a/lib/clif-backend/src/codegen.rs b/lib/clif-backend/src/codegen.rs index 1694168d2..82168a45a 100644 --- a/lib/clif-backend/src/codegen.rs +++ b/lib/clif-backend/src/codegen.rs @@ -4,7 +4,7 @@ use cranelift_codegen::ir::immediates::{Offset32, Uimm64}; use cranelift_codegen::ir::types::{self, *}; use cranelift_codegen::ir::{ self, AbiParam, ArgumentPurpose, ExtFuncData, ExternalName, FuncRef, InstBuilder, Signature, - TrapCode, + TrapCode, condcodes::IntCC, }; use cranelift_codegen::isa::TargetFrontendConfig; use cranelift_entity::{EntityRef, PrimaryMap}; @@ -57,17 +57,11 @@ pub mod converter { tables.push(convert_table(table)); } - let mut sig_registry = SigRegistry::new(); - for signature in cranelift_module.signatures { - let func_sig = convert_signature(signature); - sig_registry.register(func_sig); - } - // Convert Cranelift signature indices to Wasmer signature indices. let mut func_assoc: Map = Map::with_capacity(cranelift_module.functions.len()); for (_, signature_index) in cranelift_module.functions.iter() { - func_assoc.push(WasmerSignatureIndex::new(signature_index.index())); + func_assoc.push(cranelift_module.sig_registry.lookup_deduplicated_sigindex(WasmerSignatureIndex::new(signature_index.index()))); } let function_bodies: Vec<_> = cranelift_module @@ -95,6 +89,7 @@ pub mod converter { data_initializers, table_initializers, start_func, + sig_registry, .. } = cranelift_module; @@ -179,7 +174,7 @@ pub mod converter { } /// Converts a Cranelift signature to a Wasmer signature. - pub fn convert_signature(sig: ir::Signature) -> WasmerSignature { + pub fn convert_signature(sig: &ir::Signature) -> WasmerSignature { WasmerSignature { params: sig .params @@ -256,6 +251,8 @@ pub struct CraneliftModule { // The start function index. pub start_func: Option, + + pub sig_registry: SigRegistry, } impl CraneliftModule { @@ -286,6 +283,7 @@ impl CraneliftModule { data_initializers: Vec::new(), table_initializers: Vec::new(), start_func: None, + sig_registry: SigRegistry::new(), }; // Translate wasm to cranelift IR. @@ -441,10 +439,16 @@ impl<'environment> FuncEnvironmentTrait for FuncEnvironment<'environment> { // Based on the index provided, we need to know the offset into tables array. let table_data_offset = (index.as_u32() as i32) * (ptr_size as i32) * 2; + let table_data = func.create_global_value(ir::GlobalValueData::IAddImm { + base, + offset: (table_data_offset as i64).into(), + global_type: self.pointer_type(), + }); + // Load value at (base + table_data_offset), i.e. the address at Ctx.tables[index].data let base_gv = func.create_global_value(ir::GlobalValueData::Load { - base, - offset: Offset32::new(table_data_offset), + base: table_data, + offset: 0.into(), global_type: self.pointer_type(), readonly: false, }); @@ -452,7 +456,7 @@ impl<'environment> FuncEnvironmentTrait for FuncEnvironment<'environment> { // Load value at (base + table_data_offset), i.e. the value at Ctx.tables[index].len let bound_gv = func.create_global_value(ir::GlobalValueData::Load { base, - offset: Offset32::new(table_data_offset + ptr_size as i32), + offset: (self.pointer_bytes() as i32).into(), global_type: self.pointer_type(), readonly: false, }); @@ -462,8 +466,8 @@ impl<'environment> FuncEnvironmentTrait for FuncEnvironment<'environment> { base_gv, min_size: Uimm64::new(0), bound_gv, - element_size: Uimm64::new(u64::from(self.pointer_bytes())), - index_type: I64, + element_size: Uimm64::new(u64::from(self.pointer_bytes() * 2)), + index_type: self.pointer_type(), }) } @@ -502,14 +506,13 @@ impl<'environment> FuncEnvironmentTrait for FuncEnvironment<'environment> { /// /// Inserts instructions at `pos` to the function `callee` in the table /// `table_index` with WebAssembly signature `sig_index` - /// TODO: Generate bounds checking code. #[cfg_attr(feature = "cargo-clippy", allow(clippy::too_many_arguments))] fn translate_call_indirect( &mut self, mut pos: FuncCursor, _table_index: TableIndex, table: ir::Table, - _sig_index: SignatureIndex, + sig_index: SignatureIndex, sig_ref: ir::SigRef, callee: ir::Value, call_args: &[ir::Value], @@ -534,27 +537,26 @@ impl<'environment> FuncEnvironmentTrait for FuncEnvironment<'environment> { // The `callee` value is an index into a table of function pointers. let entry_addr = pos.ins().table_addr(ptr_type, table, callee_offset, 0); - let mut mflags = ir::MemFlags::new(); - mflags.set_notrap(); - mflags.set_aligned(); + let mflags = ir::MemFlags::trusted(); let func_ptr = pos.ins().load(ptr_type, mflags, entry_addr, 0); pos.ins().trapz(func_ptr, TrapCode::IndirectCallToNull); - // Build a value list for the indirect call instruction containing the callee, call_args, + let found_sig = pos.ins().load(I32, mflags, entry_addr, self.pointer_bytes() as i32); + let deduplicated_sig_index = self.module.sig_registry.lookup_deduplicated_sigindex(WasmerSignatureIndex::new(sig_index.index())); + let expected_sig = pos.ins().iconst(I32, deduplicated_sig_index.index() as i64); + let not_equal_flags = pos.ins().ifcmp(found_sig, expected_sig); + + pos.ins().trapif(IntCC::NotEqual, not_equal_flags, TrapCode::BadSignature); + + // Build a value list for the indirect call instruction containing the call_args // and the vmctx parameter. - let mut args = ir::ValueList::default(); - args.push(func_ptr, &mut pos.func.dfg.value_lists); - args.extend(call_args.iter().cloned(), &mut pos.func.dfg.value_lists); - args.push(vmctx, &mut pos.func.dfg.value_lists); + let mut args = Vec::with_capacity(call_args.len() + 1); + args.extend(call_args.iter().cloned()); + args.push(vmctx); - let inst = pos - .ins() - .CallIndirect(ir::Opcode::CallIndirect, INVALID, sig_ref, args) - .0; - - Ok(inst) + Ok(pos.ins().call_indirect(sig_ref, func_ptr, &args)) } /// Generates a call IR with `callee` and `call_args` and inserts it at `pos` @@ -762,6 +764,8 @@ impl<'data> ModuleEnvironment<'data> for CraneliftModule { /// Declares a function signature to the environment. fn declare_signature(&mut self, sig: &ir::Signature) { self.signatures.push(sig.clone()); + let wasmer_sig = converter::convert_signature(sig); + self.sig_registry.register(wasmer_sig); } /// Return the signature with the given index. diff --git a/lib/runtime/examples/test.rs b/lib/runtime/examples/test.rs index f1e0ff0c6..20ad160d7 100644 --- a/lib/runtime/examples/test.rs +++ b/lib/runtime/examples/test.rs @@ -5,7 +5,7 @@ use wasmer_clif_backend::CraneliftCompiler; fn main() { let mut instance = create_module_1(); - let result = instance.call("signature-explicit-reused", &[]); + let result = instance.call("signature-implicit-reused", &[]); println!("result: {:?}", result); } @@ -23,11 +23,6 @@ fn create_module_1() -> Box { (func (;5;) (type 2) (param i64 i64 f64 i64 f64 i64 f32 i32)) (func (;6;) (type 2) (param i64 i64 f64 i64 f64 i64 f32 i32)) (func (;7;) (type 0) - i32.const 1 - call_indirect (type 0) - i32.const 4 - call_indirect (type 0)) - (func (;8;) (type 0) f64.const 0x0p+0 (;=0;) i64.const 0 f64.const 0x0p+0 (;=0;) @@ -58,35 +53,8 @@ fn create_module_1() -> Box { i32.const 0 i32.const 3 call_indirect (type 3)) - (func (;9;) (type 0) - i32.const 1 - call_indirect (type 1)) - (func (;10;) (type 0) - i64.const 0 - i64.const 0 - f64.const 0x0p+0 (;=0;) - i64.const 0 - f64.const 0x0p+0 (;=0;) - i64.const 0 - f32.const 0x0p+0 (;=0;) - i32.const 0 - i32.const 5 - call_indirect (type 2) - i64.const 0 - i64.const 0 - f64.const 0x0p+0 (;=0;) - i64.const 0 - f64.const 0x0p+0 (;=0;) - i64.const 0 - f32.const 0x0p+0 (;=0;) - i32.const 0 - i32.const 6 - call_indirect (type 2)) (table (;0;) 7 7 anyfunc) - (export \"signature-explicit-reused\" (func 7)) - (export \"signature-implicit-reused\" (func 8)) - (export \"signature-explicit-duplicate\" (func 9)) - (export \"signature-implicit-duplicate\" (func 10)) + (export \"signature-implicit-reused\" (func 7)) (elem (;0;) (i32.const 0) 4 2 1 4 0 5 6)) "; let wasm_binary = wat2wasm(module_str.as_bytes()).expect("WAST not valid or malformed"); diff --git a/lib/runtime/src/backing.rs b/lib/runtime/src/backing.rs index 4a871d6ed..6a5e95fe8 100644 --- a/lib/runtime/src/backing.rs +++ b/lib/runtime/src/backing.rs @@ -15,7 +15,6 @@ pub struct LocalBacking { pub vm_memories: Box<[vm::LocalMemory]>, pub vm_tables: Box<[vm::LocalTable]>, pub vm_globals: Box<[vm::LocalGlobal]>, - pub vm_signatures: Box<[vm::SigId]>, } impl LocalBacking { @@ -27,7 +26,6 @@ impl LocalBacking { let vm_memories = Self::finalize_memories(module, &mut memories[..]); let vm_tables = Self::finalize_tables(module, imports, &mut tables[..]); let vm_globals = Self::finalize_globals(module, imports, globals); - let vm_signatures = module.sig_registry.into_vm_sigid(); Self { memories, @@ -36,7 +34,6 @@ impl LocalBacking { vm_memories, vm_tables, vm_globals, - vm_signatures, } } @@ -110,7 +107,6 @@ impl LocalBacking { for (i, &func_index) in init.elements.iter().enumerate() { let sig_index = module.func_assoc[func_index]; let vm_sig_id = vm::SigId(sig_index.index() as u32); - let func_data = if module.is_imported_function(func_index) { imports.functions[func_index.index()].clone() } else { diff --git a/lib/runtime/src/sig_registry.rs b/lib/runtime/src/sig_registry.rs index 4db35a040..268c2a1e7 100644 --- a/lib/runtime/src/sig_registry.rs +++ b/lib/runtime/src/sig_registry.rs @@ -1,41 +1,40 @@ use crate::{ - types::{FuncSig, Map, MapIndex, SigIndex}, - vm, + types::{FuncSig, Map, SigIndex}, }; -// use hashbrown::HashMap; +use hashbrown::HashMap; +#[derive(Debug)] pub struct SigRegistry { - // func_table: HashMap, + func_table: HashMap, sig_assoc: Map, + duplicated_sig_assoc: Map, } impl SigRegistry { pub fn new() -> Self { Self { - // func_table: HashMap::new(), + func_table: HashMap::new(), sig_assoc: Map::new(), + duplicated_sig_assoc: Map::new(), } } pub fn register(&mut self, func_sig: FuncSig) -> SigIndex { - self.sig_assoc.push(func_sig) - // let func_table = &mut self.func_table; - // let sig_assoc = &mut self.sig_assoc; - // *func_table - // .entry(func_sig.clone()) - // .or_insert_with(|| sig_assoc.push(func_sig)) + // self.sig_assoc.push(func_sig) + let func_table = &mut self.func_table; + let sig_assoc = &mut self.sig_assoc; + let sig_index = *func_table + .entry(func_sig.clone()) + .or_insert_with(|| sig_assoc.push(func_sig)); + self.duplicated_sig_assoc.push(sig_index); + sig_index + } + + pub fn lookup_deduplicated_sigindex(&self, sig_index: SigIndex) -> SigIndex { + self.duplicated_sig_assoc[sig_index] } pub fn lookup_func_sig(&self, sig_index: SigIndex) -> &FuncSig { &self.sig_assoc[sig_index] } - - pub(crate) fn into_vm_sigid(&self) -> Box<[vm::SigId]> { - let v: Vec<_> = self - .sig_assoc - .iter() - .map(|(sig_index, _)| vm::SigId(sig_index.index() as u32)) - .collect(); - v.into_boxed_slice() - } } diff --git a/lib/runtime/src/vm.rs b/lib/runtime/src/vm.rs index 455901db1..afb3555ac 100644 --- a/lib/runtime/src/vm.rs +++ b/lib/runtime/src/vm.rs @@ -25,9 +25,6 @@ pub struct Ctx { /// A pointer to an array of imported functions, indexed by `FuncIndex`. pub imported_funcs: *mut ImportedFunc, - /// Signature identifiers for signature-checked indirect calls. - pub signatures: *const SigId, - /// The parent instance. pub local_backing: *mut LocalBacking, } @@ -43,8 +40,7 @@ impl Ctx { imported_tables: import_backing.tables.as_mut_ptr(), imported_globals: import_backing.globals.as_mut_ptr(), imported_funcs: import_backing.functions.as_mut_ptr(), - - signatures: local_backing.vm_signatures.as_mut_ptr(), + local_backing, } } @@ -281,11 +277,6 @@ mod vm_offset_tests { Ctx::offset_imported_funcs() as usize, offset_of!(Ctx => imported_funcs).get_byte_offset(), ); - - assert_eq!( - Ctx::offset_signatures() as usize, - offset_of!(Ctx => signatures).get_byte_offset(), - ); } #[test]