diff --git a/cranelift/codegen/meta/src/shared/formats.rs b/cranelift/codegen/meta/src/shared/formats.rs index 8c54c19169..e9d5175618 100644 --- a/cranelift/codegen/meta/src/shared/formats.rs +++ b/cranelift/codegen/meta/src/shared/formats.rs @@ -16,6 +16,8 @@ pub(crate) struct Formats { pub(crate) float_compare: Rc, pub(crate) func_addr: Rc, pub(crate) heap_addr: Rc, + pub(crate) heap_load: Rc, + pub(crate) heap_store: Rc, pub(crate) int_compare: Rc, pub(crate) int_compare_imm: Rc, pub(crate) int_add_trap: Rc, @@ -206,6 +208,17 @@ impl Formats { .imm_with_name("size", &imm.uimm8) .build(), + heap_load: Builder::new("HeapLoad").imm(&imm.heap_imm).value().build(), + + heap_store: Builder::new("HeapStore") + // We have more fields for this instruction than + // `InstructionData` can hold without growing in size, so we + // push the immediates out into a side table. + .imm(&imm.heap_imm) + .value() + .value() + .build(), + // Accessing a WebAssembly table. table_addr: Builder::new("TableAddr") .imm(&entities.table) diff --git a/cranelift/codegen/meta/src/shared/immediates.rs b/cranelift/codegen/meta/src/shared/immediates.rs index 6e1e933473..9e9cdb1965 100644 --- a/cranelift/codegen/meta/src/shared/immediates.rs +++ b/cranelift/codegen/meta/src/shared/immediates.rs @@ -59,6 +59,9 @@ pub(crate) struct Immediates { /// Flags for memory operations like `load` and `store`. pub memflags: OperandKind, + /// A reference to out-of-line immediates for heap accesses. + pub heap_imm: OperandKind, + /// A trap code indicating the reason for trapping. /// /// The Rust enum type also has a `User(u16)` variant for user-provided trap codes. @@ -182,6 +185,13 @@ impl Immediates { }, memflags: new_imm("flags", "ir::MemFlags", "Memory operation flags"), + + heap_imm: new_imm( + "heap_imm", + "ir::HeapImm", + "Reference to out-of-line heap access immediates", + ), + trapcode: { let mut trapcode_values = HashMap::new(); trapcode_values.insert("stk_ovf", "StackOverflow"); diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index f6aad71b72..91a3099f50 100755 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -1155,6 +1155,55 @@ pub(crate) fn define( .operands_out(vec![addr]), ); + let heap_imm = &Operand::new("heap_imm", &imm.heap_imm); + let index = + &Operand::new("index", HeapOffset).with_doc("Dynamic index (in bytes) into the heap"); + let a = &Operand::new("a", Mem).with_doc("The value loaded from the heap"); + + ig.push( + Inst::new( + "heap_load", + r#" + Load a value from the given heap at address ``index + offset``, + trapping on out-of-bounds accesses. + + Checks that ``index + offset .. index + offset + sizeof(a)`` is + within the heap's bounds, trapping if it is not. Otherwise, when + that range is in bounds, loads the value from the heap. + + Traps on ``index + offset + sizeof(a)`` overflow. + "#, + &formats.heap_load, + ) + .operands_in(vec![heap_imm, index]) + .operands_out(vec![a]) + .can_load(true) + .can_trap(true), + ); + + let a = &Operand::new("a", Mem).with_doc("The value stored into the heap"); + + ig.push( + Inst::new( + "heap_store", + r#" + Store ``a`` into the given heap at address ``index + offset``, + trapping on out-of-bounds accesses. + + Checks that ``index + offset .. index + offset + sizeof(a)`` is + within the heap's bounds, trapping if it is not. Otherwise, when + that range is in bounds, stores the value into the heap. + + Traps on ``index + offset + sizeof(a)`` overflow. + "#, + &formats.heap_store, + ) + .operands_in(vec![heap_imm, index, a]) + .operands_out(vec![]) + .can_store(true) + .can_trap(true), + ); + // Note this instruction is marked as having other side-effects, so GVN won't try to hoist it, // which would result in it being subject to spilling. While not hoisting would generally hurt // performance, since a computed value used many times may need to be regenerated before each diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index e0818df11e..48eb4fa910 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -4,11 +4,12 @@ use crate::entity::{self, PrimaryMap, SecondaryMap}; use crate::ir; use crate::ir::builder::ReplaceBuilder; use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes}; +use crate::ir::immediates::HeapImmData; use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData}; use crate::ir::{types, ConstantData, ConstantPool, Immediate}; use crate::ir::{ - Block, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, ValueLabelAssignments, - ValueList, ValueListPool, + Block, DynamicType, FuncRef, HeapImm, Inst, SigRef, Signature, Type, Value, + ValueLabelAssignments, ValueList, ValueListPool, }; use crate::ir::{ExtFuncData, RelSourceLoc}; use crate::packed_option::ReservedValue; @@ -83,6 +84,9 @@ pub struct DataFlowGraph { /// Stores large immediates that otherwise will not fit on InstructionData pub immediates: PrimaryMap, + + /// Out-of-line heap access immediates that don't fit in `InstructionData`. + pub heap_imms: PrimaryMap, } impl DataFlowGraph { @@ -101,6 +105,7 @@ impl DataFlowGraph { values_labels: None, constants: ConstantPool::new(), immediates: PrimaryMap::new(), + heap_imms: PrimaryMap::new(), } } diff --git a/cranelift/codegen/src/ir/entities.rs b/cranelift/codegen/src/ir/entities.rs index 315ea2183a..dc63d0ec37 100644 --- a/cranelift/codegen/src/ir/entities.rs +++ b/cranelift/codegen/src/ir/entities.rs @@ -394,6 +394,34 @@ impl Heap { } } +/// An opaque reference to some out-of-line immediates for `heap_{load,store}` +/// instructions. +/// +/// These immediates are too large to store in +/// [`InstructionData`](super::instructions::InstructionData) and therefore must +/// be tracked separately in +/// [`DataFlowGraph::heap_imms`](super::dfg::DataFlowGraph). `HeapImm` provides +/// a way to reference values stored there. +/// +/// While the order is stable, it is arbitrary. +#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] +pub struct HeapImm(u32); +entity_impl!(HeapImm, "heap_imm"); + +impl HeapImm { + /// Create a new `HeapImm` reference from its number. + /// + /// This method is for use by the parser. + pub fn with_number(n: u32) -> Option { + if n < u32::MAX { + Some(Self(n)) + } else { + None + } + } +} + /// An opaque reference to a [WebAssembly /// table](https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format#WebAssembly_tables). /// diff --git a/cranelift/codegen/src/ir/immediates.rs b/cranelift/codegen/src/ir/immediates.rs index 3b3f703235..54f737b19d 100644 --- a/cranelift/codegen/src/ir/immediates.rs +++ b/cranelift/codegen/src/ir/immediates.rs @@ -4,6 +4,7 @@ //! Each type here should have a corresponding definition in the //! `cranelift-codegen/meta/src/shared/immediates` crate in the meta language. +use crate::ir; use alloc::vec::Vec; use core::cmp::Ordering; use core::convert::TryFrom; @@ -1177,6 +1178,18 @@ impl Not for Ieee64 { } } +/// Out-of-line heap access immediates. +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] +pub struct HeapImmData { + /// The memory flags for the heap access. + pub flags: ir::MemFlags, + /// The heap being accessed. + pub heap: ir::Heap, + /// The static offset added to the heap access's index. + pub offset: Uimm32, +} + #[cfg(test)] mod tests { use super::*; diff --git a/cranelift/codegen/src/ir/mod.rs b/cranelift/codegen/src/ir/mod.rs index 52a4b60373..23f952738e 100644 --- a/cranelift/codegen/src/ir/mod.rs +++ b/cranelift/codegen/src/ir/mod.rs @@ -37,8 +37,8 @@ pub use crate::ir::constant::{ConstantData, ConstantPool}; pub use crate::ir::dfg::{DataFlowGraph, ValueDef}; pub use crate::ir::dynamic_type::{dynamic_to_fixed, DynamicTypeData, DynamicTypes}; pub use crate::ir::entities::{ - Block, Constant, DynamicStackSlot, DynamicType, FuncRef, GlobalValue, Heap, Immediate, Inst, - JumpTable, SigRef, StackSlot, Table, UserExternalNameRef, Value, + Block, Constant, DynamicStackSlot, DynamicType, FuncRef, GlobalValue, Heap, HeapImm, Immediate, + Inst, JumpTable, SigRef, StackSlot, Table, UserExternalNameRef, Value, }; pub use crate::ir::extfunc::{ AbiParam, ArgumentExtension, ArgumentPurpose, ExtFuncData, Signature, diff --git a/cranelift/codegen/src/isa/aarch64/lower_inst.rs b/cranelift/codegen/src/isa/aarch64/lower_inst.rs index 1fcf7446b5..433db8407e 100644 --- a/cranelift/codegen/src/isa/aarch64/lower_inst.rs +++ b/cranelift/codegen/src/isa/aarch64/lower_inst.rs @@ -139,8 +139,8 @@ pub(crate) fn lower_insn_to_regs( panic!("Direct stack memory access not supported; should not be used by Wasm"); } - Opcode::HeapAddr => { - panic!("heap_addr should have been removed by legalization!"); + Opcode::HeapLoad | Opcode::HeapStore | Opcode::HeapAddr => { + panic!("heap access instructions should have been removed by legalization!"); } Opcode::TableAddr => { diff --git a/cranelift/codegen/src/isa/s390x/lower.rs b/cranelift/codegen/src/isa/s390x/lower.rs index cfc73c7f66..79765d0175 100644 --- a/cranelift/codegen/src/isa/s390x/lower.rs +++ b/cranelift/codegen/src/isa/s390x/lower.rs @@ -212,8 +212,8 @@ impl LowerBackend for S390xBackend { Opcode::StackLoad | Opcode::StackStore => { panic!("Direct stack memory access not supported; should not be used by Wasm"); } - Opcode::HeapAddr => { - panic!("heap_addr should have been removed by legalization!"); + Opcode::HeapLoad | Opcode::HeapStore | Opcode::HeapAddr => { + panic!("heap access instructions should have been removed by legalization!"); } Opcode::TableAddr => { panic!("table_addr should have been removed by legalization!"); diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index da0fb6efaf..f5ea23c0ac 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -549,8 +549,8 @@ fn lower_insn_to_regs( panic!("global_value should have been removed by legalization!"); } - Opcode::HeapAddr => { - panic!("heap_addr should have been removed by legalization!"); + Opcode::HeapLoad | Opcode::HeapStore | Opcode::HeapAddr => { + panic!("heap access instructions should have been removed by legalization!"); } Opcode::TableAddr => { diff --git a/cranelift/codegen/src/opts.rs b/cranelift/codegen/src/opts.rs index a64f32c0d3..7e77370e29 100644 --- a/cranelift/codegen/src/opts.rs +++ b/cranelift/codegen/src/opts.rs @@ -9,8 +9,8 @@ pub use crate::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64, pub use crate::ir::types::*; pub use crate::ir::{ dynamic_to_fixed, AtomicRmwOp, Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, Heap, - Immediate, InstructionImms, JumpTable, MemFlags, Opcode, StackSlot, Table, TrapCode, Type, - Value, + HeapImm, Immediate, InstructionImms, JumpTable, MemFlags, Opcode, StackSlot, Table, TrapCode, + Type, Value, }; use crate::isle_common_prelude_methods; use crate::machinst::isle::*; diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index d47817e09e..535fe0d99e 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -64,6 +64,7 @@ use crate::entity::SparseSet; use crate::flowgraph::{BlockPredecessor, ControlFlowGraph}; use crate::ir; use crate::ir::entities::AnyEntity; +use crate::ir::immediates::HeapImmData; use crate::ir::instructions::{BranchInfo, CallInfo, InstructionFormat, ResolvedConstraint}; use crate::ir::{ types, ArgumentPurpose, Block, Constant, DynamicStackSlot, FuncRef, Function, GlobalValue, @@ -678,6 +679,10 @@ impl<'a> Verifier<'a> { UnaryGlobalValue { global_value, .. } => { self.verify_global_value(inst, global_value, errors)?; } + HeapLoad { heap_imm, .. } | HeapStore { heap_imm, .. } => { + let HeapImmData { heap, .. } = self.func.dfg.heap_imms[heap_imm]; + self.verify_heap(inst, heap, errors)?; + } HeapAddr { heap, .. } => { self.verify_heap(inst, heap, errors)?; } diff --git a/cranelift/codegen/src/write.rs b/cranelift/codegen/src/write.rs index 725ca46c49..58d73e4ba4 100644 --- a/cranelift/codegen/src/write.rs +++ b/cranelift/codegen/src/write.rs @@ -5,6 +5,7 @@ use crate::entity::SecondaryMap; use crate::ir::entities::AnyEntity; +use crate::ir::immediates::{HeapImmData, Uimm32}; use crate::ir::{Block, DataFlowGraph, Function, Inst, SigRef, Type, Value, ValueDef}; use crate::packed_option::ReservedValue; use alloc::string::{String, ToString}; @@ -476,6 +477,47 @@ pub fn write_operands(w: &mut dyn Write, dfg: &DataFlowGraph, inst: Inst) -> fmt dynamic_stack_slot, .. } => write!(w, " {}, {}", arg, dynamic_stack_slot), + HeapLoad { + opcode: _, + heap_imm, + arg, + } => { + let HeapImmData { + flags, + heap, + offset, + } = dfg.heap_imms[heap_imm]; + write!( + w, + " {heap} {flags} {arg}{optional_offset}", + optional_offset = if offset == Uimm32::from(0) { + "".to_string() + } else { + format!("+{offset}") + } + ) + } + HeapStore { + opcode: _, + heap_imm, + args, + } => { + let HeapImmData { + flags, + heap, + offset, + } = dfg.heap_imms[heap_imm]; + let [index, value] = args; + write!( + w, + " {heap} {flags} {index}{optional_offset}, {value}", + optional_offset = if offset == Uimm32::from(0) { + "".to_string() + } else { + format!("+{offset}") + } + ) + } HeapAddr { heap, arg, diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i32_yes_guards_no_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i32_yes_guards_no_spectre.clif new file mode 100644 index 0000000000..52081f1933 --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i32_yes_guards_no_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i32, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i32 + +block0(v0: i64, v1: i32, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i32, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i32 + fn0 = %do_store(i64, i32, i32) + +block0(v0: i64, v1: i32, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: dynamic, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i32_yes_guards_yes_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i32_yes_guards_yes_spectre.clif new file mode 100644 index 0000000000..52081f1933 --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i32_yes_guards_yes_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i32, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i32 + +block0(v0: i64, v1: i32, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i32, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i32 + fn0 = %do_store(i64, i32, i32) + +block0(v0: i64, v1: i32, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: dynamic, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i64_yes_guards_no_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i64_yes_guards_no_spectre.clif new file mode 100644 index 0000000000..db20c1168f --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i64_yes_guards_no_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i64, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i64 + +block0(v0: i64, v1: i64, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i64, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i64 + fn0 = %do_store(i64, i64, i32) + +block0(v0: i64, v1: i64, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: dynamic, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i64_yes_guards_yes_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i64_yes_guards_yes_spectre.clif new file mode 100644 index 0000000000..db20c1168f --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_dynamic_i64_yes_guards_yes_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i64, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i64 + +block0(v0: i64, v1: i64, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i64, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + gv2 = load.i64 notrap aligned gv0+8 + heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0xffff_ffff, index_type i64 + fn0 = %do_store(i64, i64, i32) + +block0(v0: i64, v1: i64, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: dynamic, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_static_i32_yes_guards_no_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_static_i32_yes_guards_no_spectre.clif new file mode 100644 index 0000000000..42bd0eb49c --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_static_i32_yes_guards_no_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i32, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i32 + +block0(v0: i64, v1: i32, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i32, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i32 + fn0 = %do_store(i64, i32, i32) + +block0(v0: i64, v1: i32, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_static_i32_yes_guards_yes_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_static_i32_yes_guards_yes_spectre.clif new file mode 100644 index 0000000000..42bd0eb49c --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_static_i32_yes_guards_yes_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i32, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i32 + +block0(v0: i64, v1: i32, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i32, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i32 + fn0 = %do_store(i64, i32, i32) + +block0(v0: i64, v1: i32, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_static_i64_yes_guards_no_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_static_i64_yes_guards_no_spectre.clif new file mode 100644 index 0000000000..dcb08f9c9b --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_static_i64_yes_guards_no_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i64, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i64 + +block0(v0: i64, v1: i64, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i64, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i64 + fn0 = %do_store(i64, i64, i32) + +block0(v0: i64, v1: i64, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/heap_load_store_static_i64_yes_guards_yes_spectre.clif b/cranelift/filetests/filetests/runtests/heap_load_store_static_i64_yes_guards_yes_spectre.clif new file mode 100644 index 0000000000..dcb08f9c9b --- /dev/null +++ b/cranelift/filetests/filetests/runtests/heap_load_store_static_i64_yes_guards_yes_spectre.clif @@ -0,0 +1,40 @@ +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=true + +function %do_store(i64 vmctx, i64, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i64 + +block0(v0: i64, v1: i64, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, i64, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + + heap0 = static gv1, min 0x1000, bound 0x1000, offset_guard 0xffff_ffff, index_type i64 + fn0 = %do_store(i64, i64, i32) + +block0(v0: i64, v1: i64, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd diff --git a/cranelift/filetests/filetests/runtests/make-heap-load-store-tests.sh b/cranelift/filetests/filetests/runtests/make-heap-load-store-tests.sh new file mode 100755 index 0000000000..694ee30ae1 --- /dev/null +++ b/cranelift/filetests/filetests/runtests/make-heap-load-store-tests.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash + +# This script generates the `heap_load_store_*.clif` test files. +# +# Usage: +# +# $ ./make-heap-load-store-tests.sh + +set -e +cd $(dirname "$0") + +function generate_one_test() { + local kind=$1 + local index_type=$2 + local guard=$3 + local spectre=$4 + + local enable_spectre=true + if [[ spectre == "no" ]]; then + enable_spectre=false + fi + + local have_guards=yes + if [[ guard == "0" ]]; then + have_guards=no + fi + + local gv2="" + local bound=0x1000 + if [[ $kind == "dynamic" ]]; then + gv2="gv2 = load.i64 notrap aligned gv0+8" + bound=gv2 + fi + + local filename="heap_load_store_${kind}_${index_type}_${have_guards}_guards_${spectre}_spectre.clif" + echo "Generating $filename" + + cat < "$filename" +;; !!! GENERATED BY 'make-heap-load-store-tests.sh' DO NOT EDIT !!! + +test interpret +;; test run +;; target x86_64 +;; target s390x +;; target aarch64 +;; target riscv64 + +set enable_heap_access_spectre_mitigation=${enable_spectre} + +function %do_store(i64 vmctx, ${index_type}, i32) { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + $gv2 + heap0 = ${kind} gv1, min 0x1000, bound ${bound}, offset_guard ${guard}, index_type ${index_type} + +block0(v0: i64, v1: ${index_type}, v2: i32): + heap_store.i32 heap0 little v1+4, v2 + return +} + +function %test(i64 vmctx, ${index_type}, i32) -> i32 { + gv0 = vmctx + gv1 = load.i64 notrap aligned gv0+0 + $gv2 + heap0 = ${kind} gv1, min 0x1000, bound ${bound}, offset_guard ${guard}, index_type ${index_type} + fn0 = %do_store(i64, ${index_type}, i32) + +block0(v0: i64, v1: ${index_type}, v2: i32): + call fn0(v0, v1, v2) + v3 = heap_load.i32 heap0 little v1+4 + return v3 +} +; heap: ${kind}, size=0x1000, ptr=vmctx+0, bound=vmctx+8 +; run: %test(0, 0) == 0 +; run: %test(0, -1) == -1 +; run: %test(16, 1) == 1 +; run: %test(16, -1) == -1 +; run: %test(2049, 0xaabb_ccdd) == 0xaabb_ccdd +EOF +} + +for spectre in yes no; do + for guard in 0 0xffff_ffff; do + for index_type in i32 i64; do + for kind in static dynamic; do + generate_one_test $kind $index_type $guard $spectre + done + done + done +done diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 15c9d9e287..12c268322c 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -6,6 +6,7 @@ use crate::state::{MemoryError, State}; use crate::value::{Value, ValueConversionKind, ValueError, ValueResult}; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; +use cranelift_codegen::ir::immediates::HeapImmData; use cranelift_codegen::ir::{ types, AbiParam, Block, ExternalName, FuncRef, Function, InstructionData, Opcode, TrapCode, Type, Value as ValueRef, @@ -484,6 +485,59 @@ where } Opcode::SymbolValue => unimplemented!("SymbolValue"), Opcode::TlsValue => unimplemented!("TlsValue"), + Opcode::HeapLoad => { + if let InstructionData::HeapLoad { heap_imm, arg, .. } = inst { + let HeapImmData { + flags, + heap, + offset, + } = state.get_current_function().dfg.heap_imms[heap_imm]; + let offset = i128::from(u32::from(offset)); + let addr_ty = state.get_current_function().dfg.value_type(arg); + let index = state.get_value(arg).unwrap().into_int().unwrap(); + let load_ty = inst_context.controlling_type().unwrap(); + let load_size = i128::from(load_ty.bytes()); + assign_or_memtrap(AddressSize::try_from(addr_ty).and_then(|addr_size| { + let heap_address = index.saturating_add(offset).saturating_add(load_size); + let heap_address = + u64::try_from(heap_address).map_err(|e| MemoryError::OutOfBoundsLoad { + addr: Address::try_from(0).unwrap(), + load_size: load_size as usize, + })?; + let address = state.heap_address(addr_size, heap, heap_address)?; + state.checked_load(address, load_ty) + })) + } else { + unreachable!() + } + } + Opcode::HeapStore => { + if let InstructionData::HeapStore { heap_imm, args, .. } = inst { + let HeapImmData { + flags, + heap, + offset, + } = state.get_current_function().dfg.heap_imms[heap_imm]; + let offset = i128::from(u32::from(offset)); + let addr_ty = state.get_current_function().dfg.value_type(args[0]); + let index = state.get_value(args[0]).unwrap().into_int().unwrap(); + let value = state.get_value(args[1]).unwrap(); + let store_ty = state.get_current_function().dfg.value_type(args[1]); + let store_size = i128::from(store_ty.bytes()); + continue_or_memtrap(AddressSize::try_from(addr_ty).and_then(|addr_size| { + let heap_address = index.saturating_add(offset).saturating_add(store_size); + let heap_address = + u64::try_from(heap_address).map_err(|e| MemoryError::OutOfBoundsStore { + addr: Address::try_from(0).unwrap(), + store_size: store_size as usize, + })?; + let address = state.heap_address(addr_size, heap, heap_address)?; + state.checked_store(address, value) + })) + } else { + unreachable!() + } + } Opcode::HeapAddr => { if let InstructionData::HeapAddr { heap, diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index ff85acec72..baf0bc0a93 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -11,7 +11,9 @@ use crate::testfile::{Comment, Details, Feature, TestFile}; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::entity::{EntityRef, PrimaryMap}; use cranelift_codegen::ir::entities::{AnyEntity, DynamicType}; -use cranelift_codegen::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64}; +use cranelift_codegen::ir::immediates::{ + HeapImmData, Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64, +}; use cranelift_codegen::ir::instructions::{InstructionData, InstructionFormat, VariableArgs}; use cranelift_codegen::ir::types::INVALID; use cranelift_codegen::ir::types::*; @@ -876,6 +878,19 @@ impl<'a> Parser<'a> { } } + // Match and consume an optional uimm32 offset immediate. + // + // Note that this will match an empty string as an empty offset, and that if an offset is + // present, it must contain a plus sign. + fn optional_uimm32_offset(&mut self) -> ParseResult { + match self.token() { + Some(Token::Integer(x)) if x.starts_with('+') => { + self.match_uimm32("expected a uimm32 offset immediate") + } + _ => Ok(Uimm32::from(0)), + } + } + // Match and consume a u8 immediate. // This is used for lane numbers in SIMD vectors. fn match_uimm8(&mut self, err_msg: &str) -> ParseResult { @@ -2976,6 +2991,42 @@ impl<'a> Parser<'a> { size, } } + InstructionFormat::HeapLoad => { + let heap = self.match_heap("expected heap identifier")?; + ctx.check_heap(heap, self.loc)?; + let flags = self.optional_memflags(); + let arg = self.match_value("expected SSA value heap index")?; + let offset = self.optional_uimm32_offset()?; + let heap_imm = ctx.function.dfg.heap_imms.push(HeapImmData { + flags, + heap, + offset, + }); + InstructionData::HeapLoad { + opcode, + heap_imm, + arg, + } + } + InstructionFormat::HeapStore => { + let heap = self.match_heap("expected heap identifier")?; + ctx.check_heap(heap, self.loc)?; + let flags = self.optional_memflags(); + let index = self.match_value("expected SSA value heap index")?; + let offset = self.optional_uimm32_offset()?; + self.match_token(Token::Comma, "expected ',' between operands")?; + let value = self.match_value("expected SSA value to store")?; + let heap_imm = ctx.function.dfg.heap_imms.push(HeapImmData { + flags, + heap, + offset, + }); + InstructionData::HeapStore { + opcode, + heap_imm, + args: [index, value], + } + } InstructionFormat::TableAddr => { let table = self.match_table("expected table identifier")?; ctx.check_table(table, self.loc)?;