From 259fc54fd2447e640261151ed489933b777c15a0 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Wed, 21 May 2025 10:57:33 +0200 Subject: [PATCH 1/6] rename Locals::all field to uncached We rename because this field will no longer store _all_ locals, but only the ones that are not also stored in the `first` field. --- crates/wasmparser/src/validator/operators.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index e36dfc1f4e..19c3b429e7 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -181,7 +181,7 @@ pub(super) struct Locals { // `local.{get,set,tee}`. We do a binary search for the index desired, and // it either lies in a "hole" where the maximum index is specified later, // or it's at the end of the list meaning it's out of bounds. - all: Vec<(u32, ValType)>, + uncached: Vec<(u32, ValType)>, } /// A Wasm control flow block on the control flow stack during Wasm validation. @@ -336,7 +336,7 @@ impl OperatorValidator { locals: Locals { num_locals: 0, first: locals_first, - all: locals_all, + uncached: locals_all, }, local_inits, features: *features, @@ -521,7 +521,7 @@ impl OperatorValidator { self.local_inits }, locals_first: clear(self.locals.first), - locals_all: clear(self.locals.all), + locals_all: clear(self.locals.uncached), } } @@ -4275,7 +4275,7 @@ impl Locals { } self.first.push(ty); } - self.all.push((self.num_locals - 1, ty)); + self.uncached.push((self.num_locals - 1, ty)); true } @@ -4294,17 +4294,17 @@ impl Locals { } fn get_bsearch(&self, idx: u32) -> Option { - match self.all.binary_search_by_key(&idx, |(idx, _)| *idx) { + match self.uncached.binary_search_by_key(&idx, |(idx, _)| *idx) { // If this index would be inserted at the end of the list, then the // index is out of bounds and we return an error. - Err(i) if i == self.all.len() => None, + Err(i) if i == self.uncached.len() => None, // If `Ok` is returned we found the index exactly, or if `Err` is // returned the position is the one which is the least index // greater that `idx`, which is still the type of `idx` according // to our "compressed" representation. In both cases we access the // list at index `i`. - Ok(i) | Err(i) => Some(self.all[i].1), + Ok(i) | Err(i) => Some(self.uncached[i].1), } } } From fa18ec97c9fe32df2b0dc2d6768c6e9253dac2f5 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Wed, 21 May 2025 10:58:32 +0200 Subject: [PATCH 2/6] rename locals_all -> locals_uncached --- crates/wasmparser/src/validator/operators.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index 19c3b429e7..1f57ae253d 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -218,7 +218,7 @@ pub struct OperatorValidatorAllocations { operands: Vec, local_inits: LocalInits, locals_first: Vec, - locals_all: Vec<(u32, ValType)>, + locals_uncached: Vec<(u32, ValType)>, } /// Type storage within the validator. @@ -323,7 +323,7 @@ impl OperatorValidator { operands, local_inits, locals_first, - locals_all, + locals_uncached, } = allocs; debug_assert!(popped_types_tmp.is_empty()); debug_assert!(control.is_empty()); @@ -331,12 +331,12 @@ impl OperatorValidator { debug_assert!(local_inits.is_empty()); debug_assert!(local_inits.is_empty()); debug_assert!(locals_first.is_empty()); - debug_assert!(locals_all.is_empty()); + debug_assert!(locals_uncached.is_empty()); OperatorValidator { locals: Locals { num_locals: 0, first: locals_first, - uncached: locals_all, + uncached: locals_uncached, }, local_inits, features: *features, @@ -521,7 +521,7 @@ impl OperatorValidator { self.local_inits }, locals_first: clear(self.locals.first), - locals_all: clear(self.locals.uncached), + locals_uncached: clear(self.locals.uncached), } } From 2fee0284a0c0f2aec0416528742c0ec4ca622fed Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Wed, 21 May 2025 11:14:53 +0200 Subject: [PATCH 3/6] optimize Locals type getter for many locals --- crates/wasmparser/src/limits.rs | 2 +- crates/wasmparser/src/validator/operators.rs | 28 +++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/crates/wasmparser/src/limits.rs b/crates/wasmparser/src/limits.rs index 953981c02b..094002e2c4 100644 --- a/crates/wasmparser/src/limits.rs +++ b/crates/wasmparser/src/limits.rs @@ -27,7 +27,7 @@ pub const MAX_WASM_ELEMENT_SEGMENTS: usize = 100_000; pub const MAX_WASM_DATA_SEGMENTS: usize = 100_000; pub const MAX_WASM_STRING_SIZE: usize = 100_000; pub const MAX_WASM_FUNCTION_SIZE: usize = 128 * 1024; -pub const MAX_WASM_FUNCTION_LOCALS: usize = 50000; +pub const MAX_WASM_FUNCTION_LOCALS: u32 = 50000; pub const MAX_WASM_FUNCTION_PARAMS: usize = 1000; pub const MAX_WASM_FUNCTION_RETURNS: usize = 1000; pub const _MAX_WASM_TABLE_SIZE: usize = 10_000_000; diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index 1f57ae253d..114311466d 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -31,6 +31,7 @@ use crate::{ TryTable, UnpackedIndex, ValType, VisitOperator, WasmFeatures, WasmModuleResources, }; use crate::{prelude::*, CompositeInnerType, Ordering}; +use core::{cmp, iter}; use core::ops::{Deref, DerefMut}; #[cfg(feature = "simd")] @@ -159,7 +160,7 @@ impl LocalInits { // No science was performed in the creation of this number, feel free to change // it if you so like. -const MAX_LOCALS_TO_TRACK: usize = 50; +const MAX_LOCALS_TO_TRACK: u32 = 50; pub(super) struct Locals { // Total number of locals in the function. @@ -4262,20 +4263,23 @@ impl Locals { /// definition is unsuccessful in case the amount of total variables /// after definition exceeds the allowed maximum number. fn define(&mut self, count: u32, ty: ValType) -> bool { + if count == 0 { + return true + } + let vacant_first = MAX_LOCALS_TO_TRACK.saturating_sub(self.num_locals); match self.num_locals.checked_add(count) { - Some(n) => self.num_locals = n, + Some(num_locals) if num_locals > MAX_WASM_FUNCTION_LOCALS => return false, None => return false, + Some(num_locals) => self.num_locals = num_locals, + }; + let push_to_first = cmp::min(vacant_first, count); + self.first.extend(iter::repeat_n(ty, push_to_first as usize)); + let remaining_count = count - push_to_first; + let remaining_index = self.num_locals - 1; + if remaining_count > 0 { + self.uncached + .push((remaining_index, ty)); } - if self.num_locals > (MAX_WASM_FUNCTION_LOCALS as u32) { - return false; - } - for _ in 0..count { - if self.first.len() >= MAX_LOCALS_TO_TRACK { - break; - } - self.first.push(ty); - } - self.uncached.push((self.num_locals - 1, ty)); true } From f91a297118e18aead2f9074467e2dbd622448dcb Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Wed, 21 May 2025 11:32:23 +0200 Subject: [PATCH 4/6] apply some renamings and clean-ups --- crates/wasmparser/src/validator/operators.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index 114311466d..e884e4471c 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -4274,11 +4274,11 @@ impl Locals { }; let push_to_first = cmp::min(vacant_first, count); self.first.extend(iter::repeat_n(ty, push_to_first as usize)); - let remaining_count = count - push_to_first; - let remaining_index = self.num_locals - 1; - if remaining_count > 0 { + let num_uncached = count - push_to_first; + if num_uncached > 0 { + let max_uncached_idx = self.num_locals - 1; self.uncached - .push((remaining_index, ty)); + .push((max_uncached_idx, ty)); } true } From edba60c667ddfcac8b8eef0eb6ae97e0d23f8897 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Wed, 21 May 2025 11:40:00 +0200 Subject: [PATCH 5/6] apply rustfmt --- crates/wasmparser/src/validator/operators.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index e884e4471c..5f0386ca88 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -31,8 +31,8 @@ use crate::{ TryTable, UnpackedIndex, ValType, VisitOperator, WasmFeatures, WasmModuleResources, }; use crate::{prelude::*, CompositeInnerType, Ordering}; -use core::{cmp, iter}; use core::ops::{Deref, DerefMut}; +use core::{cmp, iter}; #[cfg(feature = "simd")] mod simd; @@ -4264,7 +4264,7 @@ impl Locals { /// after definition exceeds the allowed maximum number. fn define(&mut self, count: u32, ty: ValType) -> bool { if count == 0 { - return true + return true; } let vacant_first = MAX_LOCALS_TO_TRACK.saturating_sub(self.num_locals); match self.num_locals.checked_add(count) { @@ -4273,12 +4273,12 @@ impl Locals { Some(num_locals) => self.num_locals = num_locals, }; let push_to_first = cmp::min(vacant_first, count); - self.first.extend(iter::repeat_n(ty, push_to_first as usize)); + self.first + .extend(iter::repeat_n(ty, push_to_first as usize)); let num_uncached = count - push_to_first; if num_uncached > 0 { let max_uncached_idx = self.num_locals - 1; - self.uncached - .push((max_uncached_idx, ty)); + self.uncached.push((max_uncached_idx, ty)); } true } From 86133caf5e916c1349c67658ef38863a3de27115 Mon Sep 17 00:00:00 2001 From: Robin Freyler Date: Wed, 21 May 2025 12:02:05 +0200 Subject: [PATCH 6/6] replace repeat_n by repeat(..).take(..) due to MSRV --- crates/wasmparser/src/validator/operators.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index 5f0386ca88..1a599681ca 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -4274,7 +4274,7 @@ impl Locals { }; let push_to_first = cmp::min(vacant_first, count); self.first - .extend(iter::repeat_n(ty, push_to_first as usize)); + .extend(iter::repeat(ty).take(push_to_first as usize)); let num_uncached = count - push_to_first; if num_uncached > 0 { let max_uncached_idx = self.num_locals - 1;