From 93f33141e9cf8b17c7d2301a8daadb3886b27c1d Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sat, 8 Dec 2018 16:22:48 -0500 Subject: [PATCH] Introduce VMFunctionBody to add extra type safety. --- lib/execute/src/code.rs | 23 ++++++++++++++++------- lib/execute/src/export.rs | 6 +++--- lib/execute/src/imports.rs | 4 ++-- lib/execute/src/instance.rs | 15 +++++++++------ lib/execute/src/invoke.rs | 18 +++++++----------- lib/execute/src/lib.rs | 2 +- lib/execute/src/link.rs | 6 +++--- lib/execute/src/vmcontext.rs | 20 +++++++++++++++++++- lib/execute/src/world.rs | 6 +++--- lib/wast/src/spectest.rs | 16 ++++++++-------- 10 files changed, 71 insertions(+), 45 deletions(-) diff --git a/lib/execute/src/code.rs b/lib/execute/src/code.rs index 7f319ac95d..26e003db8d 100644 --- a/lib/execute/src/code.rs +++ b/lib/execute/src/code.rs @@ -4,9 +4,9 @@ use mmap::Mmap; use region; use std::cmp; use std::mem; -use std::slice; use std::string::String; use std::vec::Vec; +use vmcontext::VMFunctionBody; /// Memory manager for executable code. pub struct Code { @@ -33,7 +33,7 @@ impl Code { /// actually executing from it. /// /// TODO: Add an alignment flag. - fn allocate(&mut self, size: usize) -> Result<*mut u8, String> { + fn allocate(&mut self, size: usize) -> Result<&mut [u8], String> { if self.current.len() - self.position < size { self.mmaps.push(mem::replace( &mut self.current, @@ -43,17 +43,26 @@ impl Code { } let old_position = self.position; self.position += size; - Ok(self.current.as_mut_slice()[old_position..self.position].as_mut_ptr()) + Ok(&mut self.current.as_mut_slice()[old_position..self.position]) + } + + /// Convert mut a slice from u8 to VMFunctionBody. + fn as_mut_vmfunc_slice(slice: &mut [u8]) -> &mut [VMFunctionBody] { + let byte_ptr: *mut [u8] = slice; + let body_ptr = byte_ptr as *mut [VMFunctionBody]; + unsafe { &mut *body_ptr } } /// Allocate enough memory to hold a copy of `slice` and copy the data into it. /// TODO: Reorganize the code that calls this to emit code directly into the /// mmap region rather than into a Vec that we need to copy in. - pub fn allocate_copy_of_slice(&mut self, slice: &[u8]) -> Result<&mut [u8], String> { - let ptr = self.allocate(slice.len())?; - let new = unsafe { slice::from_raw_parts_mut(ptr, slice.len()) }; + pub fn allocate_copy_of_byte_slice( + &mut self, + slice: &[u8], + ) -> Result<&mut [VMFunctionBody], String> { + let new = self.allocate(slice.len())?; new.copy_from_slice(slice); - Ok(new) + Ok(Self::as_mut_vmfunc_slice(new)) } /// Make all allocated memory executable. diff --git a/lib/execute/src/export.rs b/lib/execute/src/export.rs index 2335e82d01..0639e70251 100644 --- a/lib/execute/src/export.rs +++ b/lib/execute/src/export.rs @@ -1,6 +1,6 @@ use cranelift_codegen::ir; use cranelift_wasm::Global; -use vmcontext::{VMGlobal, VMMemory, VMTable}; +use vmcontext::{VMFunctionBody, VMGlobal, VMMemory, VMTable}; use wasmtime_environ::{MemoryPlan, TablePlan}; /// The value of an export passed from one instance to another. @@ -8,7 +8,7 @@ pub enum ExportValue { /// A function export value. Function { /// The address of the native-code function. - address: *const u8, + address: *const VMFunctionBody, /// The function signature declaration, used for compatibilty checking. signature: ir::Signature, }, @@ -40,7 +40,7 @@ pub enum ExportValue { impl ExportValue { /// Construct a function export value. - pub fn function(address: *const u8, signature: ir::Signature) -> Self { + pub fn function(address: *const VMFunctionBody, signature: ir::Signature) -> Self { ExportValue::Function { address, signature } } diff --git a/lib/execute/src/imports.rs b/lib/execute/src/imports.rs index 56b53798c7..8fd06f63b5 100644 --- a/lib/execute/src/imports.rs +++ b/lib/execute/src/imports.rs @@ -1,12 +1,12 @@ use cranelift_entity::PrimaryMap; use cranelift_wasm::{FuncIndex, GlobalIndex, MemoryIndex, TableIndex}; -use vmcontext::{VMGlobal, VMMemory, VMTable}; +use vmcontext::{VMFunctionBody, VMGlobal, VMMemory, VMTable}; /// Resolved import pointers. #[derive(Debug)] pub struct Imports { /// Resolved addresses for imported functions. - pub functions: PrimaryMap, + pub functions: PrimaryMap, /// Resolved addresses for imported tables. pub tables: PrimaryMap, diff --git a/lib/execute/src/instance.rs b/lib/execute/src/instance.rs index ab701ae23a..bd9ed33dc7 100644 --- a/lib/execute/src/instance.rs +++ b/lib/execute/src/instance.rs @@ -11,7 +11,7 @@ use std::ptr; use std::slice; use std::string::String; use table::Table; -use vmcontext::{VMCallerCheckedAnyfunc, VMContext, VMGlobal, VMMemory, VMTable}; +use vmcontext::{VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMGlobal, VMMemory, VMTable}; use wasmtime_environ::{DataInitializer, Module}; /// An Instance of a WebAssemby module. @@ -40,7 +40,7 @@ pub struct Instance { imports: Imports, /// Pointers to functions in executable memory. - allocated_functions: PrimaryMap, + allocated_functions: PrimaryMap, /// Context pointer used by JIT code. vmctx: VMContext, @@ -52,7 +52,7 @@ impl Instance { /// which have been placed in executable memory. pub fn new( module: &Module, - allocated_functions: PrimaryMap, + allocated_functions: PrimaryMap, data_initializers: &[DataInitializer], imports: Imports, ) -> Result { @@ -106,14 +106,17 @@ impl Instance { } /// Return the pointer to executable memory for the given function index. - pub(crate) fn get_allocated_function(&self, index: DefinedFuncIndex) -> Option<&[u8]> { + pub(crate) fn get_allocated_function( + &self, + index: DefinedFuncIndex, + ) -> Option<&[VMFunctionBody]> { self.allocated_functions .get(index) .map(|(ptr, len)| unsafe { slice::from_raw_parts(*ptr, *len) }) } /// Return the pointer to executable memory for the given function index. - pub(crate) fn get_imported_function(&self, index: FuncIndex) -> Option<*const u8> { + pub(crate) fn get_imported_function(&self, index: FuncIndex) -> Option<*const VMFunctionBody> { self.imports.functions.get(index).cloned() } @@ -188,7 +191,7 @@ fn instantiate_memories( /// Allocate memory for just the tables of the current module. fn instantiate_tables( module: &Module, - allocated_functions: &PrimaryMap, + allocated_functions: &PrimaryMap, sig_registry: &mut SignatureRegistry, ) -> PrimaryMap { let mut tables = PrimaryMap::with_capacity(module.table_plans.len()); diff --git a/lib/execute/src/invoke.rs b/lib/execute/src/invoke.rs index 5d49228b2e..1db922f253 100644 --- a/lib/execute/src/invoke.rs +++ b/lib/execute/src/invoke.rs @@ -13,7 +13,7 @@ use std::mem; use std::ptr; use std::vec::Vec; use traphandlers::call_wasm; -use vmcontext::VMContext; +use vmcontext::{VMContext, VMFunctionBody}; use wasmtime_environ::{CompileError, Export, Module, RelocSink}; /// Calls the given named function, passing its return values and returning @@ -71,14 +71,10 @@ pub fn invoke_by_index( args: &[RuntimeValue], ) -> Result { let exec_code_buf = match module.defined_func_index(fn_index) { - Some(def_fn_index) => { - let slice = instance - .get_allocated_function(def_fn_index) - .ok_or_else(|| ActionError::Index(def_fn_index.index() as u64))?; - code.allocate_copy_of_slice(slice) - .map_err(ActionError::Resource)? - .as_ptr() - } + Some(def_fn_index) => instance + .get_allocated_function(def_fn_index) + .ok_or_else(|| ActionError::Index(def_fn_index.index() as u64))? + .as_ptr(), None => instance .get_imported_function(fn_index) .ok_or_else(|| ActionError::Index(fn_index.index() as u64))?, @@ -111,7 +107,7 @@ pub fn invoke_by_index( fn call_through_wrapper( code: &mut Code, isa: &isa::TargetIsa, - callee: *const u8, + callee: *const VMFunctionBody, instance: &mut Instance, args: &[RuntimeValue], sig: &ir::Signature, @@ -202,7 +198,7 @@ fn call_through_wrapper( assert!(reloc_sink.func_relocs.is_empty()); let exec_code_buf = code - .allocate_copy_of_slice(&code_buf) + .allocate_copy_of_byte_slice(&code_buf) .map_err(ActionError::Resource)? .as_ptr(); code.publish(); diff --git a/lib/execute/src/lib.rs b/lib/execute/src/lib.rs index 5f94575763..eb38b39a7c 100644 --- a/lib/execute/src/lib.rs +++ b/lib/execute/src/lib.rs @@ -70,7 +70,7 @@ pub use instance::Instance; pub use invoke::{invoke, invoke_by_index, invoke_start_function}; pub use link::link_module; pub use traphandlers::{call_wasm, LookupCodeSegment, RecordTrap, Unwind}; -pub use vmcontext::{VMContext, VMGlobal, VMMemory, VMTable}; +pub use vmcontext::{VMContext, VMFunctionBody, VMGlobal, VMMemory, VMTable}; pub use world::InstanceWorld; #[cfg(not(feature = "std"))] diff --git a/lib/execute/src/link.rs b/lib/execute/src/link.rs index 67c92440fb..7bb89dfe9e 100644 --- a/lib/execute/src/link.rs +++ b/lib/execute/src/link.rs @@ -8,7 +8,7 @@ use imports::Imports; use std::ptr::write_unaligned; use std::vec::Vec; use vmcontext::VMContext; -use vmcontext::{VMGlobal, VMMemory, VMTable}; +use vmcontext::{VMFunctionBody, VMGlobal, VMMemory, VMTable}; use wasmtime_environ::{ MemoryPlan, MemoryStyle, Module, Relocation, RelocationTarget, Relocations, TablePlan, TableStyle, @@ -22,7 +22,7 @@ pub struct LinkError(String); /// Links a module that has been compiled with `compiled_module` in `wasmtime-environ`. pub fn link_module( module: &Module, - allocated_functions: &PrimaryMap, + allocated_functions: &PrimaryMap, relocations: Relocations, resolver: &mut Resolver, ) -> Result { @@ -277,7 +277,7 @@ fn is_memory_compatible(exported: &MemoryPlan, imported: &MemoryPlan) -> bool { /// Performs the relocations inside the function bytecode, provided the necessary metadata. fn relocate( imports: &Imports, - allocated_functions: &PrimaryMap, + allocated_functions: &PrimaryMap, relocations: PrimaryMap>, module: &Module, ) { diff --git a/lib/execute/src/vmcontext.rs b/lib/execute/src/vmcontext.rs index 95067d4d65..b8c08b5dd9 100644 --- a/lib/execute/src/vmcontext.rs +++ b/lib/execute/src/vmcontext.rs @@ -7,6 +7,24 @@ use instance::Instance; use std::fmt; use std::ptr; +/// A placeholder byte-sized type which is just used to provide some amount of type +/// safety when dealing with pointers to JIT-compiled function bodies. Note that it's +/// deliberately not Copy, as we shouldn't be carelessly copying function body bytes +/// around. +#[repr(C)] +pub struct VMFunctionBody(u8); + +#[cfg(test)] +mod test_vmfunction_body { + use super::VMFunctionBody; + use std::mem::size_of; + + #[test] + fn check_vmfunction_body_offsets() { + assert_eq!(size_of::(), 1); + } +} + /// The fields a JIT needs to access to utilize a WebAssembly linear /// memory defined within the instance, namely the start address and the /// size in bytes. @@ -454,7 +472,7 @@ mod test_vmsignature_id { #[derive(Debug, Clone)] #[repr(C)] pub struct VMCallerCheckedAnyfunc { - pub func_ptr: *const u8, + pub func_ptr: *const VMFunctionBody, pub type_id: VMSignatureId, // If more elements are added here, remember to add offset_of tests below! } diff --git a/lib/execute/src/world.rs b/lib/execute/src/world.rs index 19e2d89da5..e8f9c9c2c4 100644 --- a/lib/execute/src/world.rs +++ b/lib/execute/src/world.rs @@ -9,7 +9,7 @@ use instance::Instance; use invoke::{invoke, invoke_start_function}; use link::link_module; use std::str; -use vmcontext::VMGlobal; +use vmcontext::{VMFunctionBody, VMGlobal}; use wasmtime_environ::{ compile_module, Compilation, CompileError, Module, ModuleEnvironment, Tunables, }; @@ -123,10 +123,10 @@ impl InstanceWorld { fn allocate_functions( code: &mut Code, compilation: Compilation, -) -> Result, String> { +) -> Result, String> { let mut result = PrimaryMap::with_capacity(compilation.functions.len()); for (_, body) in compilation.functions.into_iter() { - let slice = code.allocate_copy_of_slice(&body)?; + let slice = code.allocate_copy_of_byte_slice(body)?; result.push((slice.as_mut_ptr(), slice.len())); } Ok(result) diff --git a/lib/wast/src/spectest.rs b/lib/wast/src/spectest.rs index 0f8d89357b..6ac30a6031 100644 --- a/lib/wast/src/spectest.rs +++ b/lib/wast/src/spectest.rs @@ -4,7 +4,7 @@ use cranelift_wasm::{Global, GlobalInit, Memory, Table, TableElementType}; use std::ptr; use target_lexicon::HOST; use wasmtime_environ::{translate_signature, MemoryPlan, MemoryStyle, TablePlan, TableStyle}; -use wasmtime_execute::{ExportValue, Resolver, VMGlobal, VMMemory, VMTable}; +use wasmtime_execute::{ExportValue, Resolver, VMFunctionBody, VMGlobal, VMMemory, VMTable}; extern "C" fn spectest_print() {} @@ -79,7 +79,7 @@ impl Resolver for SpecTest { match module { "spectest" => match field { "print" => Some(ExportValue::function( - spectest_print as *const u8, + spectest_print as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![], @@ -90,7 +90,7 @@ impl Resolver for SpecTest { ), )), "print_i32" => Some(ExportValue::function( - spectest_print_i32 as *const u8, + spectest_print_i32 as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![ir::AbiParam::new(types::I32)], @@ -101,7 +101,7 @@ impl Resolver for SpecTest { ), )), "print_i64" => Some(ExportValue::function( - spectest_print_i64 as *const u8, + spectest_print_i64 as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![ir::AbiParam::new(types::I64)], @@ -112,7 +112,7 @@ impl Resolver for SpecTest { ), )), "print_f32" => Some(ExportValue::function( - spectest_print_f32 as *const u8, + spectest_print_f32 as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![ir::AbiParam::new(types::F32)], @@ -123,7 +123,7 @@ impl Resolver for SpecTest { ), )), "print_f64" => Some(ExportValue::function( - spectest_print_f64 as *const u8, + spectest_print_f64 as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![ir::AbiParam::new(types::F64)], @@ -134,7 +134,7 @@ impl Resolver for SpecTest { ), )), "print_i32_f32" => Some(ExportValue::function( - spectest_print_i32_f32 as *const u8, + spectest_print_i32_f32 as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![ @@ -148,7 +148,7 @@ impl Resolver for SpecTest { ), )), "print_f64_f64" => Some(ExportValue::function( - spectest_print_f64_f64 as *const u8, + spectest_print_f64_f64 as *const VMFunctionBody, translate_signature( ir::Signature { params: vec![