From 203d86f343177e4b460188fa2bf41834e475a3e4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 4 Dec 2019 15:19:48 -0600 Subject: [PATCH] Add tests for the interface types output of wasm-bindgen (#1898) * Add tests for the interface types output of wasm-bindgen This commit expands the test suite with assertions about the output of the interface types pass in wasm-bindgen. The goal here is to actually assert that we produce the right output and have a suite of reference files to show how the interface types output is changing over time. The `reference` test suite added in the previous PR has been updated to work for interface types as well, generating `*.wit` file assertions which are printed via the `wit-printer` crate on crates.io. Along the way a number of bugs were fixed with the interface types output, such as: * Non-determinism in output caused by iteration of a `HashMap` * Avoiding JS generation entirely in interface types mode, ensuring that we don't export extraneous intrinsics that aren't otherwise needed. * Fixing location of the stack pointer for modules where it's GC'd out. It's now rooted in the aux section of wasm-bindgen so it's available to later passes, like the multi-value pass. * Interface types emission now works in debug mode, meaning the `--release` flag is no longer required. This previously did not work because the `__wbindgen_throw` intrinsic was required in debug mode. This comes about because of the `malloc_failure` and `internal_error` functions in the anyref pass. The purpose of these functions is to signal fatal runtime errors, if any, in a way that's usable to the user. For wasm interface types though we can replace calls to these functions with `unreachable` to avoid needing to import the intrinsic. This has the accidental side effect of making `wasm_bindgen::throw_str` "just work" with wasm interface types by aborting the program, but that's not actually entirely intended. It's hoped that a split of a `wasm-bindgen-core` crate would solve this issue for the future. * Run the wasm interface types validator in tests * Add more gc roots for adapter gc * Improve stack pointer detection The stack pointer is never initialized to zero, but some other mutable globals are (TLS, thread ID, etc), so let's filter those out. --- crates/cli-support/src/js/mod.rs | 27 +-- crates/cli-support/src/lib.rs | 206 +++++++++++------- crates/cli-support/src/multivalue.rs | 51 ++++- crates/cli-support/src/throw2unreachable.rs | 87 ++++++++ crates/cli-support/src/wit/mod.rs | 3 + crates/cli-support/src/wit/nonstandard.rs | 4 + crates/cli-support/src/wit/section.rs | 34 +-- crates/cli-support/src/wit/standard.rs | 49 ++++- crates/cli/Cargo.toml | 4 +- crates/cli/tests/reference.rs | 60 ++--- .../tests/reference/interface-types-anyref.rs | 8 + .../reference/interface-types-anyref.wit | 12 + .../tests/reference/interface-types-empty.rs | 7 + .../tests/reference/interface-types-empty.wit | 10 + .../reference/interface-types-integers.rs | 46 ++++ .../reference/interface-types-integers.wit | 81 +++++++ .../reference/interface-types-interop.rs | 8 + .../reference/interface-types-interop.wit | 13 ++ .../reference/interface-types-strings.rs | 11 + .../reference/interface-types-strings.wit | 29 +++ crates/cli/tests/wasm-bindgen/main.rs | 101 +++++++++ crates/wasm-conventions/src/lib.rs | 16 +- 22 files changed, 699 insertions(+), 168 deletions(-) create mode 100644 crates/cli-support/src/throw2unreachable.rs create mode 100644 crates/cli/tests/reference/interface-types-anyref.rs create mode 100644 crates/cli/tests/reference/interface-types-anyref.wit create mode 100644 crates/cli/tests/reference/interface-types-empty.rs create mode 100644 crates/cli/tests/reference/interface-types-empty.wit create mode 100644 crates/cli/tests/reference/interface-types-integers.rs create mode 100644 crates/cli/tests/reference/interface-types-integers.wit create mode 100644 crates/cli/tests/reference/interface-types-interop.rs create mode 100644 crates/cli/tests/reference/interface-types-interop.wit create mode 100644 crates/cli/tests/reference/interface-types-strings.rs create mode 100644 crates/cli/tests/reference/interface-types-strings.wit diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 1d536ddf..7e9a0d08 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -227,7 +227,7 @@ impl<'a> Context<'a> { } => { js.push_str("let wasm;\n"); - for (id, js) in sorted_iter(&self.wasm_import_definitions) { + for (id, js) in crate::sorted_iter(&self.wasm_import_definitions) { let import = self.module.imports.get_mut(*id); import.module = format!("./{}.js", module_name); footer.push_str("\nmodule.exports."); @@ -254,7 +254,7 @@ impl<'a> Context<'a> { "import * as wasm from './{}_bg.wasm';\n", module_name )); - for (id, js) in sorted_iter(&self.wasm_import_definitions) { + for (id, js) in crate::sorted_iter(&self.wasm_import_definitions) { let import = self.module.imports.get_mut(*id); import.module = format!("./{}.js", module_name); footer.push_str("\nexport const "); @@ -328,7 +328,7 @@ impl<'a> Context<'a> { OutputMode::Node { experimental_modules: false, } => { - for (module, items) in sorted_iter(&self.js_imports) { + for (module, items) in crate::sorted_iter(&self.js_imports) { imports.push_str("const { "); for (i, (item, rename)) in items.iter().enumerate() { if i > 0 { @@ -351,7 +351,7 @@ impl<'a> Context<'a> { experimental_modules: true, } | OutputMode::Web => { - for (module, items) in sorted_iter(&self.js_imports) { + for (module, items) in crate::sorted_iter(&self.js_imports) { imports.push_str("import { "); for (i, (item, rename)) in items.iter().enumerate() { if i > 0 { @@ -450,7 +450,7 @@ impl<'a> Context<'a> { imports_init.push_str(module_name); imports_init.push_str(" = {};\n"); } - for (id, js) in sorted_iter(&self.wasm_import_definitions) { + for (id, js) in crate::sorted_iter(&self.wasm_import_definitions) { let import = self.module.imports.get_mut(*id); import.module = module_name.to_string(); imports_init.push_str("imports."); @@ -1852,7 +1852,7 @@ impl<'a> Context<'a> { } pub fn generate(&mut self) -> Result<(), Error> { - for (id, adapter) in sorted_iter(&self.wit.adapters) { + for (id, adapter) in crate::sorted_iter(&self.wit.adapters) { let instrs = match &adapter.kind { AdapterKind::Import { .. } => continue, AdapterKind::Local { instructions } => instructions, @@ -3055,21 +3055,6 @@ impl ExportedClass { } } -/// Returns a sorted iterator over a hash map, sorted based on key. -/// -/// The intention of this API is to be used whenever the iteration order of a -/// `HashMap` might affect the generated JS bindings. We want to ensure that the -/// generated output is deterministic and we do so by ensuring that iteration of -/// hash maps is consistently sorted. -fn sorted_iter(map: &HashMap) -> impl Iterator -where - K: Ord, -{ - let mut pairs = map.iter().collect::>(); - pairs.sort_by_key(|(k, _)| *k); - pairs.into_iter() -} - #[test] fn test_generate_identifier() { let mut used_names: HashMap = HashMap::new(); diff --git a/crates/cli-support/src/lib.rs b/crates/cli-support/src/lib.rs index f7a69116..23fedd9b 100755 --- a/crates/cli-support/src/lib.rs +++ b/crates/cli-support/src/lib.rs @@ -16,6 +16,7 @@ mod descriptors; mod intrinsic; mod js; mod multivalue; +mod throw2unreachable; pub mod wasm2es6js; mod wit; @@ -45,14 +46,22 @@ pub struct Bindgen { pub struct Output { module: walrus::Module, stem: String, + generated: Generated, +} + +enum Generated { + InterfaceTypes, + Js(JsGenerated), +} + +struct JsGenerated { + mode: OutputMode, js: String, ts: String, - mode: OutputMode, - typescript: bool, snippets: HashMap>, local_modules: HashMap, npm_dependencies: HashMap, - wasm_interface_types: bool, + typescript: bool, } #[derive(Clone)] @@ -354,54 +363,69 @@ impl Bindgen { } } - // We've done a whole bunch of transformations to the wasm module, many - // of which leave "garbage" lying around, so let's prune out all our - // unnecessary things here. - gc_module_and_adapters(&mut module); - - let aux = module - .customs - .delete_typed::() - .expect("aux section should be present"); - let mut adapters = module - .customs - .delete_typed::() - .unwrap(); - - // Now that our module is massaged and good to go, feed it into the JS - // shim generation which will actually generate JS for all this. - let (npm_dependencies, (js, ts)) = { - let mut cx = js::Context::new(&mut module, self, &adapters, &aux)?; - cx.generate()?; - let npm_dependencies = cx.npm_dependencies.clone(); - (npm_dependencies, cx.finalize(stem)?) - }; - + // If wasm interface types are enabled then the `__wbindgen_throw` + // intrinsic isn't available but it may be used by our runtime, so + // change all calls to this function to calls to `unreachable` instead. + // See more documentation in the pass documentation itself. if self.wasm_interface_types { - multivalue::run(&mut module, &mut adapters) - .context("failed to transform return pointers into multi-value Wasm")?; - wit::section::add(&mut module, &aux, &adapters) - .context("failed to generate a standard wasm bindings custom section")?; - } else { - if self.multi_value { + throw2unreachable::run(&mut module); + } + + // Using all of our metadata convert our module to a multi-value using + // module if applicable. + if self.multi_value { + if !self.wasm_interface_types { 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")?; } + // We've done a whole bunch of transformations to the wasm module, many + // of which leave "garbage" lying around, so let's prune out all our + // unnecessary things here. + gc_module_and_adapters(&mut module); + + // We're ready for the final emission passes now. If we're in wasm + // interface types mode then we execute the various passes there and + // generate a valid interface typess section into the wasm module. + // + // Otherwise we execute the JS generation passes to actually emit + // JS/TypeScript/etc. The output here is unused in wasm interfac + let generated = if self.wasm_interface_types { + wit::section::add(&mut module) + .context("failed to generate a standard interface types section")?; + Generated::InterfaceTypes + } else { + let aux = module + .customs + .delete_typed::() + .expect("aux section should be present"); + let adapters = module + .customs + .delete_typed::() + .unwrap(); + let mut cx = js::Context::new(&mut module, self, &adapters, &aux)?; + cx.generate()?; + let (js, ts) = cx.finalize(stem)?; + Generated::Js(JsGenerated { + snippets: aux.snippets.clone(), + local_modules: aux.local_modules.clone(), + mode: self.mode.clone(), + typescript: self.typescript, + npm_dependencies: cx.npm_dependencies.clone(), + js, + ts, + }) + }; + Ok(Output { module, stem: stem.to_string(), - snippets: aux.snippets.clone(), - local_modules: aux.local_modules.clone(), - npm_dependencies, - js, - ts, - mode: self.mode.clone(), - typescript: self.typescript, - wasm_interface_types: self.wasm_interface_types, + generated, }) } @@ -554,8 +578,10 @@ fn unexported_unused_lld_things(module: &mut Module) { impl Output { pub fn js(&self) -> &str { - assert!(!self.wasm_interface_types); - &self.js + match &self.generated { + Generated::InterfaceTypes => panic!("no js with interface types output"), + Generated::Js(gen) => &gen.js, + } } pub fn wasm(&self) -> &walrus::Module { @@ -571,10 +597,9 @@ impl Output { } fn _emit(&mut self, out_dir: &Path) -> Result<(), Error> { - let wasm_name = if self.wasm_interface_types { - self.stem.clone() - } else { - format!("{}_bg", self.stem) + let wasm_name = match &self.generated { + Generated::InterfaceTypes => self.stem.clone(), + Generated::Js(_) => format!("{}_bg", self.stem), }; let wasm_path = out_dir.join(wasm_name).with_extension("wasm"); fs::create_dir_all(out_dir)?; @@ -582,13 +607,14 @@ impl Output { fs::write(&wasm_path, wasm_bytes) .with_context(|| format!("failed to write `{}`", wasm_path.display()))?; - if self.wasm_interface_types { - return Ok(()); - } + let gen = match &self.generated { + Generated::InterfaceTypes => return Ok(()), + Generated::Js(gen) => gen, + }; // Write out all local JS snippets to the final destination now that // we've collected them from all the programs. - for (identifier, list) in self.snippets.iter() { + for (identifier, list) in gen.snippets.iter() { for (i, js) in list.iter().enumerate() { let name = format!("inline{}.js", i); let path = out_dir.join("snippets").join(identifier).join(name); @@ -598,15 +624,15 @@ impl Output { } } - for (path, contents) in self.local_modules.iter() { + for (path, contents) in gen.local_modules.iter() { let path = out_dir.join("snippets").join(path); fs::create_dir_all(path.parent().unwrap())?; fs::write(&path, contents) .with_context(|| format!("failed to write `{}`", path.display()))?; } - if self.npm_dependencies.len() > 0 { - let map = self + if gen.npm_dependencies.len() > 0 { + let map = gen .npm_dependencies .iter() .map(|(k, v)| (k, &v.1)) @@ -617,29 +643,29 @@ impl Output { // And now that we've got all our JS and TypeScript, actually write it // out to the filesystem. - let extension = if self.mode.nodejs_experimental_modules() { + let extension = if gen.mode.nodejs_experimental_modules() { "mjs" } else { "js" }; let js_path = out_dir.join(&self.stem).with_extension(extension); - fs::write(&js_path, reset_indentation(&self.js)) + fs::write(&js_path, reset_indentation(&gen.js)) .with_context(|| format!("failed to write `{}`", js_path.display()))?; - if self.typescript { + if gen.typescript { let ts_path = js_path.with_extension("d.ts"); - fs::write(&ts_path, &self.ts) + fs::write(&ts_path, &gen.ts) .with_context(|| format!("failed to write `{}`", ts_path.display()))?; } - if self.mode.nodejs() { + if gen.mode.nodejs() { let js_path = wasm_path.with_extension(extension); - let shim = self.generate_node_wasm_import(&self.module, &wasm_path); + let shim = gen.generate_node_wasm_import(&self.module, &wasm_path); fs::write(&js_path, shim) .with_context(|| format!("failed to write `{}`", js_path.display()))?; } - if self.typescript { + if gen.typescript { let ts_path = wasm_path.with_extension("d.ts"); let ts = wasm2es6js::typescript(&self.module)?; fs::write(&ts_path, ts) @@ -648,7 +674,9 @@ impl Output { Ok(()) } +} +impl JsGenerated { fn generate_node_wasm_import(&self, m: &Module, path: &Path) -> String { let mut imports = BTreeSet::new(); for import in m.imports.iter() { @@ -720,40 +748,52 @@ impl Output { } fn gc_module_and_adapters(module: &mut Module) { - // First up we execute walrus's own gc passes, and this may enable us to - // delete entries in the `implements` section of the nonstandard wasm - // interface types section. (if the import is GC'd, then the implements - // annotation is no longer needed). - // - // By deleting adapter functions that may enable us to further delete more - // functions, so we run this in a loop until we don't actually delete any - // adapter functions. loop { + // Fist up, cleanup the native wasm module. Note that roots can come + // from custom sections, namely our wasm interface types custom section + // as well as the aux section. walrus::passes::gc::run(module); + // ... and afterwards we can delete any `implements` directives for any + // imports that have been deleted. let imports_remaining = module .imports .iter() .map(|i| i.id()) .collect::>(); - let section = module + let mut section = module .customs - .get_typed_mut::() + .delete_typed::() .unwrap(); - let mut deleted_implements = Vec::new(); - section.implements.retain(|pair| { - if imports_remaining.contains(&pair.0) { - true - } else { - deleted_implements.push(pair.2); - false - } - }); - if deleted_implements.len() == 0 { + section + .implements + .retain(|pair| imports_remaining.contains(&pair.0)); + + // ... and after we delete the `implements` directive we try to + // delete some adapters themselves. If nothing is deleted, then we're + // good to go. If something is deleted though then we may have free'd up + // some functions in the main module to get deleted, so go again to gc + // things. + let aux = module.customs.get_typed::().unwrap(); + let any_removed = section.gc(aux); + module.customs.add(*section); + if !any_removed { break; } - for id in deleted_implements { - section.adapters.remove(&id); - } } } + +/// Returns a sorted iterator over a hash map, sorted based on key. +/// +/// The intention of this API is to be used whenever the iteration order of a +/// `HashMap` might affect the generated JS bindings. We want to ensure that the +/// generated output is deterministic and we do so by ensuring that iteration of +/// hash maps is consistently sorted. +fn sorted_iter(map: &HashMap) -> impl Iterator +where + K: Ord, +{ + let mut pairs = map.iter().collect::>(); + pairs.sort_by_key(|(k, _)| *k); + pairs.into_iter() +} diff --git a/crates/cli-support/src/multivalue.rs b/crates/cli-support/src/multivalue.rs index 6e2f866c..d2b76633 100644 --- a/crates/cli-support/src/multivalue.rs +++ b/crates/cli-support/src/multivalue.rs @@ -1,39 +1,60 @@ use crate::wit::{Adapter, NonstandardWitSection}; -use crate::wit::{AdapterKind, Instruction}; -use anyhow::Error; +use crate::wit::{AdapterKind, Instruction, WasmBindgenAux}; +use anyhow::{anyhow, Error}; use walrus::Module; use wasm_bindgen_multi_value_xform as multi_value_xform; use wasm_bindgen_wasm_conventions as wasm_conventions; -pub fn run(module: &mut Module, adapters: &mut NonstandardWitSection) -> Result<(), Error> { +pub fn run(module: &mut Module) -> Result<(), Error> { + let mut adapters = module + .customs + .delete_typed::() + .unwrap(); let mut to_xform = Vec::new(); let mut slots = Vec::new(); for (_, adapter) in adapters.adapters.iter_mut() { - extract_xform(adapter, &mut to_xform, &mut slots); + extract_xform(module, adapter, &mut to_xform, &mut slots); } if to_xform.is_empty() { // Early exit to avoid failing if we don't have a memory or shadow stack // pointer because this is a minimal module that doesn't use linear // memory. + module.customs.add(*adapters); return Ok(()); } - let shadow_stack_pointer = wasm_conventions::get_shadow_stack_pointer(module)?; + let shadow_stack_pointer = module + .customs + .get_typed::() + .expect("aux section should be present") + .shadow_stack_pointer + .ok_or_else(|| anyhow!("failed to find shadow stack pointer in wasm module"))?; let memory = wasm_conventions::get_memory(module)?; let wrappers = multi_value_xform::run(module, memory, shadow_stack_pointer, &to_xform)?; for (slot, id) in slots.into_iter().zip(wrappers) { - *slot = id; + match slot { + Slot::Id(s) => *s = id, + Slot::Export(e) => module.exports.get_mut(e).item = id.into(), + } } + module.customs.add(*adapters); + Ok(()) } +enum Slot<'a> { + Id(&'a mut walrus::FunctionId), + Export(walrus::ExportId), +} + fn extract_xform<'a>( + module: &Module, adapter: &'a mut Adapter, to_xform: &mut Vec<(walrus::FunctionId, usize, Vec)>, - slots: &mut Vec<&'a mut walrus::FunctionId>, + slots: &mut Vec>, ) { let instructions = match &mut adapter.kind { AdapterKind::Local { instructions } => instructions, @@ -53,10 +74,11 @@ fn extract_xform<'a>( } _ => true, }); - let id = instructions + let slot = instructions .iter_mut() .filter_map(|i| match &mut i.instr { - Instruction::Standard(wit_walrus::Instruction::CallCore(f)) => Some(f), + Instruction::Standard(wit_walrus::Instruction::CallCore(f)) => Some(Slot::Id(f)), + Instruction::CallExport(e) => Some(Slot::Export(*e)), _ => None, }) .next() @@ -64,8 +86,15 @@ fn extract_xform<'a>( // LLVM currently always uses the first parameter for the return // pointer. We hard code that here, since we have no better option. - to_xform.push((*id, 0, types)); - slots.push(id); + let id = match &slot { + Slot::Id(i) => **i, + Slot::Export(e) => match module.exports.get(*e).item { + walrus::ExportItem::Function(f) => f, + _ => panic!("found call to non-function export"), + }, + }; + to_xform.push((id, 0, types)); + slots.push(slot); return; } diff --git a/crates/cli-support/src/throw2unreachable.rs b/crates/cli-support/src/throw2unreachable.rs new file mode 100644 index 00000000..1358806a --- /dev/null +++ b/crates/cli-support/src/throw2unreachable.rs @@ -0,0 +1,87 @@ +use crate::intrinsic::Intrinsic; +use crate::wit::Instruction; +use crate::wit::{AdapterKind, AuxImport, NonstandardWitSection, WasmBindgenAux}; +use walrus::ir::*; +use walrus::Module; + +/// Runs a small pass over `Module` to replace all calls to the +/// `__wbindgen_throw` intrinsic with an `unreachable` instruction. +/// +/// This pass is executed as part of the wasm interface types support. This is +/// done to support debug mode executables with wasm interface types. Debug mode +/// executables will use malloc as well as anyref intrinsics. These intrinsics +/// internally, when they fail, abort the instance. This abort is done through +/// the `__wbindgen_throw` intrinsic in debug mode to provide a hopefully +/// useful error message. In release mode it's simply an `unreachable` +/// instruction. +/// +/// With wasm interface types we can't rely on intrinsics being available, so we +/// need to do something about this in debug mode. Our answer is to remove calls +/// to `__wbindgen_throw` and replace them with `unreachable`. +/// +/// This has the unintended side effect of making the user-visible function +/// `wasm_bindgen::throw_str` "just work", but that's hoped to get fix with a +/// split of crates like described in #1841 +pub fn run(module: &mut Module) { + // Find the adapter ID which is the import for the call to the throw + // intrinsic. + let aux = module.customs.get_typed::().unwrap(); + let throw_import = aux.import_map.iter().find(|(_, import)| match import { + AuxImport::Intrinsic(Intrinsic::Throw) => true, + _ => false, + }); + let throw_adapter = match throw_import { + Some((id, _)) => *id, + None => return, + }; + + // Find the adapter, if any, which calls this intrinsic + let wit = module.customs.get_typed::().unwrap(); + let adapter_calling_throw = wit.adapters.iter().find(|(_, adapter)| { + let instrs = match &adapter.kind { + AdapterKind::Local { instructions } => instructions, + _ => return false, + }; + instrs.iter().any(|i| match i.instr { + Instruction::CallAdapter(a) => a == throw_adapter, + _ => false, + }) + }); + let adapter_calling_throw = match adapter_calling_throw { + Some((id, _)) => *id, + None => return, + }; + + // ... then using the adapter that calls the intrinsic, find which core + // import in the wasm module it's implementing. + let import = wit + .implements + .iter() + .find(|(_, _, adapter)| *adapter == adapter_calling_throw); + let function = match import { + Some((_, function, _)) => *function, + None => return, + }; + + // .. and now replace all calls to `function` with `unreachable` + // instructions + for (_, func) in module.funcs.iter_local_mut() { + let entry = func.entry_block(); + dfs_pre_order_mut(&mut Rewrite { function }, func, entry); + } + + struct Rewrite { + function: walrus::FunctionId, + } + + impl VisitorMut for Rewrite { + fn visit_instr_mut(&mut self, instr: &mut Instr, _: &mut InstrLocId) { + match instr { + Instr::Call(c) if c.func == self.function => { + *instr = Unreachable {}.into(); + } + _ => {} + } + } + } +} diff --git a/crates/cli-support/src/wit/mod.rs b/crates/cli-support/src/wit/mod.rs index 4308ba64..8b15cc14 100644 --- a/crates/cli-support/src/wit/mod.rs +++ b/crates/cli-support/src/wit/mod.rs @@ -92,6 +92,9 @@ pub fn process( impl<'a> Context<'a> { fn init(&mut self) -> Result<(), Error> { + let stack_pointer = wasm_bindgen_wasm_conventions::get_shadow_stack_pointer(self.module)?; + self.aux.shadow_stack_pointer = Some(stack_pointer); + // Make a map from string name to ids of all exports for export in self.module.exports.iter() { if let walrus::ExportItem::Function(f) = export.item { diff --git a/crates/cli-support/src/wit/nonstandard.rs b/crates/cli-support/src/wit/nonstandard.rs index 30920146..146aab28 100644 --- a/crates/cli-support/src/wit/nonstandard.rs +++ b/crates/cli-support/src/wit/nonstandard.rs @@ -55,6 +55,7 @@ pub struct WasmBindgenAux { /// Various intrinsics used for JS glue generation pub exn_store: Option, + pub shadow_stack_pointer: Option, } pub type WasmBindgenAuxId = TypedCustomSectionId; @@ -374,5 +375,8 @@ impl walrus::CustomSection for WasmBindgenAux { if let Some(id) = self.exn_store { roots.push_func(id); } + if let Some(id) = self.shadow_stack_pointer { + roots.push_global(id); + } } } diff --git a/crates/cli-support/src/wit/section.rs b/crates/cli-support/src/wit/section.rs index adea17a3..7efd5dc4 100644 --- a/crates/cli-support/src/wit/section.rs +++ b/crates/cli-support/src/wit/section.rs @@ -24,11 +24,12 @@ use anyhow::{anyhow, bail, Context, Error}; use std::collections::HashMap; use walrus::Module; -pub fn add( - module: &mut Module, - aux: &WasmBindgenAux, - nonstandard: &NonstandardWitSection, -) -> Result<(), Error> { +pub fn add(module: &mut Module) -> Result<(), Error> { + let nonstandard = module + .customs + .delete_typed::() + .unwrap(); + let aux = module.customs.delete_typed::().unwrap(); let mut section = wit_walrus::WasmInterfaceTypes::default(); let WasmBindgenAux { extra_typescript: _, // ignore this even if it's specified @@ -42,11 +43,14 @@ pub fn add( imports_with_assert_no_shim: _, // not relevant for this purpose enums, structs, - anyref_table: _, // not relevant - anyref_alloc: _, // not relevant - anyref_drop_slice: _, // not relevant - exn_store: _, // not relevant - } = aux; + + // irrelevant ids used to track various internal intrinsics and such + anyref_table: _, + anyref_alloc: _, + anyref_drop_slice: _, + exn_store: _, + shadow_stack_pointer: _, + } = *aux; let adapter_context = |id: AdapterId| { if let Some((name, _)) = nonstandard.exports.iter().find(|p| p.1 == id) { @@ -59,11 +63,11 @@ pub fn add( import.module, import.name ); } - unreachable!() + format!("in adapter function") }; let mut us2walrus = HashMap::new(); - for (us, func) in nonstandard.adapters.iter() { + for (us, func) in crate::sorted_iter(&nonstandard.adapters) { if let Some(export) = export_map.get(us) { check_standard_export(export).context(adapter_context(*us))?; } @@ -354,11 +358,7 @@ fn check_standard_import(import: &AuxImport) -> Result<(), Error> { } AuxImport::Closure { .. } => format!("creating a `Closure` wrapper"), }; - bail!( - "cannot generate a standalone WebAssembly module which \ - contains an import of {} since it requires JS glue", - item - ); + bail!("import of {} requires JS glue", item); } fn check_standard_export(export: &AuxExport) -> Result<(), Error> { diff --git a/crates/cli-support/src/wit/standard.rs b/crates/cli-support/src/wit/standard.rs index bfc9bb93..ed497255 100644 --- a/crates/cli-support/src/wit/standard.rs +++ b/crates/cli-support/src/wit/standard.rs @@ -1,6 +1,7 @@ use crate::descriptor::VectorKind; +use crate::wit::{AuxImport, WasmBindgenAux}; use std::borrow::Cow; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use walrus::{FunctionId, ImportId, TypedCustomSectionId}; #[derive(Default, Debug)] @@ -363,6 +364,52 @@ impl NonstandardWitSection { ); return id; } + + /// Removes any dead entries in `adapters` that are no longer necessary + /// and/or no longer referenced. + /// + /// Returns `true` if any adapters were deleted, or `false` if the adapters + /// did not change. + pub fn gc(&mut self, aux: &WasmBindgenAux) -> bool { + // Populate the live set with the exports, implements directives, and + // anything transitively referenced by those adapters. + let mut live = HashSet::new(); + for (_, id) in self.exports.iter() { + self.add_live(*id, &mut live); + } + for (_, _, id) in self.implements.iter() { + self.add_live(*id, &mut live); + } + for import in aux.import_map.values() { + if let AuxImport::Closure { adapter, .. } = import { + self.add_live(*adapter, &mut live); + } + } + + // And now that we have the live set we can filter out our list of + // adapter definitions. + let before = self.adapters.len(); + self.adapters.retain(|id, _| live.contains(id)); + before != self.adapters.len() + } + + fn add_live(&self, id: AdapterId, live: &mut HashSet) { + if !live.insert(id) { + return; + } + let instructions = match &self.adapters[&id].kind { + AdapterKind::Local { instructions } => instructions, + AdapterKind::Import { .. } => return, + }; + for instr in instructions { + match instr.instr { + Instruction::StackClosure { adapter, .. } | Instruction::CallAdapter(adapter) => { + self.add_live(adapter, live); + } + _ => {} + } + } + } } impl walrus::CustomSection for NonstandardWitSection { diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 32b17797..fe2bdfb1 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -35,7 +35,9 @@ predicates = "1.0.0" rayon = "1.0" tempfile = "3.0" walrus = "0.14" -wasmprinter = "0.2" +wit-printer = "0.1" +wit-validator = "0.1" +wit-walrus = "0.1" [[test]] name = "reference" diff --git a/crates/cli/tests/reference.rs b/crates/cli/tests/reference.rs index 6bcda84e..855f52a2 100644 --- a/crates/cli/tests/reference.rs +++ b/crates/cli/tests/reference.rs @@ -74,17 +74,18 @@ fn runtest(test: &Path) -> Result<()> { repo_root().display(), test.display(), ); + let interface_types = contents.contains("// interface-types"); fs::write(td.path().join("Cargo.toml"), manifest)?; let target_dir = target_dir(); - exec( - Command::new("cargo") - .current_dir(td.path()) - .arg("build") - .arg("--target") - .arg("wasm32-unknown-unknown") - .env("CARGO_TARGET_DIR", &target_dir), - )?; + let mut cargo = Command::new("cargo"); + cargo + .current_dir(td.path()) + .arg("build") + .arg("--target") + .arg("wasm32-unknown-unknown") + .env("CARGO_TARGET_DIR", &target_dir); + exec(&mut cargo)?; let wasm = target_dir .join("wasm32-unknown-unknown") @@ -100,26 +101,33 @@ fn runtest(test: &Path) -> Result<()> { if contents.contains("// enable-anyref") { bindgen.env("WASM_BINDGEN_ANYREF", "1"); } + if interface_types { + bindgen.env("WASM_INTERFACE_TYPES", "1"); + } exec(&mut bindgen)?; - let js = fs::read_to_string(td.path().join("reference_test.js"))?; - let wat = sanitize_wasm(&td.path().join("reference_test_bg.wasm"))?; - - let js_assertion = test.with_extension("js"); - let wat_assertion = test.with_extension("wat"); - - if env::var("BLESS").is_ok() { - fs::write(js_assertion, js)?; - fs::write(wat_assertion, wat)?; - return Ok(()); + if interface_types { + let wasm = td.path().join("reference_test.wasm"); + wit_validator::validate(&fs::read(&wasm)?)?; + let wit = sanitize_wasm(&wasm)?; + assert_same(&wit, &test.with_extension("wit"))?; + } else { + let js = fs::read_to_string(td.path().join("reference_test.js"))?; + assert_same(&js, &test.with_extension("js"))?; + let wat = sanitize_wasm(&td.path().join("reference_test_bg.wasm"))?; + assert_same(&wat, &test.with_extension("wat"))?; } - let js_expected = fs::read_to_string(&js_assertion)?; - let wat_expected = fs::read_to_string(&wat_assertion)?; - - diff(&js_expected, &js)?; - diff(&wat_expected, &wat)?; + Ok(()) +} +fn assert_same(output: &str, expected: &Path) -> Result<()> { + if env::var("BLESS").is_ok() { + fs::write(expected, output)?; + } else { + let expected = fs::read_to_string(&expected)?; + diff(&expected, output)?; + } Ok(()) } @@ -127,7 +135,9 @@ fn sanitize_wasm(wasm: &Path) -> Result { // Clean up the wasm module by removing all function // implementations/instructions, data sections, etc. This'll help us largely // only deal with exports/imports which is all we're really interested in. - let mut module = walrus::Module::from_file(wasm)?; + let mut module = walrus::ModuleConfig::new() + .on_parse(wit_walrus::on_parse) + .parse_file(wasm)?; for func in module.funcs.iter_mut() { let local = match &mut func.kind { walrus::FunctionKind::Local(l) => l, @@ -155,7 +165,7 @@ fn sanitize_wasm(wasm: &Path) -> Result { module.exports.delete(id); } walrus::passes::gc::run(&mut module); - let mut wat = wasmprinter::print_bytes(&module.emit_wasm())?; + let mut wat = wit_printer::print_bytes(&module.emit_wasm())?; wat.push_str("\n"); Ok(wat) } diff --git a/crates/cli/tests/reference/interface-types-anyref.rs b/crates/cli/tests/reference/interface-types-anyref.rs new file mode 100644 index 00000000..5b69d115 --- /dev/null +++ b/crates/cli/tests/reference/interface-types-anyref.rs @@ -0,0 +1,8 @@ +// interface-types + +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub fn anyref_in_out(a: &JsValue, b: JsValue) -> JsValue { + b +} diff --git a/crates/cli/tests/reference/interface-types-anyref.wit b/crates/cli/tests/reference/interface-types-anyref.wit new file mode 100644 index 00000000..e3d4e62b --- /dev/null +++ b/crates/cli/tests/reference/interface-types-anyref.wit @@ -0,0 +1,12 @@ +(module + (type (;0;) (func (param anyref anyref) (result anyref))) + (func $anyref_in_out anyref shim (type 0) (param anyref anyref) (result anyref)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "anyref_in_out" (func $anyref_in_out anyref shim)) + (@interface type (;0;) (func (param anyref) (param anyref) (result anyref))) + (@interface func (;0;) (type 0) + arg.get 0 + arg.get 1 + call-core $anyref_in_out anyref shim) + (@interface export "anyref_in_out" (func 0))) diff --git a/crates/cli/tests/reference/interface-types-empty.rs b/crates/cli/tests/reference/interface-types-empty.rs new file mode 100644 index 00000000..a29187d7 --- /dev/null +++ b/crates/cli/tests/reference/interface-types-empty.rs @@ -0,0 +1,7 @@ +// interface-types +// +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub fn empty() {} + diff --git a/crates/cli/tests/reference/interface-types-empty.wit b/crates/cli/tests/reference/interface-types-empty.wit new file mode 100644 index 00000000..256b406f --- /dev/null +++ b/crates/cli/tests/reference/interface-types-empty.wit @@ -0,0 +1,10 @@ +(module + (type (;0;) (func)) + (func $empty (type 0)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "empty" (func $empty)) + (@interface type (;0;) (func)) + (@interface func (;0;) (type 0) + call-core $empty) + (@interface export "empty" (func 0))) diff --git a/crates/cli/tests/reference/interface-types-integers.rs b/crates/cli/tests/reference/interface-types-integers.rs new file mode 100644 index 00000000..f5016b31 --- /dev/null +++ b/crates/cli/tests/reference/interface-types-integers.rs @@ -0,0 +1,46 @@ +// interface-types + +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub fn integers(_a1: u8, _a2: i8, _a3: u16, _a4: i16, _a5: u32, _a6: i32, _a7: f32, _a8: f64) {} + +#[wasm_bindgen] +pub fn ret_i8() -> i8 { + 0 +} + +#[wasm_bindgen] +pub fn ret_u8() -> u8 { + 1 +} + +#[wasm_bindgen] +pub fn ret_i16() -> i16 { + 2 +} + +#[wasm_bindgen] +pub fn ret_u16() -> u16 { + 3 +} + +#[wasm_bindgen] +pub fn ret_i32() -> i32 { + 4 +} + +#[wasm_bindgen] +pub fn ret_u32() -> u32 { + 5 +} + +#[wasm_bindgen] +pub fn ret_f32() -> f32 { + 6.0 +} + +#[wasm_bindgen] +pub fn ret_f64() -> f64 { + 7.0 +} diff --git a/crates/cli/tests/reference/interface-types-integers.wit b/crates/cli/tests/reference/interface-types-integers.wit new file mode 100644 index 00000000..e00118c0 --- /dev/null +++ b/crates/cli/tests/reference/interface-types-integers.wit @@ -0,0 +1,81 @@ +(module + (type (;0;) (func (result i32))) + (type (;1;) (func (result f32))) + (type (;2;) (func (result f64))) + (type (;3;) (func (param i32 i32 i32 i32 i32 i32 f32 f64))) + (func $integers (type 3) (param i32 i32 i32 i32 i32 i32 f32 f64)) + (func $ret_i8 (type 0) (result i32)) + (func $ret_u8 (type 0) (result i32)) + (func $ret_i16 (type 0) (result i32)) + (func $ret_u16 (type 0) (result i32)) + (func $ret_i32 (type 0) (result i32)) + (func $ret_u32 (type 0) (result i32)) + (func $ret_f32 (type 1) (result f32)) + (func $ret_f64 (type 2) (result f64)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "integers" (func $integers)) + (export "ret_i8" (func $ret_i8)) + (export "ret_u8" (func $ret_u8)) + (export "ret_i16" (func $ret_i16)) + (export "ret_u16" (func $ret_u16)) + (export "ret_i32" (func $ret_i32)) + (export "ret_u32" (func $ret_u32)) + (export "ret_f32" (func $ret_f32)) + (export "ret_f64" (func $ret_f64)) + (@interface type (;0;) (func (param u8) (param s8) (param u16) (param s16) (param u32) (param s32) (param f32) (param f64))) + (@interface type (;1;) (func (result s8))) + (@interface type (;2;) (func (result u8))) + (@interface type (;3;) (func (result s16))) + (@interface type (;4;) (func (result u16))) + (@interface type (;5;) (func (result s32))) + (@interface type (;6;) (func (result u32))) + (@interface type (;7;) (func (result f32))) + (@interface type (;8;) (func (result f64))) + (@interface func (;0;) (type 0) + arg.get 0 + u8-to-i32 + arg.get 1 + s8-to-i32 + arg.get 2 + u16-to-i32 + arg.get 3 + s16-to-i32 + arg.get 4 + u32-to-i32 + arg.get 5 + s32-to-i32 + arg.get 6 + arg.get 7 + call-core $integers) + (@interface func (;1;) (type 1) + call-core $ret_i8 + i32-to-s8) + (@interface func (;2;) (type 2) + call-core $ret_u8 + i32-to-u8) + (@interface func (;3;) (type 3) + call-core $ret_i16 + i32-to-s16) + (@interface func (;4;) (type 4) + call-core $ret_u16 + i32-to-u16) + (@interface func (;5;) (type 5) + call-core $ret_i32 + i32-to-s32) + (@interface func (;6;) (type 6) + call-core $ret_u32 + i32-to-u32) + (@interface func (;7;) (type 7) + call-core $ret_f32) + (@interface func (;8;) (type 8) + call-core $ret_f64) + (@interface export "integers" (func 0)) + (@interface export "ret_i8" (func 1)) + (@interface export "ret_u8" (func 2)) + (@interface export "ret_i16" (func 3)) + (@interface export "ret_u16" (func 4)) + (@interface export "ret_i32" (func 5)) + (@interface export "ret_u32" (func 6)) + (@interface export "ret_f32" (func 7)) + (@interface export "ret_f64" (func 8))) diff --git a/crates/cli/tests/reference/interface-types-interop.rs b/crates/cli/tests/reference/interface-types-interop.rs new file mode 100644 index 00000000..191426d5 --- /dev/null +++ b/crates/cli/tests/reference/interface-types-interop.rs @@ -0,0 +1,8 @@ +// interface-types + +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub fn take_and_return(a: u8) -> u16 { + a.into() +} diff --git a/crates/cli/tests/reference/interface-types-interop.wit b/crates/cli/tests/reference/interface-types-interop.wit new file mode 100644 index 00000000..193b8f18 --- /dev/null +++ b/crates/cli/tests/reference/interface-types-interop.wit @@ -0,0 +1,13 @@ +(module + (type (;0;) (func (param i32) (result i32))) + (func $take_and_return (type 0) (param i32) (result i32)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "take_and_return" (func $take_and_return)) + (@interface type (;0;) (func (param u8) (result u16))) + (@interface func (;0;) (type 0) + arg.get 0 + u8-to-i32 + call-core $take_and_return + i32-to-u16) + (@interface export "take_and_return" (func 0))) diff --git a/crates/cli/tests/reference/interface-types-strings.rs b/crates/cli/tests/reference/interface-types-strings.rs new file mode 100644 index 00000000..d7a67f0f --- /dev/null +++ b/crates/cli/tests/reference/interface-types-strings.rs @@ -0,0 +1,11 @@ +// interface-types + +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub fn strings(a: &str) -> String { + String::new() +} + +#[wasm_bindgen] +pub fn many_strings(a: &str, b: String) {} diff --git a/crates/cli/tests/reference/interface-types-strings.wit b/crates/cli/tests/reference/interface-types-strings.wit new file mode 100644 index 00000000..24fb0b4c --- /dev/null +++ b/crates/cli/tests/reference/interface-types-strings.wit @@ -0,0 +1,29 @@ +(module + (type (;0;) (func (param i32) (result i32))) + (type (;1;) (func (param i32 i32))) + (type (;2;) (func (param i32 i32) (result i32 i32))) + (type (;3;) (func (param i32 i32 i32 i32))) + (func $__wbindgen_malloc (type 0) (param i32) (result i32)) + (func $many_strings (type 3) (param i32 i32 i32 i32)) + (func $__wbindgen_free (type 1) (param i32 i32)) + (func $strings multivalue shim (type 2) (param i32 i32) (result i32 i32)) + (memory (;0;) 17) + (export "memory" (memory 0)) + (export "strings" (func $strings multivalue shim)) + (export "many_strings" (func $many_strings)) + (@interface type (;0;) (func (param string) (result string))) + (@interface type (;1;) (func (param string) (param string))) + (@interface func (;0;) (type 0) + arg.get 0 + string-to-memory $__wbindgen_malloc + call-core $strings multivalue shim + defer-call-core $__wbindgen_free + memory-to-string) + (@interface func (;1;) (type 1) + arg.get 0 + string-to-memory $__wbindgen_malloc + arg.get 1 + string-to-memory $__wbindgen_malloc + call-core $many_strings) + (@interface export "strings" (func 0)) + (@interface export "many_strings" (func 1))) diff --git a/crates/cli/tests/wasm-bindgen/main.rs b/crates/cli/tests/wasm-bindgen/main.rs index 568940d7..64494d03 100644 --- a/crates/cli/tests/wasm-bindgen/main.rs +++ b/crates/cli/tests/wasm-bindgen/main.rs @@ -233,3 +233,104 @@ fn empty_interface_types() { cmd.env("WASM_INTERFACE_TYPES", "1"); cmd.assert().success(); } + +#[test] +fn bad_interface_types_export() -> anyhow::Result<()> { + let (mut cmd, _out_dir) = Project::new("bad_interface_types_export") + .file( + "src/lib.rs", + r#" + use wasm_bindgen::prelude::*; + + #[wasm_bindgen] + pub fn foo(a: Vec) {} + "#, + ) + .file( + "Cargo.toml", + &format!( + " + [package] + name = \"bad_interface_types_export\" + authors = [] + version = \"1.0.0\" + edition = '2018' + + [lib] + crate-type = [\"cdylib\"] + + [dependencies] + wasm-bindgen = {{ path = '{}' }} + + [workspace] + ", + repo_root().display(), + ), + ) + .wasm_bindgen(""); + cmd.env("WASM_INTERFACE_TYPES", "1"); + cmd.assert().failure().code(1).stderr(str::is_match( + "\ +error: failed to generate a standard interface types section + +Caused by: + 0: in function export `foo` + 1: type Vector\\(U8\\) isn't supported in standard interface types +$", + )?); + Ok(()) +} + +#[test] +fn bad_interface_types_import() -> anyhow::Result<()> { + let (mut cmd, _out_dir) = Project::new("bad_interface_types_import") + .file( + "src/lib.rs", + r#" + use wasm_bindgen::prelude::*; + + #[wasm_bindgen] + extern "C" { + pub fn foo() -> Vec; + } + + #[wasm_bindgen] + pub fn bar() { + foo(); + } + "#, + ) + .file( + "Cargo.toml", + &format!( + " + [package] + name = \"bad_interface_types_import\" + authors = [] + version = \"1.0.0\" + edition = '2018' + + [lib] + crate-type = [\"cdylib\"] + + [dependencies] + wasm-bindgen = {{ path = '{}' }} + + [workspace] + ", + repo_root().display(), + ), + ) + .wasm_bindgen(""); + cmd.env("WASM_INTERFACE_TYPES", "1"); + cmd.assert().failure().code(1).stderr(str::is_match( + "\ +error: failed to generate a standard interface types section + +Caused by: + 0: in adapter function + 1: import of global `foo` requires JS glue +$", + )?); + Ok(()) +} diff --git a/crates/wasm-conventions/src/lib.rs b/crates/wasm-conventions/src/lib.rs index 11ba8889..61d84908 100755 --- a/crates/wasm-conventions/src/lib.rs +++ b/crates/wasm-conventions/src/lib.rs @@ -9,7 +9,7 @@ #![deny(missing_docs, missing_debug_implementations)] use anyhow::{anyhow, bail, Error}; -use walrus::{GlobalId, GlobalKind, MemoryId, Module, ValType}; +use walrus::{GlobalId, GlobalKind, MemoryId, Module, ValType, InitExpr, ir::Value}; /// Get a Wasm module's canonical linear memory. pub fn get_memory(module: &Module) -> Result { @@ -39,21 +39,19 @@ pub fn get_shadow_stack_pointer(module: &Module) -> Result { .iter() .filter(|g| g.ty == ValType::I32) .filter(|g| g.mutable) + // The stack pointer is guaranteed to not be initialized to 0, and it's + // guaranteed to have an i32 initializer, so find globals which are + // locally defined, are an i32, and have a nonzero initializer .filter(|g| match g.kind { - GlobalKind::Local(_) => true, - GlobalKind::Import(_) => false, + GlobalKind::Local(InitExpr::Value(Value::I32(n))) => n != 0, + _ => false, }) .collect::>(); let ssp = match candidates.len() { 0 => bail!("could not find the shadow stack pointer for the module"), - // If we've got two mutable globals then we're in a pretty standard - // situation for threaded code where one is the stack pointer and one is the - // TLS base offset. We need to figure out which is which, and we basically - // assume LLVM's current codegen where the first is the stack pointer. - // // TODO: have an actual check here. - 1 | 2 => candidates[0].id(), + 1 => candidates[0].id(), _ => bail!("too many mutable globals to infer which is the shadow stack pointer"), };