From 46efc4a651364a2d9ef38761efbc73a77aa18ac3 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 22:39:04 -0800 Subject: [PATCH] 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());