Skip to content

Commit

Permalink
Allow enabling multi-value
Browse files Browse the repository at this point in the history
For some reason wasm-bindgen would only allow enabling multi-value if interface types were enabled, throwing an error otherwise. I couldn't see any reason why, so I changed that.

Since that also meant that multi-value + JS bindings wasn't being tested, allowing it exposed some bugs that I fixed.
  • Loading branch information
Liamolucko committed Apr 10, 2023
1 parent 578c508 commit 4e623f9
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 22 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ jobs:
env:
WASM_BINDGEN_EXTERNREF: 1
NODE_ARGS: --experimental-wasm-reftypes
- run: cargo test --target wasm32-unknown-unknown
env:
WASM_BINDGEN_MULTI_VALUE: 1

test_threads:
name: "Run wasm-bindgen crate tests with multithreading enabled"
Expand Down
39 changes: 30 additions & 9 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::js::Context;
use crate::wit::InstructionData;
use crate::wit::{Adapter, AdapterId, AdapterKind, AdapterType, Instruction};
use anyhow::{anyhow, bail, Error};
use std::fmt::Write;
use walrus::{Module, ValType};

/// A one-size-fits-all builder for processing WebIDL bindings and generating
Expand Down Expand Up @@ -39,6 +40,9 @@ pub struct JsBuilder<'a, 'b> {
/// built so far.
prelude: String,

/// Code which should go before the `try {` in a try-finally block.
pre_try: String,

/// JS code to execute in a `finally` block in case any exceptions happen.
finally: String,

Expand Down Expand Up @@ -211,10 +215,14 @@ impl<'a, 'b> Builder<'a, 'b> {
}
code.push_str(") {\n");

let mut call = js.prelude;
if js.finally.len() != 0 {
call = format!("try {{\n{}}} finally {{\n{}}}\n", call, js.finally);
}
let call = if js.finally.len() != 0 {
format!(
"{}try {{\n{}}} finally {{\n{}}}\n",
js.pre_try, js.prelude, js.finally
)
} else {
js.pre_try + &js.prelude
};

if self.catch {
js.cx.expose_handle_error()?;
Expand Down Expand Up @@ -387,6 +395,7 @@ impl<'a, 'b> JsBuilder<'a, 'b> {
cx,
args: Vec::new(),
tmp: 0,
pre_try: String::new(),
finally: String::new(),
prelude: String::new(),
stack: Vec::new(),
Expand Down Expand Up @@ -546,12 +555,25 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
let invoc = Invocation::from(instr, js.cx.module)?;
let (params, results) = invoc.params_results(js.cx);

// Pop off the number of parameters for the function we're calling
let mut args = Vec::new();
for _ in 0..params {
args.push(js.pop());
let tmp = js.tmp();
if invoc.defer() {
// If the call is deferred, the arguments to the function still need to be
// accessible in the `finally` block, so we declare variables to hold the args
// outside of the try-finally block and then set those to the args.
for (i, arg) in js.stack[js.stack.len() - params..].iter().enumerate() {
let name = format!("deferred{tmp}_{i}");
writeln!(js.pre_try, "let {name};").unwrap();
writeln!(js.prelude, "{name} = {arg};").unwrap();
args.push(name);
}
} else {
// Otherwise, pop off the number of parameters for the function we're calling.
for _ in 0..params {
args.push(js.pop());
}
args.reverse();
}
args.reverse();

// Call the function through an export of the underlying module.
let call = invoc.invoke(js.cx, &args, &mut js.prelude, log_error)?;
Expand All @@ -562,7 +584,6 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
match (invoc.defer(), results) {
(true, 0) => {
js.finally(&format!("{};", call));
js.stack.extend(args);
}
(true, _) => panic!("deferred calls must have no results"),
(false, 0) => js.prelude(&format!("{};", call)),
Expand Down
4 changes: 0 additions & 4 deletions crates/cli-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,6 @@ impl Bindgen {
// Using all of our metadata convert our module to a multi-value using
// module if applicable.
if self.multi_value {
anyhow::bail!(
"Wasm multi-value is currently only available when \
Wasm interface types is also enabled"
);
multivalue::run(&mut module)
.context("failed to transform return pointers into multi-value Wasm")?;
}
Expand Down
72 changes: 68 additions & 4 deletions crates/cli-support/src/multivalue.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::wit::{Adapter, NonstandardWitSection};
use crate::wit::{AdapterKind, Instruction, WasmBindgenAux};
use anyhow::{anyhow, Error};
use walrus::Module;
use walrus::ir::Value;
use walrus::{FunctionId, InitExpr, Module};
use wasm_bindgen_multi_value_xform as multi_value_xform;
use wasm_bindgen_wasm_conventions as wasm_conventions;

Expand Down Expand Up @@ -37,6 +38,7 @@ pub fn run(module: &mut Module) -> Result<(), Error> {
match slot {
Slot::Id(s) => *s = id,
Slot::Export(e) => module.exports.get_mut(e).item = id.into(),
Slot::TableElement(index) => set_table_entry(module, index, id),
}
}

Expand All @@ -48,6 +50,7 @@ pub fn run(module: &mut Module) -> Result<(), Error> {
enum Slot<'a> {
Id(&'a mut walrus::FunctionId),
Export(walrus::ExportId),
TableElement(u32),
}

fn extract_xform<'a>(
Expand Down Expand Up @@ -76,13 +79,13 @@ fn extract_xform<'a>(
});
let slot = instructions
.iter_mut()
.filter_map(|i| match &mut i.instr {
.find_map(|i| match &mut i.instr {
Instruction::CallCore(f) => Some(Slot::Id(f)),
Instruction::CallExport(e) => Some(Slot::Export(*e)),
Instruction::CallTableElement(index) => Some(Slot::TableElement(*index)),
_ => None,
})
.next()
.expect("should have found call-core");
.expect("adapter never calls the underlying function");

// LLVM currently always uses the first parameter for the return
// pointer. We hard code that here, since we have no better option.
Expand All @@ -92,6 +95,7 @@ fn extract_xform<'a>(
walrus::ExportItem::Function(f) => f,
_ => panic!("found call to non-function export"),
},
Slot::TableElement(func_index) => resolve_table_entry(module, *func_index),
};
to_xform.push((id, 0, types));
slots.push(slot);
Expand All @@ -104,3 +108,63 @@ fn extract_xform<'a>(
// FIXME(#1872) handle this
// if let Some(Instruction::StoreRetptr { .. }) = instructions.last() {}
}

/// Resolves an index in the function table to a function ID.
fn resolve_table_entry(module: &Module, func_index: u32) -> FunctionId {
let table_id = module
.tables
.main_function_table()
.ok()
.flatten()
.expect("there should only be one function table");
module
.tables
.get(table_id)
.elem_segments
.iter()
.find_map(|&id| {
let elem = module.elements.get(id);
let offset = match elem.kind {
walrus::ElementKind::Active { offset, .. } => match offset {
InitExpr::Value(Value::I32(value)) => value as u32,
_ => panic!("table offset was not an i32 value"),
},
_ => panic!("found non-active element section for function table"),
};
elem.members.iter().enumerate().find_map(|(i, &func_id)| {
let table_index = i as u32 + offset;
if table_index == func_index {
func_id
} else {
None
}
})
})
.expect("function in function table is not initialized")
}

/// Changes the function ID at an index in the function table.
fn set_table_entry(module: &mut Module, func_index: u32, new_id: FunctionId) {
let table_id = module
.tables
.main_function_table()
.ok()
.flatten()
.expect("there should only be one function table");
for &id in module.tables.get(table_id).elem_segments.iter() {
let elem = module.elements.get_mut(id);
let offset = match elem.kind {
walrus::ElementKind::Active { offset, .. } => match offset {
InitExpr::Value(Value::I32(value)) => value as u32,
_ => panic!("table offset was not an i32 value"),
},
_ => panic!("found non-active element section for function table"),
};
for (i, func_id) in elem.members.iter_mut().enumerate() {
let table_index = i as u32 + offset;
if table_index == func_index {
*func_id = Some(new_id);
}
}
}
}
14 changes: 9 additions & 5 deletions crates/cli/tests/reference/result-string.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,27 @@ function getStringFromWasm0(ptr, len) {
* @returns {string}
*/
export function exported() {
let deferred2_0;
let deferred2_1;
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;
var ptr1 = r0;
var len1 = r1;
if (r3) {
ptr0 = 0; len0 = 0;
ptr1 = 0; len1 = 0;
throw takeObject(r2);
}
return getStringFromWasm0(ptr0, len0);
deferred2_0 = ptr1;
deferred2_1 = len1;
return getStringFromWasm0(ptr1, len1);
} finally {
wasm.__wbindgen_add_to_stack_pointer(16);
wasm.__wbindgen_free(ptr0, len0);
wasm.__wbindgen_free(deferred2_0, deferred2_1);
}
}

Expand Down

0 comments on commit 4e623f9

Please sign in to comment.