From a11a497515c1800e81b88be836d7d8da0a89b537 Mon Sep 17 00:00:00 2001 From: David Huculak Date: Tue, 21 May 2024 04:04:15 -0400 Subject: [PATCH] Improve the performance of 'string enums' (#3915) --- CHANGELOG.md | 3 + benchmarks/index.html | 4 +- crates/backend/src/ast.rs | 8 +- crates/backend/src/codegen.rs | 123 +++++++++++++--------- crates/backend/src/encode.rs | 4 +- crates/cli-support/src/descriptor.rs | 25 ++++- crates/cli-support/src/js/binding.rs | 91 ++++++++++++++++ crates/cli-support/src/wit/incoming.rs | 26 +++++ crates/cli-support/src/wit/outgoing.rs | 32 ++++++ crates/cli-support/src/wit/standard.rs | 23 ++++ crates/macro-support/src/parser.rs | 6 +- crates/shared/src/lib.rs | 6 +- crates/shared/src/schema_hash_approval.rs | 2 +- crates/wasm-interpreter/src/lib.rs | 16 ++- crates/wasm-interpreter/tests/smoke.rs | 2 +- src/describe.rs | 1 + 16 files changed, 298 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b572de94ee8..afb6eca43f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ * Generate JS bindings for WebIDL dictionary setters instead of using `Reflect`. This increases the size of the Web API bindings but should be more performant. Also, importing getters/setters from JS now supports specifying the JS attribute name as a string, e.g. `#[wasm_bindgen(method, setter = "x-cdm-codecs")]`. [#3898](https://github.com/rustwasm/wasm-bindgen/pull/3898) +* Greatly improve the performance of sending WebIDL 'string enums' across the JavaScript boundary by converting the enum variant string to/from an int. + [#3915](https://github.com/rustwasm/wasm-bindgen/pull/3915) + ### Fixed * Fix `catch` not being thread-safe. diff --git a/benchmarks/index.html b/benchmarks/index.html index 020839f85e8..49cf021333a 100644 --- a/benchmarks/index.html +++ b/benchmarks/index.html @@ -277,12 +277,12 @@

wasm-bindgen benchmarks

- Call a custom JS function with an enum value parameter + Call a custom JS function with a string enum value parameter (?)

- Benchmarks the speed of passing enum values to javascript + Benchmarks the speed of passing string enums to javascript

diff --git a/crates/backend/src/ast.rs b/crates/backend/src/ast.rs index 95c62d63bd8..de94cabbd22 100644 --- a/crates/backend/src/ast.rs +++ b/crates/backend/src/ast.rs @@ -163,7 +163,7 @@ pub enum ImportKind { /// Importing a type/class Type(ImportType), /// Importing a JS enum - Enum(ImportEnum), + Enum(StringEnum), } /// A function being imported from JS @@ -302,10 +302,10 @@ pub struct ImportType { pub wasm_bindgen: Path, } -/// The metadata for an Enum being imported +/// The metadata for a String Enum #[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))] #[derive(Clone)] -pub struct ImportEnum { +pub struct StringEnum { /// The Rust enum's visibility pub vis: syn::Visibility, /// The Rust enum's identifiers @@ -404,7 +404,7 @@ pub struct StructField { pub wasm_bindgen: Path, } -/// Information about an Enum being exported +/// The metadata for an Enum #[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))] #[derive(Clone)] pub struct Enum { diff --git a/crates/backend/src/codegen.rs b/crates/backend/src/codegen.rs index bd7ef96f70e..d3a49b2ed89 100644 --- a/crates/backend/src/codegen.rs +++ b/crates/backend/src/codegen.rs @@ -2,7 +2,7 @@ use crate::ast; use crate::encode; use crate::Diagnostic; use once_cell::sync::Lazy; -use proc_macro2::{Ident, Literal, Span, TokenStream}; +use proc_macro2::{Ident, Span, TokenStream}; use quote::format_ident; use quote::quote_spanned; use quote::{quote, ToTokens}; @@ -1015,33 +1015,31 @@ impl ToTokens for ast::ImportType { } } -impl ToTokens for ast::ImportEnum { +impl ToTokens for ast::StringEnum { fn to_tokens(&self, tokens: &mut TokenStream) { let vis = &self.vis; - let name = &self.name; - let expect_string = format!("attempted to convert invalid {} into JSValue", name); + let enum_name = &self.name; + let name_str = enum_name.to_string(); + let name_len = name_str.len() as u32; + let name_chars = name_str.chars().map(u32::from); let variants = &self.variants; - let variant_strings = &self.variant_values; + let variant_count = self.variant_values.len() as u32; + let variant_values = &self.variant_values; + let variant_indices = (0..variant_count).collect::>(); + let invalid = variant_count; + let hole = variant_count + 1; let attrs = &self.rust_attrs; - let mut current_idx: usize = 0; - let variant_indexes: Vec = variants - .iter() - .map(|_| { - let this_index = current_idx; - current_idx += 1; - Literal::usize_unsuffixed(this_index) - }) - .collect(); - - // Borrow variant_indexes because we need to use it multiple times inside the quote! macro - let variant_indexes_ref = &variant_indexes; + let invalid_to_str_msg = format!( + "Converting an invalid string enum ({}) back to a string is currently not supported", + enum_name + ); // A vector of EnumName::VariantName tokens for this enum let variant_paths: Vec = self .variants .iter() - .map(|v| quote!(#name::#v).into_token_stream()) + .map(|v| quote!(#enum_name::#v).into_token_stream()) .collect(); // Borrow variant_paths because we need to use it multiple times inside the quote! macro @@ -1049,83 +1047,104 @@ impl ToTokens for ast::ImportEnum { let wasm_bindgen = &self.wasm_bindgen; + let describe_variants = self.variant_values.iter().map(|variant_value| { + let length = variant_value.len() as u32; + let chars = variant_value.chars().map(u32::from); + quote! { + inform(#length); + #(inform(#chars);)* + } + }); + (quote! { #(#attrs)* - #vis enum #name { - #(#variants = #variant_indexes_ref,)* + #[non_exhaustive] + #[repr(u32)] + #vis enum #enum_name { + #(#variants = #variant_indices,)* #[automatically_derived] #[doc(hidden)] - __Nonexhaustive, + __Invalid } #[automatically_derived] - impl #name { - fn from_str(s: &str) -> Option<#name> { + impl #enum_name { + fn from_str(s: &str) -> Option<#enum_name> { match s { - #(#variant_strings => Some(#variant_paths_ref),)* + #(#variant_values => Some(#variant_paths_ref),)* _ => None, } } fn to_str(&self) -> &'static str { match self { - #(#variant_paths_ref => #variant_strings,)* - #name::__Nonexhaustive => panic!(#expect_string), + #(#variant_paths_ref => #variant_values,)* + #enum_name::__Invalid => panic!(#invalid_to_str_msg), } } - #vis fn from_js_value(obj: &#wasm_bindgen::JsValue) -> Option<#name> { + #vis fn from_js_value(obj: &#wasm_bindgen::JsValue) -> Option<#enum_name> { obj.as_string().and_then(|obj_str| Self::from_str(obj_str.as_str())) } } - // It should really be using &str for all of these, but that requires some major changes to cli-support #[automatically_derived] - impl #wasm_bindgen::describe::WasmDescribe for #name { - fn describe() { - <#wasm_bindgen::JsValue as #wasm_bindgen::describe::WasmDescribe>::describe() - } - } - - #[automatically_derived] - impl #wasm_bindgen::convert::IntoWasmAbi for #name { - type Abi = <#wasm_bindgen::JsValue as #wasm_bindgen::convert::IntoWasmAbi>::Abi; + impl #wasm_bindgen::convert::IntoWasmAbi for #enum_name { + type Abi = u32; #[inline] - fn into_abi(self) -> Self::Abi { - <#wasm_bindgen::JsValue as #wasm_bindgen::convert::IntoWasmAbi>::into_abi(self.into()) + fn into_abi(self) -> u32 { + self as u32 } } #[automatically_derived] - impl #wasm_bindgen::convert::FromWasmAbi for #name { - type Abi = <#wasm_bindgen::JsValue as #wasm_bindgen::convert::FromWasmAbi>::Abi; + impl #wasm_bindgen::convert::FromWasmAbi for #enum_name { + type Abi = u32; - unsafe fn from_abi(js: Self::Abi) -> Self { - let s = <#wasm_bindgen::JsValue as #wasm_bindgen::convert::FromWasmAbi>::from_abi(js); - #name::from_js_value(&s).unwrap_or(#name::__Nonexhaustive) + unsafe fn from_abi(val: u32) -> Self { + match val { + #(#variant_indices => #variant_paths_ref,)* + #invalid => #enum_name::__Invalid, + _ => unreachable!("The JS binding should only ever produce a valid value or the specific 'invalid' value"), + } } } #[automatically_derived] - impl #wasm_bindgen::convert::OptionIntoWasmAbi for #name { + impl #wasm_bindgen::convert::OptionFromWasmAbi for #enum_name { #[inline] - fn none() -> Self::Abi { <::js_sys::Object as #wasm_bindgen::convert::OptionIntoWasmAbi>::none() } + fn is_none(val: &u32) -> bool { *val == #hole } } #[automatically_derived] - impl #wasm_bindgen::convert::OptionFromWasmAbi for #name { + impl #wasm_bindgen::convert::OptionIntoWasmAbi for #enum_name { #[inline] - fn is_none(abi: &Self::Abi) -> bool { <::js_sys::Object as #wasm_bindgen::convert::OptionFromWasmAbi>::is_none(abi) } + fn none() -> Self::Abi { #hole } + } + + #[automatically_derived] + impl #wasm_bindgen::describe::WasmDescribe for #enum_name { + fn describe() { + use #wasm_bindgen::describe::*; + inform(STRING_ENUM); + inform(#name_len); + #(inform(#name_chars);)* + inform(#variant_count); + #(#describe_variants)* + } } #[automatically_derived] - impl From<#name> for #wasm_bindgen::JsValue { - fn from(obj: #name) -> #wasm_bindgen::JsValue { - #wasm_bindgen::JsValue::from(obj.to_str()) + impl #wasm_bindgen::__rt::core::convert::From<#enum_name> for + #wasm_bindgen::JsValue + { + fn from(val: #enum_name) -> Self { + #wasm_bindgen::JsValue::from_str(val.to_str()) } } - }).to_tokens(tokens); + }) + .to_tokens(tokens); } } diff --git a/crates/backend/src/encode.rs b/crates/backend/src/encode.rs index 9341ac170dd..6710096d137 100644 --- a/crates/backend/src/encode.rs +++ b/crates/backend/src/encode.rs @@ -325,8 +325,8 @@ fn shared_import_type<'a>(i: &'a ast::ImportType, intern: &'a Interner) -> Impor } } -fn shared_import_enum<'a>(_i: &'a ast::ImportEnum, _intern: &'a Interner) -> ImportEnum { - ImportEnum {} +fn shared_import_enum<'a>(_i: &'a ast::StringEnum, _intern: &'a Interner) -> StringEnum { + StringEnum {} } fn shared_struct<'a>(s: &'a ast::Struct, intern: &'a Interner) -> Struct<'a> { diff --git a/crates/cli-support/src/descriptor.rs b/crates/cli-support/src/descriptor.rs index 62dd766ac7a..6a35786eba5 100644 --- a/crates/cli-support/src/descriptor.rs +++ b/crates/cli-support/src/descriptor.rs @@ -34,6 +34,7 @@ tys! { EXTERNREF NAMED_EXTERNREF ENUM + STRING_ENUM RUST_STRUCT CHAR OPTIONAL @@ -67,7 +68,16 @@ pub enum Descriptor { String, Externref, NamedExternref(String), - Enum { name: String, hole: u32 }, + Enum { + name: String, + hole: u32, + }, + StringEnum { + name: String, + invalid: u32, + hole: u32, + variant_values: Vec, + }, RustStruct(String), Char, Option(Box), @@ -156,6 +166,19 @@ impl Descriptor { let hole = get(data); Descriptor::Enum { name, hole } } + STRING_ENUM => { + let name = get_string(data); + let variant_count = get(data); + let invalid = variant_count; + let hole = variant_count + 1; + let variant_values = (0..variant_count).map(|_| get_string(data)).collect(); + Descriptor::StringEnum { + name, + invalid, + hole, + variant_values, + } + } RUST_STRUCT => { let name = get_string(data); Descriptor::RustStruct(name) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index ebe4e966c43..31863da50ab 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -668,6 +668,96 @@ fn instruction( } } + Instruction::WasmToStringEnum { variant_values } => { + let index = js.pop(); + + // e.g. ["a","b","c"][someIndex] + let mut enum_val_expr = String::new(); + enum_val_expr.push('['); + for variant in variant_values { + enum_val_expr.push_str(&format!("\"{variant}\",")); + } + enum_val_expr.push(']'); + enum_val_expr.push('['); + enum_val_expr.push_str(&index); + enum_val_expr.push(']'); + + js.push(enum_val_expr) + } + + Instruction::OptionWasmToStringEnum { + variant_values, + hole, + } => { + let index = js.pop(); + + let mut enum_val_expr = String::new(); + enum_val_expr.push('['); + for variant in variant_values { + enum_val_expr.push_str(&format!("\"{variant}\",")); + } + enum_val_expr.push(']'); + enum_val_expr.push('['); + enum_val_expr.push_str(&index); + enum_val_expr.push(']'); + + // e.g. someIndex === 4 ? undefined : (["a","b","c"][someIndex]) + // | + // currently, hole = variant_count + 1 + js.push(format!( + "{index} === {hole} ? undefined : ({enum_val_expr})" + )) + } + + Instruction::StringEnumToWasm { + variant_values, + invalid, + } => { + let enum_val = js.pop(); + + // e.g. {"a":0,"b":1,"c":2}[someEnumVal] ?? 3 + // | + // currently, invalid = variant_count + let mut enum_val_expr = String::new(); + enum_val_expr.push('{'); + for (i, variant) in variant_values.iter().enumerate() { + enum_val_expr.push_str(&format!("\"{variant}\":{i},")); + } + enum_val_expr.push('}'); + enum_val_expr.push('['); + enum_val_expr.push_str(&enum_val); + enum_val_expr.push(']'); + enum_val_expr.push_str(&format!(" ?? {invalid}")); + + js.push(enum_val_expr) + } + + Instruction::OptionStringEnumToWasm { + variant_values, + invalid, + hole, + } => { + let enum_val = js.pop(); + + let mut enum_val_expr = String::new(); + enum_val_expr.push('{'); + for (i, variant) in variant_values.iter().enumerate() { + enum_val_expr.push_str(&format!("\"{variant}\":{i},")); + } + enum_val_expr.push('}'); + enum_val_expr.push('['); + enum_val_expr.push_str(&enum_val); + enum_val_expr.push(']'); + enum_val_expr.push_str(&format!(" ?? {invalid}")); + + // e.g. someEnumVal == undefined ? 4 : ({"a":0,"b":1,"c":2}[someEnumVal] ?? 3) + // | + // double equals here in case it's null + js.push(format!( + "{enum_val} == undefined ? {hole} : ({enum_val_expr})" + )) + } + Instruction::MemoryToString(mem) => { let len = js.pop(); let ptr = js.pop(); @@ -1377,6 +1467,7 @@ fn adapter2ts(ty: &AdapterType, dst: &mut String) { AdapterType::NamedExternref(name) => dst.push_str(name), AdapterType::Struct(name) => dst.push_str(name), AdapterType::Enum(name) => dst.push_str(name), + AdapterType::StringEnum(name) => dst.push_str(name), AdapterType::Function => dst.push_str("any"), } } diff --git a/crates/cli-support/src/wit/incoming.rs b/crates/cli-support/src/wit/incoming.rs index 83bd3b32b16..583c1c34378 100644 --- a/crates/cli-support/src/wit/incoming.rs +++ b/crates/cli-support/src/wit/incoming.rs @@ -108,6 +108,16 @@ impl InstructionBuilder<'_, '_> { &[AdapterType::I32], ); }, + Descriptor::StringEnum { name, variant_values, invalid, .. } => { + self.instruction( + &[AdapterType::StringEnum(name.clone())], + Instruction::StringEnumToWasm { + variant_values: variant_values.clone(), + invalid: *invalid, + }, + &[AdapterType::I32], + ); + }, Descriptor::Ref(d) => self.incoming_ref(false, d)?, Descriptor::RefMut(d) => self.incoming_ref(true, d)?, Descriptor::Option(d) => self.incoming_option(d)?, @@ -296,6 +306,22 @@ impl InstructionBuilder<'_, '_> { &[AdapterType::I32], ); } + Descriptor::StringEnum { + name, + variant_values, + invalid, + hole, + } => { + self.instruction( + &[AdapterType::StringEnum(name.clone()).option()], + Instruction::OptionStringEnumToWasm { + variant_values: variant_values.clone(), + invalid: *invalid, + hole: *hole, + }, + &[AdapterType::I32], + ); + } Descriptor::RustStruct(name) => { self.instruction( &[AdapterType::Struct(name.clone()).option()], diff --git a/crates/cli-support/src/wit/outgoing.rs b/crates/cli-support/src/wit/outgoing.rs index 2c0fceefbcd..033217f61ad 100644 --- a/crates/cli-support/src/wit/outgoing.rs +++ b/crates/cli-support/src/wit/outgoing.rs @@ -74,6 +74,12 @@ impl InstructionBuilder<'_, '_> { self.output.push(AdapterType::F64); } Descriptor::Enum { name, .. } => self.outgoing_i32(AdapterType::Enum(name.clone())), + Descriptor::StringEnum { + name, + variant_values, + invalid: _, + hole: _, + } => self.outgoing_string_enum(name, variant_values), Descriptor::Char => { self.instruction( @@ -287,6 +293,21 @@ impl InstructionBuilder<'_, '_> { &[AdapterType::Enum(name.clone()).option()], ); } + Descriptor::StringEnum { + name, + invalid: _, + hole, + variant_values, + } => { + self.instruction( + &[AdapterType::I32], + Instruction::OptionWasmToStringEnum { + variant_values: variant_values.to_vec(), + hole: *hole, + }, + &[AdapterType::StringEnum(String::from(name))], + ); + } Descriptor::RustStruct(name) => { self.instruction( &[AdapterType::I32], @@ -352,6 +373,7 @@ impl InstructionBuilder<'_, '_> { | Descriptor::Boolean | Descriptor::Char | Descriptor::Enum { .. } + | Descriptor::StringEnum { .. } | Descriptor::RustStruct(_) | Descriptor::Ref(_) | Descriptor::RefMut(_) @@ -520,6 +542,16 @@ impl InstructionBuilder<'_, '_> { self.instruction(&[AdapterType::I32], instr, &[output]); } + fn outgoing_string_enum(&mut self, name: &str, variant_values: &[String]) { + self.instruction( + &[AdapterType::I32], + Instruction::WasmToStringEnum { + variant_values: variant_values.to_vec(), + }, + &[AdapterType::StringEnum(String::from(name))], + ); + } + fn outgoing_i64(&mut self, output: AdapterType) { let instr = Instruction::WasmToInt { input: walrus::ValType::I64, diff --git a/crates/cli-support/src/wit/standard.rs b/crates/cli-support/src/wit/standard.rs index 104a297abfa..e4d7aa745c1 100644 --- a/crates/cli-support/src/wit/standard.rs +++ b/crates/cli-support/src/wit/standard.rs @@ -86,6 +86,7 @@ pub enum AdapterType { Option(Box), Struct(String), Enum(String), + StringEnum(String), NamedExternref(String), Function, NonNull, @@ -140,6 +141,28 @@ pub enum Instruction { output: AdapterType, }, + /// Pops a wasm `i32` and pushes the enum variant as a string + WasmToStringEnum { + variant_values: Vec, + }, + + OptionWasmToStringEnum { + variant_values: Vec, + hole: u32, + }, + + /// pops a string and pushes the enum variant as an `i32` + StringEnumToWasm { + variant_values: Vec, + invalid: u32, + }, + + OptionStringEnumToWasm { + variant_values: Vec, + invalid: u32, + hole: u32, + }, + /// Pops a `bool` from the stack and pushes an `i32` equivalent I32FromBool, /// Pops a `string` from the stack and pushes the first character as `i32` diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index 2b161c04c3b..0bbc5df659a 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -1229,7 +1229,7 @@ impl<'a> MacroParse<&ClassMarker> for &'a mut syn::ImplItemFn { } } -fn import_enum(enum_: syn::ItemEnum, program: &mut ast::Program) -> Result<(), Diagnostic> { +fn string_enum(enum_: syn::ItemEnum, program: &mut ast::Program) -> Result<(), Diagnostic> { let mut variants = vec![]; let mut variant_values = vec![]; @@ -1263,7 +1263,7 @@ fn import_enum(enum_: syn::ItemEnum, program: &mut ast::Program) -> Result<(), D program.imports.push(ast::Import { module: None, js_namespace: None, - kind: ast::ImportKind::Enum(ast::ImportEnum { + kind: ast::ImportKind::Enum(ast::StringEnum { vis: enum_.vis, name: enum_.ident, variants, @@ -1295,7 +1295,7 @@ impl<'a> MacroParse<(&'a mut TokenStream, BindgenAttrs)> for syn::ItemEnum { }) = get_expr(expr) { opts.check_used(); - return import_enum(self, program); + return string_enum(self, program); } } let js_name = opts diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index f8ad45c7cd3..2e2d6eb0d7a 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -6,7 +6,7 @@ mod schema_hash_approval; // This gets changed whenever our schema changes. // At this time versions of wasm-bindgen and wasm-bindgen-cli are required to have the exact same // SCHEMA_VERSION in order to work together. -pub const SCHEMA_VERSION: &str = "0.2.92"; +pub const SCHEMA_VERSION: &str = "0.2.93"; #[macro_export] macro_rules! shared_api { @@ -46,7 +46,7 @@ macro_rules! shared_api { Function(ImportFunction<'a>), Static(ImportStatic<'a>), Type(ImportType<'a>), - Enum(ImportEnum), + Enum(StringEnum), } struct ImportFunction<'a> { @@ -94,7 +94,7 @@ macro_rules! shared_api { vendor_prefixes: Vec<&'a str>, } - struct ImportEnum {} + struct StringEnum {} struct Export<'a> { class: Option<&'a str>, diff --git a/crates/shared/src/schema_hash_approval.rs b/crates/shared/src/schema_hash_approval.rs index 471ccc9beac..0ba79b8eefb 100644 --- a/crates/shared/src/schema_hash_approval.rs +++ b/crates/shared/src/schema_hash_approval.rs @@ -8,7 +8,7 @@ // If the schema in this library has changed then: // 1. Bump the version in `crates/shared/Cargo.toml` // 2. Change the `SCHEMA_VERSION` in this library to this new Cargo.toml version -const APPROVED_SCHEMA_FILE_HASH: &str = "11955579329744078753"; +const APPROVED_SCHEMA_FILE_HASH: &str = "6362870592255096957"; #[test] fn schema_version() { diff --git a/crates/wasm-interpreter/src/lib.rs b/crates/wasm-interpreter/src/lib.rs index b5fda3b8172..359eb197a94 100644 --- a/crates/wasm-interpreter/src/lib.rs +++ b/crates/wasm-interpreter/src/lib.rs @@ -68,11 +68,9 @@ impl Interpreter { pub fn new(module: &Module) -> Result { let mut ret = Interpreter::default(); - // The descriptor functions shouldn't really use all that much memory - // (the LLVM call stack, now the wasm stack). To handle that let's give - // our selves a little bit of memory and set the stack pointer (global - // 0) to the top. - ret.mem = vec![0; 0x400]; + // Give ourselves some memory and set the stack pointer + // (the LLVM call stack, now the wasm stack, global 0) to the top. + ret.mem = vec![0; 0x8000]; ret.sp = ret.mem.len() as i32; // Figure out where the `__wbindgen_describe` imported function is, if @@ -299,6 +297,10 @@ impl Frame<'_> { // theory there doesn't need to be. Instr::Load(e) => { let address = stack.pop().unwrap(); + assert!( + address > 0, + "Read a negative address value from the stack. Did we run out of memory?" + ); let address = address as u32 + e.arg.offset; assert!(address % 4 == 0); stack.push(self.interp.mem[address as usize / 4]) @@ -306,6 +308,10 @@ impl Frame<'_> { Instr::Store(e) => { let value = stack.pop().unwrap(); let address = stack.pop().unwrap(); + assert!( + address > 0, + "Read a negative address value from the stack. Did we run out of memory?" + ); let address = address as u32 + e.arg.offset; assert!(address % 4 == 0); self.interp.mem[address as usize / 4] = value; diff --git a/crates/wasm-interpreter/tests/smoke.rs b/crates/wasm-interpreter/tests/smoke.rs index c14545c2ffa..6848d003932 100644 --- a/crates/wasm-interpreter/tests/smoke.rs +++ b/crates/wasm-interpreter/tests/smoke.rs @@ -91,7 +91,7 @@ fn globals() { ) "#; // __wbindgen_describe is called with a global - in Frame.eval we assume all access to globals is the stack pointer - interpret(wat, "foo", Some(&[1024])); + interpret(wat, "foo", Some(&[32768])); } #[test] diff --git a/src/describe.rs b/src/describe.rs index 23190f71987..140018a8097 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -42,6 +42,7 @@ tys! { EXTERNREF NAMED_EXTERNREF ENUM + STRING_ENUM RUST_STRUCT CHAR OPTIONAL