From 42053ddd4e77cea4a4dccf4c936ea545db49f60a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 29 Nov 2018 12:01:16 -0800 Subject: [PATCH] Move closure shims into the descriptor Currently closure shims are communicated to JS at runtime, although at runtime the same constant value is always passed to JS! More pressing, however, work in #1002 requires knowledge of closure descriptor indices at `wasm-bindgen` time which is not currently known. Since the closure descriptor shims and such are already constant values, this commit moves the descriptor function indices into the *descriptor* for a closure/function pointer. This way we can learn about these values at `wasm-bindgen` time instead of only knowing them at runtime. This should have no semantic change on users of `wasm-bindgen`, although some closure invocations may be slightly speedier because there's less arguments being transferred over the boundary. Overall though this will help #1002 as the closure shims that the Rust compiler generates may not be the exact ones we hand out to JS, but rather wrappers around them which do `anyref` business things. --- crates/backend/src/codegen.rs | 2 + crates/cli-support/src/descriptor.rs | 9 ++ crates/cli-support/src/js/closures.rs | 17 +-- crates/cli-support/src/js/js2rust.rs | 2 - crates/cli-support/src/js/mod.rs | 28 ++--- crates/cli-support/src/js/rust2js.rs | 12 +-- crates/wasm-interpreter/src/lib.rs | 2 - src/closure.rs | 43 +++----- src/convert/closures.rs | 148 +++++++++++++++----------- src/describe.rs | 66 ------------ src/lib.rs | 2 +- 11 files changed, 137 insertions(+), 194 deletions(-) diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index 3fb57485..b941c0d4 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -501,6 +501,7 @@ impl TryToTokens for ast::Export { &export, quote! { inform(FUNCTION); + inform(0); inform(#nargs); #(<#argtys as WasmDescribe>::describe();)* #describe_ret @@ -999,6 +1000,7 @@ impl<'a> ToTokens for DescribeImport<'a> { &f.shim, quote! { inform(FUNCTION); + inform(0); inform(#nargs); #(<#argtys as WasmDescribe>::describe();)* #inform_ret diff --git a/crates/cli-support/src/descriptor.rs b/crates/cli-support/src/descriptor.rs index 5c116cc1..1bcea29f 100644 --- a/crates/cli-support/src/descriptor.rs +++ b/crates/cli-support/src/descriptor.rs @@ -70,11 +70,14 @@ pub enum Descriptor { #[derive(Debug)] pub struct Function { pub arguments: Vec, + pub shim_idx: u32, pub ret: Descriptor, } #[derive(Debug)] pub struct Closure { + pub shim_idx: u32, + pub dtor_idx: u32, pub function: Function, pub mutable: bool, } @@ -293,9 +296,13 @@ fn get(a: &mut &[u32]) -> u32 { impl Closure { fn decode(data: &mut &[u32]) -> Closure { + let shim_idx = get(data); + let dtor_idx = get(data); let mutable = get(data) == REFMUT; assert_eq!(get(data), FUNCTION); Closure { + shim_idx, + dtor_idx, mutable, function: Function::decode(data), } @@ -304,11 +311,13 @@ impl Closure { impl Function { fn decode(data: &mut &[u32]) -> Function { + let shim_idx = get(data); let arguments = (0..get(data)) .map(|_| Descriptor::_decode(data)) .collect::>(); Function { arguments, + shim_idx, ret: Descriptor::_decode(data), } } diff --git a/crates/cli-support/src/js/closures.rs b/crates/cli-support/src/js/closures.rs index a3193de1..83515049 100644 --- a/crates/cli-support/src/js/closures.rs +++ b/crates/cli-support/src/js/closures.rs @@ -47,11 +47,12 @@ pub fn rewrite(input: &mut Context) -> Result<(), Error> { // If this was an imported function we didn't reorder those, so nothing // to do. if idx < old_num_imports { - return idx + idx + } else { + // ... otherwise we're injecting a number of new imports, so offset + // everything. + idx + info.code_idx_to_descriptor.len() as u32 } - // ... otherwise we're injecting a number of new imports, so offset - // everything. - idx + info.code_idx_to_descriptor.len() as u32 }).remap_module(input.module); info.delete_function_table_entries(input); @@ -252,9 +253,9 @@ impl ClosureDescriptors { input.expose_add_heap_object(); input.function_table_needed = true; let body = format!( - "function(a, b, fi, di, _ignored) {{ - const f = wasm.__wbg_function_table.get(fi); - const d = wasm.__wbg_function_table.get(di); + "function(a, b, _ignored) {{ + const f = wasm.__wbg_function_table.get({}); + const d = wasm.__wbg_function_table.get({}); const cb = {}; cb.a = a; cb.cnt = 1; @@ -262,6 +263,8 @@ impl ClosureDescriptors { real.original = cb; return addHeapObject(real); }}", + closure.shim_idx, + closure.dtor_idx, js, ); input.export(&import_name, &body, None); diff --git a/crates/cli-support/src/js/js2rust.rs b/crates/cli-support/src/js/js2rust.rs index 0d01e048..1edf41a1 100644 --- a/crates/cli-support/src/js/js2rust.rs +++ b/crates/cli-support/src/js/js2rust.rs @@ -256,7 +256,6 @@ impl<'a, 'b> Js2Rust<'a, 'b> { self.cx.expose_uint64_cvt_shim() }; self.cx.expose_uint32_memory(); - self.cx.expose_global_argument_ptr()?; self.js_arguments.push((name.clone(), "BigInt".to_string())); self.prelude(&format!( " @@ -374,7 +373,6 @@ impl<'a, 'b> Js2Rust<'a, 'b> { self.cx.expose_uint64_cvt_shim() }; self.cx.expose_uint32_memory(); - self.cx.expose_global_argument_ptr()?; self.js_arguments.push((name.clone(), "BigInt".to_string())); self.prelude(&format!( " diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 382ab39a..99698f40 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -459,8 +459,10 @@ impl<'a> Context<'a> { Ok(String::from("function(idx) { throw takeObject(idx); }")) })?; + closures::rewrite(self).with_context(|_| { + "failed to generate internal closure shims" + })?; self.unexport_unused_internal_exports(); - closures::rewrite(self)?; // Handle the `start` function, if one was specified. If we're in a // --test mode (such as wasm-bindgen-test-runner) then we skip this @@ -1682,23 +1684,6 @@ impl<'a> Context<'a> { } } - fn expose_get_global_argument(&mut self) -> Result<(), Error> { - if !self.exposed_globals.insert("get_global_argument") { - return Ok(()); - } - self.expose_uint32_memory(); - self.expose_global_argument_ptr()?; - self.global( - " - function getGlobalArgument(arg) { - const idx = globalArgumentPtr() / 4 + arg; - return getUint32Memory()[idx]; - } - ", - ); - Ok(()) - } - fn expose_global_argument_ptr(&mut self) -> Result<(), Error> { if !self.exposed_globals.insert("global_argument_ptr") { return Ok(()); @@ -2337,7 +2322,12 @@ impl<'a, 'b> SubContext<'a, 'b> { self.generate_enum(e); } for s in self.program.structs.iter() { - self.generate_struct(s)?; + self.generate_struct(s).with_context(|_| { + format!( + "failed to generate bindings for Rust struct `{}`", + s.name, + ) + })?; } for s in self.program.typescript_custom_sections.iter() { self.cx.typescript.push_str(s); diff --git a/crates/cli-support/src/js/rust2js.rs b/crates/cli-support/src/js/rust2js.rs index 254733e7..b88375d9 100644 --- a/crates/cli-support/src/js/rust2js.rs +++ b/crates/cli-support/src/js/rust2js.rs @@ -252,6 +252,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> { } if let Some((f, mutable)) = arg.stack_closure() { + let arg2 = self.shim_argument(); let (js, _ts, _js_doc) = { let mut builder = Js2Rust::new("", self.cx); if mutable { @@ -268,20 +269,19 @@ impl<'a, 'b> Rust2Js<'a, 'b> { .process(f)? .finish("function", "this.f") }; - self.cx.expose_get_global_argument()?; self.cx.function_table_needed = true; - let next_global = self.global_idx(); self.global_idx(); self.prelude(&format!( "\ let cb{0} = {js};\n\ - cb{0}.f = wasm.__wbg_function_table.get({0});\n\ - cb{0}.a = getGlobalArgument({next_global});\n\ - cb{0}.b = getGlobalArgument({next_global} + 1);\n\ + cb{0}.f = wasm.__wbg_function_table.get({idx});\n\ + cb{0}.a = {0};\n\ + cb{0}.b = {1};\n\ ", abi, + arg2, js = js, - next_global = next_global + idx = f.shim_idx, )); self.finally(&format!("cb{0}.a = cb{0}.b = 0;", abi)); self.js_arguments.push(format!("cb{0}.bind(cb{0})", abi)); diff --git a/crates/wasm-interpreter/src/lib.rs b/crates/wasm-interpreter/src/lib.rs index 387a2371..cd34a0e9 100644 --- a/crates/wasm-interpreter/src/lib.rs +++ b/crates/wasm-interpreter/src/lib.rs @@ -353,8 +353,6 @@ impl Interpreter { self.descriptor_table_idx = Some(self.stack.pop().unwrap() as u32); self.stack.pop(); self.stack.pop(); - self.stack.pop(); - self.stack.pop(); self.stack.push(0); } else { self.call(*idx, sections); diff --git a/src/closure.rs b/src/closure.rs index 359570aa..61c60fa1 100644 --- a/src/closure.rs +++ b/src/closure.rs @@ -197,20 +197,16 @@ impl Closure unsafe fn breaks_if_inlined( a: usize, b: usize, - invoke: u32, - destroy: u32, ) -> u32 { super::__wbindgen_describe_closure( a as u32, b as u32, - invoke, - destroy, describe:: as u32, ) } let idx = unsafe { - breaks_if_inlined::(a, b, T::invoke_fn(), T::destroy_fn()) + breaks_if_inlined::(a, b) }; Closure { @@ -294,9 +290,6 @@ impl Drop for Closure #[doc(hidden)] pub unsafe trait WasmClosure: 'static { fn describe(); - - fn invoke_fn() -> u32; - fn destroy_fn() -> u32; } // The memory safety here in these implementations below is a bit tricky. We @@ -322,11 +315,6 @@ macro_rules! doit { R: ReturnWasmAbi + 'static, { fn describe() { - <&Self>::describe(); - } - - #[inline] - fn invoke_fn() -> u32 { #[allow(non_snake_case)] unsafe extern "C" fn invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( a: usize, @@ -350,12 +338,10 @@ macro_rules! doit { }; ret.return_abi(&mut GlobalStack::new()) } - invoke::<$($var,)* R> as u32 - } - #[inline] - fn destroy_fn() -> u32 { - unsafe extern "C" fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( + inform(invoke::<$($var,)* R> as u32); + + unsafe extern fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( a: usize, b: usize, ) { @@ -364,7 +350,9 @@ macro_rules! doit { fields: (a, b) }.ptr)); } - destroy::<$($var,)* R> as u32 + inform(destroy::<$($var,)* R> as u32); + + <&Self>::describe(); } } @@ -373,11 +361,6 @@ macro_rules! doit { R: ReturnWasmAbi + 'static, { fn describe() { - <&mut Self>::describe(); - } - - #[inline] - fn invoke_fn() -> u32 { #[allow(non_snake_case)] unsafe extern "C" fn invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( a: usize, @@ -402,12 +385,10 @@ macro_rules! doit { }; ret.return_abi(&mut GlobalStack::new()) } - invoke::<$($var,)* R> as u32 - } - #[inline] - fn destroy_fn() -> u32 { - unsafe extern "C" fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( + inform(invoke::<$($var,)* R> as u32); + + unsafe extern fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( a: usize, b: usize, ) { @@ -416,7 +397,9 @@ macro_rules! doit { fields: (a, b) }.ptr)); } - destroy::<$($var,)* R> as u32 + inform(destroy::<$($var,)* R> as u32); + + <&mut Self>::describe(); } } )*) diff --git a/src/convert/closures.rs b/src/convert/closures.rs index 20b4f39b..b9e39054 100644 --- a/src/convert/closures.rs +++ b/src/convert/closures.rs @@ -1,93 +1,119 @@ use core::mem; use convert::{FromWasmAbi, GlobalStack, IntoWasmAbi, ReturnWasmAbi, Stack}; +use convert::slices::WasmSlice; +use describe::{inform, FUNCTION, WasmDescribe}; use throw_str; macro_rules! stack_closures { - ($( ($($var:ident)*) )*) => ($( + ($( ($cnt:tt $invoke:ident $invoke_mut:ident $($var:ident)*) )*) => ($( impl<'a, 'b, $($var,)* R> IntoWasmAbi for &'a (Fn($($var),*) -> R + 'b) where $($var: FromWasmAbi,)* R: ReturnWasmAbi { - type Abi = u32; + type Abi = WasmSlice; - fn into_abi(self, extra: &mut Stack) -> u32 { - #[allow(non_snake_case)] - unsafe extern "C" fn invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( - a: usize, - b: usize, - $($var: <$var as FromWasmAbi>::Abi),* - ) -> ::Abi { - if a == 0 { - throw_str("closure invoked recursively or destroyed already"); - } - // Scope all local variables before we call `return_abi` to - // ensure they're all destroyed as `return_abi` may throw - let ret = { - let f: &Fn($($var),*) -> R = mem::transmute((a, b)); - let mut _stack = GlobalStack::new(); - $( - let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack); - )* - f($($var),*) - }; - ret.return_abi(&mut GlobalStack::new()) - } + fn into_abi(self, _extra: &mut Stack) -> WasmSlice { unsafe { let (a, b): (usize, usize) = mem::transmute(self); - extra.push(a as u32); - extra.push(b as u32); - invoke::<$($var,)* R> as u32 + WasmSlice { ptr: a as u32, len: b as u32 } } } } + #[allow(non_snake_case)] + unsafe extern "C" fn $invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( + a: usize, + b: usize, + $($var: <$var as FromWasmAbi>::Abi),* + ) -> ::Abi { + if a == 0 { + throw_str("closure invoked recursively or destroyed already"); + } + // Scope all local variables before we call `return_abi` to + // ensure they're all destroyed as `return_abi` may throw + let ret = { + let f: &Fn($($var),*) -> R = mem::transmute((a, b)); + let mut _stack = GlobalStack::new(); + $( + let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack); + )* + f($($var),*) + }; + ret.return_abi(&mut GlobalStack::new()) + } + + impl<'a, $($var,)* R> WasmDescribe for Fn($($var),*) -> R + 'a + where $($var: FromWasmAbi,)* + R: ReturnWasmAbi + { + fn describe() { + inform(FUNCTION); + inform($invoke::<$($var,)* R> as u32); + inform($cnt); + $(<$var as WasmDescribe>::describe();)* + ::describe(); + } + } + impl<'a, 'b, $($var,)* R> IntoWasmAbi for &'a mut (FnMut($($var),*) -> R + 'b) where $($var: FromWasmAbi,)* R: ReturnWasmAbi { - type Abi = u32; + type Abi = WasmSlice; - fn into_abi(self, extra: &mut Stack) -> u32 { - #[allow(non_snake_case)] - unsafe extern "C" fn invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( - a: usize, - b: usize, - $($var: <$var as FromWasmAbi>::Abi),* - ) -> ::Abi { - if a == 0 { - throw_str("closure invoked recursively or destroyed already"); - } - // Scope all local variables before we call `return_abi` to - // ensure they're all destroyed as `return_abi` may throw - let ret = { - let f: &mut FnMut($($var),*) -> R = mem::transmute((a, b)); - let mut _stack = GlobalStack::new(); - $( - let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack); - )* - f($($var),*) - }; - ret.return_abi(&mut GlobalStack::new()) - } + fn into_abi(self, _extra: &mut Stack) -> WasmSlice { unsafe { let (a, b): (usize, usize) = mem::transmute(self); - extra.push(a as u32); - extra.push(b as u32); - invoke::<$($var,)* R> as u32 + WasmSlice { ptr: a as u32, len: b as u32 } } } } + + #[allow(non_snake_case)] + unsafe extern "C" fn $invoke_mut<$($var: FromWasmAbi,)* R: ReturnWasmAbi>( + a: usize, + b: usize, + $($var: <$var as FromWasmAbi>::Abi),* + ) -> ::Abi { + if a == 0 { + throw_str("closure invoked recursively or destroyed already"); + } + // Scope all local variables before we call `return_abi` to + // ensure they're all destroyed as `return_abi` may throw + let ret = { + let f: &mut FnMut($($var),*) -> R = mem::transmute((a, b)); + let mut _stack = GlobalStack::new(); + $( + let $var = <$var as FromWasmAbi>::from_abi($var, &mut _stack); + )* + f($($var),*) + }; + ret.return_abi(&mut GlobalStack::new()) + } + + impl<'a, $($var,)* R> WasmDescribe for FnMut($($var),*) -> R + 'a + where $($var: FromWasmAbi,)* + R: ReturnWasmAbi + { + fn describe() { + inform(FUNCTION); + inform($invoke_mut::<$($var,)* R> as u32); + inform($cnt); + $(<$var as WasmDescribe>::describe();)* + ::describe(); + } + } )*) } stack_closures! { - () - (A) - (A B) - (A B C) - (A B C D) - (A B C D E) - (A B C D E F) - (A B C D E F G) + (0 invoke0 invoke0_mut) + (1 invoke1 invoke1_mut A) + (2 invoke2 invoke2_mut A B) + (3 invoke3 invoke3_mut A B C) + (4 invoke4 invoke4_mut A B C D) + (5 invoke5 invoke5_mut A B C D E) + (6 invoke6 invoke6_mut A B C D E F) + (7 invoke7 invoke7_mut A B C D E F G) } diff --git a/src/describe.rs b/src/describe.rs index d4c53f87..6453199d 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -133,72 +133,6 @@ if_std! { } } -macro_rules! cnt { - () => { - 0 - }; - (A) => { - 1 - }; - (A B) => { - 2 - }; - (A B C) => { - 3 - }; - (A B C D) => { - 4 - }; - (A B C D E) => { - 5 - }; - (A B C D E F) => { - 6 - }; - (A B C D E F G) => { - 7 - }; -} - -macro_rules! doit { - ($( ($($var:ident)*))*) => ($( - impl<'a, $($var,)* R> WasmDescribe for Fn($($var),*) -> R + 'a - where $($var: WasmDescribe,)* - R: WasmDescribe - { - fn describe() { - inform(FUNCTION); - inform(cnt!($($var)*)); - $(<$var as WasmDescribe>::describe();)* - ::describe(); - } - } - - impl<'a, $($var,)* R> WasmDescribe for FnMut($($var),*) -> R + 'a - where $($var: WasmDescribe,)* - R: WasmDescribe - { - fn describe() { - inform(FUNCTION); - inform(cnt!($($var)*)); - $(<$var as WasmDescribe>::describe();)* - ::describe(); - } - } - )*) -} - -doit! { - () - (A) - (A B) - (A B C) - (A B C D) - (A B C D E) - (A B C D E F) - (A B C D E F G) -} - impl WasmDescribe for Option { fn describe() { inform(OPTIONAL); diff --git a/src/lib.rs b/src/lib.rs index 379e26ea..72e89142 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -483,7 +483,7 @@ externs! { fn __wbindgen_cb_forget(idx: u32) -> (); fn __wbindgen_describe(v: u32) -> (); - fn __wbindgen_describe_closure(a: u32, b: u32, c: u32, d: u32, e: u32) -> u32; + fn __wbindgen_describe_closure(a: u32, b: u32, c: u32) -> u32; fn __wbindgen_json_parse(ptr: *const u8, len: usize) -> u32; fn __wbindgen_json_serialize(idx: u32, ptr: *mut *mut u8) -> usize;