diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs index 694d8a173077..a4d8c5344772 100644 --- a/crates/cranelift/src/func_environ.rs +++ b/crates/cranelift/src/func_environ.rs @@ -1155,11 +1155,6 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { self.env .get_or_init_func_ref_table_elem(self.builder, table_index, callee); - // Check for whether the table element is null, and trap if so. - self.builder - .ins() - .trapz(funcref_ptr, ir::TrapCode::IndirectCallToNull); - // If necessary, check the signature. match self.env.module.table_plans[table_index].style { TableStyle::CallerChecksSignature => { @@ -1188,10 +1183,13 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { .load(sig_id_type, mem_flags, signatures, offset); // Load the callee ID. + // + // Note that the callee may be null in which case this load may + // trap. If so use the `IndirectCallToNull` trap code. let mem_flags = ir::MemFlags::trusted().with_readonly(); let callee_sig_id = self.builder.ins().load( sig_id_type, - mem_flags, + mem_flags.with_trap_code(Some(ir::TrapCode::IndirectCallToNull)), funcref_ptr, i32::from(self.env.offsets.ptr.vm_func_ref_type_index()), ); @@ -1205,7 +1203,11 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { } } - self.unchecked_call(sig_ref, funcref_ptr, call_args) + // Note that `funcref_ptr` at this point is guaranteed to not be null as + // we've loaded its type which would have otherwise faulted. That means + // that further loads no longer need trap metadata, so use `None` here + // for the next trap code. + self.unchecked_call(sig_ref, funcref_ptr, None, call_args) } /// Call a typed function reference. @@ -1215,19 +1217,15 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { callee: ir::Value, args: &[ir::Value], ) -> WasmResult { - // Check for whether the callee is null, and trap if so. - // // FIXME: the wasm type system tracks enough information to know whether // `callee` is a null reference or not. In some situations it can be // statically known here that `callee` cannot be null in which case this - // null check can be elided. This requires feeding type information from + // can be `None` instead. This requires feeding type information from // wasmparser's validator into this function, however, which is not // easily done at this time. - self.builder - .ins() - .trapz(callee, ir::TrapCode::NullReference); + let callee_load_trap_code = Some(ir::TrapCode::NullReference); - self.unchecked_call(sig_ref, callee, args) + self.unchecked_call(sig_ref, callee, callee_load_trap_code, args) } /// This calls a function by reference without checking the signature. @@ -1239,15 +1237,22 @@ impl<'a, 'func, 'module_env> Call<'a, 'func, 'module_env> { &mut self, sig_ref: ir::SigRef, callee: ir::Value, + callee_load_trap_code: Option, call_args: &[ir::Value], ) -> WasmResult { let pointer_type = self.env.pointer_type(); // Dereference callee pointer to get the function address. + // + // Note that this may trap if `callee` hasn't previously been verified + // to be non-null. This means that this load is annotated with an + // optional trap code provided by the caller of `unchecked_call` which + // will handle the case where this is either already known to be + // non-null or may trap. let mem_flags = ir::MemFlags::trusted().with_readonly(); let func_addr = self.builder.ins().load( pointer_type, - mem_flags, + mem_flags.with_trap_code(callee_load_trap_code), callee, i32::from(self.env.offsets.ptr.vm_func_ref_wasm_call()), ); diff --git a/crates/wasmtime/src/runtime/store.rs b/crates/wasmtime/src/runtime/store.rs index fa3966264f85..a7835356859e 100644 --- a/crates/wasmtime/src/runtime/store.rs +++ b/crates/wasmtime/src/runtime/store.rs @@ -1595,10 +1595,26 @@ impl StoreOpaque { /// always zero in these situations which means that the trapping context /// doesn't have enough information to report the fault address. pub(crate) fn wasm_fault(&self, pc: usize, addr: usize) -> Option { - // Explicitly bounds-checked memories with spectre-guards enabled will - // cause out-of-bounds accesses to get routed to address 0, so allow - // wasm instructions to fault on the null address. - if addr == 0 { + // There are a few instances where a "close to zero" pointer is loaded + // and we expect that to happen: + // + // * Explicitly bounds-checked memories with spectre-guards enabled will + // cause out-of-bounds accesses to get routed to address 0, so allow + // wasm instructions to fault on the null address. + // * `call_indirect` when invoking a null function pointer may load data + // from the a `VMFuncRef` whose address is null, meaning any field of + // `VMFuncRef` could be the address of the fault. + // + // In these situations where the address is so small it won't be in any + // instance, so skip the checks below. + if addr <= mem::size_of::() { + const _: () = { + // static-assert that `VMFuncRef` isn't too big to ensure that + // it lives solely within the first page as we currently only + // have the guarantee that the first page of memory is unmapped, + // no more. + assert!(mem::size_of::() <= 512); + }; return None; } diff --git a/tests/disas/icall-simd.wat b/tests/disas/icall-simd.wat index b312db0251f5..e6f33895d734 100644 --- a/tests/disas/icall-simd.wat +++ b/tests/disas/icall-simd.wat @@ -43,11 +43,10 @@ ;; @0033 jump block3(v20) ;; ;; block3(v15: i64): -;; @0033 trapz v15, icall_null ;; @0033 v21 = global_value.i64 gv3 ;; @0033 v22 = load.i64 notrap aligned readonly v21+64 ;; @0033 v23 = load.i32 notrap aligned readonly v22 -;; @0033 v24 = load.i32 notrap aligned readonly v15+24 +;; @0033 v24 = load.i32 icall_null aligned readonly v15+24 ;; @0033 v25 = icmp eq v24, v23 ;; @0033 trapz v25, bad_sig ;; @0033 v26 = load.i64 notrap aligned readonly v15+16 diff --git a/tests/disas/icall.wat b/tests/disas/icall.wat index 69e2fe7b1bee..ec6f668c11de 100644 --- a/tests/disas/icall.wat +++ b/tests/disas/icall.wat @@ -43,11 +43,10 @@ ;; @0033 jump block3(v20) ;; ;; block3(v15: i64): -;; @0033 trapz v15, icall_null ;; @0033 v21 = global_value.i64 gv3 ;; @0033 v22 = load.i64 notrap aligned readonly v21+64 ;; @0033 v23 = load.i32 notrap aligned readonly v22 -;; @0033 v24 = load.i32 notrap aligned readonly v15+24 +;; @0033 v24 = load.i32 icall_null aligned readonly v15+24 ;; @0033 v25 = icmp eq v24, v23 ;; @0033 trapz v25, bad_sig ;; @0033 v26 = load.i64 notrap aligned readonly v15+16 diff --git a/tests/disas/typed-funcrefs.wat b/tests/disas/typed-funcrefs.wat index 0e69d7373c2d..78eb51b3bf8f 100644 --- a/tests/disas/typed-funcrefs.wat +++ b/tests/disas/typed-funcrefs.wat @@ -113,22 +113,16 @@ ;; @0036 jump block3(v24) ;; ;; block3(v19: i64): -;; @0038 brif v19, block9, block8 -;; -;; block8 cold: -;; @0038 trap null_reference -;; -;; block9: -;; @0038 v25 = load.i64 notrap aligned readonly v19+16 +;; @0038 v25 = load.i64 null_reference aligned readonly v19+16 ;; @0038 v26 = load.i64 notrap aligned readonly v19+32 ;; @0038 v27 = call_indirect sig1, v25(v26, v0, v2, v3, v4, v5) ;; v80 = iconst.i8 0 -;; @0049 brif v80, block10, block11 ; v80 = 0 +;; @0049 brif v80, block8, block9 ; v80 = 0 ;; -;; block10 cold: +;; block8 cold: ;; @0049 trap table_oob ;; -;; block11: +;; block9: ;; @0049 v38 = load.i64 notrap aligned v0+72 ;; v81 = iconst.i8 0 ;; v78 = iconst.i64 16 @@ -148,13 +142,7 @@ ;; @0049 jump block5(v50) ;; ;; block5(v45: i64): -;; @004b brif v45, block13, block12 -;; -;; block12 cold: -;; @004b trap null_reference -;; -;; block13: -;; @004b v51 = load.i64 notrap aligned readonly v45+16 +;; @004b v51 = load.i64 null_reference aligned readonly v45+16 ;; @004b v52 = load.i64 notrap aligned readonly v45+32 ;; @004b v53 = call_indirect sig1, v51(v52, v0, v2, v3, v4, v5) ;; @0054 jump block1