Skip to content

Commit 6fa1b3a

Browse files
authored
Add InstanceEntity handle cache with eager warm-up (#1991)
* make StoreInner use StableArena for most things * fix StoreInner internal resolution API * add HandleAndCache * add HandleAndCache to InstanceEntity * add StoreInner::[try_]resolve_instance_mut * use &mut InstanceEntity for Inst in executor * add as_ptr_mut and as_mut methods to Inst type * add missing inline annotations * make HandleAndCache return reference instead of options * add new InstanceEntity API to load using its cache * fix broken intra doc links * partially use new instance cache in executor * return Option from HandleAndCache API * remove commented out line of code * switch to eager initialization for InstanceEntity cache * adjust executor for caching changes * rename methods for better parity * add missing docs and inline * use InstanceEntity::entry helper in more places * add low-level utils::load_x_ptr helpers to executor these allow using the instance cache in more places. * use the instance cache in more places in the executor * remove no longer used StoreInner APIs * rename InstanceEntity::*_v2 methods to `*_ptr` * rename Resolve[Mut] assoc methods * create Inst from &InstanceEntity (again) no more need to have mutable InstanceEntity with eager caching * remove StoreInner::[try_]resolve_instance_mut * remove unused Inst::{as_ptr_mut,as_mut} methods * fix bug in {memory,table}.copy with aliasing imports * add regression test for aliased memory imports * add StoreInner::[try_]resolve_*_ptr methods * use StoreInner::resolve_*_ptr methods to fix UB * remove unused StoreInner methods
1 parent ebe1bf7 commit 6fa1b3a

9 files changed

Lines changed: 688 additions & 321 deletions

File tree

crates/wasmi/src/engine/executor/handler/args.rs

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,7 @@ use crate::{
1919
Sp,
2020
VmState,
2121
},
22-
utils::{
23-
self,
24-
GetValue,
25-
IntoControl as _,
26-
SetValue,
27-
fetch_data,
28-
fetch_elem,
29-
fetch_global,
30-
fetch_memory,
31-
fetch_table,
32-
get_value,
33-
resolve_data_mut,
34-
resolve_elem_mut,
35-
resolve_global_mut,
36-
resolve_memory_mut,
37-
resolve_table_mut,
38-
set_value,
39-
},
22+
utils::{self, GetValue, IntoControl as _, SetValue, get_value, set_value},
4023
},
4124
},
4225
ir::{self, BoundedSlotSpan, BranchOffset},
@@ -163,20 +146,19 @@ impl Args {
163146
/// Returns the bytes of the `memory`.
164147
#[inline]
165148
pub fn fetch_memory_bytes<'a>(
166-
&self,
149+
&mut self,
167150
state: &'a mut VmState,
168-
memory: ir::MemoryAddr,
151+
addr: ir::MemoryAddr,
169152
) -> &'a mut [u8] {
170-
if utils::is_default_memory(self.instance, memory) {
153+
if utils::is_default_memory(self.instance, addr) {
171154
return self.fetch_default_memory_bytes();
172155
}
173-
let memory = utils::fetch_memory(self.instance, memory);
174-
utils::resolve_memory_mut(state.store, &memory).data_mut()
156+
self.fetch_memory(state, addr).data_mut()
175157
}
176158

177159
/// Returns the bytes of the default memory at index 0.
178160
#[inline]
179-
pub fn fetch_default_memory_bytes<'a>(&self) -> &'a mut [u8] {
161+
pub fn fetch_default_memory_bytes<'a>(&mut self) -> &'a mut [u8] {
180162
state::mem0_bytes::<'a>(self.mem0_ptr, self.mem0_len)
181163
}
182164

@@ -185,54 +167,49 @@ impl Args {
185167
pub fn fetch_memory<'a>(
186168
&mut self,
187169
state: &'a mut VmState,
188-
index: ir::MemoryAddr,
170+
addr: ir::MemoryAddr,
189171
) -> &'a mut CoreMemory {
190-
let global = fetch_memory(self.instance, index);
191-
resolve_memory_mut(state.store, &global)
172+
utils::load_memory(self.instance, state.store.inner_mut(), addr)
192173
}
193174

