From ac87c8215bdd28d6aa0e12705996238a78227f8c Mon Sep 17 00:00:00 2001 From: Cormac Relf Date: Tue, 14 Dec 2021 03:12:13 +1100 Subject: [PATCH] Fix `fn () -> Result` leaking stack space (#2710) * add Descriptor RESULT and Instruction::UnwrapResult * ResultAbi / ResultAbiUnion basic wasmresult support one WasmResult ctor per WasmAbi Remove WasmResult class * Reverse the fields on ResultAbi, remove is_ok as err can be 0 impl OptionIntoWasmAbi for JsValue * implement ResultAbi * Add tests for `-> Result` * split result.rs tests in two remove console_log * initial implementation of `-> Result` describe Result<_, JsError> implement Intrinsics::ErrorNew Revert impl From<()> for JsValue (src/lib.rs) impl WasmDescribe etc for JsError remove unused JsError * docs on JsError, move to lib.rs * Add test for returning Result<(), _> * Add failing test for returning `Result, _>` This fails because the generated LoadRetptr instructions do not have any conception of how big the previous args are. It only fails when any part of T is an f64. * Make LoadRetptr offsets factor in alignment and previously read values * Add a doc comment to UnwrapResult * Slight correction to a comment * Better error message * un-implement OptionIntoWasmAbi for JsValue, use discriminant instead * Add some documentation from the PR discussion to ResultAbi * un-implement OptionIntoWasmAbi for &'a JsValue too * bless some UI tests, not sure why * bless the CLI's output tests * fix indentation of if (is_ok === 0) { throw ... } code * add tests for async fn() -> Result<_, JsError> and custom error types * cargo fmt * fix bug where takeObject was missing * support externref in UnwrapResult * add a WASM_BINDGEN_EXTERNREF=1 test to ci * getFromExternrefTable -> takeFromExternrefTable Now we do not leak externrefs, as the take function calls drop on them. * rewrite outgoing_result There was a crash where _outgoing inserted more than one instruction, e.g. for string. In that case, the deferred free() call was using the wrong popped values, and tried to free nonsense formed by the is_ok/err pair. Now it does a similar idea, but without assuming exactly one instruction will be pushed by self._outgoing(). * rename is_ok -> is_err, which makes generated glue easier to read * update ui tests * add a crashing (if you uncomment the throw) test of Result * add result-string reference test * Fix the crashy Result by setting ptr/len to 0 on error --- .github/workflows/main.yml | 5 + crates/cli-support/src/descriptor.rs | 3 + crates/cli-support/src/externref.rs | 10 + crates/cli-support/src/intrinsic.rs | 3 + crates/cli-support/src/js/binding.rs | 77 ++++++- crates/cli-support/src/js/mod.rs | 33 +++ crates/cli-support/src/wit/incoming.rs | 1 + crates/cli-support/src/wit/mod.rs | 49 ++++- crates/cli-support/src/wit/nonstandard.rs | 1 + crates/cli-support/src/wit/outgoing.rs | 145 ++++++++++++- crates/cli-support/src/wit/section.rs | 4 + crates/cli-support/src/wit/standard.rs | 17 ++ .../tests/reference/anyref-import-catch.js | 30 ++- .../tests/reference/anyref-import-catch.wat | 7 +- crates/cli/tests/reference/import-catch.js | 52 +++-- crates/cli/tests/reference/import-catch.wat | 12 +- crates/cli/tests/reference/result-string.d.ts | 6 + crates/cli/tests/reference/result-string.js | 85 ++++++++ crates/cli/tests/reference/result-string.rs | 6 + crates/cli/tests/reference/result-string.wat | 12 ++ crates/externref-xform/src/lib.rs | 2 + crates/macro/ui-tests/async-errors.stderr | 28 +-- crates/macro/ui-tests/start-function.stderr | 28 +-- src/convert/impls.rs | 75 ++++++- src/describe.rs | 16 +- src/lib.rs | 91 +++++++++ tests/wasm/futures.js | 2 + tests/wasm/futures.rs | 22 ++ tests/wasm/main.rs | 2 + tests/wasm/result.js | 126 ++++++++++++ tests/wasm/result.rs | 190 ++++++++++++++++++ tests/wasm/result_jserror.js | 54 +++++ tests/wasm/result_jserror.rs | 118 +++++++++++ 33 files changed, 1236 insertions(+), 76 deletions(-) create mode 100644 crates/cli/tests/reference/result-string.d.ts create mode 100644 crates/cli/tests/reference/result-string.js create mode 100644 crates/cli/tests/reference/result-string.rs create mode 100644 crates/cli/tests/reference/result-string.wat create mode 100644 tests/wasm/result.js create mode 100644 tests/wasm/result.rs create mode 100644 tests/wasm/result_jserror.js create mode 100644 tests/wasm/result_jserror.rs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 80f7042c053..90fc03cb08b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -62,6 +62,11 @@ jobs: - run: cargo test --target wasm32-unknown-unknown --test wasm --features serde-serialize env: WASM_BINDGEN_WEAKREF: 1 + - run: cargo test --target wasm32-unknown-unknown + env: + WASM_BINDGEN_EXTERNREF: 1 + NODE_ARGS: --experimental-wasm-reftypes + test_wasm_bindgen_windows: name: "Run wasm-bindgen crate tests (Windows)" diff --git a/crates/cli-support/src/descriptor.rs b/crates/cli-support/src/descriptor.rs index ae65f9e9adf..a3b73a2221a 100644 --- a/crates/cli-support/src/descriptor.rs +++ b/crates/cli-support/src/descriptor.rs @@ -36,6 +36,7 @@ tys! { RUST_STRUCT CHAR OPTIONAL + RESULT UNIT CLAMPED } @@ -68,6 +69,7 @@ pub enum Descriptor { RustStruct(String), Char, Option(Box), + Result(Box), Unit, } @@ -133,6 +135,7 @@ impl Descriptor { SLICE => Descriptor::Slice(Box::new(Descriptor::_decode(data, clamped))), VECTOR => Descriptor::Vector(Box::new(Descriptor::_decode(data, clamped))), OPTIONAL => Descriptor::Option(Box::new(Descriptor::_decode(data, clamped))), + RESULT => Descriptor::Result(Box::new(Descriptor::_decode(data, clamped))), CACHED_STRING => Descriptor::CachedString, STRING => Descriptor::String, EXTERNREF => Descriptor::Externref, diff --git a/crates/cli-support/src/externref.rs b/crates/cli-support/src/externref.rs index 11127be4a03..8aff6a4853a 100644 --- a/crates/cli-support/src/externref.rs +++ b/crates/cli-support/src/externref.rs @@ -70,6 +70,7 @@ pub fn process(module: &mut Module) -> Result<()> { aux.externref_table = Some(meta.table); if module_needs_externref_metadata(&aux, section) { aux.externref_alloc = meta.alloc; + aux.externref_drop = meta.drop; aux.externref_drop_slice = meta.drop_slice; } @@ -108,6 +109,15 @@ pub fn process(module: &mut Module) -> Result<()> { } => { *table_and_alloc = meta.alloc.map(|id| (meta.table, id)); } + + Instruction::UnwrapResult { + ref mut table_and_drop, + } + | Instruction::UnwrapResultString { + ref mut table_and_drop, + } => { + *table_and_drop = meta.drop.map(|id| (meta.table, id)); + } _ => continue, }; } diff --git a/crates/cli-support/src/intrinsic.rs b/crates/cli-support/src/intrinsic.rs index 538e32d1e92..efdc131ca21 100644 --- a/crates/cli-support/src/intrinsic.rs +++ b/crates/cli-support/src/intrinsic.rs @@ -220,6 +220,9 @@ intrinsics! { #[symbol = "__wbindgen_rethrow"] #[signature = fn(Externref) -> Unit] Rethrow, + #[symbol = "__wbindgen_error_new"] + #[signature = fn(ref_string()) -> Externref] + ErrorNew, #[symbol = "__wbindgen_memory"] #[signature = fn() -> Externref] Memory, diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index 3867b6fa6a6..b0d30eae4cf 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -596,16 +596,20 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) -> } Instruction::LoadRetptr { ty, offset, mem } => { - let (mem, size) = match ty { - AdapterType::I32 => (js.cx.expose_int32_memory(*mem), 4), - AdapterType::F32 => (js.cx.expose_f32_memory(*mem), 4), - AdapterType::F64 => (js.cx.expose_f64_memory(*mem), 8), + let (mem, quads) = match ty { + AdapterType::I32 => (js.cx.expose_int32_memory(*mem), 1), + AdapterType::F32 => (js.cx.expose_f32_memory(*mem), 1), + AdapterType::F64 => (js.cx.expose_f64_memory(*mem), 2), other => bail!("invalid aggregate return type {:?}", other), }; + let size = quads * 4; + // Separate the offset and the scaled offset, because otherwise you don't guarantee + // that the variable names will be unique. + let scaled_offset = offset / quads; // If we're loading from the return pointer then we must have pushed // it earlier, and we always push the same value, so load that value // here - let expr = format!("{}()[retptr / {} + {}]", mem, size, offset); + let expr = format!("{}()[retptr / {} + {}]", mem, size, scaled_offset); js.prelude(&format!("var r{} = {};", offset, expr)); js.push(format!("r{}", offset)); } @@ -783,6 +787,69 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) -> js.push(format!("len{}", i)); } + Instruction::UnwrapResult { table_and_drop } => { + let take_object = if let Some((table, drop)) = *table_and_drop { + js.cx + .expose_take_from_externref_table(table, drop)? + .to_string() + } else { + js.cx.expose_take_object(); + "takeObject".to_string() + }; + // is_err is popped first. The original layout was: ResultAbi { + // abi: ResultAbiUnion, + // err: u32, + // is_err: u32, + // } + // So is_err is last to be added to the stack. + let is_err = js.pop(); + let err = js.pop(); + js.prelude(&format!( + " + if ({is_err}) {{ + throw {take_object}({err}); + }} + ", + take_object = take_object, + is_err = is_err, + err = err, + )); + } + + Instruction::UnwrapResultString { table_and_drop } => { + let take_object = if let Some((table, drop)) = *table_and_drop { + js.cx + .expose_take_from_externref_table(table, drop)? + .to_string() + } else { + js.cx.expose_take_object(); + "takeObject".to_string() + }; + let is_err = js.pop(); + let err = js.pop(); + let len = js.pop(); + let ptr = js.pop(); + let i = js.tmp(); + js.prelude(&format!( + " + var ptr{i} = {ptr}; + var len{i} = {len}; + if ({is_err}) {{ + ptr{i} = 0; len{i} = 0; + throw {take_object}({err}); + }} + ", + take_object = take_object, + is_err = is_err, + err = err, + i = i, + ptr = ptr, + len = len, + )); + js.push(format!("ptr{}", i)); + js.push(format!("len{}", i)); + } + Instruction::OptionString { mem, malloc, diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 724aafbef3e..b0b5136a9d9 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -2194,6 +2194,34 @@ impl<'a> Context<'a> { true } + fn expose_take_from_externref_table( + &mut self, + table: TableId, + drop: FunctionId, + ) -> Result { + let view = self.memview_table("takeFromExternrefTable", table); + assert!(self.config.externref); + if !self.should_write_global(view.to_string()) { + return Ok(view); + } + let drop = self.export_name_of(drop); + let table = self.export_name_of(table); + self.global(&format!( + " + function {view}(idx) {{ + const value = wasm.{table}.get(idx); + wasm.{drop}(idx); + return value; + }} + ", + view = view, + table = table, + drop = drop, + )); + + Ok(view) + } + fn expose_add_to_externref_table( &mut self, table: TableId, @@ -3141,6 +3169,11 @@ impl<'a> Context<'a> { format!("throw {}", args[0]) } + Intrinsic::ErrorNew => { + assert_eq!(args.len(), 1); + format!("new Error({})", args[0]) + } + Intrinsic::Module => { assert_eq!(args.len(), 0); if !self.config.mode.no_modules() && !self.config.mode.web() { diff --git a/crates/cli-support/src/wit/incoming.rs b/crates/cli-support/src/wit/incoming.rs index a911535f485..1778516caad 100644 --- a/crates/cli-support/src/wit/incoming.rs +++ b/crates/cli-support/src/wit/incoming.rs @@ -135,6 +135,7 @@ impl InstructionBuilder<'_, '_> { Descriptor::Function(_) | Descriptor::Closure(_) | + Descriptor::Result(_) | // Always behind a `Ref` Descriptor::Slice(_) => bail!( "unsupported argument type for calling Rust function from JS: {:?}", diff --git a/crates/cli-support/src/wit/mod.rs b/crates/cli-support/src/wit/mod.rs index 0bb382b9cfb..3154a48b86a 100644 --- a/crates/cli-support/src/wit/mod.rs +++ b/crates/cli-support/src/wit/mod.rs @@ -1346,9 +1346,11 @@ impl<'a> Context<'a> { }); if uses_retptr { let mem = ret.cx.memory()?; - for (i, ty) in ret.input.into_iter().enumerate() { + let mut unpacker = StructUnpacker::new(); + for ty in ret.input.into_iter() { + let offset = unpacker.read_ty(&ty)?; instructions.push(InstructionData { - instr: Instruction::LoadRetptr { offset: i, ty, mem }, + instr: Instruction::LoadRetptr { offset, ty, mem }, stack_change: StackChange::Modified { pushed: 1, popped: 0, @@ -1569,3 +1571,46 @@ fn verify_schema_matches<'a>(data: &'a [u8]) -> Result, Error> { fn concatenate_comments(comments: &[&str]) -> String { comments.iter().map(|&s| s).collect::>().join("\n") } + +/// The C struct packing algorithm, in terms of u32. +struct StructUnpacker { + next_offset: usize, +} + +impl StructUnpacker { + fn new() -> Self { + Self { next_offset: 0 } + } + fn align_up(&mut self, alignment_pow2: usize) -> usize { + let mask = alignment_pow2 - 1; + self.next_offset = (self.next_offset + mask) & (!mask); + self.next_offset + } + fn append(&mut self, quads: usize, alignment_pow2: usize) -> usize { + let ret = self.align_up(alignment_pow2); + self.next_offset += quads; + ret + } + /// Returns the offset for this member, with the offset in multiples of u32. + fn read_ty(&mut self, ty: &AdapterType) -> Result { + let (quads, alignment) = match ty { + AdapterType::I32 | AdapterType::U32 | AdapterType::F32 => (1, 1), + AdapterType::F64 => (2, 2), + other => bail!("invalid aggregate return type {:?}", other), + }; + Ok(self.append(quads, alignment)) + } +} + +#[test] +fn test_struct_packer() { + let mut unpacker = StructUnpacker::new(); + let i32___ = &AdapterType::I32; + let double = &AdapterType::F64; + let mut read_ty = |ty| unpacker.read_ty(ty).unwrap(); + assert_eq!(read_ty(i32___), 0); // u32 + assert_eq!(read_ty(i32___), 1); // u32 + assert_eq!(read_ty(double), 2); // f64, already aligned + assert_eq!(read_ty(i32___), 4); // u32, already aligned + assert_eq!(read_ty(double), 6); // f64, NOT already aligned, skips up to offset 6 +} diff --git a/crates/cli-support/src/wit/nonstandard.rs b/crates/cli-support/src/wit/nonstandard.rs index 35d966f602d..a1ac9eb5bde 100644 --- a/crates/cli-support/src/wit/nonstandard.rs +++ b/crates/cli-support/src/wit/nonstandard.rs @@ -52,6 +52,7 @@ pub struct WasmBindgenAux { pub externref_table: Option, pub function_table: Option, pub externref_alloc: Option, + pub externref_drop: Option, pub externref_drop_slice: Option, /// Various intrinsics used for JS glue generation diff --git a/crates/cli-support/src/wit/outgoing.rs b/crates/cli-support/src/wit/outgoing.rs index bba1fe81d8c..26dbb151122 100644 --- a/crates/cli-support/src/wit/outgoing.rs +++ b/crates/cli-support/src/wit/outgoing.rs @@ -18,8 +18,15 @@ impl InstructionBuilder<'_, '_> { let input_before = self.input.len(); let output_before = self.output.len(); self._outgoing(arg)?; - assert_eq!(output_before + 1, self.output.len()); + assert!(input_before < self.input.len()); + if let Descriptor::Result(arg) = arg { + if let Descriptor::Unit = &**arg { + assert_eq!(output_before, self.output.len()); + return Ok(()); + } + } + assert_eq!(output_before + 1, self.output.len()); Ok(()) } @@ -149,6 +156,7 @@ impl InstructionBuilder<'_, '_> { } Descriptor::Option(d) => self.outgoing_option(d)?, + Descriptor::Result(d) => self.outgoing_result(d)?, Descriptor::Function(_) | Descriptor::Closure(_) | Descriptor::Slice(_) => bail!( "unsupported argument type for calling JS function from Rust: {:?}", @@ -337,6 +345,141 @@ impl InstructionBuilder<'_, '_> { Ok(()) } + fn outgoing_result(&mut self, arg: &Descriptor) -> Result<(), Error> { + match arg { + Descriptor::Externref + | Descriptor::NamedExternref(_) + | Descriptor::I8 + | Descriptor::U8 + | Descriptor::I16 + | Descriptor::U16 + | Descriptor::I32 + | Descriptor::U32 + | Descriptor::F32 + | Descriptor::F64 + | Descriptor::I64 + | Descriptor::U64 + | Descriptor::Boolean + | Descriptor::Char + | Descriptor::Enum { .. } + | Descriptor::RustStruct(_) + | Descriptor::Ref(_) + | Descriptor::RefMut(_) + | Descriptor::CachedString + | Descriptor::Option(_) + | Descriptor::Vector(_) + | Descriptor::Unit => { + // We must throw before reading the Ok type, if there is an error. However, the + // structure of ResultAbi is that the Err value + discriminant come last (for + // alignment reasons). So the UnwrapResult instruction must come first, but the + // inputs must be read last. + // + // So first, push an UnwrapResult instruction without modifying the inputs list. + // + // [] + // -------------------------< + // UnwrapResult { popped: 2 } + // + self.instructions.push(InstructionData { + instr: Instruction::UnwrapResult { + table_and_drop: None, + }, + stack_change: StackChange::Modified { + popped: 2, + pushed: 0, + }, + }); + + // Then push whatever else you were going to do, modifying the inputs and + // instructions. + // + // [f64, u32, u32] + // -------------------------< + // UnwrapResult { popped: 2 } + // SomeOtherInstruction { popped: 3 } + // + // The popped numbers don't add up yet (3 != 5), but they will. + let len = self.instructions.len(); + self._outgoing(arg)?; + + // check we did not add any deferred calls, because we have undermined the idea of + // running them unconditionally in a finally {} block. String does this, but we + // special case it. + assert!(!self.instructions[len..].iter().any(|idata| matches!( + idata.instr, + Instruction::Standard(wit_walrus::Instruction::DeferCallCore(_)) + ))); + + // Finally, we add the two inputs to UnwrapResult, and everything checks out + // + // [f64, u32, u32, u32, u32] + // -------------------------< + // UnwrapResult { popped: 2 } + // SomeOtherInstruction { popped: 3 } + // + self.get(AdapterType::I32); + self.get(AdapterType::I32); + } + Descriptor::String => { + // fetch the ptr/length ... + self.get(AdapterType::I32); + self.get(AdapterType::I32); + // fetch the err/is_err + self.get(AdapterType::I32); + self.get(AdapterType::I32); + + self.instructions.push(InstructionData { + instr: Instruction::UnwrapResultString { + table_and_drop: None, + }, + stack_change: StackChange::Modified { + // 2 from UnwrapResult, 2 from ptr/len + popped: 4, + // pushes the ptr/len back on + pushed: 2, + }, + }); + + // ... then defer a call to `free` to happen later + // this will run string's DeferCallCore with the length parameter, but if is_err, + // then we have never written anything into that, so it is poison. So we'll have to + // make sure we call it with length 0, which according to __wbindgen_free's + // implementation is always safe. We do this in UnwrapResultString's + // implementation. + let free = self.cx.free()?; + let std = wit_walrus::Instruction::DeferCallCore(free); + self.instructions.push(InstructionData { + instr: Instruction::Standard(std), + stack_change: StackChange::Modified { + popped: 2, + pushed: 2, + }, + }); + + // ... and then convert it to a string type + let std = wit_walrus::Instruction::MemoryToString(self.cx.memory()?); + self.instructions.push(InstructionData { + instr: Instruction::Standard(std), + stack_change: StackChange::Modified { + popped: 2, + pushed: 1, + }, + }); + self.output.push(AdapterType::String); + } + + Descriptor::ClampedU8 + | Descriptor::Function(_) + | Descriptor::Closure(_) + | Descriptor::Slice(_) + | Descriptor::Result(_) => bail!( + "unsupported Result type for returning from exported Rust function: {:?}", + arg + ), + } + Ok(()) + } + fn outgoing_option_ref(&mut self, _mutable: bool, arg: &Descriptor) -> Result<(), Error> { match arg { Descriptor::Externref => { diff --git a/crates/cli-support/src/wit/section.rs b/crates/cli-support/src/wit/section.rs index 55042f12d98..8ee3f84019b 100644 --- a/crates/cli-support/src/wit/section.rs +++ b/crates/cli-support/src/wit/section.rs @@ -47,6 +47,7 @@ pub fn add(module: &mut Module) -> Result<(), Error> { // irrelevant ids used to track various internal intrinsics and such externref_table: _, externref_alloc: _, + externref_drop: _, externref_drop_slice: _, exn_store: _, shadow_stack_pointer: _, @@ -275,6 +276,9 @@ fn translate_instruction( | Option64FromI32 { .. } => { bail!("optional types aren't supported in wasm bindgen"); } + UnwrapResult { .. } | UnwrapResultString { .. } => { + bail!("self-unwrapping result types aren't supported in wasm bindgen"); + } MutableSliceToMemory { .. } | VectorToMemory { .. } | VectorLoad { .. } | View { .. } => { bail!("vector slices aren't supported in wasm interface types yet"); } diff --git a/crates/cli-support/src/wit/standard.rs b/crates/cli-support/src/wit/standard.rs index dc37aa4f037..412f10aafc6 100644 --- a/crates/cli-support/src/wit/standard.rs +++ b/crates/cli-support/src/wit/standard.rs @@ -219,6 +219,17 @@ pub enum Instruction { mem: walrus::MemoryId, }, + /// Pops a nullable externref; if it is non-zero, throws it. + UnwrapResult { + /// Similar to `I32FromOptionExternref`, + /// Set to `Some` by the externref pass, and we then take from the externref table. If + /// None, we use takeObject. + table_and_drop: Option<(walrus::TableId, walrus::FunctionId)>, + }, + UnwrapResultString { + table_and_drop: Option<(walrus::TableId, walrus::FunctionId)>, + }, + /// pops a `i32`, pushes `bool` BoolFromI32, /// pops `i32`, loads externref at that slot, dealloates externref, pushes `externref` @@ -504,6 +515,12 @@ impl walrus::CustomSection for NonstandardWitSection { roots.push_func(alloc); } } + UnwrapResult { table_and_drop } | UnwrapResultString { table_and_drop } => { + if let Some((table, drop)) = table_and_drop { + roots.push_table(table); + roots.push_func(drop); + } + } _ => {} } } diff --git a/crates/cli/tests/reference/anyref-import-catch.js b/crates/cli/tests/reference/anyref-import-catch.js index 88cb317f254..9e9399802f6 100644 --- a/crates/cli/tests/reference/anyref-import-catch.js +++ b/crates/cli/tests/reference/anyref-import-catch.js @@ -32,10 +32,34 @@ function handleError(f, args) { wasm.__wbindgen_exn_store(idx); } } + +let cachegetInt32Memory0 = null; +function getInt32Memory0() { + if (cachegetInt32Memory0 === null || cachegetInt32Memory0.buffer !== wasm.memory.buffer) { + cachegetInt32Memory0 = new Int32Array(wasm.memory.buffer); + } + return cachegetInt32Memory0; +} + +function takeFromExternrefTable0(idx) { + const value = wasm.__wbindgen_export_0.get(idx); + wasm.__externref_table_dealloc(idx); + return value; +} /** */ export function exported() { - wasm.exported(); + try { + const retptr = wasm.__wbindgen_add_to_stack_pointer(-16); + wasm.exported(retptr); + var r0 = getInt32Memory0()[retptr / 4 + 0]; + var r1 = getInt32Memory0()[retptr / 4 + 1]; + if (r1) { + throw takeFromExternrefTable0(r0); + } + } finally { + wasm.__wbindgen_add_to_stack_pointer(16); + } } export function __wbg_foo_8d66ddef0ff279d6() { return handleError(function () { @@ -46,10 +70,6 @@ export function __wbindgen_throw(arg0, arg1) { throw new Error(getStringFromWasm0(arg0, arg1)); }; -export function __wbindgen_rethrow(arg0) { - throw arg0; -}; - export function __wbindgen_init_externref_table() { const table = wasm.__wbindgen_export_0; const offset = table.grow(4); diff --git a/crates/cli/tests/reference/anyref-import-catch.wat b/crates/cli/tests/reference/anyref-import-catch.wat index fc7af2360ad..6b1b5b7e2cf 100644 --- a/crates/cli/tests/reference/anyref-import-catch.wat +++ b/crates/cli/tests/reference/anyref-import-catch.wat @@ -2,10 +2,13 @@ (type (;0;) (func)) (type (;1;) (func (result i32))) (type (;2;) (func (param i32))) + (type (;3;) (func (param i32) (result i32))) (import "./reference_test_bg.js" "__wbindgen_init_externref_table" (func (;0;) (type 0))) (func $__wbindgen_exn_store (type 2) (param i32)) + (func $__externref_table_dealloc (type 2) (param i32)) + (func $exported (type 2) (param i32)) (func $__externref_table_alloc (type 1) (result i32)) - (func $exported (type 0)) + (func $__wbindgen_add_to_stack_pointer (type 3) (param i32) (result i32)) (table (;0;) 32 externref) (memory (;0;) 17) (export "memory" (memory 0)) @@ -13,4 +16,6 @@ (export "__wbindgen_export_0" (table 0)) (export "__wbindgen_exn_store" (func $__wbindgen_exn_store)) (export "__externref_table_alloc" (func $__externref_table_alloc)) + (export "__wbindgen_add_to_stack_pointer" (func $__wbindgen_add_to_stack_pointer)) + (export "__externref_table_dealloc" (func $__externref_table_dealloc)) (export "__wbindgen_start" (func 0))) diff --git a/crates/cli/tests/reference/import-catch.js b/crates/cli/tests/reference/import-catch.js index fc3f89ef203..adaf3c2ae34 100644 --- a/crates/cli/tests/reference/import-catch.js +++ b/crates/cli/tests/reference/import-catch.js @@ -4,22 +4,8 @@ const heap = new Array(32).fill(undefined); heap.push(undefined, null, true, false); -function getObject(idx) { return heap[idx]; } - let heap_next = heap.length; -function dropObject(idx) { - if (idx < 36) return; - heap[idx] = heap_next; - heap_next = idx; -} - -function takeObject(idx) { - const ret = getObject(idx); - dropObject(idx); - return ret; -} - function addHeapObject(obj) { if (heap_next === heap.length) heap.push(heap.length + 1); const idx = heap_next; @@ -36,17 +22,45 @@ function handleError(f, args) { wasm.__wbindgen_exn_store(addHeapObject(e)); } } + +let cachegetInt32Memory0 = null; +function getInt32Memory0() { + if (cachegetInt32Memory0 === null || cachegetInt32Memory0.buffer !== wasm.memory.buffer) { + cachegetInt32Memory0 = new Int32Array(wasm.memory.buffer); + } + return cachegetInt32Memory0; +} + +function getObject(idx) { return heap[idx]; } + +function dropObject(idx) { + if (idx < 36) return; + heap[idx] = heap_next; + heap_next = idx; +} + +function takeObject(idx) { + const ret = getObject(idx); + dropObject(idx); + return ret; +} /** */ export function exported() { - wasm.exported(); + try { + const retptr = wasm.__wbindgen_add_to_stack_pointer(-16); + wasm.exported(retptr); + var r0 = getInt32Memory0()[retptr / 4 + 0]; + var r1 = getInt32Memory0()[retptr / 4 + 1]; + if (r1) { + throw takeObject(r0); + } + } finally { + wasm.__wbindgen_add_to_stack_pointer(16); + } } export function __wbg_foo_8d66ddef0ff279d6() { return handleError(function () { foo(); }, arguments) }; -export function __wbindgen_rethrow(arg0) { - throw takeObject(arg0); -}; - diff --git a/crates/cli/tests/reference/import-catch.wat b/crates/cli/tests/reference/import-catch.wat index 702b6ae029c..1298798b7a7 100644 --- a/crates/cli/tests/reference/import-catch.wat +++ b/crates/cli/tests/reference/import-catch.wat @@ -1,9 +1,11 @@ (module - (type (;0;) (func)) - (type (;1;) (func (param i32))) - (func $__wbindgen_exn_store (type 1) (param i32)) - (func $exported (type 0)) + (type (;0;) (func (param i32))) + (type (;1;) (func (param i32) (result i32))) + (func $__wbindgen_exn_store (type 0) (param i32)) + (func $exported (type 0) (param i32)) + (func $__wbindgen_add_to_stack_pointer (type 1) (param i32) (result i32)) (memory (;0;) 17) (export "memory" (memory 0)) (export "exported" (func $exported)) - (export "__wbindgen_exn_store" (func $__wbindgen_exn_store))) + (export "__wbindgen_exn_store" (func $__wbindgen_exn_store)) + (export "__wbindgen_add_to_stack_pointer" (func $__wbindgen_add_to_stack_pointer))) diff --git a/crates/cli/tests/reference/result-string.d.ts b/crates/cli/tests/reference/result-string.d.ts new file mode 100644 index 00000000000..50967eed5cf --- /dev/null +++ b/crates/cli/tests/reference/result-string.d.ts @@ -0,0 +1,6 @@ +/* tslint:disable */ +/* eslint-disable */ +/** +* @returns {string} +*/ +export function exported(): string; diff --git a/crates/cli/tests/reference/result-string.js b/crates/cli/tests/reference/result-string.js new file mode 100644 index 00000000000..d395375ff9f --- /dev/null +++ b/crates/cli/tests/reference/result-string.js @@ -0,0 +1,85 @@ +import * as wasm from './reference_test_bg.wasm'; + +const heap = new Array(32).fill(undefined); + +heap.push(undefined, null, true, false); + +let heap_next = heap.length; + +function addHeapObject(obj) { + if (heap_next === heap.length) heap.push(heap.length + 1); + const idx = heap_next; + heap_next = heap[idx]; + + heap[idx] = obj; + return idx; +} + +let cachegetInt32Memory0 = null; +function getInt32Memory0() { + if (cachegetInt32Memory0 === null || cachegetInt32Memory0.buffer !== wasm.memory.buffer) { + cachegetInt32Memory0 = new Int32Array(wasm.memory.buffer); + } + return cachegetInt32Memory0; +} + +function getObject(idx) { return heap[idx]; } + +function dropObject(idx) { + if (idx < 36) return; + heap[idx] = heap_next; + heap_next = idx; +} + +function takeObject(idx) { + const ret = getObject(idx); + dropObject(idx); + return ret; +} + +const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder; + +let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true }); + +cachedTextDecoder.decode(); + +let cachegetUint8Memory0 = null; +function getUint8Memory0() { + if (cachegetUint8Memory0 === null || cachegetUint8Memory0.buffer !== wasm.memory.buffer) { + cachegetUint8Memory0 = new Uint8Array(wasm.memory.buffer); + } + return cachegetUint8Memory0; +} + +function getStringFromWasm0(ptr, len) { + return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len)); +} +/** +* @returns {string} +*/ +export function exported() { + try { + const retptr = wasm.__wbindgen_add_to_stack_pointer(-16); + wasm.exported(retptr); + var r0 = getInt32Memory0()[retptr / 4 + 0]; + var r1 = getInt32Memory0()[retptr / 4 + 1]; + var r2 = getInt32Memory0()[retptr / 4 + 2]; + var r3 = getInt32Memory0()[retptr / 4 + 3]; + var ptr0 = r0; + var len0 = r1; + if (r3) { + ptr0 = 0; len0 = 0; + throw takeObject(r2); + } + return getStringFromWasm0(ptr0, len0); + } finally { + wasm.__wbindgen_add_to_stack_pointer(16); + wasm.__wbindgen_free(ptr0, len0); + } +} + +export function __wbindgen_number_new(arg0) { + var ret = arg0; + return addHeapObject(ret); +}; + diff --git a/crates/cli/tests/reference/result-string.rs b/crates/cli/tests/reference/result-string.rs new file mode 100644 index 00000000000..eb965cbde2b --- /dev/null +++ b/crates/cli/tests/reference/result-string.rs @@ -0,0 +1,6 @@ +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub fn exported() -> Result { + Err(JsValue::from(5i32)) +} diff --git a/crates/cli/tests/reference/result-string.wat b/crates/cli/tests/reference/result-string.wat new file mode 100644 index 00000000000..efe5e65189c --- /dev/null +++ b/crates/cli/tests/reference/result-string.wat @@ -0,0 +1,12 @@ +(module + (type (;0;) (func (param i32))) + (type (;1;) (func (param i32) (result i32))) + (type (;2;) (func (param i32 i32))) + (func $exported (type 0) (param i32)) + (func $__wbindgen_free (type 2) (param i32 i32)) + (func $__wbindgen_add_to_stack_pointer (type 1) (param i32) (result i32)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "exported" (func $exported)) + (export "__wbindgen_add_to_stack_pointer" (func $__wbindgen_add_to_stack_pointer)) + (export "__wbindgen_free" (func $__wbindgen_free))) diff --git a/crates/externref-xform/src/lib.rs b/crates/externref-xform/src/lib.rs index 563cabfdd18..168c108fe8e 100644 --- a/crates/externref-xform/src/lib.rs +++ b/crates/externref-xform/src/lib.rs @@ -56,6 +56,7 @@ pub struct Context { pub struct Meta { pub table: TableId, pub alloc: Option, + pub drop: Option, pub drop_slice: Option, pub live_count: Option, } @@ -263,6 +264,7 @@ impl Context { Ok(Meta { table, alloc: heap_alloc, + drop: heap_dealloc, drop_slice, live_count, }) diff --git a/crates/macro/ui-tests/async-errors.stderr b/crates/macro/ui-tests/async-errors.stderr index 8399f793093..492492a181e 100644 --- a/crates/macro/ui-tests/async-errors.stderr +++ b/crates/macro/ui-tests/async-errors.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `Result<(), ()>: IntoJsResult` is not satisfied - --> $DIR/async-errors.rs:30:1 + --> ui-tests/async-errors.rs:30:1 | 30 | #[wasm_bindgen] | ^^^^^^^^^^^^^^^ the trait `IntoJsResult` is not implemented for `Result<(), ()>` @@ -8,14 +8,14 @@ error[E0277]: the trait bound `Result<(), ()>: IntoJsResult` is not satisfied as IntoJsResult> as IntoJsResult> note: required by `into_js_result` - --> $DIR/lib.rs:1512:9 + --> $WORKSPACE/src/lib.rs | -1512 | fn into_js_result(self) -> Result; + | fn into_js_result(self) -> Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Result<(), BadType>: IntoJsResult` is not satisfied - --> $DIR/async-errors.rs:32:1 + --> ui-tests/async-errors.rs:32:1 | 32 | #[wasm_bindgen] | ^^^^^^^^^^^^^^^ the trait `IntoJsResult` is not implemented for `Result<(), BadType>` @@ -24,14 +24,14 @@ error[E0277]: the trait bound `Result<(), BadType>: IntoJsResult` is not satisfi as IntoJsResult> as IntoJsResult> note: required by `into_js_result` - --> $DIR/lib.rs:1512:9 + --> $WORKSPACE/src/lib.rs | -1512 | fn into_js_result(self) -> Result; + | fn into_js_result(self) -> Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `wasm_bindgen::JsValue: From` is not satisfied - --> $DIR/async-errors.rs:34:1 + --> ui-tests/async-errors.rs:34:1 | 34 | #[wasm_bindgen] | ^^^^^^^^^^^^^^^ the trait `From` is not implemented for `wasm_bindgen::JsValue` @@ -40,19 +40,19 @@ error[E0277]: the trait bound `wasm_bindgen::JsValue: From` is not sati > > > - > - and 72 others + > + and 73 others = note: required because of the requirements on the impl of `Into` for `BadType` = note: required because of the requirements on the impl of `IntoJsResult` for `BadType` note: required by `into_js_result` - --> $DIR/lib.rs:1512:9 + --> $WORKSPACE/src/lib.rs | -1512 | fn into_js_result(self) -> Result; + | fn into_js_result(self) -> Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Result: IntoJsResult` is not satisfied - --> $DIR/async-errors.rs:36:1 + --> ui-tests/async-errors.rs:36:1 | 36 | #[wasm_bindgen] | ^^^^^^^^^^^^^^^ the trait `IntoJsResult` is not implemented for `Result` @@ -61,8 +61,8 @@ error[E0277]: the trait bound `Result: IntoJsRes as IntoJsResult> as IntoJsResult> note: required by `into_js_result` - --> $DIR/lib.rs:1512:9 + --> $WORKSPACE/src/lib.rs | -1512 | fn into_js_result(self) -> Result; + | fn into_js_result(self) -> Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/crates/macro/ui-tests/start-function.stderr b/crates/macro/ui-tests/start-function.stderr index 326233e0e32..8f459863d2e 100644 --- a/crates/macro/ui-tests/start-function.stderr +++ b/crates/macro/ui-tests/start-function.stderr @@ -1,17 +1,17 @@ error: the start function cannot have arguments - --> $DIR/start-function.rs:7:13 + --> ui-tests/start-function.rs:7:13 | 7 | pub fn foo2(x: u32) {} | ^^^^^^ error: the start function cannot have generics - --> $DIR/start-function.rs:10:12 + --> ui-tests/start-function.rs:10:12 | 10 | pub fn foo3() {} | ^^^ error[E0277]: the trait bound `Result: wasm_bindgen::__rt::Start` is not satisfied - --> $DIR/start-function.rs:15:1 + --> ui-tests/start-function.rs:15:1 | 15 | #[wasm_bindgen(start)] | ^^^^^^^^^^^^^^^^^^^^^^ the trait `wasm_bindgen::__rt::Start` is not implemented for `Result` @@ -19,14 +19,14 @@ error[E0277]: the trait bound `Result: wasm_bindgen:: = help: the following implementations were found: as wasm_bindgen::__rt::Start> note: required by `start` - --> $DIR/lib.rs:1548:9 + --> $WORKSPACE/src/lib.rs | -1548 | fn start(self); + | fn start(self); | ^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Result: wasm_bindgen::__rt::Start` is not satisfied - --> $DIR/start-function.rs:18:1 + --> ui-tests/start-function.rs:18:1 | 18 | #[wasm_bindgen(start)] | ^^^^^^^^^^^^^^^^^^^^^^ the trait `wasm_bindgen::__rt::Start` is not implemented for `Result` @@ -34,14 +34,14 @@ error[E0277]: the trait bound `Result as wasm_bindgen::__rt::Start> note: required by `start` - --> $DIR/lib.rs:1548:9 + --> $WORKSPACE/src/lib.rs | -1548 | fn start(self); + | fn start(self); | ^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Result: wasm_bindgen::__rt::Start` is not satisfied - --> $DIR/start-function.rs:27:1 + --> ui-tests/start-function.rs:27:1 | 27 | #[wasm_bindgen(start)] | ^^^^^^^^^^^^^^^^^^^^^^ the trait `wasm_bindgen::__rt::Start` is not implemented for `Result` @@ -49,14 +49,14 @@ error[E0277]: the trait bound `Result: wasm_bindgen:: = help: the following implementations were found: as wasm_bindgen::__rt::Start> note: required by `start` - --> $DIR/lib.rs:1548:9 + --> $WORKSPACE/src/lib.rs | -1548 | fn start(self); + | fn start(self); | ^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Result: wasm_bindgen::__rt::Start` is not satisfied - --> $DIR/start-function.rs:30:1 + --> ui-tests/start-function.rs:30:1 | 30 | #[wasm_bindgen(start)] | ^^^^^^^^^^^^^^^^^^^^^^ the trait `wasm_bindgen::__rt::Start` is not implemented for `Result` @@ -64,8 +64,8 @@ error[E0277]: the trait bound `Result as wasm_bindgen::__rt::Start> note: required by `start` - --> $DIR/lib.rs:1548:9 + --> $WORKSPACE/src/lib.rs | -1548 | fn start(self); + | fn start(self); | ^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/src/convert/impls.rs b/src/convert/impls.rs index 89f74dbfa16..429a32896af 100644 --- a/src/convert/impls.rs +++ b/src/convert/impls.rs @@ -4,7 +4,7 @@ use core::mem::{self, ManuallyDrop}; use crate::convert::traits::WasmAbi; use crate::convert::{FromWasmAbi, IntoWasmAbi, RefFromWasmAbi}; use crate::convert::{OptionFromWasmAbi, OptionIntoWasmAbi, ReturnWasmAbi}; -use crate::{Clamped, JsValue}; +use crate::{Clamped, JsError, JsValue}; unsafe impl WasmAbi for () {} @@ -396,14 +396,79 @@ impl IntoWasmAbi for () { } } -impl ReturnWasmAbi for Result { - type Abi = T::Abi; +/// This is an encoding of a Result. It can only store things that can be decoded by the JS +/// bindings. +/// +/// At the moment, we do not write the exact struct packing layout of everything into the +/// glue/descriptions of datatypes, so T cannot be arbitrary. The current requirements of the +/// struct unpacker (StructUnpacker), which apply to ResultAbi as a whole, are as follows: +/// +/// - repr(C), of course +/// - u32/i32/f32/f64 fields at the "leaf fields" of the "field tree" +/// - layout equivalent to a completely flattened repr(C) struct, constructed by an in order +/// traversal of all the leaf fields in it. +/// +/// This means that you can't embed struct A(u32, f64) as struct B(u32, A); because the "completely +/// flattened" struct AB(u32, u32, f64) would miss the 4 byte padding that is actually present +/// within B and then as a consequence also miss the 4 byte padding within A that repr(C) inserts. +/// +/// The enemy is padding. Padding is only required when there is an `f64` field. So the enemy is +/// `f64` after anything else, particularly anything arbitrary. There is no smaller sized type, so +/// we don't need to worry about 1-byte integers, etc. It's best, therefore, to place your f64s +/// first in your structs, that's why we have `abi` first, although here it doesn't matter as the +/// other two fields total 8 bytes anyway. +/// +#[repr(C)] +pub struct ResultAbi { + /// This field is the same size/align as `T`. + abi: ResultAbiUnion, + /// Order of args here is such that we can pop() the possible error first, deal with it and + /// move on. Later fields are popped off the stack first. + err: u32, + is_err: u32, +} + +#[repr(C)] +pub union ResultAbiUnion { + // ManuallyDrop is #[repr(transparent)] + ok: std::mem::ManuallyDrop, + err: (), +} +unsafe impl WasmAbi for ResultAbi {} +unsafe impl WasmAbi for ResultAbiUnion {} + +impl> ReturnWasmAbi for Result { + type Abi = ResultAbi; #[inline] fn return_abi(self) -> Self::Abi { match self { - Ok(v) => v.into_abi(), - Err(e) => crate::throw_val(e), + Ok(v) => { + let abi = ResultAbiUnion { + ok: std::mem::ManuallyDrop::new(v.into_abi()), + }; + ResultAbi { + abi, + is_err: 0, + err: 0, + } + } + Err(e) => { + let jsval = e.into(); + ResultAbi { + abi: ResultAbiUnion { err: () }, + is_err: 1, + err: jsval.into_abi(), + } + } } } } + +impl IntoWasmAbi for JsError { + type Abi = ::Abi; + + fn into_abi(self) -> Self::Abi { + self.value.into_abi() + } +} diff --git a/src/describe.rs b/src/describe.rs index 2d424f8f3f7..84fab48f779 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -3,7 +3,7 @@ #![doc(hidden)] -use crate::{Clamped, JsValue}; +use crate::{Clamped, JsError, JsValue}; use cfg_if::cfg_if; macro_rules! tys { @@ -42,6 +42,7 @@ tys! { RUST_STRUCT CHAR OPTIONAL + RESULT UNIT CLAMPED } @@ -170,11 +171,10 @@ impl WasmDescribe for () { } } -// Note that this is only for `ReturnWasmAbi for Result`, which -// throws the result, so we only need to inform about the `T`. -impl WasmDescribe for Result { +impl> WasmDescribe for Result { fn describe() { - T::describe() + inform(RESULT); + T::describe(); } } @@ -184,3 +184,9 @@ impl WasmDescribe for Clamped { T::describe(); } } + +impl WasmDescribe for JsError { + fn describe() { + JsValue::describe(); + } +} diff --git a/src/lib.rs b/src/lib.rs index 087a85bd589..15a622c84cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,6 +60,8 @@ pub mod prelude { if_std! { pub use crate::closure::Closure; } + + pub use crate::JsError; } pub mod convert; @@ -924,6 +926,7 @@ externs! { fn __wbindgen_throw(a: *const u8, b: usize) -> !; fn __wbindgen_rethrow(a: u32) -> !; + fn __wbindgen_error_new(a: *const u8, b: usize) -> u32; fn __wbindgen_cb_drop(idx: u32) -> u32; @@ -1591,3 +1594,91 @@ impl DerefMut for Clamped { &mut self.0 } } + +/// Convenience type for use on exported `fn() -> Result` functions, where you wish to +/// throw a JavaScript `Error` object. +/// +/// You can get wasm_bindgen to throw basic errors by simply returning +/// `Err(JsError::new("message"))` from such a function. +/// +/// For more complex error handling, `JsError` implements `From where T: std::error::Error` by +/// converting it to a string, so you can use it with `?`. Many Rust error types already do this, +/// and you can use [`thiserror`](https://crates.io/crates/thiserror) to derive Display +/// implementations easily or use any number of boxed error types that implement it already. +/// +/// +/// To allow JavaScript code to catch only your errors, you may wish to add a subclass of `Error` +/// in a JS module, and then implement `Into` directly on a type and instantiate that +/// subclass. In that case, you would not need `JsError` at all. +/// +/// ### Basic example +/// +/// ```rust,no_run +/// use wasm_bindgen::prelude::*; +/// +/// #[wasm_bindgen] +/// pub fn throwing_function() -> Result<(), JsError> { +/// Err(JsError::new("message")) +/// } +/// ``` +/// +/// ### Complex Example +/// +/// ```rust,no_run +/// use wasm_bindgen::prelude::*; +/// +/// #[derive(Debug, Clone)] +/// enum MyErrorType { +/// SomeError, +/// } +/// +/// use core::fmt; +/// impl std::error::Error for MyErrorType {} +/// impl fmt::Display for MyErrorType { +/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +/// write!(f, "display implementation becomes the error message") +/// } +/// } +/// +/// fn internal_api() -> Result<(), MyErrorType> { +/// Err(MyErrorType::SomeError) +/// } +/// +/// #[wasm_bindgen] +/// pub fn throwing_function() -> Result<(), JsError> { +/// internal_api()?; +/// Ok(()) +/// } +/// +/// ``` +#[derive(Clone)] +pub struct JsError { + value: JsValue, +} + +impl JsError { + /// Construct a JavaScript `Error` object with a string message + #[inline] + pub fn new(s: &str) -> JsError { + Self { + value: unsafe { JsValue::_new(crate::__wbindgen_error_new(s.as_ptr(), s.len())) }, + } + } +} + +if_std! { + impl From for JsError + where + E: std::error::Error, + { + fn from(error: E) -> Self { + JsError::new(&error.to_string()) + } + } +} + +impl From for JsValue { + fn from(error: JsError) -> Self { + error.value + } +} diff --git a/tests/wasm/futures.js b/tests/wasm/futures.js index 9da0aace80d..386125583a4 100644 --- a/tests/wasm/futures.js +++ b/tests/wasm/futures.js @@ -13,6 +13,8 @@ exports.call_exports = async function() { await assert.rejects(wasm.async_throw_7(), /7/); await assert.rejects(wasm.async_throw_custom(), /\[object Object\]/); await assert.rejects(wasm.async_throw_message(), /async message/); + await assert.rejects(wasm.async_throw_jserror(), /async message/); + await assert.rejects(wasm.async_throw_custom_error(), /custom error/); }; exports.call_promise = async function() { diff --git a/tests/wasm/futures.rs b/tests/wasm/futures.rs index 32a9edbfb5e..adf3324e826 100644 --- a/tests/wasm/futures.rs +++ b/tests/wasm/futures.rs @@ -84,6 +84,28 @@ pub async fn async_throw_message() -> Result<(), JsValue> { Err(js_sys::Error::new("async message").into()) } +#[wasm_bindgen] +pub async fn async_throw_jserror() -> Result { + Err(JsError::new("async message")) +} + +pub struct AsyncCustomError { + pub val: JsValue, +} + +impl Into for AsyncCustomError { + fn into(self) -> JsValue { + self.val + } +} + +#[wasm_bindgen] +pub async fn async_throw_custom_error() -> Result { + Err(AsyncCustomError { + val: JsValue::from("custom error"), + }) +} + #[wasm_bindgen_test] async fn test_promise() { assert_eq!(call_promise().await.as_string(), Some(String::from("ok"))) diff --git a/tests/wasm/main.rs b/tests/wasm/main.rs index e0064cebc2c..a90f9f3afef 100644 --- a/tests/wasm/main.rs +++ b/tests/wasm/main.rs @@ -34,6 +34,8 @@ pub mod no_shims; pub mod node; pub mod option; pub mod optional_primitives; +pub mod result; +pub mod result_jserror; pub mod rethrow; pub mod simple; pub mod slice; diff --git a/tests/wasm/result.js b/tests/wasm/result.js new file mode 100644 index 00000000000..2bf9f378f28 --- /dev/null +++ b/tests/wasm/result.js @@ -0,0 +1,126 @@ +const wasm = require('wasm-bindgen-test.js'); +const assert = require('assert'); + +exports.error_new = function(message) { + return new Error(message) +} + +exports.call_ok = function() { + assert.doesNotThrow(() => { + let five = wasm.return_my_ok(); + assert.strictEqual(five, 5); + }) +} + +exports.call_err = function() { + assert.throws(() => wasm.return_my_err(), { + message: "MyError::Variant" + }); +} + +function check_inflight(struct) { + assert.strictEqual(struct.is_inflight(), false); +} + +exports.all_struct_methods = function() { + let struct; + assert.throws(() => wasm.Struct.new_err(), { + message: "MyError::Variant" + }); + assert.doesNotThrow(() => { + struct = wasm.Struct.new(); + }); + check_inflight(struct); + assert.doesNotThrow(() => { + let five = struct.return_ok(); + assert.strictEqual(five, 5); + }); + check_inflight(struct); + assert.throws(() => struct.return_err(), { + message: "MyError::Variant" + }); + check_inflight(struct); +} + +exports.call_return_string = function() { + assert.doesNotThrow(() => { + let ok = wasm.return_string(); + assert.strictEqual(ok, "string here"); + }) +} + +exports.call_jsvalue_ok = function() { + assert.doesNotThrow(() => { + let five = wasm.return_jsvalue_ok(); + assert.strictEqual(five, 5); + }) +} + +exports.call_jsvalue_err = function() { + try { + wasm.return_jsvalue_err(); + assert.fail("should have thrown"); + } catch (e) { + assert.strictEqual(e, -1); + } +} + +exports.call_string_ok = function() { + assert.doesNotThrow(() => { + let ok = wasm.return_string_ok(); + assert.strictEqual(ok, "Ok"); + }) +} + +exports.call_string_err = function() { + // the behaviour of Result is so finicky that it's not obvious + // how to to reproduce reliably but also pass the test suite. + assert.throws(() => e = wasm.return_string_err(), e => { + // one thing we can do (uncomment to test) + // is to throw an error in here. + // throw new Error("should not cause a SIGBUS in Node") + return e === "Er"; + }); +} + +exports.call_enum_ok = function() { + assert.doesNotThrow(() => { + let ok = wasm.return_enum_ok(); + assert.strictEqual(ok, 2); + }) +} + +exports.call_enum_err = function() { + assert.throws(() => { + wasm.return_enum_err(); + }, { + message: "MyError::Variant" + }) +} + +exports.call_unit = function() { + assert.doesNotThrow(() => { + wasm.return_unit_ok(); + }); + assert.throws(() => { + wasm.return_unit_err(); + }, { + message: "MyError::Variant" + }); +} + +exports.call_option = function() { + assert.doesNotThrow(() => { + let o = wasm.return_option_ok_some(); + assert.strictEqual(o, 10.0); + }); + assert.doesNotThrow(() => { + let o = wasm.return_option_ok_none(); + assert.strictEqual(o, undefined); + }); + assert.throws(() => { + wasm.return_option_err(); + }, { + message: "MyError::Variant" + }); +} diff --git a/tests/wasm/result.rs b/tests/wasm/result.rs new file mode 100644 index 00000000000..c2c043ab877 --- /dev/null +++ b/tests/wasm/result.rs @@ -0,0 +1,190 @@ +use std::fmt; +use wasm_bindgen::prelude::*; +use wasm_bindgen_test::*; + +#[derive(Clone, Debug)] +pub enum MyError { + Variant, + InflightShouldBeFalse, +} +// shouldn't technically need this, surely +impl std::error::Error for MyError {} +impl fmt::Display for MyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "MyError::{:?}", self) + } +} + +#[wasm_bindgen(module = "tests/wasm/result.js")] +extern "C" { + fn error_new(message: &str) -> JsValue; +} + +impl Into for MyError { + fn into(self) -> JsValue { + error_new(&format!("{}", self)) + } +} + +macro_rules! call_test { + ($test_fn:ident, $js_fn:ident) => { + #[wasm_bindgen_test] + fn $test_fn() { + #[wasm_bindgen(module = "tests/wasm/result.js")] + extern "C" { + fn $js_fn(); + } + $js_fn(); + } + }; +} + +#[wasm_bindgen] +pub fn return_my_err() -> Result { + let e = Err(MyError::Variant)?; + Ok(e) +} +call_test!(test_err, call_err); + +#[wasm_bindgen] +pub fn return_my_ok() -> Result { + Ok(5) +} +call_test!(test_ok, call_ok); + +#[wasm_bindgen] +pub struct Struct { + inflight: bool, +} + +call_test!(test_struct, all_struct_methods); + +struct ResetOnDrop<'a> { + flag: &'a mut bool, +} + +impl<'a> Drop for ResetOnDrop<'a> { + fn drop(&mut self) { + *self.flag = false; + } +} + +impl<'a> ResetOnDrop<'a> { + fn new(flag: &'a mut bool) -> Result { + if *flag { + return Err(MyError::InflightShouldBeFalse); + } + Ok(Self { flag }) + } +} + +#[wasm_bindgen] +impl Struct { + #[wasm_bindgen] + pub fn new() -> Result { + Ok(Struct { inflight: false }) + } + + #[wasm_bindgen] + pub fn new_err() -> Result { + Err(MyError::Variant.into()) + } + + #[wasm_bindgen] + pub fn return_ok(&mut self) -> Result { + let _guard = ResetOnDrop::new(&mut self.inflight)?; + Ok(5) + } + + #[wasm_bindgen] + pub fn return_err(&mut self) -> Result { + let guard = ResetOnDrop::new(&mut self.inflight)?; + let err = Err(MyError::Variant); + let nope = err?; + // we are checking both for the flag being reset (from js, via is_inflight) + // and for the running of drop code + drop(guard); + Ok(nope) + } + + #[wasm_bindgen] + pub fn is_inflight(&self) -> bool { + self.inflight + } +} + +// check some more Ok types +#[wasm_bindgen] +pub fn return_string() -> Result { + Ok("string here".into()) +} +call_test!(test_return_string, call_return_string); + +// now we check that jsvalue works, as it did before + +#[wasm_bindgen] +pub fn return_jsvalue_ok() -> Result { + Ok(5) +} +call_test!(test_jsvalue_ok, call_jsvalue_ok); + +#[wasm_bindgen] +pub fn return_jsvalue_err() -> Result { + Err(JsValue::from(-1i32)) +} +call_test!(test_jsvalue_err, call_jsvalue_err); + +// test strings (they have a deferred free, in a finally block: tricky) +#[wasm_bindgen] +pub fn return_string_ok() -> Result { + Ok("Ok".into()) +} +call_test!(test_string_ok, call_string_ok); +#[wasm_bindgen] +pub fn return_string_err() -> Result { + Err("Er".into()) +} +call_test!(test_string_err, call_string_err); + +// test enums +#[wasm_bindgen] +pub enum MyEnum { + One = 1, + Two = 2, +} +#[wasm_bindgen] +pub fn return_enum_ok() -> Result { + Ok(MyEnum::Two) +} +call_test!(test_enum_ok, call_enum_ok); +#[wasm_bindgen] +pub fn return_enum_err() -> Result { + Err(MyError::Variant) +} +call_test!(test_enum_err, call_enum_err); + +// T = Unit +#[wasm_bindgen] +pub fn return_unit_ok() -> Result<(), MyError> { + Ok(()) +} +#[wasm_bindgen] +pub fn return_unit_err() -> Result<(), MyError> { + Err(MyError::Variant) +} +call_test!(test_unit, call_unit); + +// T = Option +#[wasm_bindgen] +pub fn return_option_ok_some() -> Result, MyError> { + Ok(Some(10f64)) +} +#[wasm_bindgen] +pub fn return_option_ok_none() -> Result, MyError> { + Ok(None) +} +#[wasm_bindgen] +pub fn return_option_err() -> Result, MyError> { + Err(MyError::Variant) +} +call_test!(test_option, call_option); diff --git a/tests/wasm/result_jserror.js b/tests/wasm/result_jserror.js new file mode 100644 index 00000000000..d27736d9cc0 --- /dev/null +++ b/tests/wasm/result_jserror.js @@ -0,0 +1,54 @@ +const wasm = require('wasm-bindgen-test.js'); +const assert = require('assert'); + +exports.call_ok = function() { + assert.doesNotThrow(() => { + let five = wasm.return_ok(); + assert.strictEqual(five, 5); + }) +} + +exports.call_err = function() { + assert.throws(() => wasm.return_err(), { + message: "MyError::Variant" + }); +} + +exports.call_make_an_error = function() { + assert.doesNotThrow(() => { + let e = wasm.make_an_error() + assert.strictEqual(e.message, "un-thrown error"); + }); +} + +function check_inflight(struct) { + assert.strictEqual(struct.is_inflight(), false); +} + +exports.all_struct_methods = function() { + let struct; + assert.throws(() => wasm.MyStruct.new_err(), { + message: "MyError::Variant" + }); + assert.doesNotThrow(() => { + struct = wasm.MyStruct.new(); + }); + check_inflight(struct); + assert.doesNotThrow(() => { + let five = struct.return_ok(); + assert.strictEqual(five, 5); + }); + check_inflight(struct); + assert.throws(() => struct.return_err(), { + message: "MyError::Variant" + }); + check_inflight(struct); +} + +exports.call_return_string = function() { + assert.doesNotThrow(() => { + let ok = wasm.jserror_return_string(); + assert.strictEqual(ok, "string here"); + }) +} + diff --git a/tests/wasm/result_jserror.rs b/tests/wasm/result_jserror.rs new file mode 100644 index 00000000000..f714ac2f133 --- /dev/null +++ b/tests/wasm/result_jserror.rs @@ -0,0 +1,118 @@ +use wasm_bindgen::prelude::*; +use wasm_bindgen::JsError; +use wasm_bindgen_test::*; + +use std::fmt; + +#[derive(Clone, Debug)] +enum MyError { + Variant, + InflightShouldBeFalse, +} +// shouldn't technically need this, surely +impl std::error::Error for MyError {} +impl fmt::Display for MyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "MyError::{:?}", self) + } +} + +macro_rules! call_test { + ($test_fn:ident, $js_fn:ident) => { + #[wasm_bindgen_test] + fn $test_fn() { + #[wasm_bindgen(module = "tests/wasm/result_jserror.js")] + extern "C" { + fn $js_fn(); + } + $js_fn(); + } + }; +} + +#[wasm_bindgen] +pub fn return_err() -> Result { + let e = Err(MyError::Variant)?; + Ok(e) +} +call_test!(test_err, call_err); + +#[wasm_bindgen] +pub fn return_ok() -> Result { + Ok(5) +} +call_test!(test_ok, call_ok); + +#[wasm_bindgen] +pub fn make_an_error() -> JsError { + JsError::new("un-thrown error").into() +} +call_test!(test_make_an_error, call_make_an_error); + +#[wasm_bindgen] +pub struct MyStruct { + inflight: bool, +} + +call_test!(test_struct, all_struct_methods); + +struct ResetOnDrop<'a> { + flag: &'a mut bool, +} + +impl<'a> Drop for ResetOnDrop<'a> { + fn drop(&mut self) { + *self.flag = false; + } +} + +impl<'a> ResetOnDrop<'a> { + fn new(flag: &'a mut bool) -> Result { + if *flag { + return Err(MyError::InflightShouldBeFalse); + } + Ok(Self { flag }) + } +} + +#[wasm_bindgen] +impl MyStruct { + #[wasm_bindgen] + pub fn new() -> Result { + Ok(MyStruct { inflight: false }) + } + + #[wasm_bindgen] + pub fn new_err() -> Result { + Err(MyError::Variant.into()) + } + + #[wasm_bindgen] + pub fn return_ok(&mut self) -> Result { + let _guard = ResetOnDrop::new(&mut self.inflight)?; + Ok(5) + } + + #[wasm_bindgen] + pub fn return_err(&mut self) -> Result { + let guard = ResetOnDrop::new(&mut self.inflight)?; + let err = Err(MyError::Variant); + let nope = err?; + // we are checking both for the flag being reset (from js, via is_inflight) + // and for the running of drop code + drop(guard); + Ok(nope) + } + + #[wasm_bindgen] + pub fn is_inflight(&self) -> bool { + self.inflight + } +} + +// check some more Ok types +#[wasm_bindgen] +pub fn jserror_return_string() -> Result { + Ok("string here".into()) +} +call_test!(test_return_string, call_return_string);