From 63f8b739dbaab71ed5955b0ca2259b7badd9127d Mon Sep 17 00:00:00 2001 From: daxpedda Date: Wed, 22 May 2024 02:44:12 +0200 Subject: [PATCH 1/2] Fix missing target features when using xforms --- CHANGELOG.md | 5 ++- crates/externref-xform/Cargo.toml | 1 + crates/externref-xform/src/lib.rs | 3 ++ crates/multi-value-xform/Cargo.toml | 1 + crates/multi-value-xform/src/lib.rs | 3 ++ crates/wasm-conventions/Cargo.toml | 3 ++ crates/wasm-conventions/src/lib.rs | 69 ++++++++++++++++++++++++++++- 7 files changed, 83 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47e952f69d0..ba338a203f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,9 +45,12 @@ * Fix MSRV compilation. [#3927](https://github.com/rustwasm/wasm-bindgen/pull/3927) -* Fixed `clippy::empty_docs` lint. +* Fix `clippy::empty_docs` lint. [#3946](https://github.com/rustwasm/wasm-bindgen/pull/3946) +* Fix missing target features in module when enabling reference types or multi-value transformation. + [#3967](https://github.com/rustwasm/wasm-bindgen/pull/3967) + -------------------------------------------------------------------------------- ## [0.2.92](https://github.com/rustwasm/wasm-bindgen/compare/0.2.91...0.2.92) diff --git a/crates/externref-xform/Cargo.toml b/crates/externref-xform/Cargo.toml index eb09f087eca..173967be1ee 100644 --- a/crates/externref-xform/Cargo.toml +++ b/crates/externref-xform/Cargo.toml @@ -15,6 +15,7 @@ rust-version = "1.57" [dependencies] anyhow = "1.0" walrus = "0.20.2" +wasm-bindgen-wasm-conventions = { path = "../wasm-conventions", version = "=0.2.92" } [dev-dependencies] rayon = "1.0" diff --git a/crates/externref-xform/src/lib.rs b/crates/externref-xform/src/lib.rs index f861f366f0c..57c912fb7ba 100644 --- a/crates/externref-xform/src/lib.rs +++ b/crates/externref-xform/src/lib.rs @@ -101,6 +101,9 @@ impl Context { /// large the function table is so we know what indexes to hand out when /// we're appending entries. pub fn prepare(&mut self, module: &mut Module) -> Result<(), Error> { + // Insert reference types to the target features section. + wasm_bindgen_wasm_conventions::insert_target_feature(module, "reference-types"); + // Figure out what the maximum index of functions pointers are. We'll // be adding new entries to the function table later (maybe) so // precalculate this ahead of time. diff --git a/crates/multi-value-xform/Cargo.toml b/crates/multi-value-xform/Cargo.toml index a363d87760e..6f2a807ff21 100644 --- a/crates/multi-value-xform/Cargo.toml +++ b/crates/multi-value-xform/Cargo.toml @@ -15,6 +15,7 @@ rust-version = "1.57" [dependencies] anyhow = "1.0" walrus = "0.20.2" +wasm-bindgen-wasm-conventions = { path = "../wasm-conventions", version = "=0.2.92" } [dev-dependencies] rayon = "1.0" diff --git a/crates/multi-value-xform/src/lib.rs b/crates/multi-value-xform/src/lib.rs index 2bfb5bea374..61393592a66 100644 --- a/crates/multi-value-xform/src/lib.rs +++ b/crates/multi-value-xform/src/lib.rs @@ -117,6 +117,9 @@ pub fn run( shadow_stack_pointer: walrus::GlobalId, to_xform: &[(walrus::FunctionId, usize, Vec)], ) -> Result, anyhow::Error> { + // Insert multi-value to the target features section. + wasm_bindgen_wasm_conventions::insert_target_feature(module, "multivalue"); + let mut wrappers = Vec::new(); for (func, return_pointer_index, results) in to_xform { wrappers.push(xform_one( diff --git a/crates/wasm-conventions/Cargo.toml b/crates/wasm-conventions/Cargo.toml index 02bfc3d762a..cd2b474032a 100644 --- a/crates/wasm-conventions/Cargo.toml +++ b/crates/wasm-conventions/Cargo.toml @@ -11,5 +11,8 @@ edition = "2018" rust-version = "1.57" [dependencies] +leb128 = "0.2" walrus = "0.20.2" +# Matching the version `walrus` depends on. +wasmparser = "0.80" anyhow = "1.0" diff --git a/crates/wasm-conventions/src/lib.rs b/crates/wasm-conventions/src/lib.rs index d906e0dec63..ef91a6418fc 100755 --- a/crates/wasm-conventions/src/lib.rs +++ b/crates/wasm-conventions/src/lib.rs @@ -6,11 +6,14 @@ //! * The shadow stack pointer //! * The canonical linear memory that contains the shadow stack +use std::io::Cursor; + use anyhow::{anyhow, bail, Result}; use walrus::{ ir::Value, ElementId, FunctionBuilder, FunctionId, FunctionKind, GlobalId, GlobalKind, - InitExpr, MemoryId, Module, ValType, + InitExpr, MemoryId, Module, RawCustomSection, ValType, }; +use wasmparser::BinaryReader; /// Get a Wasm module's canonical linear memory. pub fn get_memory(module: &Module) -> Result { @@ -148,3 +151,67 @@ pub fn get_or_insert_start_builder(module: &mut Module) -> &mut FunctionBuilder .unwrap_local_mut() .builder_mut() } + +pub fn insert_target_feature(module: &mut Module, new_feature: &str) { + // Taken from . + assert!(new_feature.len() <= 100_000); + + // Try to find an existing section. + let section = module + .customs + .iter_mut() + .find(|(_, custom)| custom.name() == "target_features"); + + // If one exists, check if the target feature is already present. + let section = if let Some((_, section)) = section { + let section: &mut RawCustomSection = section.as_any_mut().downcast_mut().unwrap(); + let mut reader = BinaryReader::new(§ion.data); + // The first integer contains the target feature count. + let count = reader.read_var_u32().unwrap(); + + // Try to find if the target feature is already present. + for _ in 0..count { + // First byte is the prefix. + let prefix_index = reader.current_position(); + let prefix = reader.read_u8().unwrap() as u8; + // Read the feature. + let length = reader.read_var_u32().unwrap(); + let feature = reader.read_bytes(length as usize).unwrap(); + + // If we found the target feature, we are done here. + if feature == new_feature.as_bytes() { + // Make sure we set any existing prefix to "enabled". + if prefix == b'-' { + section.data[prefix_index] = b'+'; + } + + return; + } + } + + section + } else { + let mut data = Vec::new(); + leb128::write::unsigned(&mut data, 0).unwrap(); + let id = module.customs.add(RawCustomSection { + name: String::from("target_features"), + data, + }); + module.customs.get_mut(id).unwrap() + }; + + // If we couldn't find the target feature, insert it. + + // The first byte contains an integer describing the target feature count, which we increase by one. + let mut data = Cursor::new(§ion.data); + let count = leb128::read::unsigned(&mut data).unwrap(); + let mut new_count = Vec::new(); + leb128::write::unsigned(&mut new_count, count + 1).unwrap(); + section.data.splice(0..data.position() as usize, new_count); + // Then we insert the "enabled" prefix at the end. + section.data.push(b'+'); + // The next byte contains the length of the target feature string. + leb128::write::unsigned(&mut section.data, new_feature.len() as u64).unwrap(); + // Lastly the target feature string is inserted. + section.data.extend(new_feature.as_bytes()); +} From 7bfa18ec9b752f3c0b0fd970cce0af12b58c82d5 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sun, 26 May 2024 09:04:04 +0200 Subject: [PATCH 2/2] Return an error on invalid section --- crates/externref-xform/src/lib.rs | 5 +++-- crates/multi-value-xform/src/lib.rs | 5 ++++- crates/wasm-conventions/src/lib.rs | 23 ++++++++++++++--------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/crates/externref-xform/src/lib.rs b/crates/externref-xform/src/lib.rs index 57c912fb7ba..ab9ee589173 100644 --- a/crates/externref-xform/src/lib.rs +++ b/crates/externref-xform/src/lib.rs @@ -15,7 +15,7 @@ //! goal at least is to have valid wasm modules coming in that don't use //! `externref` and valid wasm modules going out which use `externref` at the fringes. -use anyhow::{anyhow, bail, Error}; +use anyhow::{anyhow, bail, Context as _, Error}; use std::cmp; use std::collections::{BTreeMap, HashMap, HashSet}; use std::mem; @@ -102,7 +102,8 @@ impl Context { /// we're appending entries. pub fn prepare(&mut self, module: &mut Module) -> Result<(), Error> { // Insert reference types to the target features section. - wasm_bindgen_wasm_conventions::insert_target_feature(module, "reference-types"); + wasm_bindgen_wasm_conventions::insert_target_feature(module, "reference-types") + .context("failed to parse `target_features` custom section")?; // Figure out what the maximum index of functions pointers are. We'll // be adding new entries to the function table later (maybe) so diff --git a/crates/multi-value-xform/src/lib.rs b/crates/multi-value-xform/src/lib.rs index 61393592a66..510aef3553c 100644 --- a/crates/multi-value-xform/src/lib.rs +++ b/crates/multi-value-xform/src/lib.rs @@ -92,6 +92,8 @@ #![deny(missing_docs, missing_debug_implementations)] +use anyhow::Context; + /// Run the transformation. /// /// See the module-level docs for details on the transformation. @@ -118,7 +120,8 @@ pub fn run( to_xform: &[(walrus::FunctionId, usize, Vec)], ) -> Result, anyhow::Error> { // Insert multi-value to the target features section. - wasm_bindgen_wasm_conventions::insert_target_feature(module, "multivalue"); + wasm_bindgen_wasm_conventions::insert_target_feature(module, "multivalue") + .context("failed to parse `target_features` custom section")?; let mut wrappers = Vec::new(); for (func, return_pointer_index, results) in to_xform { diff --git a/crates/wasm-conventions/src/lib.rs b/crates/wasm-conventions/src/lib.rs index ef91a6418fc..2161a0d3b43 100755 --- a/crates/wasm-conventions/src/lib.rs +++ b/crates/wasm-conventions/src/lib.rs @@ -8,7 +8,7 @@ use std::io::Cursor; -use anyhow::{anyhow, bail, Result}; +use anyhow::{anyhow, bail, Context, Result}; use walrus::{ ir::Value, ElementId, FunctionBuilder, FunctionId, FunctionKind, GlobalId, GlobalKind, InitExpr, MemoryId, Module, RawCustomSection, ValType, @@ -152,9 +152,9 @@ pub fn get_or_insert_start_builder(module: &mut Module) -> &mut FunctionBuilder .builder_mut() } -pub fn insert_target_feature(module: &mut Module, new_feature: &str) { +pub fn insert_target_feature(module: &mut Module, new_feature: &str) -> Result<()> { // Taken from . - assert!(new_feature.len() <= 100_000); + anyhow::ensure!(new_feature.len() <= 100_000, "feature name too long"); // Try to find an existing section. let section = module @@ -164,19 +164,22 @@ pub fn insert_target_feature(module: &mut Module, new_feature: &str) { // If one exists, check if the target feature is already present. let section = if let Some((_, section)) = section { - let section: &mut RawCustomSection = section.as_any_mut().downcast_mut().unwrap(); + let section: &mut RawCustomSection = section + .as_any_mut() + .downcast_mut() + .context("failed to read section")?; let mut reader = BinaryReader::new(§ion.data); // The first integer contains the target feature count. - let count = reader.read_var_u32().unwrap(); + let count = reader.read_var_u32()?; // Try to find if the target feature is already present. for _ in 0..count { // First byte is the prefix. let prefix_index = reader.current_position(); - let prefix = reader.read_u8().unwrap() as u8; + let prefix = reader.read_u8()? as u8; // Read the feature. - let length = reader.read_var_u32().unwrap(); - let feature = reader.read_bytes(length as usize).unwrap(); + let length = reader.read_var_u32()?; + let feature = reader.read_bytes(length as usize)?; // If we found the target feature, we are done here. if feature == new_feature.as_bytes() { @@ -185,7 +188,7 @@ pub fn insert_target_feature(module: &mut Module, new_feature: &str) { section.data[prefix_index] = b'+'; } - return; + return Ok(()); } } @@ -214,4 +217,6 @@ pub fn insert_target_feature(module: &mut Module, new_feature: &str) { leb128::write::unsigned(&mut section.data, new_feature.len() as u64).unwrap(); // Lastly the target feature string is inserted. section.data.extend(new_feature.as_bytes()); + + Ok(()) }