194175
/// Returns an exclusive reference to the global at `index`.
195176
#[inline]
196177
pub fn fetch_global<'a>(
197178
&mut self,
198179
state: &'a mut VmState,
199-
index: ir::GlobalAddr,
180+
addr: ir::GlobalAddr,
200181
) -> &'a mut CoreGlobal {
201-
let global = fetch_global(self.instance, index);
202-
resolve_global_mut(state.store, &global)
182+
utils::load_global(self.instance, state.store.inner_mut(), addr)
203183
}
204184

205185
/// Returns an exclusive reference to the table at `index`.
206186
#[inline]
207187
pub fn fetch_table<'a>(
208188
&mut self,
209189
state: &'a mut VmState,
210-
index: ir::TableAddr,
190+
addr: ir::TableAddr,
211191
) -> &'a mut CoreTable {
212-
let table = fetch_table(self.instance, index);
213-
resolve_table_mut(state.store, &table)
192+
utils::load_table(self.instance, state.store.inner_mut(), addr)
214193
}
215194

216195
/// Returns an exclusive reference to the element segment at `index`.
217196
#[inline]
218197
pub fn fetch_elem<'a>(
219198
&mut self,
220199
state: &'a mut VmState,
221-
index: ir::ElemAddr,
200+
addr: ir::ElemAddr,
222201
) -> &'a mut CoreElementSegment {
223-
let elem = fetch_elem(self.instance, index);
224-
resolve_elem_mut(state.store, &elem)
202+
utils::load_elem(self.instance, state.store.inner_mut(), addr)
225203
}
226204

227205
/// Returns an exclusive reference to the data segment at `index`.
228206
#[inline]
229207
pub fn fetch_data<'a>(
230208
&mut self,
231209
state: &'a mut VmState,
232-
index: ir::DataAddr,
210+
addr: ir::DataAddr,
233211
) -> &'a mut DataSegmentEntity {
234-
let elem = fetch_data(self.instance, index);
235-
resolve_data_mut(state.store, &elem)
212+
utils::load_data(self.instance, state.store.inner_mut(), addr)
236213
}
237214

