From 3f5800ac541262e5d0e40e3b93170c50118ec323 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 16:32:08 -0700 Subject: [PATCH 1/7] Bump the versions --- Cargo.lock | 96 ++++++++++++++++------------ lib/compiler-cranelift/Cargo.toml | 8 +-- lib/compiler-cranelift/src/config.rs | 2 +- 3 files changed, 59 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c567a71b40..de99ceb57e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -769,29 +769,28 @@ dependencies = [ [[package]] name = "cranelift-bforest" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a2ab4512dfd3a6f4be184403a195f76e81a8a9f9e6c898e19d2dc3ce20e0115" +checksum = "2b5bb9245ec7dcc04d03110e538d31f0969d301c9d673145f4b4d5c3478539a3" dependencies = [ "cranelift-entity", ] [[package]] name = "cranelift-codegen" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98b022ed2a5913a38839dfbafe6cf135342661293b08049843362df4301261dc" +checksum = "ebb18d10e5ddac43ba4ca8fd4e310938569c3e484cc01b6372b27dc5bb4dfd28" dependencies = [ - "arrayvec 0.7.4", "bumpalo", "cranelift-bforest", "cranelift-codegen-meta", "cranelift-codegen-shared", - "cranelift-egraph", + "cranelift-control", "cranelift-entity", "cranelift-isle", - "gimli 0.26.2", - "hashbrown 0.12.3", + "gimli 0.28.0", + "hashbrown 0.14.2", "log", "regalloc2", "smallvec", @@ -800,47 +799,42 @@ dependencies = [ [[package]] name = "cranelift-codegen-meta" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "639307b45434ad112a98f8300c0f0ab085cbefcd767efcdef9ef19d4c0756e74" +checksum = "7a3ce6d22982c1b9b6b012654258bab1a13947bb12703518bef06b1a4867c3d6" dependencies = [ "cranelift-codegen-shared", ] [[package]] name = "cranelift-codegen-shared" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "278e52e29c53fcf32431ef08406c295699a70306d05a0715c5b1bf50e33a9ab7" +checksum = "47220fd4f9a0ce23541652b6f16f83868d282602c600d14934b2a4c166b4bd80" [[package]] -name = "cranelift-egraph" -version = "0.91.1" +name = "cranelift-control" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "624b54323b06e675293939311943ba82d323bb340468ce1889be5da7932c8d73" +checksum = "ed5a4c42672aea9b6e820046b52e47a1c05d3394a6cdf4cb3c3c4b702f954bd2" dependencies = [ - "cranelift-entity", - "fxhash", - "hashbrown 0.12.3", - "indexmap 1.9.3", - "log", - "smallvec", + "arbitrary", ] [[package]] name = "cranelift-entity" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a59bcbca89c3f1b70b93ab3cbba5e5e0cbf3e63dadb23c7525cb142e21a9d4c" +checksum = "0b4e9a3296fc827f9d35135dc2c0c8dd8d8359eb1ef904bae2d55d5bcb0c9f94" [[package]] name = "cranelift-frontend" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d70abacb8cfef3dc8ff7e8836e9c1d70f7967dfdac824a4cd5e30223415aca6" +checksum = "33ec537d0f0b8e084517f3e7bfa1d89af343d7c7df455573fca9f272d4e01267" dependencies = [ "cranelift-codegen", - "hashbrown 0.12.3", + "hashbrown 0.14.2", "log", "smallvec", "target-lexicon 0.12.12", @@ -848,9 +842,9 @@ dependencies = [ [[package]] name = "cranelift-isle" -version = "0.91.1" +version = "0.101.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "393bc73c451830ff8dbb3a07f61843d6cb41a084f9996319917c0b291ed785bb" +checksum = "45bab6d69919d210a50331d35cc6ce111567bc040aebac63a8ae130d0400a075" [[package]] name = "crc" @@ -1476,6 +1470,12 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4443176a9f2c162692bd3d352d745ef9413eec5782a80d8fd6f8a1ac692a07f7" +[[package]] +name = "fallible-iterator" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" + [[package]] name = "fallible-streaming-iterator" version = "0.1.9" @@ -1676,15 +1676,6 @@ dependencies = [ "slab", ] -[[package]] -name = "fxhash" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c31b6d751ae2c7f11320402d34e41349dd1016f8d5d45e48c4312bc8625af50c" -dependencies = [ - "byteorder", -] - [[package]] name = "generic-array" version = "0.14.7" @@ -1734,7 +1725,7 @@ version = "0.26.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22030e2c5a68ec659fde1e949a745124b48e6fa8b045b7ed5bd1fe4ccc5c4e5d" dependencies = [ - "fallible-iterator", + "fallible-iterator 0.2.0", "indexmap 1.9.3", "stable_deref_trait", ] @@ -1744,6 +1735,11 @@ name = "gimli" version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6fb8d784f27acf97159b40fc4db5ecd8aa23b9ad5ef69cdd136d3bc80665f0c0" +dependencies = [ + "fallible-iterator 0.3.0", + "indexmap 2.0.2", + "stable_deref_trait", +] [[package]] name = "glob" @@ -1887,6 +1883,15 @@ dependencies = [ "ahash 0.7.7", ] +[[package]] +name = "hashbrown" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43a3c133739dddd0d2990f9a4bdf8eb4b21ef50e4851ca85ab661199821d510e" +dependencies = [ + "ahash 0.8.6", +] + [[package]] name = "hashbrown" version = "0.14.2" @@ -3409,12 +3414,13 @@ checksum = "a0d51660a68078997855ba5602f73ab3a5031bd7ad480a9d4c90fbbf04e1fff0" [[package]] name = "regalloc2" -version = "0.5.1" +version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "300d4fbfb40c1c66a78ba3ddd41c1110247cf52f97b87d0f2fc9209bd49b030c" +checksum = "ad156d539c879b7a24a363a2016d77961786e71f48f2e2fc8302a92abd2429a6" dependencies = [ - "fxhash", + "hashbrown 0.13.2", "log", + "rustc-hash", "slice-group-by", "smallvec", ] @@ -3630,7 +3636,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "01e213bc3ecb39ac32e81e51ebe31fd888a940515173e3a18a35f8c6e896422a" dependencies = [ "bitflags 1.3.2", - "fallible-iterator", + "fallible-iterator 0.2.0", "fallible-streaming-iterator", "hashlink", "libsqlite3-sys", @@ -3643,6 +3649,12 @@ version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "rustc_version" version = "0.3.3" diff --git a/lib/compiler-cranelift/Cargo.toml b/lib/compiler-cranelift/Cargo.toml index b0e62de63fa..b56fa9fc26f 100644 --- a/lib/compiler-cranelift/Cargo.toml +++ b/lib/compiler-cranelift/Cargo.toml @@ -16,9 +16,9 @@ version.workspace = true [dependencies] wasmer-compiler = { path = "../compiler", version = "=4.2.3", features = ["translator", "compiler"], default-features = false } wasmer-types = { path = "../types", version = "=4.2.3", default-features = false, features = ["std"] } -cranelift-entity = { version = "0.91.1", default-features = false } -cranelift-codegen = { version = "0.91.1", default-features = false, features = ["x86", "arm64", "riscv64"] } -cranelift-frontend = { version = "0.91.1", default-features = false } +cranelift-entity = { version = "0.101.4", default-features = false } +cranelift-codegen = { version = "0.101.4", default-features = false, features = ["x86", "arm64", "riscv64"] } +cranelift-frontend = { version = "0.101.4", default-features = false } tracing = "0.1" hashbrown = { version = "0.11", optional = true } rayon = { version = "1.5", optional = true } @@ -28,7 +28,7 @@ smallvec = "1.6" target-lexicon = { version = "0.12.2", default-features = false } [dev-dependencies] -cranelift-codegen = { version = "0.91.1", features = ["all-arch"] } +cranelift-codegen = { version = "0.101.4", features = ["all-arch"] } lazy_static = "1.4" [badges] diff --git a/lib/compiler-cranelift/src/config.rs b/lib/compiler-cranelift/src/config.rs index 0ce14a4cebe..bf5cc5d0bc6 100644 --- a/lib/compiler-cranelift/src/config.rs +++ b/lib/compiler-cranelift/src/config.rs @@ -66,7 +66,7 @@ impl Cranelift { } /// Generates the ISA for the provided target - pub fn isa(&self, target: &Target) -> CodegenResult> { + pub fn isa(&self, target: &Target) -> CodegenResult> { let mut builder = lookup(target.triple().clone()).expect("construct Cranelift ISA for triple"); // Cpu Features From bb235be5d09bc2474f5977483fe47919b4c07d6c Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 17:11:28 -0700 Subject: [PATCH 2/7] Fixed most heap issues --- Cargo.lock | 2 +- lib/compiler-cranelift/Cargo.toml | 2 +- lib/compiler-cranelift/src/compiler.rs | 21 ++-- lib/compiler-cranelift/src/func_environ.rs | 29 ++--- lib/compiler-cranelift/src/heap.rs | 108 ++++++++++++++++++ lib/compiler-cranelift/src/lib.rs | 1 + .../src/translator/code_translator.rs | 3 +- .../src/translator/func_environ.rs | 19 +-- .../src/translator/func_state.rs | 5 +- 9 files changed, 155 insertions(+), 35 deletions(-) create mode 100644 lib/compiler-cranelift/src/heap.rs diff --git a/Cargo.lock b/Cargo.lock index de99ceb57e7..58b3a53173b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5804,7 +5804,7 @@ dependencies = [ "cranelift-codegen", "cranelift-entity", "cranelift-frontend", - "gimli 0.26.2", + "gimli 0.28.0", "hashbrown 0.11.2", "lazy_static", "more-asserts", diff --git a/lib/compiler-cranelift/Cargo.toml b/lib/compiler-cranelift/Cargo.toml index b56fa9fc26f..cde58e839b2 100644 --- a/lib/compiler-cranelift/Cargo.toml +++ b/lib/compiler-cranelift/Cargo.toml @@ -23,7 +23,7 @@ tracing = "0.1" hashbrown = { version = "0.11", optional = true } rayon = { version = "1.5", optional = true } more-asserts = "0.2" -gimli = { version = "0.26", optional = true } +gimli = { version = "0.28.0", optional = true } smallvec = "1.6" target-lexicon = { version = "0.12.2", default-features = false } diff --git a/lib/compiler-cranelift/src/compiler.rs b/lib/compiler-cranelift/src/compiler.rs index b2a880f6953..478ac759f49 100644 --- a/lib/compiler-cranelift/src/compiler.rs +++ b/lib/compiler-cranelift/src/compiler.rs @@ -13,7 +13,7 @@ use crate::translator::{ signature_to_cranelift_ir, CraneliftUnwindInfo, FuncTranslator, }; use cranelift_codegen::ir::{ExternalName, UserFuncName}; -use cranelift_codegen::{ir, MachReloc}; +use cranelift_codegen::{ir, FinalizedMachReloc, FinalizedRelocTarget}; use cranelift_codegen::{Context, MachTrap}; #[cfg(feature = "unwind")] use gimli::write::{Address, EhFrame, FrameTable}; @@ -415,24 +415,30 @@ impl Compiler for CraneliftCompiler { } } -fn mach_reloc_to_reloc(module: &ModuleInfo, reloc: &MachReloc) -> Relocation { - let &MachReloc { +fn mach_reloc_to_reloc(module: &ModuleInfo, reloc: &FinalizedMachReloc) -> Relocation { + let &FinalizedMachReloc { offset, kind, - ref name, addend, + target, } = reloc; - let reloc_target = if let ExternalName::User(extname_ref) = *name { + let name = match target { + FinalizedRelocTarget::ExternalName(external_name) => external_name, + FinalizedRelocTarget::Func(CodeOffset) => { + unimplemented!("relocations to offset in the same function are not yet supported") + } + }; + let reloc_target: RelocationTarget = if let ExternalName::User(extname_ref) = name { //debug_assert_eq!(namespace, 0); RelocationTarget::LocalFunc( module .local_func_index(FunctionIndex::from_u32(extname_ref.as_u32())) .expect("The provided function should be local"), ) - } else if let ExternalName::LibCall(libcall) = *name { + } else if let ExternalName::LibCall(libcall) = name { RelocationTarget::LibCall(irlibcall_to_libcall(libcall)) } else { - panic!("unrecognized external name") + panic!("unrecognized external target") }; Relocation { kind: irreloc_to_relocationkind(kind), @@ -464,6 +470,7 @@ fn translate_ir_trapcode(trap: ir::TrapCode) -> TrapCode { ir::TrapCode::BadConversionToInteger => TrapCode::BadConversionToInteger, ir::TrapCode::UnreachableCodeReached => TrapCode::UnreachableCodeReached, ir::TrapCode::Interrupt => unimplemented!("Interrupts not supported"), + ir::TrapCode::NullReference => unimplemented!("Null reference not supported"), ir::TrapCode::User(_user_code) => unimplemented!("User trap code not supported"), // ir::TrapCode::Interrupt => TrapCode::Interrupt, // ir::TrapCode::User(user_code) => TrapCode::User(user_code), diff --git a/lib/compiler-cranelift/src/func_environ.rs b/lib/compiler-cranelift/src/func_environ.rs index 98f371cca3c..251f9ea0b36 100644 --- a/lib/compiler-cranelift/src/func_environ.rs +++ b/lib/compiler-cranelift/src/func_environ.rs @@ -1,6 +1,7 @@ // This file contains code from external sources. // Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md +use crate::heap::{Heap, HeapData, HeapStyle}; use crate::translator::{ type_to_irtype, FuncEnvironment as BaseFuncEnvironment, GlobalVariable, TargetEnvironment, }; @@ -1072,7 +1073,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro unreachable!("we don't make any custom globals") } - fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult { + fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult { let pointer_type = self.pointer_type(); let (ptr, base_offset, current_length_offset) = { @@ -1113,7 +1114,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro }); ( Uimm64::new(offset_guard_size), - ir::HeapStyle::Dynamic { + HeapStyle::Dynamic { bound_gv: heap_bound, }, false, @@ -1124,8 +1125,8 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro offset_guard_size, } => ( Uimm64::new(offset_guard_size), - ir::HeapStyle::Static { - bound: Uimm64::new(bound.bytes().0 as u64), + HeapStyle::Static { + bound: bound.bytes().0 as u64, }, true, ), @@ -1137,10 +1138,10 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro global_type: pointer_type, readonly: readonly_base, }); - Ok(func.create_heap(ir::HeapData { + Ok(func.create_heap(HeapData { base: heap_base, min_size: 0.into(), - offset_guard_size, + offset_guard_size: offset_guard_size.into(), style: heap_style, index_type: I32, })) @@ -1334,7 +1335,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor<'_>, index: MemoryIndex, - _heap: ir::Heap, + _heap: Heap, val: ir::Value, ) -> WasmResult { let (func_sig, index_arg, func_idx) = self.get_memory_grow_func(pos.func, index); @@ -1350,7 +1351,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor<'_>, index: MemoryIndex, - _heap: ir::Heap, + _heap: Heap, ) -> WasmResult { let (func_sig, index_arg, func_idx) = self.get_memory_size_func(pos.func, index); let memory_index = pos.ins().iconst(I32, index_arg as i64); @@ -1365,9 +1366,9 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor, src_index: MemoryIndex, - _src_heap: ir::Heap, + _src_heap: Heap, _dst_index: MemoryIndex, - _dst_heap: ir::Heap, + _dst_heap: Heap, dst: ir::Value, src: ir::Value, len: ir::Value, @@ -1388,7 +1389,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor, memory_index: MemoryIndex, - _heap: ir::Heap, + _heap: Heap, dst: ir::Value, val: ir::Value, len: ir::Value, @@ -1412,7 +1413,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor, memory_index: MemoryIndex, - _heap: ir::Heap, + _heap: Heap, seg_index: u32, dst: ir::Value, src: ir::Value, @@ -1536,7 +1537,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor, index: MemoryIndex, - _heap: ir::Heap, + _heap: Heap, addr: ir::Value, expected: ir::Value, timeout: ir::Value, @@ -1560,7 +1561,7 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro &mut self, mut pos: FuncCursor, index: MemoryIndex, - _heap: ir::Heap, + _heap: Heap, addr: ir::Value, count: ir::Value, ) -> WasmResult { diff --git a/lib/compiler-cranelift/src/heap.rs b/lib/compiler-cranelift/src/heap.rs new file mode 100644 index 00000000000..b253c62dcee --- /dev/null +++ b/lib/compiler-cranelift/src/heap.rs @@ -0,0 +1,108 @@ +//! Heaps to implement WebAssembly linear memories. + +use cranelift_codegen::ir::{GlobalValue, Type}; +use cranelift_entity::entity_impl; + +/// An opaque reference to a [`HeapData`][crate::HeapData]. +/// +/// While the order is stable, it is arbitrary. +#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[cfg_attr( + feature = "enable-serde", + derive(serde_derive::Serialize, serde_derive::Deserialize) +)] +pub struct Heap(u32); +entity_impl!(Heap, "heap"); + +/// A heap implementing a WebAssembly linear memory. +/// +/// Code compiled from WebAssembly runs in a sandbox where it can't access all +/// process memory. Instead, it is given a small set of memory areas to work in, +/// and all accesses are bounds checked. `cranelift-wasm` models this through +/// the concept of *heaps*. +/// +/// Heap addresses can be smaller than the native pointer size, for example +/// unsigned `i32` offsets on a 64-bit architecture. +/// +/// A heap appears as three consecutive ranges of address space: +/// +/// 1. The *mapped pages* are the accessible memory range in the heap. A heap +/// may have a minimum guaranteed size which means that some mapped pages are +/// always present. +/// +/// 2. The *unmapped pages* is a possibly empty range of address space that may +/// be mapped in the future when the heap is grown. They are addressable but +/// not accessible. +/// +/// 3. The *offset-guard pages* is a range of address space that is guaranteed +/// to always cause a trap when accessed. It is used to optimize bounds +/// checking for heap accesses with a shared base pointer. They are +/// addressable but not accessible. +/// +/// The *heap bound* is the total size of the mapped and unmapped pages. This is +/// the bound that `heap_addr` checks against. Memory accesses inside the heap +/// bounds can trap if they hit an unmapped page (which is not accessible). +/// +/// Two styles of heaps are supported, *static* and *dynamic*. They behave +/// differently when resized. +/// +/// #### Static heaps +/// +/// A *static heap* starts out with all the address space it will ever need, so +/// it never moves to a different address. At the base address is a number of +/// mapped pages corresponding to the heap's current size. Then follows a number +/// of unmapped pages where the heap can grow up to its maximum size. After the +/// unmapped pages follow the offset-guard pages which are also guaranteed to +/// generate a trap when accessed. +/// +/// #### Dynamic heaps +/// +/// A *dynamic heap* can be relocated to a different base address when it is +/// resized, and its bound can move dynamically. The offset-guard pages move +/// when the heap is resized. The bound of a dynamic heap is stored in a global +/// value. +#[derive(Clone, PartialEq, Hash)] +#[cfg_attr( + feature = "enable-serde", + derive(serde_derive::Serialize, serde_derive::Deserialize) +)] +pub struct HeapData { + /// The address of the start of the heap's storage. + pub base: GlobalValue, + + /// Guaranteed minimum heap size in bytes. Heap accesses before `min_size` + /// don't need bounds checking. + pub min_size: u64, + + /// Size in bytes of the offset-guard pages following the heap. + pub offset_guard_size: u64, + + /// Heap style, with additional style-specific info. + pub style: HeapStyle, + + /// The index type for the heap. + pub index_type: Type, +} + +/// Style of heap including style-specific information. +#[derive(Clone, PartialEq, Hash)] +#[cfg_attr( + feature = "enable-serde", + derive(serde_derive::Serialize, serde_derive::Deserialize) +)] +pub enum HeapStyle { + /// A dynamic heap can be relocated to a different base address when it is + /// grown. + Dynamic { + /// Global value providing the current bound of the heap in bytes. + bound_gv: GlobalValue, + }, + + /// A static heap has a fixed base address and a number of not-yet-allocated + /// pages before the offset-guard pages. + Static { + /// Heap bound in bytes. The offset-guard pages are allocated after the + /// bound. + bound: u64, + }, +} diff --git a/lib/compiler-cranelift/src/lib.rs b/lib/compiler-cranelift/src/lib.rs index bb644035c76..0a115ce4730 100644 --- a/lib/compiler-cranelift/src/lib.rs +++ b/lib/compiler-cranelift/src/lib.rs @@ -51,6 +51,7 @@ mod debug; #[cfg(feature = "unwind")] mod dwarf; mod func_environ; +mod heap; mod trampoline; mod translator; diff --git a/lib/compiler-cranelift/src/translator/code_translator.rs b/lib/compiler-cranelift/src/translator/code_translator.rs index 293283e2b77..0f395decf00 100644 --- a/lib/compiler-cranelift/src/translator/code_translator.rs +++ b/lib/compiler-cranelift/src/translator/code_translator.rs @@ -77,6 +77,7 @@ use super::func_environ::{FuncEnvironment, GlobalVariable, ReturnMode}; use super::func_state::{ControlStackFrame, ElseData, FuncTranslationState}; use super::translation_utils::{block_with_params, f32_translation, f64_translation}; +use crate::heap::Heap; use crate::{hash_map, HashMap}; use core::cmp; use core::convert::TryFrom; @@ -2193,7 +2194,7 @@ fn translate_unreachable_operator( /// Get the address+offset to use for a heap access. fn get_heap_addr( - heap: ir::Heap, + heap: Heap, addr32: ir::Value, offset: u32, width: u32, diff --git a/lib/compiler-cranelift/src/translator/func_environ.rs b/lib/compiler-cranelift/src/translator/func_environ.rs index dc0967ef854..8537c0ac647 100644 --- a/lib/compiler-cranelift/src/translator/func_environ.rs +++ b/lib/compiler-cranelift/src/translator/func_environ.rs @@ -6,6 +6,7 @@ use super::func_state::FuncTranslationState; use super::translation_utils::reference_type; +use crate::heap::Heap; use core::convert::From; use cranelift_codegen::cursor::FuncCursor; use cranelift_codegen::ir::immediates::Offset32; @@ -115,7 +116,7 @@ pub trait FuncEnvironment: TargetEnvironment { /// by `index`. /// /// The index space covers both imported and locally declared memories. - fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult; + fn make_heap(&mut self, func: &mut ir::Function, index: MemoryIndex) -> WasmResult; /// Set up the necessary preamble definitions in `func` to access the table identified /// by `index`. @@ -205,7 +206,7 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, index: MemoryIndex, - heap: ir::Heap, + heap: Heap, val: ir::Value, ) -> WasmResult; @@ -219,7 +220,7 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, index: MemoryIndex, - heap: ir::Heap, + heap: Heap, ) -> WasmResult; /// Translate a `memory.copy` WebAssembly instruction. @@ -231,9 +232,9 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, src_index: MemoryIndex, - src_heap: ir::Heap, + src_heap: Heap, dst_index: MemoryIndex, - dst_heap: ir::Heap, + dst_heap: Heap, dst: ir::Value, src: ir::Value, len: ir::Value, @@ -247,7 +248,7 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, index: MemoryIndex, - heap: ir::Heap, + heap: Heap, dst: ir::Value, val: ir::Value, len: ir::Value, @@ -263,7 +264,7 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, index: MemoryIndex, - heap: ir::Heap, + heap: Heap, seg_index: u32, dst: ir::Value, src: ir::Value, @@ -417,7 +418,7 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, index: MemoryIndex, - heap: ir::Heap, + heap: Heap, addr: ir::Value, expected: ir::Value, timeout: ir::Value, @@ -433,7 +434,7 @@ pub trait FuncEnvironment: TargetEnvironment { &mut self, pos: FuncCursor, index: MemoryIndex, - heap: ir::Heap, + heap: Heap, addr: ir::Value, count: ir::Value, ) -> WasmResult; diff --git a/lib/compiler-cranelift/src/translator/func_state.rs b/lib/compiler-cranelift/src/translator/func_state.rs index 4b42bb1e521..3ea8f5de0fd 100644 --- a/lib/compiler-cranelift/src/translator/func_state.rs +++ b/lib/compiler-cranelift/src/translator/func_state.rs @@ -10,6 +10,7 @@ //! value and control stacks during the translation of a single function. use super::func_environ::{FuncEnvironment, GlobalVariable}; +use crate::heap::Heap; use crate::{HashMap, Occupied, Vacant}; use cranelift_codegen::ir::{self, Block, Inst, Value}; use std::vec::Vec; @@ -233,7 +234,7 @@ pub struct FuncTranslationState { globals: HashMap, // Map of heaps that have been created by `FuncEnvironment::make_heap`. - heaps: HashMap, + heaps: HashMap, // Map of tables that have been created by `FuncEnvironment::make_table`. tables: HashMap, @@ -473,7 +474,7 @@ impl FuncTranslationState { func: &mut ir::Function, index: u32, environ: &mut FE, - ) -> WasmResult { + ) -> WasmResult { let index = MemoryIndex::from_u32(index); match self.heaps.entry(index) { Occupied(entry) => Ok(*entry.get()), From c1c9cdc7622c9ce0098502ec1a0322ab6df99c95 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 17:19:34 -0700 Subject: [PATCH 3/7] Fix compile_and_emit --- lib/compiler-cranelift/src/compiler.rs | 4 ++-- lib/compiler-cranelift/src/trampoline/dynamic_function.rs | 2 +- lib/compiler-cranelift/src/trampoline/function_call.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/compiler-cranelift/src/compiler.rs b/lib/compiler-cranelift/src/compiler.rs index 478ac759f49..5cadfa34660 100644 --- a/lib/compiler-cranelift/src/compiler.rs +++ b/lib/compiler-cranelift/src/compiler.rs @@ -160,7 +160,7 @@ impl Compiler for CraneliftCompiler { let mut code_buf: Vec = Vec::new(); context - .compile_and_emit(&*isa, &mut code_buf) + .compile_and_emit(&*isa, &mut code_buf, &mut Default::default()) .map_err(|error| CompileError::Codegen(error.inner.to_string()))?; let result = context.compiled_code().unwrap(); @@ -272,7 +272,7 @@ impl Compiler for CraneliftCompiler { let mut code_buf: Vec = Vec::new(); context - .compile_and_emit(&*isa, &mut code_buf) + .compile_and_emit(&*isa, &mut code_buf, &mut Default::default()) .map_err(|error| CompileError::Codegen(error.inner.to_string()))?; let result = context.compiled_code().unwrap(); diff --git a/lib/compiler-cranelift/src/trampoline/dynamic_function.rs b/lib/compiler-cranelift/src/trampoline/dynamic_function.rs index f7f84edc324..381d768a8ea 100644 --- a/lib/compiler-cranelift/src/trampoline/dynamic_function.rs +++ b/lib/compiler-cranelift/src/trampoline/dynamic_function.rs @@ -105,7 +105,7 @@ pub fn make_trampoline_dynamic_function( let mut code_buf = Vec::new(); context - .compile_and_emit(isa, &mut code_buf) + .compile_and_emit(isa, &mut code_buf, &mut Default::default()) .map_err(|error| CompileError::Codegen(error.inner.to_string()))?; let unwind_info = compiled_function_unwind_info(isa, &context)?.maybe_into_to_windows_unwind(); diff --git a/lib/compiler-cranelift/src/trampoline/function_call.rs b/lib/compiler-cranelift/src/trampoline/function_call.rs index 8d6741d3db4..551ca0ba0f8 100644 --- a/lib/compiler-cranelift/src/trampoline/function_call.rs +++ b/lib/compiler-cranelift/src/trampoline/function_call.rs @@ -103,7 +103,7 @@ pub fn make_trampoline_function_call( let mut code_buf = Vec::new(); context - .compile_and_emit(isa, &mut code_buf) + .compile_and_emit(isa, &mut code_buf, &mut Default::default()) .map_err(|error| CompileError::Codegen(error.inner.to_string()))?; let unwind_info = compiled_function_unwind_info(isa, &context)?.maybe_into_to_windows_unwind(); From d50321d25f4a6e421af60577f2ab285f503018fa Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 17:27:47 -0700 Subject: [PATCH 4/7] Fixed jumptable --- .../src/translator/code_translator.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/compiler-cranelift/src/translator/code_translator.rs b/lib/compiler-cranelift/src/translator/code_translator.rs index 0f395decf00..97561de7d30 100644 --- a/lib/compiler-cranelift/src/translator/code_translator.rs +++ b/lib/compiler-cranelift/src/translator/code_translator.rs @@ -459,7 +459,7 @@ pub fn translate_operator( } }; let val = state.pop1(); - let mut data = JumpTableData::with_capacity(targets.len() as usize); + let mut data = Vec::with_capacity(targets.len() as usize); if jump_args_count == 0 { // No jump arguments for depth in targets.targets() { @@ -470,16 +470,17 @@ pub fn translate_operator( frame.set_branched_to_exit(); frame.br_destination() }; - data.push_entry(block); + data.push(builder.func.dfg.block_call(block, &[])); } - let jt = builder.create_jump_table(data); let block = { let i = state.control_stack.len() - 1 - (default as usize); let frame = &mut state.control_stack[i]; frame.set_branched_to_exit(); frame.br_destination() }; - builder.ins().br_table(val, block, jt); + let block = builder.func.dfg.block_call(block, &[]); + let jt = builder.create_jump_table(JumpTableData::new(block, &data)); + builder.ins().br_table(val, jt); } else { // Here we have jump arguments, but Cranelift's br_table doesn't support them // We then proceed to split the edges going out of the br_table @@ -496,7 +497,7 @@ pub fn translate_operator( *entry.insert(block) } }; - data.push_entry(branch_block); + data.push(builder.func.dfg.block_call(branch_block, &[])); } let default_branch_block = match dest_block_map.entry(default as usize) { hash_map::Entry::Occupied(entry) => *entry.get(), @@ -506,8 +507,9 @@ pub fn translate_operator( *entry.insert(block) } }; - let jt = builder.create_jump_table(data); - builder.ins().br_table(val, default_branch_block, jt); + let default_branch_block = builder.func.dfg.block_call(default_branch_block, &[]); + let jt = builder.create_jump_table(JumpTableData::new(default_branch_block, &data)); + builder.ins().br_table(val, jt); for (depth, dest_block) in dest_block_sequence { builder.switch_to_block(dest_block); builder.seal_block(dest_block); @@ -517,7 +519,7 @@ pub fn translate_operator( frame.set_branched_to_exit(); frame.br_destination() }; - let destination_args = state.peekn(return_count); + let destination_args = state.peekn_mut(return_count); canonicalise_then_jump(builder, real_dest_block, destination_args); } state.popn(return_count); From 701af1b29443f7c51026bd2e5a8f14d293954d72 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 17:33:01 -0700 Subject: [PATCH 5/7] Fixed heaps --- lib/compiler-cranelift/src/func_environ.rs | 7 +++++-- lib/compiler-cranelift/src/heap.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/compiler-cranelift/src/func_environ.rs b/lib/compiler-cranelift/src/func_environ.rs index 251f9ea0b36..0c880e21cfd 100644 --- a/lib/compiler-cranelift/src/func_environ.rs +++ b/lib/compiler-cranelift/src/func_environ.rs @@ -50,6 +50,9 @@ pub struct FuncEnvironment<'module_environment> { /// The module function signatures signatures: &'module_environment PrimaryMap, + /// Heaps implementing WebAssembly linear memories. + heaps: PrimaryMap, + /// The Cranelift global holding the vmctx address. vmctx: Option, @@ -1138,9 +1141,9 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro global_type: pointer_type, readonly: readonly_base, }); - Ok(func.create_heap(HeapData { + Ok(self.heaps.push(HeapData { base: heap_base, - min_size: 0.into(), + min_size: 0, offset_guard_size: offset_guard_size.into(), style: heap_style, index_type: I32, diff --git a/lib/compiler-cranelift/src/heap.rs b/lib/compiler-cranelift/src/heap.rs index b253c62dcee..29c1671510b 100644 --- a/lib/compiler-cranelift/src/heap.rs +++ b/lib/compiler-cranelift/src/heap.rs @@ -1,7 +1,7 @@ //! Heaps to implement WebAssembly linear memories. use cranelift_codegen::ir::{GlobalValue, Type}; -use cranelift_entity::entity_impl; +use wasmer_types::entity::entity_impl; /// An opaque reference to a [`HeapData`][crate::HeapData]. /// From 6c58cf825a2331bbbc96c72ecd0d8981d239fec1 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 18:28:04 -0700 Subject: [PATCH 6/7] Added heaps into FuncEnvironment --- lib/compiler-cranelift/src/func_environ.rs | 5 +++++ .../src/translator/code_translator.rs | 15 ++++++++------- .../src/translator/func_environ.rs | 5 ++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/compiler-cranelift/src/func_environ.rs b/lib/compiler-cranelift/src/func_environ.rs index 0c880e21cfd..b52079d6209 100644 --- a/lib/compiler-cranelift/src/func_environ.rs +++ b/lib/compiler-cranelift/src/func_environ.rs @@ -140,6 +140,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> { module, signatures, type_stack: vec![], + heaps: PrimaryMap::new(), vmctx: None, memory32_size_sig: None, table_size_sig: None, @@ -1610,4 +1611,8 @@ impl<'module_environment> BaseFuncEnvironment for FuncEnvironment<'module_enviro fn get_function_sig(&self, sig_index: SignatureIndex) -> Option<&FunctionType> { self.module.signatures.get(sig_index) } + + fn get_heap(&self, heap: Heap) -> &HeapData { + &self.heaps[heap] + } } diff --git a/lib/compiler-cranelift/src/translator/code_translator.rs b/lib/compiler-cranelift/src/translator/code_translator.rs index 97561de7d30..87c70873fe8 100644 --- a/lib/compiler-cranelift/src/translator/code_translator.rs +++ b/lib/compiler-cranelift/src/translator/code_translator.rs @@ -77,7 +77,7 @@ use super::func_environ::{FuncEnvironment, GlobalVariable, ReturnMode}; use super::func_state::{ControlStackFrame, ElseData, FuncTranslationState}; use super::translation_utils::{block_with_params, f32_translation, f64_translation}; -use crate::heap::Heap; +use crate::heap::{Heap, HeapData}; use crate::{hash_map, HashMap}; use core::cmp; use core::convert::TryFrom; @@ -2195,15 +2195,16 @@ fn translate_unreachable_operator( } /// Get the address+offset to use for a heap access. -fn get_heap_addr( +fn get_heap_addr( heap: Heap, addr32: ir::Value, offset: u32, width: u32, - addr_ty: Type, + environ: &mut FE, builder: &mut FunctionBuilder, ) -> (ir::Value, i32) { - let offset_guard_size: u64 = builder.func.heaps[heap].offset_guard_size.into(); + let addr_ty: Type = environ.pointer_type(); + let offset_guard_size: u64 = environ.get_heap(heap).offset_guard_size; // How exactly the bounds check is performed here and what it's performed // on is a bit tricky. Generally we want to rely on access violations (e.g. @@ -2294,7 +2295,7 @@ fn prepare_load( addr32, memarg.offset as u32, loaded_bytes, - environ.pointer_type(), + environ, builder, ); @@ -2346,7 +2347,7 @@ fn translate_store( addr32, memarg.offset as u32, mem_op_size(opcode, val_ty), - environ.pointer_type(), + environ, builder, ); // See the comments in `prepare_load` about the flags. @@ -2450,7 +2451,7 @@ fn finalise_atomic_mem_addr( final_lma, /*offset=*/ 0, access_ty.bytes(), - environ.pointer_type(), + environ, builder, ); diff --git a/lib/compiler-cranelift/src/translator/func_environ.rs b/lib/compiler-cranelift/src/translator/func_environ.rs index 8537c0ac647..a61c4dbae7e 100644 --- a/lib/compiler-cranelift/src/translator/func_environ.rs +++ b/lib/compiler-cranelift/src/translator/func_environ.rs @@ -6,7 +6,7 @@ use super::func_state::FuncTranslationState; use super::translation_utils::reference_type; -use crate::heap::Heap; +use crate::heap::{Heap, HeapData}; use core::convert::From; use cranelift_codegen::cursor::FuncCursor; use cranelift_codegen::ir::immediates::Offset32; @@ -490,4 +490,7 @@ pub trait FuncEnvironment: TargetEnvironment { /// Get the type of a function with the given signature index. fn get_function_sig(&self, sig_index: SignatureIndex) -> Option<&FunctionType>; + + /// Get the heap data for the given heap. + fn get_heap(&self, heap: Heap) -> &HeapData; } From 6df1e38bd9af4fba2219cf1e54aba975f0f39b77 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 1 Nov 2023 18:56:09 -0700 Subject: [PATCH 7/7] Trying to fix code_translator --- .../src/translator/code_translator.rs | 19 +- .../code_translator/bounds_checks.rs | 387 ++++++++++++++++++ 2 files changed, 402 insertions(+), 4 deletions(-) create mode 100644 lib/compiler-cranelift/src/translator/code_translator/bounds_checks.rs diff --git a/lib/compiler-cranelift/src/translator/code_translator.rs b/lib/compiler-cranelift/src/translator/code_translator.rs index 87c70873fe8..3d903861e71 100644 --- a/lib/compiler-cranelift/src/translator/code_translator.rs +++ b/lib/compiler-cranelift/src/translator/code_translator.rs @@ -74,6 +74,8 @@ //! //! ("Relax verification to allow I8X16 to act as a default vector type") +mod bounds_checks; + use super::func_environ::{FuncEnvironment, GlobalVariable, ReturnMode}; use super::func_state::{ControlStackFrame, ElseData, FuncTranslationState}; use super::translation_utils::{block_with_params, f32_translation, f64_translation}; @@ -2204,7 +2206,8 @@ fn get_heap_addr( builder: &mut FunctionBuilder, ) -> (ir::Value, i32) { let addr_ty: Type = environ.pointer_type(); - let offset_guard_size: u64 = environ.get_heap(heap).offset_guard_size; + let heap_data = environ.get_heap(heap); + let offset_guard_size: u64 = heap_data.offset_guard_size; // How exactly the bounds check is performed here and what it's performed // on is a bit tricky. Generally we want to rely on access violations (e.g. @@ -2264,9 +2267,17 @@ fn get_heap_addr( }; debug_assert!(adjusted_offset > 0); // want to bounds check at least 1 byte let check_size = u32::try_from(adjusted_offset).unwrap_or(u32::MAX); - let base = builder - .ins() - .heap_addr(addr_ty, heap, addr32, 0u32, check_size as u8); + + let access_size = check_size as u8; + let base = bounds_checks::bounds_check_and_compute_addr( + builder, + environ, + &heap_data, + addr32, + offset, + access_size, + ) + .unwrap(); // Native load/store instructions take a signed `Offset32` immediate, so adjust the base // pointer if necessary. diff --git a/lib/compiler-cranelift/src/translator/code_translator/bounds_checks.rs b/lib/compiler-cranelift/src/translator/code_translator/bounds_checks.rs new file mode 100644 index 00000000000..050b2bd0d70 --- /dev/null +++ b/lib/compiler-cranelift/src/translator/code_translator/bounds_checks.rs @@ -0,0 +1,387 @@ +//! Implementation of Wasm to CLIF memory access translation. +//! +//! Given +//! +//! * a dynamic Wasm memory index operand, +//! * a static offset immediate, and +//! * a static access size, +//! +//! bounds check the memory access and translate it into a native memory access. +//! +//! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +//! !!! !!! +//! !!! THIS CODE IS VERY SUBTLE, HAS MANY SPECIAL CASES, AND IS ALSO !!! +//! !!! ABSOLUTELY CRITICAL FOR MAINTAINING THE SAFETY OF THE WASM HEAP !!! +//! !!! SANDBOX. !!! +//! !!! !!! +//! !!! A good rule of thumb is to get two reviews on any substantive !!! +//! !!! changes in here. !!! +//! !!! !!! +//! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + +use crate::heap::{HeapData, HeapStyle}; +use crate::translator::func_environ::FuncEnvironment; +use cranelift_codegen::{ + cursor::{Cursor, FuncCursor}, + ir::{self, condcodes::IntCC, InstBuilder, RelSourceLoc}, +}; +use cranelift_frontend::FunctionBuilder; +use wasmer_types::WasmResult; + +/// Helper used to emit bounds checks (as necessary) and compute the native +/// address of a heap access. +/// +/// Returns the `ir::Value` holding the native address of the heap access, or +/// `None` if the heap access will unconditionally trap. +pub fn bounds_check_and_compute_addr( + builder: &mut FunctionBuilder, + env: &mut Env, + heap: &HeapData, + // Dynamic operand indexing into the heap. + index: ir::Value, + // Static immediate added to the index. + offset: u32, + // Static size of the heap access. + access_size: u8, +) -> WasmResult +where + Env: FuncEnvironment + ?Sized, +{ + let index = cast_index_to_pointer_ty( + index, + heap.index_type, + env.pointer_type(), + &mut builder.cursor(), + ); + let offset_and_size = offset_plus_size(offset, access_size); + let spectre_mitigations_enabled = false; + // let spectre_mitigations_enabled = env.heap_access_spectre_mitigation(); + + // We need to emit code that will trap (or compute an address that will trap + // when accessed) if + // + // index + offset + access_size > bound + // + // or if the `index + offset + access_size` addition overflows. + // + // Note that we ultimately want a 64-bit integer (we only target 64-bit + // architectures at the moment) and that `offset` is a `u32` and + // `access_size` is a `u8`. This means that we can add the latter together + // as `u64`s without fear of overflow, and we only have to be concerned with + // whether adding in `index` will overflow. + // + // Finally, the following right-hand sides of the matches do have a little + // bit of duplicated code across them, but I think writing it this way is + // worth it for readability and seeing very clearly each of our cases for + // different bounds checks and optimizations of those bounds checks. It is + // intentionally written in a straightforward case-matching style that will + // hopefully make it easy to port to ISLE one day. + Ok(match heap.style { + // ====== Dynamic Memories ====== + // + // 1. First special case for when `offset + access_size == 1`: + // + // index + 1 > bound + // ==> index >= bound + HeapStyle::Dynamic { bound_gv } if offset_and_size == 1 => { + let bound = builder.ins().global_value(env.pointer_type(), bound_gv); + let oob = builder + .ins() + .icmp(IntCC::UnsignedGreaterThanOrEqual, index, bound); + explicit_check_oob_condition_and_compute_addr( + &mut builder.cursor(), + heap, + env.pointer_type(), + index, + offset, + spectre_mitigations_enabled, + oob, + ) + } + + // 2. Second special case for when we know that there are enough guard + // pages to cover the offset and access size. + // + // The precise should-we-trap condition is + // + // index + offset + access_size > bound + // + // However, if we instead check only the partial condition + // + // index > bound + // + // then the most out of bounds that the access can be, while that + // partial check still succeeds, is `offset + access_size`. + // + // However, when we have a guard region that is at least as large as + // `offset + access_size`, we can rely on the virtual memory + // subsystem handling these out-of-bounds errors at + // runtime. Therefore, the partial `index > bound` check is + // sufficient for this heap configuration. + // + // Additionally, this has the advantage that a series of Wasm loads + // that use the same dynamic index operand but different static + // offset immediates -- which is a common code pattern when accessing + // multiple fields in the same struct that is in linear memory -- + // will all emit the same `index > bound` check, which we can GVN. + HeapStyle::Dynamic { bound_gv } if offset_and_size <= heap.offset_guard_size => { + let bound = builder.ins().global_value(env.pointer_type(), bound_gv); + let oob = builder.ins().icmp(IntCC::UnsignedGreaterThan, index, bound); + explicit_check_oob_condition_and_compute_addr( + &mut builder.cursor(), + heap, + env.pointer_type(), + index, + offset, + spectre_mitigations_enabled, + oob, + ) + } + + // 3. Third special case for when `offset + access_size <= min_size`. + // + // We know that `bound >= min_size`, so we can do the following + // comparison, without fear of the right-hand side wrapping around: + // + // index + offset + access_size > bound + // ==> index > bound - (offset + access_size) + HeapStyle::Dynamic { bound_gv } if offset_and_size <= heap.min_size.into() => { + let bound = builder.ins().global_value(env.pointer_type(), bound_gv); + let adjusted_bound = builder.ins().iadd_imm(bound, -(offset_and_size as i64)); + let oob = builder + .ins() + .icmp(IntCC::UnsignedGreaterThan, index, adjusted_bound); + explicit_check_oob_condition_and_compute_addr( + &mut builder.cursor(), + heap, + env.pointer_type(), + index, + offset, + spectre_mitigations_enabled, + oob, + ) + } + + // 4. General case for dynamic memories: + // + // index + offset + access_size > bound + // + // And we have to handle the overflow case in the left-hand side. + HeapStyle::Dynamic { bound_gv } => { + let access_size_val = builder + .ins() + .iconst(env.pointer_type(), offset_and_size as i64); + let adjusted_index = builder.ins().uadd_overflow_trap( + index, + access_size_val, + ir::TrapCode::HeapOutOfBounds, + ); + let bound = builder.ins().global_value(env.pointer_type(), bound_gv); + let oob = builder + .ins() + .icmp(IntCC::UnsignedGreaterThan, adjusted_index, bound); + explicit_check_oob_condition_and_compute_addr( + &mut builder.cursor(), + heap, + env.pointer_type(), + index, + offset, + spectre_mitigations_enabled, + oob, + ) + } + + // ====== Static Memories ====== + // + // With static memories we know the size of the heap bound at compile + // time. + // + // 1. First special case: trap immediately if `offset + access_size > + // bound`, since we will end up being out-of-bounds regardless of the + // given `index`. + HeapStyle::Static { bound } if offset_and_size > bound.into() => { + // env.before_unconditionally_trapping_memory_access(builder)?; + // builder.ins().trap(ir::TrapCode::HeapOutOfBounds); + unreachable!("trap should have been unconditional") + } + + // 2. Second special case for when we can completely omit explicit + // bounds checks for 32-bit static memories. + // + // First, let's rewrite our comparison to move all of the constants + // to one side: + // + // index + offset + access_size > bound + // ==> index > bound - (offset + access_size) + // + // We know the subtraction on the right-hand side won't wrap because + // we didn't hit the first special case. + // + // Additionally, we add our guard pages (if any) to the right-hand + // side, since we can rely on the virtual memory subsystem at runtime + // to catch out-of-bound accesses within the range `bound .. bound + + // guard_size`. So now we are dealing with + // + // index > bound + guard_size - (offset + access_size) + // + // Note that `bound + guard_size` cannot overflow for + // correctly-configured heaps, as otherwise the heap wouldn't fit in + // a 64-bit memory space. + // + // The complement of our should-this-trap comparison expression is + // the should-this-not-trap comparison expression: + // + // index <= bound + guard_size - (offset + access_size) + // + // If we know the right-hand side is greater than or equal to + // `u32::MAX`, then + // + // index <= u32::MAX <= bound + guard_size - (offset + access_size) + // + // This expression is always true when the heap is indexed with + // 32-bit integers because `index` cannot be larger than + // `u32::MAX`. This means that `index` is always either in bounds or + // within the guard page region, neither of which require emitting an + // explicit bounds check. + HeapStyle::Static { bound } + if heap.index_type == ir::types::I32 + && u64::from(u32::MAX) + <= u64::from(bound) + u64::from(heap.offset_guard_size) - offset_and_size => + { + compute_addr( + &mut builder.cursor(), + heap, + env.pointer_type(), + index, + offset, + ) + } + + // 3. General case for static memories. + // + // We have to explicitly test whether + // + // index > bound - (offset + access_size) + // + // and trap if so. + // + // Since we have to emit explicit bounds checks, we might as well be + // precise, not rely on the virtual memory subsystem at all, and not + // factor in the guard pages here. + HeapStyle::Static { bound } => { + // NB: this subtraction cannot wrap because we didn't hit the first + // special case. + let adjusted_bound = u64::from(bound) - offset_and_size; + let oob = + builder + .ins() + .icmp_imm(IntCC::UnsignedGreaterThan, index, adjusted_bound as i64); + explicit_check_oob_condition_and_compute_addr( + &mut builder.cursor(), + heap, + env.pointer_type(), + index, + offset, + spectre_mitigations_enabled, + oob, + ) + } + }) +} + +fn cast_index_to_pointer_ty( + index: ir::Value, + index_ty: ir::Type, + pointer_ty: ir::Type, + pos: &mut FuncCursor, +) -> ir::Value { + if index_ty == pointer_ty { + return index; + } + // Note that using 64-bit heaps on a 32-bit host is not currently supported, + // would require at least a bounds check here to ensure that the truncation + // from 64-to-32 bits doesn't lose any upper bits. For now though we're + // mostly interested in the 32-bit-heaps-on-64-bit-hosts cast. + assert!(index_ty.bits() < pointer_ty.bits()); + + // Convert `index` to `addr_ty`. + let extended_index = pos.ins().uextend(pointer_ty, index); + + // Add debug value-label alias so that debuginfo can name the extended + // value as the address + let loc = pos.srcloc(); + let loc = RelSourceLoc::from_base_offset(pos.func.params.base_srcloc(), loc); + pos.func + .stencil + .dfg + .add_value_label_alias(extended_index, loc, index); + + extended_index +} + +/// Emit explicit checks on the given out-of-bounds condition for the Wasm +/// address and return the native address. +/// +/// This function deduplicates explicit bounds checks and Spectre mitigations +/// that inherently also implement bounds checking. +fn explicit_check_oob_condition_and_compute_addr( + pos: &mut FuncCursor, + heap: &HeapData, + addr_ty: ir::Type, + index: ir::Value, + offset: u32, + // Whether Spectre mitigations are enabled for heap accesses. + spectre_mitigations_enabled: bool, + // The `i8` boolean value that is non-zero when the heap access is out of + // bounds (and therefore we should trap) and is zero when the heap access is + // in bounds (and therefore we can proceed). + oob_condition: ir::Value, +) -> ir::Value { + if !spectre_mitigations_enabled { + pos.ins() + .trapnz(oob_condition, ir::TrapCode::HeapOutOfBounds); + } + + let mut addr = compute_addr(pos, heap, addr_ty, index, offset); + + if spectre_mitigations_enabled { + let null = pos.ins().iconst(addr_ty, 0); + addr = pos.ins().select_spectre_guard(oob_condition, null, addr); + } + + addr +} + +/// Emit code for the native address computation of a Wasm address, +/// without any bounds checks or overflow checks. +/// +/// It is the caller's responsibility to ensure that any necessary bounds and +/// overflow checks are emitted, and that the resulting address is never used +/// unless they succeed. +fn compute_addr( + pos: &mut FuncCursor, + heap: &HeapData, + addr_ty: ir::Type, + index: ir::Value, + offset: u32, +) -> ir::Value { + debug_assert_eq!(pos.func.dfg.value_type(index), addr_ty); + + let heap_base = pos.ins().global_value(addr_ty, heap.base); + let base_and_index = pos.ins().iadd(heap_base, index); + if offset == 0 { + base_and_index + } else { + // NB: The addition of the offset immediate must happen *before* the + // `select_spectre_guard`, if any. If it happens after, then we + // potentially are letting speculative execution read the whole first + // 4GiB of memory. + pos.ins().iadd_imm(base_and_index, offset as i64) + } +} + +#[inline] +fn offset_plus_size(offset: u32, size: u8) -> u64 { + // Cannot overflow because we are widening to `u64`. + offset as u64 + size as u64 +}