Fix fn () -> Result<T, JsValue> leaking stack space

.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)"

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<Descriptor>),
+    Result(Box<Descriptor>),
     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,

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,
             };
         }

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,

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<T>,
+            //    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,

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<MemView, Error> {
+        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() {

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: {:?}",

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<Option<&'a str>, Error> {
 fn concatenate_comments(comments: &[&str]) -> String {
     comments.iter().map(|&s| s).collect::<Vec<_>>().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<usize, Error> {
+        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
+}

crates/cli-support/src/wit/nonstandard.rs

@@ -52,6 +52,7 @@ pub struct WasmBindgenAux {
     pub externref_table: Option<walrus::TableId>,
     pub function_table: Option<walrus::TableId>,
     pub externref_alloc: Option<walrus::FunctionId>,
+    pub externref_drop: Option<walrus::FunctionId>,
     pub externref_drop_slice: Option<walrus::FunctionId>,
 
     /// Various intrinsics used for JS glue generation