238215
/// Reloads the data pointer and length of the default memory at index 0 from `state`.
@@ -270,7 +247,7 @@ impl Args {
270247
/// Resolves the [`Func`] at `table[index]` of type `func_type` using `state`.
271248
#[inline]
272249
pub fn resolve_indirect_func<Idx>(
273-
&self,
250+
&mut self,
274251
state: &mut VmState<'_>,
275252
index: Idx,
276253
table: ir::TableAddr,

crates/wasmi/src/engine/executor/handler/exec.rs

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -487,15 +487,24 @@ execution_handler! {
487487
memory_copy_within(state, &mut args, ip, dst_memory, dst_index, src_index, len)?;
488488
dispatch!(state, args)
489489
}
490-
let dst_memory = utils::fetch_memory(instance, dst_memory);
491-
let src_memory = utils::fetch_memory(instance, src_memory);
492-
let (src_memory, dst_memory, fuel) = state
493-
.store
494-
.inner_mut()
495-
.resolve_memory_pair_and_fuel(&src_memory, &dst_memory);
496-
// These accesses just perform the bounds checks required by the Wasm spec.
497-
let src_bytes = utils::memory_slice(src_memory, src_index, len).into_control()?;
498-
let dst_bytes = utils::memory_slice_mut(dst_memory, dst_index, len).into_control()?;
490+
let src_ptr = unsafe { utils::load_memory_ptr(instance, src_memory) };
491+
let mut dst_ptr = unsafe { utils::load_memory_ptr(instance, dst_memory) };
492+
if src_ptr == dst_ptr {
493+
// Distinct memory indices can still resolve to the same store entity (e.g. the same
494+
// memory imported under two names), so branch on the resolved entity before forming
495+
// the aliasing references below.
496+
memory_copy_within(state, &mut args, ip, dst_memory, dst_index, src_index, len)?;
497+
dispatch!(state, args)
498+
}
499+
// SAFETY: `src_ptr != dst_ptr`, so these are distinct entities whose references do not
500+
// alias. These accesses just perform the bounds checks required by the Wasm spec.
501+
let src_memory = unsafe { src_ptr.as_ref() };
502+
let dst_memory = unsafe { dst_ptr.as_mut() };
503+
let fuel = state.store.inner_mut().fuel_mut();
504+
let src_bytes = utils::memory_slice(src_memory, src_index, len)
505+
.into_control()?;
506+
let dst_bytes = utils::memory_slice_mut(dst_memory, dst_index, len)
507+
.into_control()?;
499508
consume_fuel!(
500509
state,
501510
ip,
@@ -517,8 +526,9 @@ fn memory_copy_within(
517526
src_index: usize,
518527
len: usize,
519528
) -> Control<(), Break> {
520-
let memory = utils::fetch_memory(args.instance, dst_memory);
521-
let (memory, fuel) = state.store.inner_mut().resolve_memory_and_fuel_mut(&memory);
529+
// SAFETY: `args.instance` is live and warmed up; `memory` is the only entity accessed here.
530+
let memory = unsafe { utils::load_memory_ptr(args.instance, dst_memory).as_mut() };
531+
let fuel = state.store.inner_mut().fuel_mut();
522532
// These accesses just perform the bounds checks required by the Wasm spec.
523533
utils::memory_slice(memory, src_index, len).into_control()?;
524534
utils::memory_slice(memory, dst_index, len).into_control()?;
@@ -558,8 +568,9 @@ execution_handler! {
558568
let Ok(len) = usize::try_from(len) else {
559569
trap!(TrapCode::MemoryOutOfBounds)
560570
};
561-
let memory = utils::fetch_memory(instance, memory);
562-
let (memory, fuel) = state.store.inner_mut().resolve_memory_and_fuel_mut(&memory);
571+
// SAFETY: `instance` is live and warmed up; `memory` is the only entity accessed here.
572+
let memory = unsafe { utils::load_memory_ptr(instance, memory).as_mut() };
573+
let fuel = state.store.inner_mut().fuel_mut();
563574
let slice = utils::memory_slice_mut(memory, dst, len).into_control()?;
564575
consume_fuel!(state, ip, args, fuel, |costs| costs.fuel_for_copying_values::<u8>(len as u64));
565576
slice.fill(value);
@@ -599,13 +610,11 @@ execution_handler! {
599610
let Ok(len) = usize::try_from(len) else {
600611
trap!(TrapCode::MemoryOutOfBounds)
601612
};
602-
let (memory, data, fuel) = state
603-
.store
604-
.inner_mut()
605-
.resolve_memory_init_params(
606-
&utils::fetch_memory(instance, memory),
607-
&utils::fetch_data(instance, data),
608-
);
613+
// SAFETY: `instance` is live and warmed up. `memory` and `data` live in different
614+
// arenas, so their references are to distinct entities and cannot alias.
615+
let memory = unsafe { utils::load_memory_ptr(instance, memory).as_mut() };
616+
let data = unsafe { utils::load_data_ptr(instance, data).as_ref() };
617+
let fuel = state.store.inner_mut().fuel_mut();
609618
let memory = utils::memory_slice_mut(memory, dst_index, len).into_control()?;
610619
let Some(data) = data
611620
.bytes()
@@ -731,32 +740,26 @@ execution_handler! {
731740
let dst: u64 = args.get(dst);
732741
let src: u64 = args.get(src);
733742
let len: u64 = args.get(len);
734-
if dst_table == src_table {
735-
// Case: copy within the same table
736-
let table = utils::fetch_table(instance, dst_table);
737-
let (table, fuel) = state.store.inner_mut().resolve_table_and_fuel_mut(&table);
738-
if let Err(error) = table.copy_within(dst, src, len, Some(fuel)) {
739-
let trap_code = match error {
740-
TableError::CopyOutOfBounds => TrapCode::TableOutOfBounds,
741-
TableError::OutOfSystemMemory => TrapCode::OutOfSystemMemory,
742-
TableError::OutOfFuel { required_fuel } => {
743-
args.set_ip(ip);
744-
out_of_fuel!(state, args, required_fuel)
745-
}
746-
_ => panic!("table.copy: unexpected error: {error}"),
747-
};
748-
trap!(trap_code)
749-
}
750-
dispatch!(state, args)
751-
}
752-
// Case: copy between two different tables
753-
let dst_table = utils::fetch_table(instance, dst_table);
754-
let src_table = utils::fetch_table(instance, src_table);
755-
let (dst_table, src_table, fuel) = state
756-
.store
757-
.inner_mut()
758-
.resolve_table_pair_and_fuel(&dst_table, &src_table);
759-
if let Err(error) = CoreTable::copy(dst_table, dst, src_table, src, len, Some(fuel)) {
743+
let src_ptr = unsafe { utils::load_table_ptr(instance, src_table) };
744+
let mut dst_ptr = unsafe { utils::load_table_ptr(instance, dst_table) };
745+
// Distinct table indices can still resolve to the same store entity (e.g. the same table
746+
// imported under two names), so branch on the resolved entity, not just the index.
747+
let result = if src_ptr == dst_ptr {
748+
// Case: copy within the same table.
749+
// SAFETY: `dst_ptr` is warmed up and is the only entity accessed here.
750+
let table = unsafe { dst_ptr.as_mut() };
751+
let fuel = state.store.inner_mut().fuel_mut();
752+
table.copy_within(dst, src, len, Some(fuel))
753+
} else {
754+
// Case: copy between two distinct tables.
755+
// SAFETY: `src_ptr != dst_ptr`, so these are distinct entities whose references do
756+
// not alias.
757+
let src_table = unsafe { src_ptr.as_ref() };
758+
let dst_table = unsafe { dst_ptr.as_mut() };
759+
let fuel = state.store.inner_mut().fuel_mut();
760+
CoreTable::copy(dst_table, dst, src_table, src, len, Some(fuel))
761+
};
762+
if let Err(error) = result {
760763
let trap_code = match error {
761764
TableError::CopyOutOfBounds => TrapCode::TableOutOfBounds,
762765
TableError::OutOfSystemMemory => TrapCode::OutOfSystemMemory,
@@ -794,8 +797,9 @@ execution_handler! {
794797
let dst: u64 = args.get(dst);
795798
let len: u64 = args.get(len);
796799
let value: RawRef = args.get(value);
797-
let table = utils::fetch_table(instance, table);
798-
let (table, fuel) = state.store.inner_mut().resolve_table_and_fuel_mut(&table);
800+
// SAFETY: `instance` is live and warmed up; `table` is the only entity accessed here.
801+
let table = unsafe { utils::load_table_ptr(instance, table).as_mut() };
802+
let fuel = state.store.inner_mut().fuel_mut();
799803
if let Err(error) = table.fill_raw(dst, value, len, Some(fuel)) {
800804
let trap_code = match error {
801805
TableError::OutOfSystemMemory => TrapCode::OutOfSystemMemory,
@@ -835,12 +839,11 @@ execution_handler! {
835839
let dst: u64 = args.get(dst);
836840
let src: u32 = args.get(src);
837841
let len: u32 = args.get(len);
838-
let table = utils::fetch_table(args.instance, table);
839-
let elem = utils::fetch_elem(args.instance, elem);
840-
let (table, element, fuel) = state
841-
.store
842-
.inner_mut()
843-
.resolve_table_init_params(&table, &elem);
842+
// SAFETY: `args.instance` is live and warmed up. `table` and `elem` live in different
843+
// arenas, so their references are to distinct entities and cannot alias.
844+
let table = unsafe { utils::load_table_ptr(args.instance, table).as_mut() };
845+
let element = unsafe { utils::load_elem_ptr(args.instance, elem).as_ref() };
846+
let fuel = state.store.inner_mut().fuel_mut();
844847
if let Err(error) = table.init(element.as_ref(), dst, src, len, Some(fuel)) {
845848
let trap_code = match error {
846849
TableError::OutOfSystemMemory => TrapCode::OutOfSystemMemory,

0 commit comments

Comments
 (0)