From e2eb364f836f85d672fb89172483a6a4b97c08f7 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 20:35:25 -0800 Subject: [PATCH 1/7] Scripts: operators, JSON.stringify and join count a flattened string against the heap limit Flattening a concatenated string copies its units into a new buffer. The string built-ins reserve that copy with flat_charged, but several other paths flattened without it, so a script could build copies the heap limit never saw: - the relational operators (`<`, `>`, `<=`, `>=`) on two strings, `==` between a string and a number, unary `+` and every other ToNumber of a string, and reading a character by index (`s[0]`) - a property name made from an object whose toString returns a long string, and a long string used as a property name, which now reserves its copy before it is made instead of after - direct `eval` of a concatenated source - `JSON.stringify` of a string value and its `space` argument - `Array.prototype.join`'s separator and each string element it writes Property names and eval source reserve the copy's heap bytes without taking fuel. --- crates/js/src/builtins/array.rs | 6 +++-- crates/js/src/builtins/json.rs | 8 +++++-- crates/js/src/interp.rs | 40 +++++++++++++++++++++------------ crates/js/src/text_tests.rs | 17 ++++++++++++++ 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/crates/js/src/builtins/array.rs b/crates/js/src/builtins/array.rs index 2f81bf766..263656437 100644 --- a/crates/js/src/builtins/array.rs +++ b/crates/js/src/builtins/array.rs @@ -13,6 +13,7 @@ use super::{ }; use crate::heap::{Class, NativeKind, ObjId}; use crate::interp::{Abrupt, Interp, JsResult, strict_eq}; +use crate::string_builtins::flat_charged; use crate::value::{Str, Value, to_uint32}; pub(super) fn install(interp: &mut Interp) -> JsResult<()> { @@ -278,7 +279,7 @@ fn join_with(interp: &mut Interp, o: ObjId, separator: &Value, item: fn(&mut Int if len == 0 { return Ok(Value::str("")); } - let flat = sep.flat(); + let flat = flat_charged(interp, &sep)?; let sep = &flat[..]; // A result that cannot fit is refused before any element is read. let least = (sep.len() as u64).saturating_mul(u64::from(len - 1)); @@ -301,7 +302,8 @@ fn join_elements(interp: &mut Interp, o: ObjId, len: u32, sep: &[u16], item: fn( let Some(s) = item(interp, v)? else { continue }; out.push_repeated(interp, sep, k.saturating_sub(seps))?; seps = k; - out.push_units(interp, &s.flat())?; + let units = flat_charged(interp, &s)?; + out.push_units(interp, &units)?; } out.push_repeated(interp, sep, u64::from(len - 1).saturating_sub(seps))?; Ok(out.finish(interp)) diff --git a/crates/js/src/builtins/json.rs b/crates/js/src/builtins/json.rs index 4375303fe..991495ba9 100644 --- a/crates/js/src/builtins/json.rs +++ b/crates/js/src/builtins/json.rs @@ -13,6 +13,7 @@ use std::collections::HashSet; use super::{Builder, ELEMENT_BYTES, arg, data, key_of, length_of, method, new_array, new_string, to_integer}; use crate::heap::{Class, Key, OBJECT_BYTES, OPEN, ObjId, Object, Prop, key, string_bytes}; use crate::interp::{Desc, ErrorType, Hint, Interp, JsResult}; +use crate::string_builtins::flat_charged; use crate::value::{Str, Value, number_to_str}; pub(super) fn install(interp: &mut Interp) -> JsResult<()> { @@ -473,7 +474,7 @@ fn settings(interp: &mut Interp, replacer: &Value, space: &Value) -> JsResult s.gap = t.flat().iter().take(10).copied().collect(), + Value::String(t) => s.gap = flat_charged(interp, &t)?.iter().take(10).copied().collect(), _ => {} } Ok(s) @@ -632,7 +633,10 @@ fn write_primitive(interp: &mut Interp, out: &mut Builder, v: &Value) -> JsResul Value::Bool(b) => out.push_str(interp, if *b { "true" } else { "false" }), Value::Number(n) if n.is_finite() => out.push_units(interp, &number_to_str(*n).flat()), Value::Number(_) => out.push_str(interp, "null"), - Value::String(s) => write_quoted(interp, out, &s.flat()), + Value::String(s) => { + let units = flat_charged(interp, s)?; + write_quoted(interp, out, &units) + } Value::Undefined | Value::Object(_) => Ok(()), } } diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index afb6cde63..afca1a102 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -26,6 +26,7 @@ use crate::heap::{ ArgMap, Binding, CONFIGURABLE, Class, ENUMERABLE, ENV_BYTES, Env, EnvId, ForIn, Func, HIDDEN, Heap, Key, NativeKind, OBJECT_BYTES, OPEN, ObjId, Object, PROP_BYTES, Prop, PropMap, ROPE_BYTES, Roots, WRITABLE, code_bytes, key, string_bytes, }; +use crate::string_builtins::flat_charged; use crate::value::{Str, Value, array_index, index_key, number_to_str, string_to_number, to_int32, to_uint32}; use crate::{LimitKind, Limits, Options}; @@ -1290,7 +1291,7 @@ impl Interp { if let Some(i) = array_index(k) && (i as usize) < s.len() { - let unit = s.flat().get(i as usize).copied().unwrap_or(0); + let unit = flat_charged(self, s)?.get(i as usize).copied().unwrap_or(0); return Ok(Value::String(Str::from_units(vec![unit]))); } } @@ -2397,7 +2398,7 @@ impl Interp { }; let strict = self.strict(); self.check_source_units(s.len()).map_err(|e| self.compile_error_to_abrupt(e))?; - let units = s.flat(); + let units = self.flat_reserved(s)?; let code = self.compile_source(&units, strict, CodeKind::Eval).map_err(|e| self.compile_error_to_abrupt(e))?; self.charge_code(&code, declaration_bytes(&code).saturating_add(ENV_BYTES))?; let frame = self.frame()?; @@ -2562,7 +2563,7 @@ impl Interp { Value::Null => 0.0, Value::Bool(b) => f64::from(u8::from(*b)), Value::Number(n) => *n, - Value::String(s) => string_to_number(&s.flat()), + Value::String(s) => string_to_number(&flat_charged(self, s)?), Value::Object(_) => { let p = self.to_primitive(v, Hint::Number)?; return self.to_number(&p); @@ -2596,19 +2597,27 @@ impl Interp { #[allow(clippy::wrong_self_convention)] pub(crate) fn to_key(&mut self, v: &Value) -> JsResult { match v { - Value::String(s) => { - let rope = s.is_rope(); - let flat = s.flat(); - if rope { - self.heap.bytes += string_bytes(flat.len()); - } - Ok(flat) - } + Value::String(s) => self.flat_reserved(s), Value::Number(n) if *n >= 0.0 && *n < 4_294_967_295.0 && n.fract() == 0.0 => Ok(index_key(*n as u32)), - _ => Ok(self.to_str(v)?.flat()), + _ => { + let s = self.to_str(v)?; + self.flat_reserved(&s) + } } } + /// The code units of `s`. Flattening a rope makes a copy of its units, so for a rope that + /// copy's bytes are reserved and added to the heap first, without taking fuel. Everything + /// the caller still needs must be reachable from the roots. + fn flat_reserved(&mut self, s: &Str) -> JsResult { + if s.is_rope() { + let bytes = string_bytes(s.len()); + self.reserve_heap(bytes)?; + self.heap.bytes = self.heap.bytes.saturating_add(bytes); + } + Ok(s.flat()) + } + /// ToObject (section 9.9). #[allow(clippy::wrong_self_convention)] pub(crate) fn to_object(&mut self, v: &Value) -> JsResult { @@ -2731,7 +2740,8 @@ impl Interp { (self.to_primitive(x, Hint::Number)?, py) }; if let (Value::String(a), Value::String(b)) = (&px, &py) { - return Ok(Some(*a.flat() < *b.flat())); + let (a, b) = (flat_charged(self, a)?, flat_charged(self, b)?); + return Ok(Some(*a < *b)); } let nx = self.to_number(&px)?; let ny = self.to_number(&py)?; @@ -2756,7 +2766,9 @@ impl Interp { | (Value::String(_), Value::String(_)) | (Value::Bool(_), Value::Bool(_)) | (Value::Object(_), Value::Object(_)) => return Ok(strict_eq(&x, &y)), - (Value::Number(n), Value::String(s)) | (Value::String(s), Value::Number(n)) => return Ok(*n == string_to_number(&s.flat())), + (Value::Number(n), Value::String(s)) | (Value::String(s), Value::Number(n)) => { + return Ok(*n == string_to_number(&flat_charged(self, s)?)); + } (Value::Bool(v), _) => x = Value::Number(f64::from(u8::from(*v))), (_, Value::Bool(v)) => y = Value::Number(f64::from(u8::from(*v))), (Value::Number(_) | Value::String(_), Value::Object(_)) => y = self.to_primitive(&y, Hint::Default)?, diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index c873bdaf5..3134e318d 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -309,6 +309,23 @@ fn flattening_a_rope_counts_against_the_heap() { } } +#[test] +fn operators_and_builtins_count_a_flattened_rope_against_the_heap() { + // Three ropes of about 524,288 units, each 1 MiB once flat, with a 3 MiB heap. + let build = "var s = 'x'; for (var i = 0; i < 19; i++) s = s + s; var t = s + 'y', u = s + 'z';"; + let limits = Limits { heap_bytes: 3 << 20, ..Limits::default() }; + let not_stopped: Vec<_> = + ["X < 'a'", "X == 1", "+X", "X[0]", "({})[{ toString: function () { return X; } }]", "JSON.stringify(0, null, X)", "[1].join(X)"] + .into_iter() + .filter(|read| { + let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); + let result = Realm::with_options(Options::default(), limits.clone()).run(&src); + !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) + }) + .collect(); + assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); +} + #[test] fn compiling_a_pattern_counts_against_the_heap() { let src = "var s = '[\\\\s\\\\S]a*'; for (var i = 0; i < 14; i++) s = s + s; new RegExp(s).source.length"; From 44eb3be71301e8973e5f96f2346b925a991d1ee3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 21:21:45 -0800 Subject: [PATCH 2/7] Scripts: a flattened string stays counted while the next copy is made - the relational operators root both strings while flattening them, so the first copy still counts when the second one starts a collection - join roots its separator for the whole join and each element while it is written, and JSON.stringify roots a string while it writes it - a String object flattens its string when it is made, with the copy counted, so reading one of its characters copies nothing --- crates/js/src/builtins/array.rs | 31 ++++++++++++--------- crates/js/src/builtins/json.rs | 8 +++--- crates/js/src/interp.rs | 13 ++++++--- crates/js/src/string_builtins.rs | 2 ++ crates/js/src/text_tests.rs | 46 +++++++++++++++++++++++++------- 5 files changed, 71 insertions(+), 29 deletions(-) diff --git a/crates/js/src/builtins/array.rs b/crates/js/src/builtins/array.rs index 263656437..838fc4ff9 100644 --- a/crates/js/src/builtins/array.rs +++ b/crates/js/src/builtins/array.rs @@ -279,17 +279,20 @@ fn join_with(interp: &mut Interp, o: ObjId, separator: &Value, item: fn(&mut Int if len == 0 { return Ok(Value::str("")); } - let flat = flat_charged(interp, &sep)?; - let sep = &flat[..]; - // A result that cannot fit is refused before any element is read. - let least = (sep.len() as u64).saturating_mul(u64::from(len - 1)); - if least > (interp.limits.heap_bytes / 4) as u64 { - return Err(Abrupt::Limit(crate::LimitKind::Heap)); - } - interp.join_stack.push(o); - let r = join_elements(interp, o, len, sep, item); - interp.join_stack.pop(); - r + // The separator stays rooted for the whole join, so its flattened copy stays counted. + interp.rooted(&[Value::String(sep.clone())], |interp| { + let flat = flat_charged(interp, &sep)?; + let sep = &flat[..]; + // A result that cannot fit is refused before any element is read. + let least = (sep.len() as u64).saturating_mul(u64::from(len - 1)); + if least > (interp.limits.heap_bytes / 4) as u64 { + return Err(Abrupt::Limit(crate::LimitKind::Heap)); + } + interp.join_stack.push(o); + let r = join_elements(interp, o, len, sep, item); + interp.join_stack.pop(); + r + }) } fn join_elements(interp: &mut Interp, o: ObjId, len: u32, sep: &[u16], item: fn(&mut Interp, Value) -> JsResult>) -> JsResult { @@ -302,8 +305,10 @@ fn join_elements(interp: &mut Interp, o: ObjId, len: u32, sep: &[u16], item: fn( let Some(s) = item(interp, v)? else { continue }; out.push_repeated(interp, sep, k.saturating_sub(seps))?; seps = k; - let units = flat_charged(interp, &s)?; - out.push_units(interp, &units)?; + interp.rooted(&[Value::String(s.clone())], |interp| { + let units = flat_charged(interp, &s)?; + out.push_units(interp, &units) + })?; } out.push_repeated(interp, sep, u64::from(len - 1).saturating_sub(seps))?; Ok(out.finish(interp)) diff --git a/crates/js/src/builtins/json.rs b/crates/js/src/builtins/json.rs index 991495ba9..364ec9969 100644 --- a/crates/js/src/builtins/json.rs +++ b/crates/js/src/builtins/json.rs @@ -633,10 +633,10 @@ fn write_primitive(interp: &mut Interp, out: &mut Builder, v: &Value) -> JsResul Value::Bool(b) => out.push_str(interp, if *b { "true" } else { "false" }), Value::Number(n) if n.is_finite() => out.push_units(interp, &number_to_str(*n).flat()), Value::Number(_) => out.push_str(interp, "null"), - Value::String(s) => { - let units = flat_charged(interp, s)?; - write_quoted(interp, out, &units) - } + Value::String(s) => interp.rooted(std::slice::from_ref(v), |i| { + let units = flat_charged(i, s)?; + write_quoted(i, out, &units) + }), Value::Undefined | Value::Object(_) => Ok(()), } } diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index afca1a102..9801cc904 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -2626,7 +2626,11 @@ impl Interp { Value::Undefined | Value::Null => return Err(self.type_error("cannot convert undefined or null to an object")), Value::Bool(b) => (Class::Boolean(*b), self.intrinsics.boolean_proto), Value::Number(n) => (Class::Number(*n), self.intrinsics.number_proto), - Value::String(s) => (Class::String(s.clone()), self.intrinsics.string_proto), + // Flattened now, with its copy counted, so reading a character copies nothing. + Value::String(s) => { + flat_charged(self, s)?; + (Class::String(s.clone()), self.intrinsics.string_proto) + } }; Ok(self.heap.alloc_object(Object::new(class, Some(proto)))) } @@ -2740,8 +2744,11 @@ impl Interp { (self.to_primitive(x, Hint::Number)?, py) }; if let (Value::String(a), Value::String(b)) = (&px, &py) { - let (a, b) = (flat_charged(self, a)?, flat_charged(self, b)?); - return Ok(Some(*a < *b)); + // Both stay rooted, so the first copy is still counted while the second is made. + return self.rooted(&[px.clone(), py.clone()], |i| { + let (a, b) = (flat_charged(i, a)?, flat_charged(i, b)?); + Ok(Some(*a < *b)) + }); } let nx = self.to_number(&px)?; let ny = self.to_number(&py)?; diff --git a/crates/js/src/string_builtins.rs b/crates/js/src/string_builtins.rs index b3758bdf5..ba9c79155 100644 --- a/crates/js/src/string_builtins.rs +++ b/crates/js/src/string_builtins.rs @@ -214,6 +214,8 @@ fn string_ctor(interp: &mut Interp, _: Value, args: &[Value], construct: bool) - if !construct { return Ok(Value::String(s)); } + // Flattened now, with its copy counted, so reading a character copies nothing. + flat_charged(interp, &s)?; let proto = interp.intrinsics.string_proto; Ok(Value::Object(interp.heap.alloc_object(Object::new(Class::String(s), Some(proto))))) } diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index 3134e318d..03b4fa433 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -314,15 +314,43 @@ fn operators_and_builtins_count_a_flattened_rope_against_the_heap() { // Three ropes of about 524,288 units, each 1 MiB once flat, with a 3 MiB heap. let build = "var s = 'x'; for (var i = 0; i < 19; i++) s = s + s; var t = s + 'y', u = s + 'z';"; let limits = Limits { heap_bytes: 3 << 20, ..Limits::default() }; - let not_stopped: Vec<_> = - ["X < 'a'", "X == 1", "+X", "X[0]", "({})[{ toString: function () { return X; } }]", "JSON.stringify(0, null, X)", "[1].join(X)"] - .into_iter() - .filter(|read| { - let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); - let result = Realm::with_options(Options::default(), limits.clone()).run(&src); - !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) - }) - .collect(); + let not_stopped: Vec<_> = [ + "X < 'a'", + "X == 1", + "+X", + "X[0]", + "new String(X)[0]", + "({})[{ toString: function () { return X; } }]", + "JSON.stringify(0, null, X)", + "[1].join(X)", + ] + .into_iter() + .filter(|read| { + let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); + let result = Realm::with_options(Options::default(), limits.clone()).run(&src); + !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) + }) + .collect(); + assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); +} + +#[test] +fn a_flattened_temporary_stays_counted_while_the_next_copy_is_made() { + // Ropes of 262,145 units, each about half the heap once flat: one fits, two don't. Each is + // made by a function, so nothing but the native code holds it while it is flattened. + let build = "var s = 'x'; for (var i = 0; i < 18; i++) s = s + s; function f(c) { return s + c; }"; + let limits = Limits { heap_bytes: 4 * 262_146, ..Limits::default() }; + let run = |read: &str| Realm::with_options(Options::default(), limits.clone()).run(&format!("{build} {read}; 0")); + assert!(run("f('a')[0]").is_ok(), "one flattened copy fits"); + let not_stopped: Vec<_> = [ + "({ valueOf: function () { return f('a'); } }) < ({ valueOf: function () { return f('b'); } })", + "JSON.stringify({ toJSON: function () { return f('a'); } })", + "[1, 2].join({ toString: function () { return f('a'); } })", + "[{ toString: function () { return f('a'); } }].join('')", + ] + .into_iter() + .filter(|read| !matches!(run(read), Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap))) + .collect(); assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); } From 3dc597f25948f0a0675774f27a775b93c25a69db Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 21:53:50 -0800 Subject: [PATCH 3/7] Scripts: a call and new String keep their operands rooted while a rope is flattened Boxing the string receiver of a non-strict function flattens it, and reserving the copy can collect. The callee, receiver and arguments are already off the stack at that point, so they stay in the temporary roots until the frame holds them; before, an argument object could be freed and its cell reused by the wrapper. new String(value) roots the converted string the same way, so what the rope holds is still counted while its copy is reserved. --- crates/js/src/interp.rs | 42 ++++++++++++++++++++------------ crates/js/src/string_builtins.rs | 5 ++-- crates/js/src/text_tests.rs | 29 ++++++++++++++++++++++ 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index 9801cc904..82d0c8f9a 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -2216,26 +2216,19 @@ impl Interp { let arguments = if code.uses_arguments { OBJECT_BYTES.saturating_add(args.len().saturating_add(4).saturating_mul(binding_bytes(10))) } else { 0 }; let need = declaration_bytes(&code).saturating_add(ENV_BYTES).saturating_add(params).saturating_add(arguments); - if self.heap.bytes.saturating_add(need) > self.limits.heap_bytes { - let mark = self.temp_roots.len(); + // The operands are off the stack. Reserving the heap collects, and so can boxing a string + // receiver, which flattens it; the operands stay rooted until the frame holds them. + let collects = self.heap.bytes.saturating_add(need) > self.limits.heap_bytes || (!code.strict && matches!(this, Value::String(_))); + let mark = self.temp_roots.len(); + if collects { self.temp_roots.push(Value::Object(callee)); self.temp_roots.push(this.clone()); self.temp_roots.extend(construct.map(Value::Object)); self.temp_roots.extend(args.iter().cloned()); - let reserved = self.reserve_heap(need); - self.temp_roots.truncate(mark); - reserved?; } - let this = if code.strict { - this - } else { - match this { - Value::Undefined | Value::Null => Value::Object(self.global), - Value::Object(_) => this, - prim => Value::Object(self.to_object(&prim)?), - } - }; - let fenv = self.instantiate_function(&code, env, &args, callee)?; + let received = self.receive_call(&code, env, this, &args, callee, need); + self.temp_roots.truncate(mark); + let (this, fenv) = received?; let base = self.stack.len(); self.frames.push(Frame { code, @@ -2253,6 +2246,25 @@ impl Interp { Ok(()) } + /// Reserves the `need` bytes of a call, boxes a primitive receiver for non-strict code and + /// binds the parameters, returning the receiver and the function's environment. + fn receive_call(&mut self, code: &Rc, env: EnvId, this: Value, args: &[Value], callee: ObjId, need: usize) -> JsResult<(Value, EnvId)> { + if self.heap.bytes.saturating_add(need) > self.limits.heap_bytes { + self.reserve_heap(need)?; + } + let this = if code.strict { + this + } else { + match this { + Value::Undefined | Value::Null => Value::Object(self.global), + Value::Object(_) => this, + prim => Value::Object(self.to_object(&prim)?), + } + }; + let fenv = self.instantiate_function(code, env, args, callee)?; + Ok((this, fenv)) + } + /// Declaration binding instantiation for function code (section 10.5). Each parameter and /// declaration takes a unit of fuel. fn instantiate_function(&mut self, code: &Rc, outer: EnvId, args: &[Value], callee: ObjId) -> JsResult { diff --git a/crates/js/src/string_builtins.rs b/crates/js/src/string_builtins.rs index ba9c79155..85a27e326 100644 --- a/crates/js/src/string_builtins.rs +++ b/crates/js/src/string_builtins.rs @@ -214,8 +214,9 @@ fn string_ctor(interp: &mut Interp, _: Value, args: &[Value], construct: bool) - if !construct { return Ok(Value::String(s)); } - // Flattened now, with its copy counted, so reading a character copies nothing. - flat_charged(interp, &s)?; + // Flattened now, with its copy counted, so reading a character copies nothing. The string + // stays rooted, so what it holds is still counted if reserving the copy collects. + interp.rooted(&[Value::String(s.clone())], |interp| flat_charged(interp, &s))?; let proto = interp.intrinsics.string_proto; Ok(Value::Object(interp.heap.alloc_object(Object::new(Class::String(s), Some(proto))))) } diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index 03b4fa433..0c2f051f8 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -354,6 +354,35 @@ fn a_flattened_temporary_stays_counted_while_the_next_copy_is_made() { assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); } +#[test] +fn a_string_wrapper_counts_what_its_rope_holds_while_the_copy_is_made() { + // `mk` returns a rope whose left half is a flat string of about half the heap, held by + // nothing else. Its flattened copy is the other half, so the two don't fit together. + let build = "var s = 'x'; for (var i = 0; i < 18; i++) s = s + s; function f(c) { return s + c; } \ + function mk() { var a = f('a'); a[0]; return a + 'b'; }"; + let limits = Limits { heap_bytes: 4 * 262_146, ..Limits::default() }; + let src = format!("{build} new String({{ toString: mk }}); 0"); + let result = Realm::with_options(Options::default(), limits).run(&src); + assert!(matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)), "{result:?}"); +} + +#[test] +fn a_call_keeps_its_operands_while_a_string_receiver_is_boxed() { + // Boxing the rope receiver of a non-strict function flattens it, and reserving the copy + // collects the garbage string `g` left. The callee and the argument object are off the + // stack by then, and must survive the collection. + let build = "var s = 'x'; for (var i = 0; i < 17; i++) s = s + s; function f(c) { return s + c; } \ + function mk() { var a = f('a'); a[0]; return a + 'b'; } 0"; + let mut realm = Realm::with_options(Options::default(), Limits::default()); + assert_eq!(realm.run(build), Ok(JsValue::Number(0.0))); + let interp = realm.interp(); + interp.collect_garbage(); + interp.limits.heap_bytes = interp.heap.bytes + 2 * crate::heap::string_bytes(131_073) + (64 << 10); + interp.reset_gc_threshold(); + let src = "(function () { var g = f('g'); g[0]; })(); (function (o) { return o.x; }).call(mk(), { x: 1 })"; + assert_eq!(realm.run(src), Ok(JsValue::Number(1.0))); +} + #[test] fn compiling_a_pattern_counts_against_the_heap() { let src = "var s = '[\\\\s\\\\S]a*'; for (var i = 0; i < 14; i++) s = s + s; new RegExp(s).source.length"; From 46efc4a651364a2d9ef38761efbc73a77aa18ac3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 22:39:04 -0800 Subject: [PATCH 4/7] Scripts: a String object counts its rope's copy when a character is read - wrapping a string no longer flattens it, so Object(s).length and new String(s) on a large concatenated string make no copy and cannot collect while a call's operands are off the stack - reading a String object's character by index, `in` and hasOwnProperty count the flattened copy at the next limit check, and the run ends before the copy when the fuel for it is not left - unary plus on an object whose valueOf returns a concatenated string, and a property key from toString, keep that string rooted while its copy is reserved - the unshift comment cites step 6 of section 15.4.4.13 --- crates/js/src/builtins/array.rs | 2 +- crates/js/src/interp.rs | 80 ++++++++++++++++++-------------- crates/js/src/realm.rs | 1 + crates/js/src/string_builtins.rs | 3 -- crates/js/src/text_tests.rs | 55 +++++++++++++++++++--- 5 files changed, 94 insertions(+), 47 deletions(-) diff --git a/crates/js/src/builtins/array.rs b/crates/js/src/builtins/array.rs index 838fc4ff9..bfb4de5ea 100644 --- a/crates/js/src/builtins/array.rs +++ b/crates/js/src/builtins/array.rs @@ -479,7 +479,7 @@ fn unshift(interp: &mut Interp, this: Value, args: &[Value], _: bool) -> JsResul with_this(interp, &this, |interp, o| { let len = u64::from(length_of(interp, o)?); let count = args.len() as u64; - // With no arguments each element is still read and written back in place (steps 6 and 7). + // With no arguments each element is still read and written back in place (step 6). let maps = vec![Map::Shift(-1), Map::Shift(count as i64 - 1)]; let mut it = Indices::new(interp, o, 1, len + 1, false, maps)?; while let Some(k) = it.next(interp)? { diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index 82d0c8f9a..ace43fe24 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -205,6 +205,11 @@ pub(crate) struct Interp { pub(crate) date_host: crate::date_builtins::DateHost, /// The bytes native code holds outside the heap's cells through [`HeldBytes`]. pub(crate) held: Rc, + /// Bytes of the copies [`Self::get_own_property`] made flattening the rope of a String + /// object, which the heap's total adds before the next limit check. + flattened: std::cell::Cell, + /// Fuel for the units those copies took, which the next step takes. + pub(crate) flattened_fuel: std::cell::Cell, /// The global code's default XML namespace URI (ECMA-357 section 12.1). pub(crate) xml_default_uri: Key, /// Set by the host to end the current slice of a resumable run at the next safe point. @@ -450,6 +455,8 @@ impl Interp { files: None, date_host: crate::date_builtins::DateHost::default(), held: Rc::default(), + flattened: std::cell::Cell::new(0), + flattened_fuel: std::cell::Cell::new(0), xml_default_uri: Rc::from(Vec::new()), pause: Rc::default(), in_slice: false, @@ -569,6 +576,7 @@ impl Interp { fn tick(&mut self) -> JsResult<()> { self.use_fuel()?; + crate::regexp_vm::use_fuel_n(self, self.flattened_fuel.replace(0))?; self.settle_held(); if self.heap.bytes > self.gc_threshold { self.collect_garbage(); @@ -595,10 +603,12 @@ impl Interp { Ok(()) } - /// Takes the bytes that dropped [`HeldBytes`] released off the heap's total. + /// Takes the bytes that dropped [`HeldBytes`] released off the heap's total, and adds the + /// bytes of the ropes [`Self::get_own_property`] flattened. fn settle_held(&mut self) { let released = self.held.released.replace(0); - self.heap.bytes = self.heap.bytes.saturating_sub(released); + let flattened = self.flattened.replace(0); + self.heap.bytes = self.heap.bytes.saturating_sub(released).saturating_add(flattened); } /// Collects garbage now and sets when the next collection runs. @@ -664,6 +674,7 @@ impl Interp { self.heap.collect(&roots); // The collection counts the live total again, so bytes already released are not in it. self.held.released.set(0); + self.flattened.set(0); self.heap.bytes = self.heap.bytes.saturating_add(self.held_bytes).saturating_add(self.held.now.get()); } @@ -1214,6 +1225,17 @@ impl Interp { if let Some(i) = array_index(k) && (i as usize) < s.len() { + if s.is_rope() { + // Flattening copies the units. The run ends here when the fuel for the + // copy is not left; otherwise the next step takes it, and the next limit + // check adds the copy's bytes. + let fuel = self.flattened_fuel.get().saturating_add((s.len() / crate::string_builtins::UNITS_PER_FUEL) as u64 + 1); + if fuel > self.fuel { + return Err(Abrupt::Limit(LimitKind::Fuel)); + } + self.flattened_fuel.set(fuel); + self.flattened.set(self.flattened.get().saturating_add(string_bytes(s.len()))); + } let unit = s.flat().get(i as usize).copied().unwrap_or(0); return Ok(Some(Prop::Data { value: Value::String(Str::from_units(vec![unit])), flags: ENUMERABLE })); } @@ -2216,19 +2238,26 @@ impl Interp { let arguments = if code.uses_arguments { OBJECT_BYTES.saturating_add(args.len().saturating_add(4).saturating_mul(binding_bytes(10))) } else { 0 }; let need = declaration_bytes(&code).saturating_add(ENV_BYTES).saturating_add(params).saturating_add(arguments); - // The operands are off the stack. Reserving the heap collects, and so can boxing a string - // receiver, which flattens it; the operands stay rooted until the frame holds them. - let collects = self.heap.bytes.saturating_add(need) > self.limits.heap_bytes || (!code.strict && matches!(this, Value::String(_))); - let mark = self.temp_roots.len(); - if collects { + if self.heap.bytes.saturating_add(need) > self.limits.heap_bytes { + let mark = self.temp_roots.len(); self.temp_roots.push(Value::Object(callee)); self.temp_roots.push(this.clone()); self.temp_roots.extend(construct.map(Value::Object)); self.temp_roots.extend(args.iter().cloned()); + let reserved = self.reserve_heap(need); + self.temp_roots.truncate(mark); + reserved?; } - let received = self.receive_call(&code, env, this, &args, callee, need); - self.temp_roots.truncate(mark); - let (this, fenv) = received?; + let this = if code.strict { + this + } else { + match this { + Value::Undefined | Value::Null => Value::Object(self.global), + Value::Object(_) => this, + prim => Value::Object(self.to_object(&prim)?), + } + }; + let fenv = self.instantiate_function(&code, env, &args, callee)?; let base = self.stack.len(); self.frames.push(Frame { code, @@ -2246,25 +2275,6 @@ impl Interp { Ok(()) } - /// Reserves the `need` bytes of a call, boxes a primitive receiver for non-strict code and - /// binds the parameters, returning the receiver and the function's environment. - fn receive_call(&mut self, code: &Rc, env: EnvId, this: Value, args: &[Value], callee: ObjId, need: usize) -> JsResult<(Value, EnvId)> { - if self.heap.bytes.saturating_add(need) > self.limits.heap_bytes { - self.reserve_heap(need)?; - } - let this = if code.strict { - this - } else { - match this { - Value::Undefined | Value::Null => Value::Object(self.global), - Value::Object(_) => this, - prim => Value::Object(self.to_object(&prim)?), - } - }; - let fenv = self.instantiate_function(code, env, args, callee)?; - Ok((this, fenv)) - } - /// Declaration binding instantiation for function code (section 10.5). Each parameter and /// declaration takes a unit of fuel. fn instantiate_function(&mut self, code: &Rc, outer: EnvId, args: &[Value], callee: ObjId) -> JsResult { @@ -2578,7 +2588,8 @@ impl Interp { Value::String(s) => string_to_number(&flat_charged(self, s)?), Value::Object(_) => { let p = self.to_primitive(v, Hint::Number)?; - return self.to_number(&p); + // Rooted, so what a string holds is still counted while its copy is reserved. + return self.rooted(std::slice::from_ref(&p), |i| i.to_number(&p)); } }) } @@ -2613,7 +2624,8 @@ impl Interp { Value::Number(n) if *n >= 0.0 && *n < 4_294_967_295.0 && n.fract() == 0.0 => Ok(index_key(*n as u32)), _ => { let s = self.to_str(v)?; - self.flat_reserved(&s) + // Rooted, so what the string holds is still counted while its copy is reserved. + self.rooted(&[Value::String(s.clone())], |i| i.flat_reserved(&s)) } } } @@ -2638,11 +2650,7 @@ impl Interp { Value::Undefined | Value::Null => return Err(self.type_error("cannot convert undefined or null to an object")), Value::Bool(b) => (Class::Boolean(*b), self.intrinsics.boolean_proto), Value::Number(n) => (Class::Number(*n), self.intrinsics.number_proto), - // Flattened now, with its copy counted, so reading a character copies nothing. - Value::String(s) => { - flat_charged(self, s)?; - (Class::String(s.clone()), self.intrinsics.string_proto) - } + Value::String(s) => (Class::String(s.clone()), self.intrinsics.string_proto), }; Ok(self.heap.alloc_object(Object::new(class, Some(proto)))) } diff --git a/crates/js/src/realm.rs b/crates/js/src/realm.rs index f274c42ea..431e9d8f4 100644 --- a/crates/js/src/realm.rs +++ b/crates/js/src/realm.rs @@ -230,6 +230,7 @@ impl Realm { return Err(RunError { kind: RunErrorKind::Internal, message: message.clone(), line: 0, column: 0 }); } interp.fuel = interp.limits.fuel; + interp.flattened_fuel.set(0); interp.source_units = 0; interp.throw_site = None; interp.last_thrown = Value::Undefined; diff --git a/crates/js/src/string_builtins.rs b/crates/js/src/string_builtins.rs index 85a27e326..b3758bdf5 100644 --- a/crates/js/src/string_builtins.rs +++ b/crates/js/src/string_builtins.rs @@ -214,9 +214,6 @@ fn string_ctor(interp: &mut Interp, _: Value, args: &[Value], construct: bool) - if !construct { return Ok(Value::String(s)); } - // Flattened now, with its copy counted, so reading a character copies nothing. The string - // stays rooted, so what it holds is still counted if reserving the copy collects. - interp.rooted(&[Value::String(s.clone())], |interp| flat_charged(interp, &s))?; let proto = interp.intrinsics.string_proto; Ok(Value::Object(interp.heap.alloc_object(Object::new(Class::String(s), Some(proto))))) } diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index 0c2f051f8..3d3455544 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -355,22 +355,63 @@ fn a_flattened_temporary_stays_counted_while_the_next_copy_is_made() { } #[test] -fn a_string_wrapper_counts_what_its_rope_holds_while_the_copy_is_made() { +fn a_converted_rope_counts_what_it_holds_while_its_copy_is_made() { // `mk` returns a rope whose left half is a flat string of about half the heap, held by // nothing else. Its flattened copy is the other half, so the two don't fit together. let build = "var s = 'x'; for (var i = 0; i < 18; i++) s = s + s; function f(c) { return s + c; } \ function mk() { var a = f('a'); a[0]; return a + 'b'; }"; let limits = Limits { heap_bytes: 4 * 262_146, ..Limits::default() }; - let src = format!("{build} new String({{ toString: mk }}); 0"); - let result = Realm::with_options(Options::default(), limits).run(&src); - assert!(matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)), "{result:?}"); + let not_stopped: Vec<_> = ["+({ valueOf: mk })", "({})[{ toString: mk }]"] + .into_iter() + .filter(|read| { + let result = Realm::with_options(Options::default(), limits.clone()).run(&format!("{build} {read}; 0")); + !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) + }) + .collect(); + assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); +} + +#[test] +fn a_string_object_counts_its_flattened_rope_against_the_heap() { + // The same three ropes, each wrapped in a String object and read by index. + let build = "var s = 'x'; for (var i = 0; i < 19; i++) s = s + s; var t = s + 'y', u = s + 'z'; \ + String.prototype.first = function () { return this[0]; };"; + let limits = Limits { heap_bytes: 3 << 20, ..Limits::default() }; + // Wrapping alone reads no units, so it makes no copy. + let lengths = + Realm::with_options(Options::default(), limits.clone()).run(&format!("{build} Object(s).length + new String(t).length + Object(u).length")); + assert_eq!(lengths, Ok(JsValue::Number(1_572_866.0))); + for read in ["Object(X)[0]", "new String(X)[0]", "0 in Object(X)", "Object(X).hasOwnProperty(0)", "X.first()"] { + let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); + let e = fail_with(Realm::with_options(Options::default(), limits.clone()), &src); + assert_eq!(e.kind, RunErrorKind::Limit(LimitKind::Heap), "{read}: {e:?}"); + } + let wrapped = + Realm::with_options(Options::default(), Limits::default()).run(&format!("{build} Object(t)[524288] + new String(u)[524288] + t.first()")); + assert_eq!(wrapped, Ok(JsValue::String("yzx".into()))); +} + +#[test] +fn a_string_object_takes_fuel_for_the_rope_it_flattens() { + let limits = Limits { fuel: 40_000, ..Limits::default() }; + let build = "var s = ' '; for (var i = 0; i < 20; i++) s = s + s; Object(s).length"; + assert!(Realm::with_options(Options::default(), limits.clone()).run(build).is_ok()); + for read in ["Object(s)[0]", "0 in new String(s)"] { + let e = fail_with(Realm::with_options(Options::default(), limits.clone()), &format!("{build}; {read}")); + assert_eq!(e.kind, RunErrorKind::Limit(LimitKind::Fuel), "{read}: {e:?}"); + } + // A run that ends with an error right after the read leaves the next run its whole fuel. + let mut realm = Realm::with_options(Options::default(), Limits { fuel: 100_000, ..Limits::default() }); + let defined = realm.run("var s = ' '; for (var i = 0; i < 20; i++) s = s + s; Object.defineProperty(new String(s), 0, { value: 'a' })"); + assert!(matches!(defined, Err(ref e) if e.kind == RunErrorKind::Thrown), "{defined:?}"); + assert!(realm.run("for (var j = 0; j < 5000; j++) {} j").is_ok()); } #[test] fn a_call_keeps_its_operands_while_a_string_receiver_is_boxed() { - // Boxing the rope receiver of a non-strict function flattens it, and reserving the copy - // collects the garbage string `g` left. The callee and the argument object are off the - // stack by then, and must survive the collection. + // `g` leaves a garbage string, so the heap is close to its limit when the rope receiver of a + // non-strict function is boxed. The callee and the argument object are off the stack by + // then, and must survive anything boxing the receiver does. let build = "var s = 'x'; for (var i = 0; i < 17; i++) s = s + s; function f(c) { return s + c; } \ function mk() { var a = f('a'); a[0]; return a + 'b'; } 0"; let mut realm = Realm::with_options(Options::default(), Limits::default()); From 40283f4a380955ac22639e31bcd6354c5931b75c Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 22:49:15 -0800 Subject: [PATCH 5/7] Scripts: loose equality and the JSON space string keep a converted string rooted while it is flattened --- crates/js/src/builtins/json.rs | 3 ++- crates/js/src/interp.rs | 4 +++- crates/js/src/text_tests.rs | 19 ++++++++++++------- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/js/src/builtins/json.rs b/crates/js/src/builtins/json.rs index 364ec9969..d32344946 100644 --- a/crates/js/src/builtins/json.rs +++ b/crates/js/src/builtins/json.rs @@ -474,7 +474,8 @@ fn settings(interp: &mut Interp, replacer: &Value, space: &Value) -> JsResult s.gap = flat_charged(interp, &t)?.iter().take(10).copied().collect(), + // Rooted, so what the string holds is still counted while its copy is reserved. + Value::String(t) => s.gap = interp.rooted(&[Value::String(t.clone())], |i| flat_charged(i, &t))?.iter().take(10).copied().collect(), _ => {} } Ok(s) diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index 19d2eef22..b399ebc23 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -2816,7 +2816,9 @@ impl Interp { | (Value::Bool(_), Value::Bool(_)) | (Value::Object(_), Value::Object(_)) => return Ok(strict_eq(&x, &y)), (Value::Number(n), Value::String(s)) | (Value::String(s), Value::Number(n)) => { - return Ok(*n == string_to_number(&flat_charged(self, s)?)); + // Rooted, so what the string holds is still counted while its copy is reserved. + let units = self.rooted(&[x.clone(), y.clone()], |i| flat_charged(i, s))?; + return Ok(*n == string_to_number(&units)); } (Value::Bool(v), _) => x = Value::Number(f64::from(u8::from(*v))), (_, Value::Bool(v)) => y = Value::Number(f64::from(u8::from(*v))), diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index 3d3455544..6fccc085f 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -361,13 +361,18 @@ fn a_converted_rope_counts_what_it_holds_while_its_copy_is_made() { let build = "var s = 'x'; for (var i = 0; i < 18; i++) s = s + s; function f(c) { return s + c; } \ function mk() { var a = f('a'); a[0]; return a + 'b'; }"; let limits = Limits { heap_bytes: 4 * 262_146, ..Limits::default() }; - let not_stopped: Vec<_> = ["+({ valueOf: mk })", "({})[{ toString: mk }]"] - .into_iter() - .filter(|read| { - let result = Realm::with_options(Options::default(), limits.clone()).run(&format!("{build} {read}; 0")); - !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) - }) - .collect(); + let not_stopped: Vec<_> = [ + "+({ valueOf: mk })", + "({})[{ toString: mk }]", + "({ valueOf: mk }) == 1", + "JSON.stringify(0, null, (function () { var o = new String(''); o.toString = mk; return o; })())", + ] + .into_iter() + .filter(|read| { + let result = Realm::with_options(Options::default(), limits.clone()).run(&format!("{build} {read}; 0")); + !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) + }) + .collect(); assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); } From c2c560eeaaf9ff7d4ad36df2dcde0ca46a4ca19f Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 22:57:35 -0800 Subject: [PATCH 6/7] Scripts: a String object's character is read along its rope when the copy does not fit When the flattened copy of a String object's concatenated string would pass the heap limit before a collection can run, the indexed read walks the rope to the unit instead, taking a unit of fuel for each node it visits, so no copy is made. --- crates/js/src/interp.rs | 38 +++++++++++++++++++++--------- crates/js/src/text_tests.rs | 46 ++++++++++++++++++++----------------- crates/js/src/value.rs | 28 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 32 deletions(-) diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index b399ebc23..696df4778 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -209,7 +209,8 @@ pub(crate) struct Interp { /// Bytes of the copies [`Self::get_own_property`] made flattening the rope of a String /// object, which the heap's total adds before the next limit check. flattened: std::cell::Cell, - /// Fuel for the units those copies took, which the next step takes. + /// Fuel for the units those copies took and the rope nodes it read in place, which the next + /// step takes. pub(crate) flattened_fuel: std::cell::Cell, /// The global code's default XML namespace URI (ECMA-357 section 12.1). pub(crate) xml_default_uri: Key, @@ -1246,18 +1247,33 @@ impl Interp { if let Some(i) = array_index(k) && (i as usize) < s.len() { - if s.is_rope() { - // Flattening copies the units. The run ends here when the fuel for the - // copy is not left; otherwise the next step takes it, and the next limit - // check adds the copy's bytes. - let fuel = self.flattened_fuel.get().saturating_add((s.len() / crate::string_builtins::UNITS_PER_FUEL) as u64 + 1); - if fuel > self.fuel { - return Err(Abrupt::Limit(LimitKind::Fuel)); + let unit = if s.is_rope() { + let owed = self.flattened_fuel.get(); + let flattened = self.flattened.get().saturating_add(string_bytes(s.len())); + if self.heap.bytes.saturating_add(flattened) > self.limits.heap_bytes { + // The copy does not fit without a collection, which needs `&mut self`: + // the unit is read along the rope instead, a unit of fuel per node. + let Some((unit, visited)) = s.unit_at(i as usize, self.fuel.saturating_sub(owed)) else { + return Err(Abrupt::Limit(LimitKind::Fuel)); + }; + self.flattened_fuel.set(owed.saturating_add(visited)); + unit + } else { + // Flattening copies the units. The run ends here when the fuel for + // the copy is not left; otherwise the next step takes it, and the + // next limit check adds the copy's bytes. + let fuel = owed.saturating_add((s.len() / crate::string_builtins::UNITS_PER_FUEL) as u64 + 1); + if fuel > self.fuel { + return Err(Abrupt::Limit(LimitKind::Fuel)); + } + self.flattened_fuel.set(fuel); + self.flattened.set(flattened); + s.flat().get(i as usize).copied() } - self.flattened_fuel.set(fuel); - self.flattened.set(self.flattened.get().saturating_add(string_bytes(s.len()))); + } else { + s.flat().get(i as usize).copied() } - let unit = s.flat().get(i as usize).copied().unwrap_or(0); + .unwrap_or(0); return Ok(Some(Prop::Data { value: Value::String(Str::from_units(vec![unit])), flags: ENUMERABLE })); } } diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index 6fccc085f..d2627cfb5 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -314,23 +314,15 @@ fn operators_and_builtins_count_a_flattened_rope_against_the_heap() { // Three ropes of about 524,288 units, each 1 MiB once flat, with a 3 MiB heap. let build = "var s = 'x'; for (var i = 0; i < 19; i++) s = s + s; var t = s + 'y', u = s + 'z';"; let limits = Limits { heap_bytes: 3 << 20, ..Limits::default() }; - let not_stopped: Vec<_> = [ - "X < 'a'", - "X == 1", - "+X", - "X[0]", - "new String(X)[0]", - "({})[{ toString: function () { return X; } }]", - "JSON.stringify(0, null, X)", - "[1].join(X)", - ] - .into_iter() - .filter(|read| { - let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); - let result = Realm::with_options(Options::default(), limits.clone()).run(&src); - !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) - }) - .collect(); + let not_stopped: Vec<_> = + ["X < 'a'", "X == 1", "+X", "X[0]", "({})[{ toString: function () { return X; } }]", "JSON.stringify(0, null, X)", "[1].join(X)"] + .into_iter() + .filter(|read| { + let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); + let result = Realm::with_options(Options::default(), limits.clone()).run(&src); + !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)) + }) + .collect(); assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}"); } @@ -386,10 +378,22 @@ fn a_string_object_counts_its_flattened_rope_against_the_heap() { let lengths = Realm::with_options(Options::default(), limits.clone()).run(&format!("{build} Object(s).length + new String(t).length + Object(u).length")); assert_eq!(lengths, Ok(JsValue::Number(1_572_866.0))); - for read in ["Object(X)[0]", "new String(X)[0]", "0 in Object(X)", "Object(X).hasOwnProperty(0)", "X.first()"] { - let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); - let e = fail_with(Realm::with_options(Options::default(), limits.clone()), &src); - assert_eq!(e.kind, RunErrorKind::Limit(LimitKind::Heap), "{read}: {e:?}"); + // Two copies fit. The third read finds no room for its copy and reads the unit along the + // rope, so the live heap stays within the limit. + for (read, expected) in [ + ("Object(X)[0]", "xxx"), + ("new String(X)[0]", "xxx"), + ("0 in Object(X)", "truetruetrue"), + ("Object(X).hasOwnProperty(0)", "truetruetrue"), + ("X.first()", "xxx"), + ("Object(X)[524288]", "undefinedyz"), + ] { + let src = format!("{build} '' + ({}) + ({}) + ({})", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u")); + let mut realm = Realm::with_options(Options::default(), limits.clone()); + assert_eq!(realm.run(&src), Ok(JsValue::String(expected.into())), "{read}"); + let interp = realm.interp(); + interp.collect_garbage(); + assert!(interp.heap.bytes <= limits.heap_bytes, "{read}: {} bytes live", interp.heap.bytes); } let wrapped = Realm::with_options(Options::default(), Limits::default()).run(&format!("{build} Object(t)[524288] + new String(u)[524288] + t.first()")); diff --git a/crates/js/src/value.rs b/crates/js/src/value.rs index e164001ad..a0fbcd4a5 100644 --- a/crates/js/src/value.rs +++ b/crates/js/src/value.rs @@ -163,6 +163,34 @@ impl Str { flat } + /// The code unit at `i` read along the rope without flattening it, visiting at most + /// `max_nodes` nodes: `None` when that is not enough, otherwise the unit (when `i` is in + /// range) and the number of nodes visited. + pub(crate) fn unit_at(&self, mut i: usize, max_nodes: u64) -> Option<(Option, u64)> { + let mut node = self.clone(); + let mut visited = 0u64; + loop { + visited += 1; + if visited > max_nodes { + return None; + } + let next = match &*node.0.borrow() { + Node::Flat(f) => return Some((f.get(i).copied(), visited)), + Node::Concat { left, right, .. } => { + let left_len = left.len(); + if i < left_len { + left.clone() + } else { + i -= left_len; + right.clone() + } + } + Node::Taken => return Some((None, visited)), + }; + node = next; + } + } + pub(crate) fn to_rust_string(&self) -> String { String::from_utf16_lossy(&self.flat()) } From 49c8a380a1e2ed2f44b39d23fc1068cfea09cf5a Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 23:02:45 -0800 Subject: [PATCH 7/7] Scripts: flattening a rope keeps it rooted while its copy is reserved flat_charged and the property-key path now root the rope itself, so a string held only by native code (a getter's result for ++o.x, a valueOf result, a toString key) still counts what it holds when the reservation collects. The per-call roots for these cases are removed. --- crates/js/src/builtins/json.rs | 3 +-- crates/js/src/interp.rs | 17 +++++++---------- crates/js/src/string_builtins.rs | 5 +++-- crates/js/src/text_tests.rs | 1 + 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/js/src/builtins/json.rs b/crates/js/src/builtins/json.rs index d32344946..364ec9969 100644 --- a/crates/js/src/builtins/json.rs +++ b/crates/js/src/builtins/json.rs @@ -474,8 +474,7 @@ fn settings(interp: &mut Interp, replacer: &Value, space: &Value) -> JsResult s.gap = interp.rooted(&[Value::String(t.clone())], |i| flat_charged(i, &t))?.iter().take(10).copied().collect(), + Value::String(t) => s.gap = flat_charged(interp, &t)?.iter().take(10).copied().collect(), _ => {} } Ok(s) diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index 696df4778..ff821b217 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -2626,8 +2626,7 @@ impl Interp { Value::String(s) => string_to_number(&flat_charged(self, s)?), Value::Object(_) => { let p = self.to_primitive(v, Hint::Number)?; - // Rooted, so what a string holds is still counted while its copy is reserved. - return self.rooted(std::slice::from_ref(&p), |i| i.to_number(&p)); + return self.to_number(&p); } }) } @@ -2662,19 +2661,19 @@ impl Interp { Value::Number(n) if *n >= 0.0 && *n < 4_294_967_295.0 && n.fract() == 0.0 => Ok(index_key(*n as u32)), _ => { let s = self.to_str(v)?; - // Rooted, so what the string holds is still counted while its copy is reserved. - self.rooted(&[Value::String(s.clone())], |i| i.flat_reserved(&s)) + self.flat_reserved(&s) } } } /// The code units of `s`. Flattening a rope makes a copy of its units, so for a rope that - /// copy's bytes are reserved and added to the heap first, without taking fuel. Everything - /// the caller still needs must be reachable from the roots. + /// copy's bytes are reserved and added to the heap first, without taking fuel. The rope stays + /// rooted while the copy is reserved; everything else the caller still needs must be + /// reachable from the roots. fn flat_reserved(&mut self, s: &Str) -> JsResult { if s.is_rope() { let bytes = string_bytes(s.len()); - self.reserve_heap(bytes)?; + self.rooted(&[Value::String(s.clone())], |i| i.reserve_heap(bytes))?; self.heap.bytes = self.heap.bytes.saturating_add(bytes); } Ok(s.flat()) @@ -2832,9 +2831,7 @@ impl Interp { | (Value::Bool(_), Value::Bool(_)) | (Value::Object(_), Value::Object(_)) => return Ok(strict_eq(&x, &y)), (Value::Number(n), Value::String(s)) | (Value::String(s), Value::Number(n)) => { - // Rooted, so what the string holds is still counted while its copy is reserved. - let units = self.rooted(&[x.clone(), y.clone()], |i| flat_charged(i, s))?; - return Ok(*n == string_to_number(&units)); + return Ok(*n == string_to_number(&flat_charged(self, s)?)); } (Value::Bool(v), _) => x = Value::Number(f64::from(u8::from(*v))), (_, Value::Bool(v)) => y = Value::Number(f64::from(u8::from(*v))), diff --git a/crates/js/src/string_builtins.rs b/crates/js/src/string_builtins.rs index b3758bdf5..e1baed6c2 100644 --- a/crates/js/src/string_builtins.rs +++ b/crates/js/src/string_builtins.rs @@ -109,10 +109,11 @@ fn push_held(interp: &mut Interp, held: &mut HeldBytes, list: &mut Vec, it } /// The code units of `s`. Flattening a rope makes a copy of its units, so for a rope that copy's -/// bytes are reserved and added to the heap, and its units charged as fuel, first. +/// bytes are reserved and added to the heap, and its units charged as fuel, first. The rope stays +/// rooted while the copy is reserved, so what it holds is still counted. pub(crate) fn flat_charged(interp: &mut Interp, s: &Str) -> JsResult> { if s.is_rope() { - reserve_units(interp, s.len())?; + interp.rooted(&[Value::String(s.clone())], |interp| reserve_units(interp, s.len()))?; } Ok(s.flat()) } diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index d2627cfb5..01fc613cc 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -357,6 +357,7 @@ fn a_converted_rope_counts_what_it_holds_while_its_copy_is_made() { "+({ valueOf: mk })", "({})[{ toString: mk }]", "({ valueOf: mk }) == 1", + "++({ get x() { return mk(); } }).x", "JSON.stringify(0, null, (function () { var o = new String(''); o.toString = mk; return o; })())", ] .into_iter()