diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index 458d68f40c..afb6cde635 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; @@ -619,9 +625,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) { @@ -784,6 +797,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..28157d6eea 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,58 @@ 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 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