Implement call_indirect signature checking.

The call_indirect.wast spec test now passes.

lib/environ/src/environ.rs

@@ -1,3 +1,4 @@
+use cast;
 use cranelift_codegen::cursor::FuncCursor;
 use cranelift_codegen::ir;
 use cranelift_codegen::ir::condcodes::*;
@@ -109,6 +110,9 @@ pub struct FuncEnvironment<'module_environment> {
     /// The Cranelift global holding the base address of the globals vector.
     globals_base: Option<ir::GlobalValue>,
 
+    /// The Cranelift global holding the base address of the signature IDs vector.
+    signature_ids_base: Option<ir::GlobalValue>,
+
     /// The external function declaration for implementing wasm's `memory.size`.
     memory_size_extfunc: Option<FuncRef>,
 
@@ -131,9 +135,10 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
             memories_base: None,
             tables_base: None,
             globals_base: None,
+            signature_ids_base: None,
             memory_size_extfunc: None,
             memory_grow_extfunc: None,
-            offsets: VMOffsets::new(isa.frontend_config().pointer_bytes()),
+            offsets: VMOffsets::new(isa.pointer_bytes()),
         }
     }
 
@@ -145,10 +150,6 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
         real_call_args
     }
 
-    fn pointer_bytes(&self) -> u8 {
-        self.isa.pointer_bytes()
-    }
-
     fn vmctx(&mut self, func: &mut Function) -> ir::GlobalValue {
         self.vmctx.unwrap_or_else(|| {
             let vmctx = func.create_global_value(ir::GlobalValueData::VMContext);
@@ -309,12 +310,14 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
     }
 
     fn make_global(&mut self, func: &mut ir::Function, index: GlobalIndex) -> GlobalVariable {
+        let pointer_type = self.pointer_type();
+
         let vmctx = self.vmctx(func);
         let globals_base = self.globals_base.unwrap_or_else(|| {
             let new_base = func.create_global_value(ir::GlobalValueData::Load {
                 base: vmctx,
                 offset: Offset32::new(i32::from(self.offsets.vmctx_globals())),
-                global_type: self.pointer_type(),
+                global_type: pointer_type,
                 readonly: true,
             });
             self.globals_base = Some(new_base);
@@ -323,7 +326,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         let gv = func.create_global_value(ir::GlobalValueData::IAddImm {
             base: globals_base,
             offset: Imm64::new(i64::from(self.offsets.index_vmglobal(index.as_u32()))),
-            global_type: self.pointer_type(),
+            global_type: pointer_type,
         });
         GlobalVariable::Memory {
             gv,
@@ -332,12 +335,14 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
     }
 
     fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> ir::Heap {
+        let pointer_type = self.pointer_type();
+
         let vmctx = self.vmctx(func);
         let memories_base = self.memories_base.unwrap_or_else(|| {
             let new_base = func.create_global_value(ir::GlobalValueData::Load {
                 base: vmctx,
                 offset: Offset32::new(i32::from(self.offsets.vmctx_memories())),
-                global_type: self.pointer_type(),
+                global_type: pointer_type,
                 readonly: true,
             });
             self.memories_base = Some(new_base);
@@ -383,7 +388,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         let heap_base = func.create_global_value(ir::GlobalValueData::Load {
             base: memories_base,
             offset: Offset32::new(self.offsets.index_vmmemory_base(index.as_u32())),
-            global_type: self.pointer_type(),
+            global_type: pointer_type,
             readonly: readonly_base,
         });
         func.create_heap(ir::HeapData {
@@ -396,12 +401,14 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
     }
 
     fn make_table(&mut self, func: &mut ir::Function, index: TableIndex) -> ir::Table {
+        let pointer_type = self.pointer_type();
+
         let vmctx = self.vmctx(func);
         let tables_base = self.tables_base.unwrap_or_else(|| {
             let new_base = func.create_global_value(ir::GlobalValueData::Load {
                 base: vmctx,
                 offset: Offset32::new(i32::from(self.offsets.vmctx_tables())),
-                global_type: self.pointer_type(),
+                global_type: pointer_type,
                 readonly: true,
             });
             self.tables_base = Some(new_base);
@@ -410,7 +417,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         let base_gv = func.create_global_value(ir::GlobalValueData::Load {
             base: tables_base,
             offset: Offset32::new(self.offsets.index_vmtable_base(index.as_u32())),
-            global_type: self.pointer_type(),
+            global_type: pointer_type,
             readonly: false,
         });
         let bound_gv = func.create_global_value(ir::GlobalValueData::Load {
@@ -421,7 +428,9 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         });
 
         let element_size = match self.module.table_plans[index].style {
-            TableStyle::CallerChecksSignature => 2 * u64::from(self.pointer_bytes()),
+            TableStyle::CallerChecksSignature => {
+                u64::from(self.offsets.size_of_vmcaller_checked_anyfunc())
+            }
         };
 
         func.create_table(ir::TableData {
@@ -461,40 +470,70 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         callee: ir::Value,
         call_args: &[ir::Value],
     ) -> WasmResult<ir::Inst> {
-        // FIXME: Cranelift's call_indirect doesn't implement signature checking,
+        let pointer_type = self.pointer_type();
-        // so we need to implement it ourselves.
-        debug_assert_eq!(
-            table_index.index(),
-            0,
-            "non-default tables not supported yet"
-        );
 
-        let table_entry_addr = pos.ins().table_addr(self.pointer_type(), table, callee, 0);
+        let table_entry_addr = pos.ins().table_addr(pointer_type, table, callee, 0);
 
         // Dereference table_entry_addr to get the function address.
         let mut mem_flags = ir::MemFlags::new();
         mem_flags.set_notrap();
         mem_flags.set_aligned();
-        let func_addr = pos
+        let func_addr = pos.ins().load(
-            .ins()
+            pointer_type,
-            .load(self.pointer_type(), mem_flags, table_entry_addr, 0);
+            mem_flags,
+            table_entry_addr,
+            i32::from(self.offsets.vmcaller_checked_anyfunc_func_ptr()),
+        );
 
         // If necessary, check the signature.
         match self.module.table_plans[table_index].style {
             TableStyle::CallerChecksSignature => {
-                // Dereference table_type_addr to get the function signature id.
+                let sig_id_size = self.offsets.size_of_vmsignature_id();
+                let sig_id_type = Type::int(u16::from(sig_id_size) * 8).unwrap();
+
+                let vmctx = self.vmctx(pos.func);
+                let signature_ids_base = self.globals_base.unwrap_or_else(|| {
+                    let new_base = pos.func.create_global_value(ir::GlobalValueData::Load {
+                        base: vmctx,
+                        offset: Offset32::new(i32::from(self.offsets.vmctx_signature_ids())),
+                        global_type: pointer_type,
+                        readonly: true,
+                    });
+                    self.signature_ids_base = Some(new_base);
+                    new_base
+                });
+                let sig_ids = pos.ins().global_value(pointer_type, signature_ids_base);
+
+                // Load the caller ID.
+                // TODO: Factor this out into a MemFlags constructor, as it's used a lot.
                 let mut mem_flags = ir::MemFlags::new();
                 mem_flags.set_notrap();
                 mem_flags.set_aligned();
-                let callee_sig = pos.ins().load(
+                let caller_sig_id = pos.ins().load(
-                    self.pointer_type(),
+                    sig_id_type,
+                    mem_flags,
+                    sig_ids,
+                    cast::i32(
+                        sig_index
+                            .as_u32()
+                            .checked_mul(u32::from(sig_id_size))
+                            .unwrap(),
+                    ).unwrap(),
+                );
+
+                // Load the callee ID.
+                let mut mem_flags = ir::MemFlags::new();
+                mem_flags.set_notrap();
+                mem_flags.set_aligned();
+                let callee_sig_id = pos.ins().load(
+                    sig_id_type,
                     mem_flags,
                     table_entry_addr,
-                    i32::from(self.pointer_bytes()),
+                    i32::from(self.offsets.vmcaller_checked_anyfunc_type_id()),
                 );
-                let cmp =
+
-                    pos.ins()
+                // Check that they match.
-                        .icmp_imm(IntCC::Equal, callee_sig, i64::from(sig_index.as_u32()));
+                let cmp = pos.ins().icmp(IntCC::Equal, callee_sig_id, caller_sig_id);
                 pos.ins().trapz(cmp, ir::TrapCode::BadSignature);
             }
         }

lib/environ/src/lib.rs

@@ -36,6 +36,7 @@ extern crate cranelift_wasm;
 #[cfg(not(feature = "std"))]
 #[macro_use]
 extern crate alloc;
+extern crate cast;
 
 mod compilation;
 mod environ;

lib/environ/src/vmoffsets.rs

@@ -55,6 +55,32 @@ impl VMOffsets {
     }
 }
 
+/// Offsets for `wasmtime_execute::VMSignatureId`.
+impl VMOffsets {
+    /// Return the size of `VMSignatureId`.
+    pub fn size_of_vmsignature_id(&self) -> u8 {
+        4
+    }
+}
+
+/// Offsets for `wasmtime_execute::VMCallerCheckedAnyfunc`.
+impl VMOffsets {
+    /// The offset of the `func_ptr` field.
+    pub fn vmcaller_checked_anyfunc_func_ptr(&self) -> u8 {
+        0 * self.pointer_size
+    }
+
+    /// The offset of the `type_id` field.
+    pub fn vmcaller_checked_anyfunc_type_id(&self) -> u8 {
+        1 * self.pointer_size
+    }
+
+    /// Return the size of `VMTable`.
+    pub fn size_of_vmcaller_checked_anyfunc(&self) -> u8 {
+        2 * self.pointer_size
+    }
+}
+
 /// Offsets for `wasmtime_execute::VMContext`.
 impl VMOffsets {
     /// The offset of the `memories` field.
@@ -72,10 +98,15 @@ impl VMOffsets {
         2 * self.pointer_size
     }
 
+    /// The offset of the `signature_ids` field.
+    pub fn vmctx_signature_ids(&self) -> u8 {
+        3 * self.pointer_size
+    }
+
     /// Return the size of `VMContext`.
     #[allow(dead_code)]
     pub fn size_of_vmctx(&self) -> u8 {
-        3 * self.pointer_size
+        4 * self.pointer_size
     }
 
     /// Return the offset from the `memories` pointer to `VMMemory` index `index`.

lib/execute/Cargo.toml

@@ -20,6 +20,7 @@ lazy_static = "1.2.0"
 libc = { version = "0.2.44", default-features = false }
 errno = "0.2.4"
 memoffset = "0.2.1"
+cast = { version = "0.2.2", default-features = false }
 
 [build-dependencies]
 cmake = "0.1.35"

lib/execute/src/instance.rs

@@ -5,9 +5,10 @@ use cranelift_entity::EntityRef;
 use cranelift_entity::PrimaryMap;
 use cranelift_wasm::{GlobalIndex, MemoryIndex, TableIndex};
 use memory::LinearMemory;
+use sig_registry::SignatureRegistry;
 use std::string::String;
-use table::{AnyFunc, Table};
+use table::Table;
-use vmcontext::{VMContext, VMGlobal, VMMemory, VMTable};
+use vmcontext::{VMCallerCheckedAnyfunc, VMContext, VMGlobal, VMMemory, VMTable};
 use wasmtime_environ::{Compilation, DataInitializer, Module};
 
 /// An Instance of a WebAssemby module.
@@ -19,6 +20,10 @@ pub struct Instance {
     /// WebAssembly table data.
     tables: PrimaryMap<TableIndex, Table>,
 
+    /// Function Signature IDs.
+    /// FIXME: This should be shared across instances rather than per-Instance.
+    sig_registry: SignatureRegistry,
+
     /// Memory base address vector pointed to by vmctx.
     vmctx_memories: PrimaryMap<MemoryIndex, VMMemory>,
 
@@ -39,8 +44,9 @@ impl Instance {
         compilation: &Compilation,
         data_initializers: &[DataInitializer],
     ) -> Result<Self, String> {
+        let mut sig_registry = SignatureRegistry::new();
         let mut memories = instantiate_memories(module, data_initializers)?;
-        let mut tables = instantiate_tables(module, compilation);
+        let mut tables = instantiate_tables(module, compilation, &mut sig_registry);
 
         let mut vmctx_memories = memories
             .values_mut()
@@ -57,14 +63,21 @@ impl Instance {
         let vmctx_memories_ptr = vmctx_memories.values_mut().into_slice().as_mut_ptr();
         let vmctx_globals_ptr = vmctx_globals.values_mut().into_slice().as_mut_ptr();
         let vmctx_tables_ptr = vmctx_tables.values_mut().into_slice().as_mut_ptr();
+        let signature_ids_ptr = sig_registry.vmsignature_ids();
 
         Ok(Self {
             memories,
             tables,
+            sig_registry,
             vmctx_memories,
             vmctx_globals,
             vmctx_tables,
-            vmctx: VMContext::new(vmctx_memories_ptr, vmctx_globals_ptr, vmctx_tables_ptr),
+            vmctx: VMContext::new(
+                vmctx_memories_ptr,
+                vmctx_globals_ptr,
+                vmctx_tables_ptr,
+                signature_ids_ptr,
+            ),
         })
     }
 
@@ -139,7 +152,11 @@ fn instantiate_memories(
 }
 
 /// Allocate memory for just the tables of the current module.
-fn instantiate_tables(module: &Module, compilation: &Compilation) -> PrimaryMap<TableIndex, Table> {
+fn instantiate_tables(
+    module: &Module,
+    compilation: &Compilation,
+    sig_registry: &mut SignatureRegistry,
+) -> PrimaryMap<TableIndex, Table> {
     let mut tables = PrimaryMap::with_capacity(module.table_plans.len());
     for table in module.table_plans.values() {
         tables.push(Table::new(table));
@@ -150,14 +167,14 @@ fn instantiate_tables(module: &Module, compilation: &Compilation) -> PrimaryMap<
         let slice = &mut tables[init.table_index].as_mut();
         let subslice = &mut slice[init.offset..init.offset + init.elements.len()];
         for (i, func_idx) in init.elements.iter().enumerate() {
-            // FIXME: Implement cross-module signature checking.
+            let callee_sig = module.functions[*func_idx];
-            let type_id = module.functions[*func_idx];
             let code_buf = &compilation.functions[module.defined_func_index(*func_idx).expect(
                 "table element initializer with imported function not supported yet",
             )];
-            subslice[i] = AnyFunc {
+            let type_id = sig_registry.register(callee_sig, &module.signatures[callee_sig]);
+            subslice[i] = VMCallerCheckedAnyfunc {
                 func_ptr: code_buf.as_ptr(),
-                type_id: type_id.index(),
+                type_id,
             };
         }
     }

lib/execute/src/lib.rs

@@ -42,6 +42,7 @@ extern crate lazy_static;
 extern crate libc;
 #[macro_use]
 extern crate memoffset;
+extern crate cast;
 
 mod code;
 mod execute;
@@ -50,6 +51,7 @@ mod invoke;
 mod libcalls;
 mod memory;
 mod mmap;
+mod sig_registry;
 mod signalhandlers;
 mod table;
 mod traphandlers;

lib/execute/src/sig_registry.rs

@@ -0,0 +1,43 @@
+//! Implement a registry of function signatures, for fast indirect call
+//! signature checking.
+
+use cast;
+use cranelift_codegen::ir;
+use cranelift_entity::SecondaryMap;
+use cranelift_wasm::SignatureIndex;
+use std::collections::{hash_map, HashMap};
+use vmcontext::VMSignatureId;
+
+#[derive(Debug)]
+pub struct SignatureRegistry {
+    signature_hash: HashMap<ir::Signature, VMSignatureId>,
+    signature_ids: SecondaryMap<SignatureIndex, VMSignatureId>,
+}
+
+impl SignatureRegistry {
+    pub fn new() -> Self {
+        Self {
+            signature_hash: HashMap::new(),
+            signature_ids: SecondaryMap::new(),
+        }
+    }
+
+    pub fn vmsignature_ids(&mut self) -> *mut VMSignatureId {
+        self.signature_ids.values_mut().into_slice().as_mut_ptr()
+    }
+
+    /// Register the given signature.
+    pub fn register(&mut self, sig_index: SignatureIndex, sig: &ir::Signature) -> VMSignatureId {
+        let len = self.signature_hash.len();
+        let sig_id = match self.signature_hash.entry(sig.clone()) {
+            hash_map::Entry::Occupied(entry) => *entry.get(),
+            hash_map::Entry::Vacant(entry) => {
+                let sig_id = cast::u32(len).unwrap();
+                entry.insert(sig_id);
+                sig_id
+            }
+        };
+        self.signature_ids[sig_index] = sig_id;
+        sig_id
+    }
+}

lib/execute/src/signatures.rs

@@ -1,2 +0,0 @@
-//! Implement a registry of function signatures, for fast indirect call
-//! signature checking.

lib/execute/src/table.rs

@@ -3,30 +3,13 @@
 //! `Table` is to WebAssembly tables what `LinearMemory` is to WebAssembly linear memories.
 
 use cranelift_wasm::TableElementType;
-use std::ptr;
+use vmcontext::{VMCallerCheckedAnyfunc, VMTable};
-use vmcontext::VMTable;
 use wasmtime_environ::{TablePlan, TableStyle};
 
-#[derive(Debug, Clone)]
-#[repr(C)]
-pub struct AnyFunc {
-    pub func_ptr: *const u8,
-    pub type_id: usize,
-}
-
-impl Default for AnyFunc {
-    fn default() -> Self {
-        Self {
-            func_ptr: ptr::null(),
-            type_id: 0,
-        }
-    }
-}
-
 /// A table instance.
 #[derive(Debug)]
 pub struct Table {
-    vec: Vec<AnyFunc>,
+    vec: Vec<VMCallerCheckedAnyfunc>,
     maximum: Option<u32>,
 }
 
@@ -43,7 +26,10 @@ impl Table {
         match plan.style {
             TableStyle::CallerChecksSignature => {
                 let mut vec = Vec::new();
-                vec.resize(plan.table.minimum as usize, AnyFunc::default());
+                vec.resize(
+                    plan.table.minimum as usize,
+                    VMCallerCheckedAnyfunc::default(),
+                );
 
                 Self {
                     vec,
@@ -58,14 +44,14 @@ impl Table {
     }
 }
 
-impl AsRef<[AnyFunc]> for Table {
+impl AsRef<[VMCallerCheckedAnyfunc]> for Table {
-    fn as_ref(&self) -> &[AnyFunc] {
+    fn as_ref(&self) -> &[VMCallerCheckedAnyfunc] {
         self.vec.as_slice()
     }
 }
 
-impl AsMut<[AnyFunc]> for Table {
+impl AsMut<[VMCallerCheckedAnyfunc]> for Table {
-    fn as_mut(&mut self) -> &mut [AnyFunc] {
+    fn as_mut(&mut self) -> &mut [VMCallerCheckedAnyfunc] {
         self.vec.as_mut_slice()
     }
 }

lib/execute/src/vmcontext.rs

@@ -5,6 +5,7 @@ use cranelift_entity::EntityRef;
 use cranelift_wasm::{GlobalIndex, MemoryIndex, TableIndex};
 use instance::Instance;
 use std::mem::size_of;
+use std::ptr;
 use std::slice;
 
 /// The main fields a JIT needs to access to utilize a WebAssembly linear,
@@ -171,6 +172,69 @@ impl VMTable {
     }
 }
 
+/// The type of the `type_id` field in `VMCallerCheckedAnyfunc`.
+pub type VMSignatureId = u32;
+
+#[cfg(test)]
+mod test_vmsignature_id {
+    use super::VMSignatureId;
+    use std::mem::size_of;
+    use wasmtime_environ::VMOffsets;
+
+    #[test]
+    fn check_vmcaller_checked_anyfunc_offsets() {
+        let offsets = VMOffsets::new(size_of::<*mut u8>() as u8);
+        assert_eq!(
+            size_of::<VMSignatureId>(),
+            usize::from(offsets.size_of_vmsignature_id())
+        );
+    }
+}
+
+/// The VM caller-checked "anyfunc" record, for caller-side signature checking.
+/// It consists of the actual function pointer and a signature id to be checked
+/// by the caller.
+#[derive(Debug, Clone)]
+#[repr(C)]
+pub struct VMCallerCheckedAnyfunc {
+    pub func_ptr: *const u8,
+    pub type_id: VMSignatureId,
+    // If more elements are added here, remember to add offset_of tests below!
+}
+
+#[cfg(test)]
+mod test_vmcaller_checked_anyfunc {
+    use super::VMCallerCheckedAnyfunc;
+    use std::mem::size_of;
+    use wasmtime_environ::VMOffsets;
+
+    #[test]
+    fn check_vmcaller_checked_anyfunc_offsets() {
+        let offsets = VMOffsets::new(size_of::<*mut u8>() as u8);
+        assert_eq!(
+            size_of::<VMCallerCheckedAnyfunc>(),
+            usize::from(offsets.size_of_vmcaller_checked_anyfunc())
+        );
+        assert_eq!(
+            offset_of!(VMCallerCheckedAnyfunc, func_ptr),
+            usize::from(offsets.vmcaller_checked_anyfunc_func_ptr())
+        );
+        assert_eq!(
+            offset_of!(VMCallerCheckedAnyfunc, type_id),
+            usize::from(offsets.vmcaller_checked_anyfunc_type_id())
+        );
+    }
+}
+
+impl Default for VMCallerCheckedAnyfunc {
+    fn default() -> Self {
+        Self {
+            func_ptr: ptr::null_mut(),
+            type_id: 0,
+        }
+    }
+}
+
 /// The VM "context", which is pointed to by the `vmctx` arg in Cranelift.
 /// This has pointers to the globals, memories, tables, and other runtime
 /// state associated with the current instance.
@@ -185,6 +249,8 @@ pub struct VMContext {
     /// A pointer to an array of `VMTable` instances, indexed by
     /// WebAssembly table index.
     tables: *mut VMTable,
+    /// Signature identifiers for signature-checking indirect calls.
+    signature_ids: *mut u32,
     // If more elements are added here, remember to add offset_of tests below!
 }
 
@@ -210,16 +276,26 @@ mod test {
             offset_of!(VMContext, tables),
             usize::from(offsets.vmctx_tables())
         );
+        assert_eq!(
+            offset_of!(VMContext, signature_ids),
+            usize::from(offsets.vmctx_signature_ids())
+        );
     }
 }
 
 impl VMContext {
     /// Create a new `VMContext` instance.
-    pub fn new(memories: *mut VMMemory, globals: *mut VMGlobal, tables: *mut VMTable) -> Self {
+    pub fn new(
+        memories: *mut VMMemory,
+        globals: *mut VMGlobal,
+        tables: *mut VMTable,
+        signature_ids: *mut u32,
+    ) -> Self {
         Self {
             memories,
             globals,
             tables,
+            signature_ids,
         }
     }
 

lib/wast/build.rs

@@ -93,8 +93,8 @@ fn ignore(testsuite: &str, name: &str) -> bool {
     match testsuite {
         "spec_testsuite" => match name {
             // These are the remaining spec testsuite failures.
-            "call_indirect" | "data" | "elem" | "exports" | "func" | "func_ptrs" | "globals"
+            "data" | "elem" | "exports" | "func" | "func_ptrs" | "globals" | "imports"
-            | "imports" | "linking" | "names" | "start" => true,
+            | "linking" | "names" | "start" => true,
             _ => false,
         },
         _ => false,