diff --git a/changelog.d/10144-wasm-review-hardening.md b/changelog.d/10144-wasm-review-hardening.md new file mode 100644 index 0000000000..0d30f14a65 --- /dev/null +++ b/changelog.d/10144-wasm-review-hardening.md @@ -0,0 +1,9 @@ +--- +category: Runtime +title: Harden WebAssembly host bindings +--- + +`WebAssembly.instantiate(Module)` now returns the specified Promise while +preserving its optional-imports dispatch, Wasm table function wrappers release +their host handles when collected, and imported callbacks can no longer +re-enter the shared host store unsafely. diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index d2fbcaf4dd..d9946ee5ab 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -95,9 +95,42 @@ fn get_closure_props() -> &'static Mutex> { CLOSURE_PROPS.get_or_init(|| Mutex::new(new_ptr_hash_map())) } +#[cfg(feature = "wasm-host")] +per_test_global! { + /// Host-owned funcref handles whose JavaScript wrappers are closures. + /// The closure address is a weak owner key; move/death hooks rekey or + /// release the handle without keeping the wrapper alive. + static WASM_FUNCREF_EXTERNALS: OnceLock>> = OnceLock::new(); +} + +#[cfg(feature = "wasm-host")] +fn get_wasm_funcref_externals() -> &'static Mutex> { + WASM_FUNCREF_EXTERNALS.get_or_init(|| Mutex::new(new_ptr_hash_map())) +} + +#[cfg(feature = "wasm-host")] +fn drop_wasm_funcref_external(handle: usize) { + crate::webassembly::drop_host_extern_handle(handle); +} + +#[cfg(feature = "wasm-host")] +pub(crate) fn register_wasm_funcref_external(owner: usize, handle: usize) { + if owner == 0 || handle == 0 { + drop_wasm_funcref_external(handle); + return; + } + note_young_closure_owner(owner, 0); + let replaced = match get_wasm_funcref_externals().lock() { + Ok(mut externals) => externals.insert(owner, handle), + Err(_) => Some(handle), + }; + if let Some(replaced) = replaced { + drop_wasm_funcref_external(replaced); + } +} + crate::perry_thread_local! { - /// #9754: this thread's young-entry log for the three closure side tables - /// (`CLOSURE_PROPS`, `CLOSURE_STATIC_PROTOTYPES`, `CLOSURE_DELETED_KEYS`) — + /// #9754: this thread's young-entry log for the closure side tables — /// the owners whose entry may hold a pointer a minor can act on, as key /// or as value. Thread-local although the tables are process-global: an /// entry's addresses belong to the inserting thread's heap, and only that @@ -322,25 +355,41 @@ pub(crate) fn clear_closure_side_tables_for_dead_ptr(ptr: usize) { if let Ok(mut deleted) = get_closure_deleted_keys().lock() { deleted.remove(&ptr); } + #[cfg(feature = "wasm-host")] + let external = get_wasm_funcref_externals() + .lock() + .ok() + .and_then(|mut externals| externals.remove(&ptr)); + #[cfg(feature = "wasm-host")] + if let Some(external) = external { + drop_wasm_funcref_external(external); + } } -/// Cheap sweep gate: true when any of the three closure side tables has +/// Cheap sweep gate: true when any closure side table has /// entries, so the per-dead-object `clear_dead_payload` dispatch can be /// skipped entirely on the (overwhelmingly common) runs that never attach /// props to closures. Mirrors `object::overflow_fields_is_empty`. pub(crate) fn closure_dynamic_side_tables_nonempty() -> bool { - get_closure_props().lock().is_ok_and(|m| !m.is_empty()) + let dynamic = get_closure_props().lock().is_ok_and(|m| !m.is_empty()) || get_closure_prototypes().lock().is_ok_and(|m| !m.is_empty()) || get_closure_deleted_keys() .lock() - .is_ok_and(|m| !m.is_empty()) + .is_ok_and(|m| !m.is_empty()); + #[cfg(feature = "wasm-host")] + return dynamic + || get_wasm_funcref_externals() + .lock() + .is_ok_and(|m| !m.is_empty()); + #[cfg(not(feature = "wasm-host"))] + dynamic } /// Death pruning for tenured/uncollected-by-sweep closures (2026-07-09 GC /// audit wave 2): the sweep's dead-payload arm above only fires for headers /// the ordinary sweep reclaims; closures dying in the ACTIVE nursery block, /// in bulk block resets, or in copied-minor from-space never reach it. This -/// registry-style pass walks the three tables with one of the GC's deadness +/// registry-style pass walks the tables with one of the GC's deadness /// predicates (`gc::dead_owner`, narrowed to `GC_TYPE_CLOSURE`). The tables /// are process-global: foreign threads' closure addresses don't attribute /// and are skipped (documented residual). @@ -361,6 +410,24 @@ pub(crate) fn prune_dead_closure_side_table_owners(is_dead_closure: &dyn Fn(usiz if let Ok(mut deleted) = get_closure_deleted_keys().lock() { deleted.retain(|owner, _| !is_dead(*owner)); } + #[cfg(feature = "wasm-host")] + let removed = if let Ok(mut externals) = get_wasm_funcref_externals().lock() { + let mut removed = Vec::new(); + externals.retain(|owner, handle| { + let keep = !is_dead(*owner); + if !keep { + removed.push(*handle); + } + keep + }); + removed + } else { + Vec::new() + }; + #[cfg(feature = "wasm-host")] + for external in removed { + drop_wasm_funcref_external(external); + } } /// [`prune_dead_closure_side_table_owners`] for a MINOR: only a young owner @@ -401,6 +468,18 @@ pub(crate) fn closure_dynamic_props_owner_moved(old_owner: usize, new_owner: usi deleted.entry(new_owner).or_default().extend(keys); } } + #[cfg(feature = "wasm-host")] + let replaced = get_wasm_funcref_externals() + .lock() + .ok() + .and_then(|mut externals| { + let handle = externals.remove(&old_owner)?; + externals.insert(new_owner, handle) + }); + #[cfg(feature = "wasm-host")] + if let Some(replaced) = replaced { + drop_wasm_funcref_external(replaced); + } } pub(crate) fn visit_closure_dynamic_prop_values_mut(owner: usize, mut visit: impl FnMut(&mut f64)) { @@ -497,6 +576,10 @@ pub fn scan_closure_dynamic_props_roots_mut(visitor: &mut crate::gc::RuntimeRoot if let Ok(deleted) = get_closure_deleted_keys().lock() { owners.extend(deleted.keys().copied()); } + #[cfg(feature = "wasm-host")] + if let Ok(externals) = get_wasm_funcref_externals().lock() { + owners.extend(externals.keys().copied()); + } owners.sort_unstable(); owners.dedup(); let table_len = owners.len() as u64; @@ -541,7 +624,14 @@ fn scan_closure_side_tables_young(visitor: &mut crate::gc::RuntimeRootVisitor<'_ .lock() .map(|m| m.len()) .unwrap_or(0); - (props + prototypes + deleted) as u64 + #[cfg(feature = "wasm-host")] + let externals = get_wasm_funcref_externals() + .lock() + .map(|m| m.len()) + .unwrap_or(0); + #[cfg(not(feature = "wasm-host"))] + let externals = 0; + (props + prototypes + deleted + externals) as u64 }; #[cfg(any(debug_assertions, test))] debug_assert_closure_young_log_complete(); @@ -608,6 +698,14 @@ fn debug_assert_closure_young_log_complete() { } } } + #[cfg(feature = "wasm-host")] + if let Ok(externals) = get_wasm_funcref_externals().lock() { + for &owner in externals.keys() { + if addr_is_minor_collectible(owner) { + relevant.push(owner); + } + } + } CLOSURE_YOUNG_OWNERS.with(|log| { log.borrow() .debug_assert_logged(CLOSURE_YOUNG_LOG_NAME, &relevant) @@ -685,6 +783,21 @@ fn scan_closure_owner( } } + #[cfg(feature = "wasm-host")] + if let Ok(mut externals) = get_wasm_funcref_externals().lock() { + if externals.contains_key(&owner) { + let mut new_owner = owner; + if visitor.visit_metadata_usize_slot(&mut new_owner) && new_owner != owner { + if let Some(handle) = externals.remove(&owner) { + if let Some(replaced) = externals.insert(new_owner, handle) { + drop_wasm_funcref_external(replaced); + } + } + current_owner = new_owner; + } + } + } + relevant |= addr_is_minor_collectible(current_owner); (current_owner, relevant) } @@ -1129,6 +1242,20 @@ pub(crate) fn test_clear_closure_side_tables() { if let Ok(mut deleted) = get_closure_deleted_keys().lock() { deleted.clear(); } + #[cfg(feature = "wasm-host")] + let externals = get_wasm_funcref_externals() + .lock() + .map(|mut externals| { + externals + .drain() + .map(|(_, handle)| handle) + .collect::>() + }) + .unwrap_or_default(); + #[cfg(feature = "wasm-host")] + for external in externals { + drop_wasm_funcref_external(external); + } } /// Snapshot every dynamic property on a closure as `(name, value)` pairs in diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index 93177371db..4effa0b213 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -73,6 +73,8 @@ pub(crate) use box_captures::{ box_capture_count, clone_closure_box_captures, closure_box_captures_owner_moved, prune_dead_closure_box_capture_owners, visit_closure_box_payload_slots_mut, }; +#[cfg(feature = "wasm-host")] +pub(crate) use dynamic_props::register_wasm_funcref_external; #[cfg(test)] pub(crate) use dynamic_props::test_clear_closure_side_tables; pub(crate) use dynamic_props::{ diff --git a/crates/perry-runtime/src/object/global_this_webassembly.rs b/crates/perry-runtime/src/object/global_this_webassembly.rs index ec7def9da0..ddfb50b6d7 100644 --- a/crates/perry-runtime/src/object/global_this_webassembly.rs +++ b/crates/perry-runtime/src/object/global_this_webassembly.rs @@ -1010,7 +1010,6 @@ pub(super) fn create_webassembly_namespace() -> f64 { 1, true, ); - crate::closure::js_register_closure_arity(webassembly_instantiate_thunk as *const u8, 2); install_webassembly_static_fn( ns_obj, "validate", @@ -1025,6 +1024,9 @@ pub(super) fn create_webassembly_namespace() -> f64 { 1, true, ); + // The optional imports object is a real second call argument, while + // `WebAssembly.instantiate.length` stays 1. + crate::closure::js_register_closure_arity(webassembly_instantiate_thunk as *const u8, 2); install_webassembly_static_fn( ns_obj, "compileStreaming", @@ -1401,6 +1403,11 @@ mod tests { let grow = js_object_get_field_by_name_f64(memory_proto, named_key(b"grow")); let grow_jv = crate::value::JSValue::from_bits(grow.to_bits()); assert!(grow_jv.is_pointer(), "Memory.prototype.grow must exist"); + assert_eq!( + crate::closure::lookup_closure_arity(webassembly_instantiate_thunk as *const u8), + Some(2), + "the install helper must not overwrite instantiate's two-argument dispatch" + ); } #[cfg(not(feature = "wasm-host"))] diff --git a/crates/perry-runtime/src/webassembly.rs b/crates/perry-runtime/src/webassembly.rs index 4af8fc234d..7a426840db 100644 --- a/crates/perry-runtime/src/webassembly.rs +++ b/crates/perry-runtime/src/webassembly.rs @@ -138,6 +138,12 @@ fn emit_error_to_stderr(prefix: &str, err: *mut c_char) { } } +pub(crate) fn drop_host_extern_handle(handle: usize) { + if handle != 0 { + unsafe { perry_wasm_host_extern_drop(handle as *mut c_void) }; + } +} + /// Consume (and free) a host error C-string into a `WebAssembly.`- /// shaped error value: an ordinary `ErrorHeader` whose `.name` is /// `CompileError` / `LinkError` — the same shape the graceful-fail @@ -818,27 +824,27 @@ fn wasm_import_value( } let scope = crate::gc::RuntimeHandleScope::new(); let imports = scope.root_nanbox_f64(instance_imports(context)); - let imports_value = JSValue::from_bits(imports.get_nanbox_f64().to_bits()); - if !imports_value.is_pointer() { + if !JSValue::from_bits(imports.get_nanbox_f64().to_bits()).is_pointer() { return nanbox_undefined(); } let module_bytes = unsafe { std::slice::from_raw_parts(module, module_len) }; let module_key = scope.root_string_ptr(named_key(module_bytes)); let module_value = scope.root_nanbox_f64(module_key.with_const_ptr( |k: *const crate::string::StringHeader| { + let imports_value = JSValue::from_bits(imports.get_nanbox_f64().to_bits()); crate::object::js_object_get_field_by_name_f64( imports_value.as_pointer::(), k, ) }, )); - let module_object = JSValue::from_bits(module_value.get_nanbox_f64().to_bits()); - if !module_object.is_pointer() { + if !JSValue::from_bits(module_value.get_nanbox_f64().to_bits()).is_pointer() { return nanbox_undefined(); } let name_bytes = unsafe { std::slice::from_raw_parts(name, name_len) }; let name_key = scope.root_string_ptr(named_key(name_bytes)); name_key.with_const_ptr(|k: *const crate::string::StringHeader| { + let module_object = JSValue::from_bits(module_value.get_nanbox_f64().to_bits()); crate::object::js_object_get_field_by_name_f64( module_object.as_pointer::(), k, @@ -1033,6 +1039,7 @@ fn make_export_function( .get_raw_mut_ptr::() .is_null() { + drop_host_extern_handle(external as usize); return nanbox_undefined(); } crate::closure::js_register_closure_arity(func_ptr, declared_arity); @@ -1049,6 +1056,7 @@ fn make_export_function( closure_ptr, std::str::from_utf8(name).unwrap_or("wasm"), ); + let closure_ptr = closure.get_raw_mut_ptr::(); crate::value::js_nanbox_pointer(closure_ptr as i64) } @@ -1159,8 +1167,9 @@ extern "C" fn js_wasm_table_get(closure: *const crate::closure::ClosureHeader, i } else { f64::from_bits(bits) }; - let values_ptr = - values_value.as_pointer::() as *mut crate::array::ArrayHeader; + let values_ptr = JSValue::from_bits(values.get_nanbox_f64().to_bits()) + .as_pointer::() + as *mut crate::array::ArrayHeader; crate::array::js_array_set_f64(values_ptr, index as u32, value); value } @@ -1314,22 +1323,38 @@ fn make_table_method( fn make_table_function(external: *mut c_void) -> f64 { let arity = unsafe { perry_wasm_host_func_arity(external) }; if arity == usize::MAX { + drop_host_extern_handle(external as usize); return nanbox_undefined(); } let (func_ptr, declared_arity) = wasm_export_call_shim_for_arity(arity); - let closure = crate::closure::js_closure_alloc(func_ptr, 7); - if closure.is_null() { + let scope = crate::gc::RuntimeHandleScope::new(); + let closure = scope.root_raw_mut_ptr(crate::closure::js_closure_alloc(func_ptr, 7)); + if closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| closure.is_null()) { + drop_host_extern_handle(external as usize); return nanbox_undefined(); } crate::closure::js_register_closure_arity(func_ptr, declared_arity); for index in 0..6 { - crate::closure::js_closure_set_capture_f64(closure, index, nanbox_undefined()); + closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| { + crate::closure::js_closure_set_capture_f64(closure, index, nanbox_undefined()) + }); } - crate::closure::js_closure_set_capture_f64(closure, 0, 0.0); - crate::closure::js_closure_set_capture_f64(closure, 5, 0.0); - crate::closure::js_closure_set_capture_f64(closure, 6, external as usize as f64); - crate::object::set_bound_native_closure_name(closure, "wasm-table-function"); - crate::value::js_nanbox_pointer(closure as i64) + closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| { + crate::closure::js_closure_set_capture_f64(closure, 0, 0.0) + }); + closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| { + crate::closure::js_closure_set_capture_f64(closure, 5, 0.0) + }); + closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| { + crate::closure::js_closure_set_capture_f64(closure, 6, external as usize as f64) + }); + closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| { + crate::object::set_bound_native_closure_name(closure, "wasm-table-function") + }); + closure.with_mut_ptr(|closure: *mut crate::closure::ClosureHeader| { + crate::closure::register_wasm_funcref_external(closure as usize, external as usize); + crate::value::js_nanbox_pointer(closure as i64) + }) } fn make_table_object(external: *mut c_void, inst: *mut c_void, name: f64, receiver: f64) -> f64 { @@ -1820,9 +1845,10 @@ pub extern "C" fn js_webassembly_instance_new( ) } -/// `WebAssembly.instantiate(bytes, imports?)` returns the standard instance -/// result shape. Imported numeric functions are resolved from the JS imports -/// object by module/name and called synchronously by the wasmi host. +/// `WebAssembly.instantiate(source, imports?)` resolves to an `Instance` for a +/// compiled `Module`, or the standard `{ module, instance }` result for bytes. +/// Imported numeric functions are resolved from the JS imports object by +/// module/name and called synchronously by the wasmi host. #[no_mangle] pub extern "C" fn js_webassembly_instantiate(bytes_jsval: f64, imports_jsval: f64) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); @@ -1845,7 +1871,20 @@ pub extern "C" fn js_webassembly_instantiate(bytes_jsval: f64, imports_jsval: f6 "WebAssembly.instantiate(): instantiation failed", )); } - return make_instance_value(module, inst, imports.get_nanbox_f64(), nanbox_undefined()); + let instance = scope.root_nanbox_f64(make_instance_value( + module, + inst, + imports.get_nanbox_f64(), + nanbox_undefined(), + )); + let promise = scope.root_raw_mut_ptr(crate::promise::js_promise_new()); + crate::promise::js_promise_resolve( + promise.get_raw_mut_ptr::(), + instance.get_nanbox_f64(), + ); + return crate::value::js_nanbox_pointer( + promise.get_raw_mut_ptr::() as i64, + ); } let Some((ptr, len)) = extract_bytes(bytes_jsval) else { return rejected_promise_value(wasm_type_error_value( diff --git a/crates/perry-runtime/src/webassembly_calls.rs b/crates/perry-runtime/src/webassembly_calls.rs index 73f8826c19..07d1a5e376 100644 --- a/crates/perry-runtime/src/webassembly_calls.rs +++ b/crates/perry-runtime/src/webassembly_calls.rs @@ -17,10 +17,18 @@ extern "C" fn js_wasm_instance_result_then( 1, ) }); + let (fulfilled, value) = match outcome { + Ok(result) => (true, result), + Err(reason) => (false, reason), + }; + let value = scope.root_nanbox_f64(value); let promise = scope.root_raw_mut_ptr(crate::promise::js_promise_new()); - promise.with_mut_ptr(|p: *mut crate::promise::Promise| match outcome { - Ok(result) => crate::promise::js_promise_resolve(p, result), - Err(reason) => crate::promise::js_promise_reject(p, reason), + promise.with_mut_ptr(|p: *mut crate::promise::Promise| { + if fulfilled { + crate::promise::js_promise_resolve(p, value.get_nanbox_f64()); + } else { + crate::promise::js_promise_reject(p, value.get_nanbox_f64()); + } }); promise .with_mut_ptr(|p: *mut crate::promise::Promise| crate::value::js_nanbox_pointer(p as i64)) diff --git a/crates/perry-runtime/src/webassembly_host.rs b/crates/perry-runtime/src/webassembly_host.rs index d33233b389..16d47c5c84 100644 --- a/crates/perry-runtime/src/webassembly_host.rs +++ b/crates/perry-runtime/src/webassembly_host.rs @@ -45,6 +45,7 @@ extern "C" { out_err: *mut *mut c_char, ) -> *mut c_void; pub(super) fn perry_wasm_host_module_drop(module: *mut c_void); + pub(super) fn perry_wasm_host_extern_drop(handle: *mut c_void); pub(super) fn perry_wasm_host_module_exports_len(module: *mut c_void) -> usize; pub(super) fn perry_wasm_host_module_export_at( module: *mut c_void, diff --git a/crates/perry-wasm-host/src/externals.rs b/crates/perry-wasm-host/src/externals.rs index 97296b6485..48c2dd8343 100644 --- a/crates/perry-wasm-host/src/externals.rs +++ b/crates/perry-wasm-host/src/externals.rs @@ -46,6 +46,7 @@ pub extern "C" fn perry_wasm_host_global_new(kind: u8, mutable: i32, bits: u64) ); extern_handle(global.into()) }) + .unwrap_or(std::ptr::null_mut()) } #[no_mangle] @@ -70,6 +71,7 @@ pub extern "C" fn perry_wasm_host_global_get( } 1 }) + .unwrap_or(0) } #[no_mangle] @@ -78,7 +80,7 @@ pub extern "C" fn perry_wasm_host_global_set(handle: *mut c_void, kind: u8, bits return 0; }; let value = val_from_kind_bits(kind, bits); - with_host_runtime(|runtime| global.set(&mut runtime.store, value).is_ok() as i32) + with_host_runtime(|runtime| global.set(&mut runtime.store, value).is_ok() as i32).unwrap_or(0) } #[no_mangle] @@ -90,6 +92,7 @@ pub extern "C" fn perry_wasm_host_memory_new(initial: u32, maximum: u32) -> *mut .map(|memory| extern_handle(memory.into())) .unwrap_or(std::ptr::null_mut()) }) + .unwrap_or(std::ptr::null_mut()) } #[no_mangle] @@ -106,6 +109,7 @@ pub extern "C" fn perry_wasm_host_memory_span(handle: *mut c_void, out_len: *mut } memory.data_ptr(&runtime.store) }) + .unwrap_or(std::ptr::null_mut()) } #[no_mangle] @@ -119,6 +123,7 @@ pub extern "C" fn perry_wasm_host_memory_grow(handle: *mut c_void, delta: u32) - .map(|pages| pages as i64) .unwrap_or(-1) }) + .unwrap_or(-1) } #[no_mangle] @@ -139,6 +144,7 @@ pub extern "C" fn perry_wasm_host_table_new( .map(|table| extern_handle(table.into())) .unwrap_or(std::ptr::null_mut()) }) + .unwrap_or(std::ptr::null_mut()) } fn table_from_handle(handle: *mut c_void) -> Option { @@ -148,7 +154,7 @@ fn table_from_handle(handle: *mut c_void) -> Option
{ } } -fn table_value_for_store( +pub(crate) fn table_value_for_store( store: &mut Store<()>, table: Table, bits: u64, @@ -175,6 +181,7 @@ pub extern "C" fn perry_wasm_host_table_len(handle: *mut c_void) -> usize { return usize::MAX; }; with_host_runtime(|runtime| usize::try_from(table.size(&runtime.store)).unwrap_or(usize::MAX)) + .unwrap_or(usize::MAX) } #[no_mangle] @@ -215,6 +222,7 @@ pub extern "C" fn perry_wasm_host_table_get( } 1 }) + .unwrap_or(0) } #[no_mangle] @@ -235,6 +243,7 @@ pub extern "C" fn perry_wasm_host_table_set( }; table.set(&mut runtime.store, index as u64, value).is_ok() as i32 }) + .unwrap_or(0) } #[no_mangle] @@ -266,6 +275,7 @@ pub extern "C" fn perry_wasm_host_table_grow( unsafe { *out_old_len = old_len }; 1 }) + .unwrap_or(0) } #[no_mangle] @@ -273,7 +283,7 @@ pub extern "C" fn perry_wasm_host_func_arity(handle: *mut c_void) -> usize { let Some(Extern::Func(function)) = extern_from_handle(handle) else { return usize::MAX; }; - with_host_runtime(|runtime| function.ty(&runtime.store).params().len()) + with_host_runtime(|runtime| function.ty(&runtime.store).params().len()).unwrap_or(usize::MAX) } /// Invoke a function obtained from a funcref table. This is the generic @@ -361,6 +371,11 @@ pub extern "C" fn perry_wasm_host_func_call( }) }) .collect() + }) + .unwrap_or_else(|| { + Err(WasmHostError::Runtime( + "host runtime is already in use".into(), + )) }); let values = match call_result { Ok(values) => values, diff --git a/crates/perry-wasm-host/src/host_runtime.rs b/crates/perry-wasm-host/src/host_runtime.rs new file mode 100644 index 0000000000..7a637d1b30 --- /dev/null +++ b/crates/perry-wasm-host/src/host_runtime.rs @@ -0,0 +1,46 @@ +//! Thread-local shared wasmi engine/store access. + +use std::cell::{Cell, UnsafeCell}; +use wasmi::{Engine, Store}; + +/// All WebAssembly objects in one JavaScript agent share an engine and store. +/// The borrow flag rejects JavaScript re-entry before a second mutable +/// reference to that store can be created. +pub(super) struct HostRuntime { + pub(super) engine: Engine, + pub(super) store: Store<()>, +} + +thread_local! { + static HOST_RUNTIME: UnsafeCell = UnsafeCell::new({ + let engine = Engine::default(); + let store = Store::new(&engine, ()); + HostRuntime { engine, store } + }); + static HOST_RUNTIME_BORROWED: Cell = const { Cell::new(false) }; +} + +struct HostRuntimeBorrowGuard; + +impl HostRuntimeBorrowGuard { + fn enter() -> Option { + HOST_RUNTIME_BORROWED.with(|borrowed| { + if borrowed.replace(true) { + None + } else { + Some(Self) + } + }) + } +} + +impl Drop for HostRuntimeBorrowGuard { + fn drop(&mut self) { + HOST_RUNTIME_BORROWED.with(|borrowed| borrowed.set(false)); + } +} + +pub(super) fn with_host_runtime(f: impl FnOnce(&mut HostRuntime) -> R) -> Option { + let _guard = HostRuntimeBorrowGuard::enter()?; + Some(HOST_RUNTIME.with(|runtime| unsafe { f(&mut *runtime.get()) })) +} diff --git a/crates/perry-wasm-host/src/lib.rs b/crates/perry-wasm-host/src/lib.rs index 642be0a1b2..cc1556b491 100644 --- a/crates/perry-wasm-host/src/lib.rs +++ b/crates/perry-wasm-host/src/lib.rs @@ -11,7 +11,7 @@ //! That keeps the wasmi version surface small and lets us swap engines //! (wasmtime, etc.) behind the same shape later. -use std::cell::{RefCell, UnsafeCell}; +use std::cell::RefCell; use std::collections::HashMap; use std::sync::{ atomic::{AtomicI32, AtomicU64, Ordering}, @@ -23,6 +23,9 @@ use wasmi::{ Mutability, Ref, Store, Table, TableType, Val, ValType, }; +mod host_runtime; +use host_runtime::with_host_runtime; + /// Numeric WebAssembly value used by the public Rust call API. JavaScript /// import callbacks additionally marshal `externref`, while `funcref` values /// cross the C boundary as opaque external handles. @@ -86,31 +89,6 @@ fn trace_module(module: &ModuleInner, event: &str) { ); } -/// All WebAssembly objects in one JavaScript agent share an engine and store. -/// -/// Emscripten side modules import the main module's memory, table, functions, -/// and mutable globals. wasmi external handles can only be linked into the -/// store that owns them, so the former one-store-per-instance layout could -/// never represent that graph. JavaScript execution in Perry is thread-local; -/// mirroring that here gives each Worker an independent WebAssembly agent -/// while allowing every instance on a worker to exchange externals. -struct HostRuntime { - engine: Engine, - store: Store<()>, -} - -thread_local! { - static HOST_RUNTIME: UnsafeCell = UnsafeCell::new({ - let engine = Engine::default(); - let store = Store::new(&engine, ()); - HostRuntime { engine, store } - }); -} - -fn with_host_runtime(f: impl FnOnce(&mut HostRuntime) -> R) -> R { - HOST_RUNTIME.with(|runtime| unsafe { f(&mut *runtime.get()) }) -} - /// Opaque instance backed by its JavaScript agent's shared store. Wasm /// instances still own their defined state, while imported externals retain /// identity across an Emscripten main/side-module graph. @@ -200,20 +178,17 @@ thread_local! { static ACTIVE_INSTANCE_TABLES: RefCell> = const { RefCell::new(Vec::new()) }; } -fn begin_instance_call(inst: &mut WasmInstanceHandle) { +fn begin_instance_call(inst: &mut WasmInstanceHandle, store: &Store<()>) { let instance_id = inst as *mut WasmInstanceHandle as usize; let mut lengths = HashMap::new(); for export in inst.inner._module.0.module.exports() { - let ExternType::Table(table_type) = export.ty() else { + let ExternType::Table(_) = export.ty() else { continue; }; - if table_type.element() != ValType::ExternRef { - continue; - } - let Some(table) = inst.inner.instance.get_table(inst.store(), export.name()) else { + let Some(table) = inst.inner.instance.get_table(store, export.name()) else { continue; }; - if let Ok(len) = usize::try_from(table.size(inst.store())) { + if let Ok(len) = usize::try_from(table.size(store)) { lengths.insert(export.name().to_string(), len); } } @@ -353,7 +328,7 @@ impl std::error::Error for WasmHostError {} /// Cheap byte-level magic check (`\0asm\01\0\0\0`). Mirrors `WebAssembly.validate` /// — for the MVP we delegate to wasmi's full module decode. pub fn validate(bytes: &[u8]) -> bool { - with_host_runtime(|runtime| Module::new(&runtime.engine, bytes).is_ok()) + with_host_runtime(|runtime| Module::new(&runtime.engine, bytes).is_ok()).unwrap_or(false) } /// Compile bytes to a module. No imports resolved at this stage. @@ -371,6 +346,11 @@ pub fn compile(bytes: &[u8]) -> Result { trace_module(&inner, "compiled"); Ok(WasmModuleHandle(inner)) }) + .unwrap_or_else(|| { + Err(WasmHostError::Compile( + "host runtime is already in use".into(), + )) + }) } /// Instantiate with the module's imported numeric functions routed through an @@ -397,7 +377,6 @@ fn instantiate_with_import_callbacks( import_context_value: u64, ) -> Result { trace_module(&module.0, "instantiating"); - let store = with_host_runtime(|runtime| &mut runtime.store as *mut Store<()>); let import_context = Arc::new(AtomicU64::new(import_context_value)); // i32::MIN is not a valid WASI process status and acts as "not exited". let exit_code = Arc::new(AtomicI32::new(i32::MIN)); @@ -542,10 +521,15 @@ fn instantiate_with_import_callbacks( ) .map_err(|e| WasmHostError::Link(e.to_string()))?; } - let instance = linker - .instantiate_and_start(unsafe { &mut *store }, &module.0.module) - .map_err(|e| WasmHostError::Link(e.to_string()))?; - let memory = instance.get_memory(unsafe { &*store }, "memory"); + let (store, instance, memory) = with_host_runtime(|runtime| { + let store = &mut runtime.store; + let instance = linker + .instantiate_and_start(&mut *store, &module.0.module) + .map_err(|e| WasmHostError::Link(e.to_string()))?; + let memory = instance.get_memory(&*store, "memory"); + Ok((store as *mut Store<()>, instance, memory)) + }) + .unwrap_or_else(|| Err(WasmHostError::Link("host runtime is already in use".into())))?; trace_module(&module.0, "instantiated"); Ok(WasmInstanceHandle { inner: Box::new(InstanceInner { @@ -586,22 +570,26 @@ fn coerce_numeric_value(value: WasmVal, expected: ValType) -> Option { /// once per instance. `None` when the instance has no function export by that /// name. fn resolve_export(inst: &mut WasmInstanceHandle, name: &str) -> Option { - if let Some(&index) = inst.inner.export_handles.get(name) { - return Some(index); - } - let func = inst.inner.instance.get_func(inst.store(), name)?; - let ty = func.ty(inst.store()); - let index = inst.inner.exports.len(); - inst.inner.exports.push(CachedExport { - name: name.to_string(), - func, - params: ty.params().to_vec().into_boxed_slice(), - results: ty.results().to_vec().into_boxed_slice(), - args: Vec::new(), - outs: Vec::new(), - }); - inst.inner.export_handles.insert(name.to_string(), index); - Some(index) + with_host_runtime(|runtime| { + debug_assert_eq!(inst.inner.store, &mut runtime.store as *mut Store<()>); + if let Some(&index) = inst.inner.export_handles.get(name) { + return Some(index); + } + let func = inst.inner.instance.get_func(&runtime.store, name)?; + let ty = func.ty(&runtime.store); + let index = inst.inner.exports.len(); + inst.inner.exports.push(CachedExport { + name: name.to_string(), + func, + params: ty.params().to_vec().into_boxed_slice(), + results: ty.results().to_vec().into_boxed_slice(), + args: Vec::new(), + outs: Vec::new(), + }); + inst.inner.export_handles.insert(name.to_string(), index); + Some(index) + }) + .flatten() } /// Call a previously resolved export. Results are left in the cached entry's @@ -641,20 +629,28 @@ fn call_resolved_export( .extend(entry.results.iter().copied().map(Val::default)); } - begin_instance_call(inst); - let call_result = { + let call_result = with_host_runtime(|runtime| { + begin_instance_call(inst, &runtime.store); // Split the borrow: the call needs the store mutably while reading the // cached argument buffer and filling the cached result buffer, and all // three are disjoint fields of the same `InstanceInner`. - let store = inst.inner.store; + debug_assert_eq!(inst.inner.store, &mut runtime.store as *mut Store<()>); let inner = &mut *inst.inner; let CachedExport { func, args, outs, .. } = &mut inner.exports[index]; - func.call(unsafe { &mut *store }, args, outs) - }; + func.call(&mut runtime.store, args, outs) + }) + .map_or_else( + || { + Err(WasmHostError::Runtime( + "host runtime is already in use".into(), + )) + }, + |result| result.map_err(|e| WasmHostError::Runtime(e.to_string())), + ); let table_result = finish_instance_call(inst); - call_result.map_err(|e| WasmHostError::Runtime(e.to_string()))?; + call_result?; table_result?; Ok(()) } @@ -1032,6 +1028,14 @@ pub(crate) fn extern_from_handle(handle: *mut c_void) -> Option { (!handle.is_null()).then(|| unsafe { (*(handle as *const WasmExternHandle)).item }) } +/// Release an opaque external handle after its JavaScript wrapper dies. +#[no_mangle] +pub extern "C" fn perry_wasm_host_extern_drop(handle: *mut c_void) { + if !handle.is_null() { + unsafe { drop(Box::from_raw(handle as *mut WasmExternHandle)) }; + } +} + mod externals; /// Return the byte length of the exported `memory`, or zero when absent. @@ -1342,7 +1346,7 @@ mod tests { ]; /// `(module (import "env" "f" (func $f (result f64))) /// (func (export "call") (result f64) call $f))`. - const IMPORT_F64_RESULT_WASM: &[u8] = &[ + pub(super) const IMPORT_F64_RESULT_WASM: &[u8] = &[ 0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x01, 0x05, 0x01, 0x60, 0x00, 0x01, 0x7c, 0x02, 0x09, 0x01, 0x03, 0x65, 0x6e, 0x76, 0x01, 0x66, 0x00, 0x00, 0x03, 0x02, 0x01, 0x00, 0x07, 0x08, 0x01, 0x04, 0x63, 0x61, 0x6c, 0x6c, 0x00, 0x01, 0x0a, 0x06, 0x01, 0x04, 0x00, @@ -1973,5 +1977,7 @@ mod tests { } } +#[cfg(test)] +mod reentrancy_tests; #[cfg(test)] mod shared_import_tests; diff --git a/crates/perry-wasm-host/src/reentrancy_tests.rs b/crates/perry-wasm-host/src/reentrancy_tests.rs new file mode 100644 index 0000000000..6a83f72650 --- /dev/null +++ b/crates/perry-wasm-host/src/reentrancy_tests.rs @@ -0,0 +1,42 @@ +use super::*; + +unsafe extern "C" fn tries_nested_store_access( + _context: u64, + _module: *const u8, + _module_len: usize, + _name: *const u8, + _name_len: usize, + _arg_kinds: *const u8, + _arg_bits: *const u64, + _arg_count: usize, + _result_kinds: *const u8, + result_bits: *mut u64, + result_count: usize, +) -> i32 { + assert!( + externals::perry_wasm_host_global_new(WASM_VAL_KIND_I32, 1, 0).is_null(), + "nested access must fail before borrowing the active store again" + ); + assert_eq!(result_count, 1); + *result_bits = 9.5f64.to_bits(); + 1 +} + +#[test] +fn javascript_import_callback_cannot_reborrow_the_shared_store() { + let module = compile(super::tests::IMPORT_F64_RESULT_WASM).expect("compile import module"); + let mut instance = + instantiate_with_import_callback(&module, Some(tries_nested_store_access), 0) + .expect("instantiate import module"); + assert_eq!( + call_export(&mut instance, "call", &[]).expect("outer wasm call remains usable"), + [WasmVal::F64(9.5)] + ); + + let handle = externals::perry_wasm_host_global_new(WASM_VAL_KIND_I32, 1, 0); + assert!( + !handle.is_null(), + "the borrow guard must clear after the call" + ); + perry_wasm_host_extern_drop(handle); +} diff --git a/crates/perry-wasm-host/src/tables.rs b/crates/perry-wasm-host/src/tables.rs index 3e277be3c6..bdbe57f1ac 100644 --- a/crates/perry-wasm-host/src/tables.rs +++ b/crates/perry-wasm-host/src/tables.rs @@ -65,7 +65,10 @@ pub extern "C" fn perry_wasm_host_instance_table_get( *out_bits = value.bits; *out_is_null = value.is_null as i32; if !out_external.is_null() { - *out_external = value.external; + *out_external = match extern_from_handle(value.external) { + Some(Extern::Func(function)) => extern_handle(function.into()), + _ => std::ptr::null_mut(), + }; } } return 1; @@ -115,18 +118,7 @@ pub(crate) fn table_value( is_null: i32, external: *mut c_void, ) -> Option { - let element = table.ty(inst.store()).element(); - if is_null != 0 { - return Some(Val::default(element)); - } - match element { - ValType::ExternRef => Some(Val::from(ExternRef::new(inst.store_mut(), bits))), - ValType::FuncRef => match extern_from_handle(external) { - Some(Extern::Func(function)) => Some(Val::FuncRef(Ref::Val(function))), - _ => None, - }, - _ => None, - } + crate::externals::table_value_for_store(inst.store_mut(), table, bits, is_null, external) } #[no_mangle] diff --git a/crates/perry/src/commands/compile/collect_modules/tests.rs b/crates/perry/src/commands/compile/collect_modules/tests.rs index 6d03afcd5c..cd9305ce38 100644 --- a/crates/perry/src/commands/compile/collect_modules/tests.rs +++ b/crates/perry/src/commands/compile/collect_modules/tests.rs @@ -3,7 +3,7 @@ use super::{ collect_js_module_imports, collect_modules, env_defines_for_lowering, - expand_dynamic_import_glob, package_has_unsupported_node_addon, + expand_dynamic_import_glob, file_loader_import_sources, package_has_unsupported_node_addon, refuse_compile_package_native_addon, }; use crate::commands::compile::{CompilationContext, DefineValue}; @@ -788,6 +788,16 @@ fn assert_dynamic_asset_import(source: &str, filename: &str, bytes: &[u8]) { assert!(ctx.native_modules.contains_key(&canonical_asset)); } +#[test] +fn invalid_wtf8_static_asset_specifier_is_not_collected_as_empty() { + let module = perry_parser::parse_typescript( + r#"import asset from "\uD800" with { type: "file" };"#, + "entry.ts", + ) + .expect("lone surrogate import source is valid JavaScript syntax"); + assert!(file_loader_import_sources(&module).is_empty()); +} + #[test] fn dynamic_wasm_import_attribute_returns_embedded_path() { assert_dynamic_asset_import( diff --git a/crates/perry/src/commands/compile/collect_modules_helpers.rs b/crates/perry/src/commands/compile/collect_modules_helpers.rs index 494223c2df..ebc0fff0df 100644 --- a/crates/perry/src/commands/compile/collect_modules_helpers.rs +++ b/crates/perry/src/commands/compile/collect_modules_helpers.rs @@ -81,7 +81,8 @@ pub(super) fn file_loader_import_sources(module: &swc_ecma_ast::Module) -> HashS }; let attributes = import.with.as_deref()?; requests_asset_path(attributes) - .then(|| import.src.value.as_str().unwrap_or("").to_string()) + .then(|| import.src.value.as_str().map(str::to_owned)) + .flatten() }) .collect(); diff --git a/crates/perry/tests/issue_5234_wasm_esm_import.rs b/crates/perry/tests/issue_5234_wasm_esm_import.rs index a8f620488c..e329e0693c 100644 --- a/crates/perry/tests/issue_5234_wasm_esm_import.rs +++ b/crates/perry/tests/issue_5234_wasm_esm_import.rs @@ -250,6 +250,9 @@ console.log("tableCycle=" + runTableCycle()); const fileModule = new WebAssembly.Module(readFileSync(addWasmPath)); const fileInstance = new WebAssembly.Instance(fileModule); console.log("fileInstance=" + fileInstance.exports.add(9, 12)); +WebAssembly.instantiate(fileModule).then((instance) => { + console.log("moduleThen=" + instance.exports.add(10, 11)); +}); const importedModule = new WebAssembly.Module(readFileSync(importedWasmPath)); const importedInstance = new WebAssembly.Instance(importedModule, { @@ -472,6 +475,7 @@ fn wasm_esm_import_instantiates_and_exposes_exports() { assert!(stdout.contains("table=2:5:true"), "stdout:\n{stdout}"); assert!(stdout.contains("tableCycle=4:true"), "stdout:\n{stdout}"); assert!(stdout.contains("fileInstance=21"), "stdout:\n{stdout}"); + assert!(stdout.contains("moduleThen=21"), "stdout:\n{stdout}"); assert!(stdout.contains("fileImported=21"), "stdout:\n{stdout}"); assert!(stdout.contains("instanceLength=1"), "stdout:\n{stdout}"); assert!(stdout.contains("sharedGlobal=7"), "stdout:\n{stdout}");