diff --git a/cranelift/filetests/filetests/wasm/table-get.wat b/cranelift/filetests/filetests/wasm/table-get.wat index 20abed7298c3..13261b9951c1 100644 --- a/cranelift/filetests/filetests/wasm/table-get.wat +++ b/cranelift/filetests/filetests/wasm/table-get.wat @@ -17,30 +17,28 @@ ;; gv0 = vmctx ;; gv1 = load.i64 notrap aligned readonly gv0 ;; gv2 = load.i32 notrap aligned readonly gv0 -;; table0 = dynamic gv1, min 0, bound gv2, element_size 16, index_type i32 ;; ;; block0(v0: i64): +;; v12 -> v0 ;; v13 -> v0 -;; v14 -> v0 ;; @0051 v2 = iconst.i32 0 -;; @0053 v5 = load.i32 notrap aligned readonly v13 -;; @0053 v6 = icmp uge v2, v5 ; v2 = 0 -;; @0053 brif v6, block2, block3 +;; @0053 v3 = load.i32 notrap aligned readonly v12 +;; @0053 v4 = icmp uge v2, v3 ; v2 = 0 +;; @0053 brif v4, block2, block3 ;; ;; block2 cold: ;; @0053 trap table_oob ;; ;; block3: -;; @0053 v7 = uextend.i64 v2 ; v2 = 0 -;; @0053 v8 = load.i64 notrap aligned readonly v14 -;; v15 = iconst.i64 4 -;; @0053 v9 = ishl v7, v15 ; v15 = 4 -;; @0053 v10 = iadd v8, v9 -;; @0053 v11 = icmp.i32 uge v2, v5 ; v2 = 0 -;; @0053 v12 = select_spectre_guard v11, v8, v10 -;; v3 -> v12 -;; @0053 v4 = load.r64 notrap aligned table v3 -;; v1 -> v4 +;; @0053 v5 = uextend.i64 v2 ; v2 = 0 +;; @0053 v6 = load.i64 notrap aligned readonly v13 +;; v14 = iconst.i64 4 +;; @0053 v7 = ishl v5, v14 ; v14 = 4 +;; @0053 v8 = iadd v6, v7 +;; @0053 v9 = icmp.i32 uge v2, v3 ; v2 = 0 +;; @0053 v10 = select_spectre_guard v9, v6, v8 +;; @0053 v11 = load.r64 notrap aligned table v10 +;; v1 -> v11 ;; @0055 jump block1 ;; ;; block1: @@ -51,29 +49,27 @@ ;; gv0 = vmctx ;; gv1 = load.i64 notrap aligned readonly gv0 ;; gv2 = load.i32 notrap aligned readonly gv0 -;; table0 = dynamic gv1, min 0, bound gv2, element_size 16, index_type i32 ;; ;; block0(v0: i32, v1: i64): +;; v12 -> v1 ;; v13 -> v1 -;; v14 -> v1 -;; @005a v5 = load.i32 notrap aligned readonly v13 -;; @005a v6 = icmp uge v0, v5 -;; @005a brif v6, block2, block3 +;; @005a v3 = load.i32 notrap aligned readonly v12 +;; @005a v4 = icmp uge v0, v3 +;; @005a brif v4, block2, block3 ;; ;; block2 cold: ;; @005a trap table_oob ;; ;; block3: -;; @005a v7 = uextend.i64 v0 -;; @005a v8 = load.i64 notrap aligned readonly v14 -;; v15 = iconst.i64 4 -;; @005a v9 = ishl v7, v15 ; v15 = 4 -;; @005a v10 = iadd v8, v9 -;; @005a v11 = icmp.i32 uge v0, v5 -;; @005a v12 = select_spectre_guard v11, v8, v10 -;; v3 -> v12 -;; @005a v4 = load.r64 notrap aligned table v3 -;; v2 -> v4 +;; @005a v5 = uextend.i64 v0 +;; @005a v6 = load.i64 notrap aligned readonly v13 +;; v14 = iconst.i64 4 +;; @005a v7 = ishl v5, v14 ; v14 = 4 +;; @005a v8 = iadd v6, v7 +;; @005a v9 = icmp.i32 uge v0, v3 +;; @005a v10 = select_spectre_guard v9, v6, v8 +;; @005a v11 = load.r64 notrap aligned table v10 +;; v2 -> v11 ;; @005c jump block1 ;; ;; block1: diff --git a/cranelift/filetests/filetests/wasm/table-set.wat b/cranelift/filetests/filetests/wasm/table-set.wat index 650dfc04adbc..a1ef1f18dbd8 100644 --- a/cranelift/filetests/filetests/wasm/table-set.wat +++ b/cranelift/filetests/filetests/wasm/table-set.wat @@ -19,29 +19,27 @@ ;; gv0 = vmctx ;; gv1 = load.i64 notrap aligned readonly gv0 ;; gv2 = load.i32 notrap aligned readonly gv0 -;; table0 = dynamic gv1, min 0, bound gv2, element_size 16, index_type i32 ;; ;; block0(v0: r64, v1: i64): +;; v11 -> v1 ;; v12 -> v1 -;; v13 -> v1 ;; @0051 v2 = iconst.i32 0 -;; @0055 v4 = load.i32 notrap aligned readonly v12 -;; @0055 v5 = icmp uge v2, v4 ; v2 = 0 -;; @0055 brif v5, block2, block3 +;; @0055 v3 = load.i32 notrap aligned readonly v11 +;; @0055 v4 = icmp uge v2, v3 ; v2 = 0 +;; @0055 brif v4, block2, block3 ;; ;; block2 cold: ;; @0055 trap table_oob ;; ;; block3: -;; @0055 v6 = uextend.i64 v2 ; v2 = 0 -;; @0055 v7 = load.i64 notrap aligned readonly v13 -;; v14 = iconst.i64 4 -;; @0055 v8 = ishl v6, v14 ; v14 = 4 -;; @0055 v9 = iadd v7, v8 -;; @0055 v10 = icmp.i32 uge v2, v4 ; v2 = 0 -;; @0055 v11 = select_spectre_guard v10, v7, v9 -;; v3 -> v11 -;; @0055 store.r64 notrap aligned table v0, v3 +;; @0055 v5 = uextend.i64 v2 ; v2 = 0 +;; @0055 v6 = load.i64 notrap aligned readonly v12 +;; v13 = iconst.i64 4 +;; @0055 v7 = ishl v5, v13 ; v13 = 4 +;; @0055 v8 = iadd v6, v7 +;; @0055 v9 = icmp.i32 uge v2, v3 ; v2 = 0 +;; @0055 v10 = select_spectre_guard v9, v6, v8 +;; @0055 store.r64 notrap aligned table v0, v10 ;; @0057 jump block1 ;; ;; block1: @@ -52,28 +50,26 @@ ;; gv0 = vmctx ;; gv1 = load.i64 notrap aligned readonly gv0 ;; gv2 = load.i32 notrap aligned readonly gv0 -;; table0 = dynamic gv1, min 0, bound gv2, element_size 16, index_type i32 ;; ;; block0(v0: i32, v1: r64, v2: i64): +;; v11 -> v2 ;; v12 -> v2 -;; v13 -> v2 -;; @005e v4 = load.i32 notrap aligned readonly v12 -;; @005e v5 = icmp uge v0, v4 -;; @005e brif v5, block2, block3 +;; @005e v3 = load.i32 notrap aligned readonly v11 +;; @005e v4 = icmp uge v0, v3 +;; @005e brif v4, block2, block3 ;; ;; block2 cold: ;; @005e trap table_oob ;; ;; block3: -;; @005e v6 = uextend.i64 v0 -;; @005e v7 = load.i64 notrap aligned readonly v13 -;; v14 = iconst.i64 4 -;; @005e v8 = ishl v6, v14 ; v14 = 4 -;; @005e v9 = iadd v7, v8 -;; @005e v10 = icmp.i32 uge v0, v4 -;; @005e v11 = select_spectre_guard v10, v7, v9 -;; v3 -> v11 -;; @005e store.r64 notrap aligned table v1, v3 +;; @005e v5 = uextend.i64 v0 +;; @005e v6 = load.i64 notrap aligned readonly v12 +;; v13 = iconst.i64 4 +;; @005e v7 = ishl v5, v13 ; v13 = 4 +;; @005e v8 = iadd v6, v7 +;; @005e v9 = icmp.i32 uge v0, v3 +;; @005e v10 = select_spectre_guard v9, v6, v8 +;; @005e store.r64 notrap aligned table v1, v10 ;; @0060 jump block1 ;; ;; block1: diff --git a/cranelift/wasm/src/environ/dummy.rs b/cranelift/wasm/src/environ/dummy.rs index a381ce33c451..c2f4d7ee4c35 100644 --- a/cranelift/wasm/src/environ/dummy.rs +++ b/cranelift/wasm/src/environ/dummy.rs @@ -8,18 +8,17 @@ use crate::environ::{FuncEnvironment, GlobalVariable, ModuleEnvironment, TargetEnvironment}; use crate::func_translator::FuncTranslator; use crate::state::FuncTranslationState; -use crate::WasmValType; use crate::{ DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex, GlobalInit, Heap, HeapData, HeapStyle, Memory, MemoryIndex, Table, TableIndex, TypeConvert, TypeIndex, WasmFuncType, WasmHeapType, WasmResult, }; +use crate::{TableData, WasmValType}; use cranelift_codegen::cursor::FuncCursor; -use cranelift_codegen::ir::immediates::{Offset32, Uimm64}; +use cranelift_codegen::ir::immediates::Offset32; use cranelift_codegen::ir::{self, InstBuilder}; use cranelift_codegen::ir::{types::*, UserFuncName}; use cranelift_codegen::isa::{CallConv, TargetFrontendConfig}; -use cranelift_entity::packed_option::ReservedValue; use cranelift_entity::{EntityRef, PrimaryMap, SecondaryMap}; use cranelift_frontend::FunctionBuilder; use std::boxed::Box; @@ -214,7 +213,7 @@ pub struct DummyFuncEnvironment<'dummy_environment> { pub heaps: PrimaryMap, /// Cranelift tables we have created to implement Wasm tables. - tables: SecondaryMap, + tables: SecondaryMap>, } impl<'dummy_environment> DummyFuncEnvironment<'dummy_environment> { @@ -227,7 +226,7 @@ impl<'dummy_environment> DummyFuncEnvironment<'dummy_environment> { mod_info, expected_reachability, heaps: Default::default(), - tables: SecondaryMap::with_default(ir::Table::reserved_value()), + tables: Default::default(), } } @@ -251,7 +250,7 @@ impl<'dummy_environment> DummyFuncEnvironment<'dummy_environment> { } fn ensure_table_exists(&mut self, func: &mut ir::Function, index: TableIndex) { - if !self.tables[index].is_reserved_value() { + if self.tables[index].is_some() { return; } @@ -271,12 +270,10 @@ impl<'dummy_environment> DummyFuncEnvironment<'dummy_environment> { flags: ir::MemFlags::trusted().with_readonly(), }); - self.tables[index] = func.create_table(ir::TableData { + self.tables[index] = Some(TableData { base_gv, - min_size: Uimm64::new(0), bound_gv, - element_size: Uimm64::from(u64::from(self.pointer_bytes()) * 2), - index_type: I32, + element_size: u32::from(self.pointer_bytes()) * 2, }); } } @@ -603,8 +600,8 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ ) -> WasmResult { let pointer_type = self.pointer_type(); self.ensure_table_exists(builder.func, table_index); - let table = self.tables[table_index]; - let table_entry_addr = builder.ins().table_addr(pointer_type, table, index, 0); + let table = self.tables[table_index].as_ref().unwrap(); + let table_entry_addr = table.prepare_table_addr(builder, index, pointer_type, true); let flags = ir::MemFlags::trusted().with_table(); let value = builder .ins() @@ -621,8 +618,8 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ ) -> WasmResult<()> { let pointer_type = self.pointer_type(); self.ensure_table_exists(builder.func, table_index); - let table = self.tables[table_index]; - let table_entry_addr = builder.ins().table_addr(pointer_type, table, index, 0); + let table = self.tables[table_index].as_ref().unwrap(); + let table_entry_addr = table.prepare_table_addr(builder, index, pointer_type, true); let flags = ir::MemFlags::trusted().with_table(); builder.ins().store(flags, value, table_entry_addr, 0); Ok(()) diff --git a/cranelift/wasm/src/lib.rs b/cranelift/wasm/src/lib.rs index 63954a660f85..b2aa934edcee 100644 --- a/cranelift/wasm/src/lib.rs +++ b/cranelift/wasm/src/lib.rs @@ -39,6 +39,7 @@ mod heap; mod module_translator; mod sections_translator; mod state; +mod table; mod translation_utils; pub use crate::environ::{ @@ -49,6 +50,7 @@ pub use crate::func_translator::FuncTranslator; pub use crate::heap::{Heap, HeapData, HeapStyle}; pub use crate::module_translator::translate_module; pub use crate::state::FuncTranslationState; +pub use crate::table::TableData; pub use crate::translation_utils::*; pub use cranelift_frontend::FunctionBuilder; pub use wasmtime_types::*; diff --git a/cranelift/wasm/src/table.rs b/cranelift/wasm/src/table.rs new file mode 100644 index 000000000000..f52070773c8b --- /dev/null +++ b/cranelift/wasm/src/table.rs @@ -0,0 +1,77 @@ +use cranelift_codegen::ir::{self, condcodes::IntCC, InstBuilder}; +use cranelift_frontend::FunctionBuilder; + +/// An implementation of a WebAssembly table. +#[derive(Clone)] +pub struct TableData { + /// Global value giving the address of the start of the table. + pub base_gv: ir::GlobalValue, + + /// Global value giving the current bound of the table, in elements. + pub bound_gv: ir::GlobalValue, + + /// The size of a table element, in bytes. + pub element_size: u32, +} + +impl TableData { + /// Return a CLIF value containing a native pointer to the beginning of the + /// given index within this table. + pub fn prepare_table_addr( + &self, + pos: &mut FunctionBuilder, + mut index: ir::Value, + addr_ty: ir::Type, + enable_table_access_spectre_mitigation: bool, + ) -> ir::Value { + let index_ty = pos.func.dfg.value_type(index); + + // Start with the bounds check. Trap if `index + 1 > bound`. + let bound = pos.ins().global_value(index_ty, self.bound_gv); + + // `index > bound - 1` is the same as `index >= bound`. + let oob = pos + .ins() + .icmp(IntCC::UnsignedGreaterThanOrEqual, index, bound); + pos.ins().trapnz(oob, ir::TrapCode::TableOutOfBounds); + + // If Spectre mitigations are enabled, we will use a comparison to + // short-circuit the computed table element address to the start + // of the table on the misspeculation path when out-of-bounds. + let spectre_oob_cmp = if enable_table_access_spectre_mitigation { + Some((index, bound)) + } else { + None + }; + + // Convert `index` to `addr_ty`. + if index_ty != addr_ty { + index = pos.ins().uextend(addr_ty, index); + } + + // Add the table base address base + let base = pos.ins().global_value(addr_ty, self.base_gv); + + let element_size = self.element_size; + let offset = if element_size == 1 { + index + } else if element_size.is_power_of_two() { + pos.ins() + .ishl_imm(index, i64::from(element_size.trailing_zeros())) + } else { + pos.ins().imul_imm(index, element_size as i64) + }; + + let element_addr = pos.ins().iadd(base, offset); + + if let Some((index, bound)) = spectre_oob_cmp { + let cond = pos + .ins() + .icmp(IntCC::UnsignedGreaterThanOrEqual, index, bound); + // If out-of-bounds, choose the table base on the misspeculation path. + pos.ins().select_spectre_guard(cond, base, element_addr) + } else { + element_addr + } + } +} diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs index 73576ee57929..46c11c8766c1 100644 --- a/crates/cranelift/src/func_environ.rs +++ b/crates/cranelift/src/func_environ.rs @@ -2,21 +2,20 @@ use cfg_if::cfg_if; use cranelift_codegen::cursor::FuncCursor; use cranelift_codegen::ir; use cranelift_codegen::ir::condcodes::*; -use cranelift_codegen::ir::immediates::{Imm64, Offset32, Uimm64}; +use cranelift_codegen::ir::immediates::{Imm64, Offset32}; use cranelift_codegen::ir::pcc::Fact; use cranelift_codegen::ir::types::*; use cranelift_codegen::ir::{ AbiParam, ArgumentPurpose, Function, InstBuilder, MemFlags, Signature, UserFuncName, Value, }; use cranelift_codegen::isa::{self, CallConv, TargetFrontendConfig, TargetIsa}; -use cranelift_entity::packed_option::ReservedValue; use cranelift_entity::{EntityRef, PrimaryMap, SecondaryMap}; use cranelift_frontend::FunctionBuilder; use cranelift_frontend::Variable; use cranelift_wasm::{ FuncIndex, FuncTranslationState, GlobalIndex, GlobalVariable, Heap, HeapData, HeapStyle, - MemoryIndex, TableIndex, TargetEnvironment, TypeIndex, WasmHeapType, WasmRefType, WasmResult, - WasmValType, + MemoryIndex, TableData, TableIndex, TargetEnvironment, TypeIndex, WasmHeapType, WasmRefType, + WasmResult, WasmValType, }; use std::mem; use wasmparser::Operator; @@ -137,7 +136,7 @@ pub struct FuncEnvironment<'module_environment> { heaps: PrimaryMap, /// Cranelift tables we have created to implement Wasm tables. - tables: SecondaryMap, + tables: SecondaryMap>, /// The Cranelift global holding the vmctx address. vmctx: Option, @@ -212,7 +211,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { module: &translation.module, types, heaps: PrimaryMap::default(), - tables: SecondaryMap::with_default(ir::Table::reserved_value()), + tables: SecondaryMap::default(), vmctx: None, pcc_vmctx_memtype: None, builtin_function_signatures, @@ -875,7 +874,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { /// /// The index space covers both imported and locally declared tables. fn ensure_table_exists(&mut self, func: &mut ir::Function, index: TableIndex) { - if !self.tables[index].is_reserved_value() { + if self.tables[index].is_some() { return; } @@ -923,17 +922,14 @@ impl<'module_environment> FuncEnvironment<'module_environment> { flags: MemFlags::trusted(), }); - let element_size = u64::from( - self.reference_type(self.module.table_plans[index].table.wasm_ty.heap_type) - .bytes(), - ); + let element_size = self + .reference_type(self.module.table_plans[index].table.wasm_ty.heap_type) + .bytes(); - self.tables[index] = func.create_table(ir::TableData { + self.tables[index] = Some(TableData { base_gv, - min_size: Uimm64::new(0), bound_gv, - element_size: Uimm64::new(element_size), - index_type: I32, + element_size, }); } @@ -945,13 +941,18 @@ impl<'module_environment> FuncEnvironment<'module_environment> { ) -> ir::Value { let pointer_type = self.pointer_type(); self.ensure_table_exists(builder.func, table_index); - let table = self.tables[table_index]; + let table_data = self.tables[table_index].as_ref().unwrap(); // To support lazy initialization of table // contents, we check for a null entry here, and // if null, we take a slow-path that invokes a // libcall. - let table_entry_addr = builder.ins().table_addr(pointer_type, table, index, 0); + let table_entry_addr = table_data.prepare_table_addr( + builder, + index, + pointer_type, + self.isa.flags().enable_table_access_spectre_mitigation(), + ); let flags = ir::MemFlags::trusted().with_table(); let value = builder.ins().load(pointer_type, flags, table_entry_addr, 0); // Mask off the "initialized bit". See documentation on @@ -1430,8 +1431,13 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m // Load the table element. self.ensure_table_exists(builder.func, table_index); - let table = self.tables[table_index]; - let elem_addr = builder.ins().table_addr(pointer_type, table, index, 0); + let table_data = self.tables[table_index].as_ref().unwrap(); + let elem_addr = table_data.prepare_table_addr( + builder, + index, + pointer_type, + self.isa.flags().enable_table_access_spectre_mitigation(), + ); let flags = ir::MemFlags::trusted().with_table(); let elem = builder.ins().load(reference_type, flags, elem_addr, 0); @@ -1532,26 +1538,31 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m let pointer_type = self.pointer_type(); let plan = &self.module.table_plans[table_index]; self.ensure_table_exists(builder.func, table_index); - let table = self.tables[table_index]; + let table_data = self.tables[table_index].as_ref().unwrap(); match plan.table.wasm_ty.heap_type { - WasmHeapType::Func | WasmHeapType::Concrete(_) | WasmHeapType::NoFunc => match plan - .style - { - TableStyle::CallerChecksSignature => { - let table_entry_addr = builder.ins().table_addr(pointer_type, table, index, 0); - // Set the "initialized bit". See doc-comment on - // `FUNCREF_INIT_BIT` in - // crates/environ/src/ref_bits.rs for details. - let value_with_init_bit = builder - .ins() - .bor_imm(value, Imm64::from(FUNCREF_INIT_BIT as i64)); - let flags = ir::MemFlags::trusted().with_table(); - builder - .ins() - .store(flags, value_with_init_bit, table_entry_addr, 0); - Ok(()) + WasmHeapType::Func | WasmHeapType::Concrete(_) | WasmHeapType::NoFunc => { + match plan.style { + TableStyle::CallerChecksSignature => { + let table_entry_addr = table_data.prepare_table_addr( + builder, + index, + pointer_type, + self.isa.flags().enable_table_access_spectre_mitigation(), + ); + // Set the "initialized bit". See doc-comment on + // `FUNCREF_INIT_BIT` in + // crates/environ/src/ref_bits.rs for details. + let value_with_init_bit = builder + .ins() + .bor_imm(value, Imm64::from(FUNCREF_INIT_BIT as i64)); + let flags = ir::MemFlags::trusted().with_table(); + builder + .ins() + .store(flags, value_with_init_bit, table_entry_addr, 0); + Ok(()) + } } - }, + } #[cfg(feature = "gc")] WasmHeapType::Extern => { @@ -1600,7 +1611,12 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m // bounds checks. This is the first thing we do, because we // don't want to modify any ref counts if this `table.set` is // going to trap. - let table_entry_addr = builder.ins().table_addr(pointer_type, table, index, 0); + let table_entry_addr = table_data.prepare_table_addr( + builder, + index, + pointer_type, + self.isa.flags().enable_table_access_spectre_mitigation(), + ); // If value is not null, increment `value`'s ref count. // @@ -2482,9 +2498,8 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m table_index: TableIndex, ) -> WasmResult { self.ensure_table_exists(pos.func, table_index); - let table = self.tables[table_index]; - let size_gv = pos.func.tables[table].bound_gv; - Ok(pos.ins().global_value(ir::types::I32, size_gv)) + let table_data = self.tables[table_index].as_ref().unwrap(); + Ok(pos.ins().global_value(ir::types::I32, table_data.bound_gv)) } fn translate_table_copy(