From f04aa61fc1e3f899822c0f230c399450a30ab602 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 12:19:32 -0800 Subject: [PATCH 1/2] Scripts: a return from a finally block drops the discarded error's throw site A `return` inside a finally block discards the pending exception, but the saved throw site for it stayed in the list of collection roots. The code it pointed to then stayed live after the function was gone, which could stop a script with the heap limit when that code came from `Function` or `eval`. Returning from a frame now drops the saved sites for the slots it leaves. Follows #6653. --- crates/js/src/interp.rs | 2 ++ crates/js/src/interp_tests.rs | 41 ++++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index b04702be58..d9a351c774 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -779,6 +779,8 @@ impl Interp { fn return_value(&mut self, v: Value) -> JsResult> { let Some(frame) = self.frames.pop() else { return internal("return without a frame") }; self.stack.truncate(frame.base); + // A `return` in a finally block leaves the block, and the value takes its exception's slot. + self.drop_left_finally_sites(frame.base); let v = match frame.construct { Some(o) if !matches!(v, Value::Object(_)) => Value::Object(o), _ => v, diff --git a/crates/js/src/interp_tests.rs b/crates/js/src/interp_tests.rs index d2a12b5fb0..9d673dc78e 100644 --- a/crates/js/src/interp_tests.rs +++ b/crates/js/src/interp_tests.rs @@ -3,7 +3,7 @@ use std::rc::Rc; use proptest::prelude::*; use crate::bytecode::{Code, Handler, Op, Source, verify}; -use crate::{JsValue, LimitKind, Limits, Options, Realm, RunError, RunErrorKind}; +use crate::{JsValue, LimitKind, Limits, Options, Progress, Realm, RunError, RunErrorKind}; fn run(src: &str) -> JsValue { match Realm::new().run(src) { @@ -357,6 +357,45 @@ fn function_source_text_counts_toward_the_heap() { assert_eq!(realm.run("(function f(a) { return a; }).toString()"), Ok(JsValue::String("function f(a) { return a; }".into()))); } +#[test] +fn a_function_that_returns_from_a_finally_block_does_not_keep_its_code() { + // The function's source holds a 256 Ki-unit comment. Its result stays on the stack while the + // second call runs, after the function was dropped. A collection then leaves its code out of + // the live total, as it does for a function whose finally block ran without an exception. + let live = |body: &str| { + let mut r = small(Limits::default()); + let src = format!( + "var s = 'x'; for (var i = 0; i < 18; i++) s += s;\n\ + var f = Function('/*' + s + '*/ try {{ {body} }} finally {{ return 0; }}'); s = null;\n\ + f() + (f = null, (function () {{ var a = 0; for (var i = 0; i < 50; i++) a += i; return a; }})())" + ); + if let Err(e) = r.start(&src) { + panic!("{e:?}"); + } + let mut bytes = None; + loop { + match r.advance(1) { + Ok(Progress::Paused) => { + let i = r.interp(); + if i.frames.len() == 2 && i.stack.len() > 1 { + i.collect_garbage(); + bytes = Some(i.heap.bytes); + } + } + Ok(Progress::Done(v)) => { + assert_eq!(v, JsValue::Number(1225.0)); + break; + } + Err(e) => panic!("{e:?}"), + } + } + let Some(bytes) = bytes else { panic!("the second call never paused") }; + bytes + }; + let (plain, thrown) = (live(""), live("throw 1;")); + assert!(thrown < plain + 4096, "{thrown} > {plain}"); +} + #[test] fn for_in_names_take_fuel_and_heap_under_later_limits() { // The first run, under the default limits, builds an object with many properties. Later runs From db6b55ab0a8da708b54e321e48785529b28c64ac Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 14:03:21 -0800 Subject: [PATCH 2/2] Scripts: a return looks only at the last saved finally site The saved throw sites of running finally blocks are kept in rising slot order, so leaving a frame or a block now removes entries from the end of the list and stops at the first one below the slot. Before, every return looked at every saved site, so a call made inside many nested finally blocks took time in proportion to their count. --- crates/js/src/interp.rs | 17 +++++++++++++++-- crates/js/src/interp_tests.rs | 13 +++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index d9a351c774..af17e6586b 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -300,6 +300,12 @@ pub(crate) const MAX_APPLY_ARGS: usize = 1 << 16; /// The estimated bytes of a name's entries in the list and the set `for-in` builds. const KEY_ENTRY_BYTES: usize = 32; +#[cfg(test)] +thread_local! { + /// Tests: the [`Interp::finally_sites`] entries that leaving a block or a frame has looked at. + pub(crate) static SITE_CHECKS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// The estimated bytes of the index names `0` to `len - 1` while `for-in` lists them. fn index_keys_bytes(len: usize) -> usize { let mut total = 0usize; @@ -614,9 +620,16 @@ impl Interp { } /// Removes the [`Self::finally_sites`] entries whose stack slot is at or above `len`: those of - /// `finally` blocks that were left. + /// `finally` blocks that were left. The slots rise along the list, so only its end is looked at. fn drop_left_finally_sites(&mut self, len: usize) { - self.finally_sites.retain(|&(_, slot)| slot < len); + while let Some(&(_, slot)) = self.finally_sites.last() { + #[cfg(test)] + SITE_CHECKS.with(|c| c.set(c.get() + 1)); + if slot < len { + break; + } + self.finally_sites.pop(); + } } pub(crate) fn collect_garbage(&mut self) { diff --git a/crates/js/src/interp_tests.rs b/crates/js/src/interp_tests.rs index 9d673dc78e..28157d6eea 100644 --- a/crates/js/src/interp_tests.rs +++ b/crates/js/src/interp_tests.rs @@ -396,6 +396,19 @@ fn a_function_that_returns_from_a_finally_block_does_not_keep_its_code() { assert!(thrown < plain + 4096, "{thrown} > {plain}"); } +#[test] +fn a_return_looks_only_at_the_finally_sites_of_its_own_frame() { + // 200 finally blocks run at once, each for its own exception, and the innermost calls a + // function 1,000 times. Each return looks at the last saved site only, not at all 200. + let src = "function g() { return 1; }\n\ + function f(n) { try { throw n; } finally { if (n) { f(n - 1); } else { for (var i = 0; i < 1000; i++) g(); } } }\n\ + try { f(199); } catch (e) { e; }"; + crate::interp::SITE_CHECKS.with(|c| c.set(0)); + assert_eq!(small(Limits::default()).run(src), Ok(JsValue::Number(0.0))); + let checks = crate::interp::SITE_CHECKS.with(std::cell::Cell::get); + assert!(checks < 10_000, "{checks} saved sites looked at"); +} + #[test] fn for_in_names_take_fuel_and_heap_under_later_limits() { // The first run, under the default limits, builds an object with many properties. Later runs