From dea30d6328aa39fe491f494e80a104df7bc1a4e7 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 19:14:05 -0800 Subject: [PATCH] Scripts: a finally block left early drops its saved throw site A finally block that break or continue leaves no longer keeps the saved site of the exception it ran for. A Jump now removes the sites at or above the stack's length, as a return already does, so a collection after the stack grows back over that slot does not root and count the old code. New tests in crates/js/src/host_tests.rs: an error passed on by a finally block that collects garbage under a 1 MiB heap is reported at its throw; the saved site keeps direct eval code live through a collection; a host abort inside a finally block leaves no saved site; and break, continue and return out of a finally block drop the site. --- crates/js/src/host_tests.rs | 98 +++++++++++++++++++++++++++++++++++++ crates/js/src/interp.rs | 12 +++-- docs/target-app-parity.md | 1 + 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/crates/js/src/host_tests.rs b/crates/js/src/host_tests.rs index 6fc6c4e04..6aaa847ce 100644 --- a/crates/js/src/host_tests.rs +++ b/crates/js/src/host_tests.rs @@ -436,6 +436,104 @@ fn a_marked_host_error_that_passes_through_a_finally_block_says_where_it_was_rai assert_eq!(r.error_mark(), Some(7)); } +/// A realm with a 1 MiB heap, the size that the `finally` blocks of these tests collect garbage in. +fn small_heap_realm() -> Realm { + let mut r = Realm::new(); + r.set_limits(Limits { fuel: 200_000_000, heap_bytes: 1 << 20, ..Limits::default() }); + r +} + +/// Begins a run of `source` and advances it one instruction at a time until a `finally` block runs +/// for an exception whose throw site is saved. +fn start_until_a_throw_site_is_saved(r: &mut Realm, source: &str) { + r.start(source).unwrap(); + while r.interp().finally_sites.is_empty() { + assert_eq!(r.advance(1), Ok(Progress::Paused), "{source}"); + } +} + +/// The collections that a `finally` block's allocations cause under a small heap limit keep the +/// site its exception was raised at, and the run's end leaves no saved site behind. +#[test] +fn an_error_that_passes_through_a_finally_block_that_collects_garbage_says_where_it_was_raised() { + let mut r = small_heap_realm(); + let source = "try {\n throw new Error('original');\n} finally {\n for (var i = 0; i < 20000; i++) { var waste = [i, i + 1, i + 2]; }\n}"; + start_until_a_throw_site_is_saved(&mut r, source); + r.collect_garbage(); + let collections = r.interp().heap.collections; + let e = loop { + match r.advance(1_000) { + Ok(Progress::Paused) => assert_eq!(r.interp().finally_sites.len(), 1, "the site stays saved while the block runs"), + Ok(Progress::Done(v)) => panic!("the run ended with {v:?}"), + Err(e) => break e, + } + }; + assert!(r.interp().heap.collections > collections, "the block's allocations collect garbage"); + assert_eq!((e.kind, e.line, e.column, e.message.as_str()), (RunErrorKind::Thrown, 2, 3, "Error: original")); + assert!(r.interp().finally_sites.is_empty()); +} + +/// The code of a saved throw site stays live through a collection: direct `eval` code that +/// threw is not part of the code of any frame still running. +#[test] +fn a_saved_throw_site_keeps_its_code_live() { + let mut r = small_heap_realm(); + let source = "try {\n eval(\"var unused = 'padding padding padding padding';\\nthrow new Error('original');\");\n} finally {\n for (var i = 0; i < 2000; i++) { var waste = [i]; }\n}"; + start_until_a_throw_site_is_saved(&mut r, source); + let with_site = r.collect_garbage(); + let sites = std::mem::take(&mut r.interp().finally_sites); + let without_site = r.collect_garbage(); + r.interp().finally_sites = sites; + assert_eq!(r.collect_garbage(), with_site, "the saved site's code is counted again"); + assert!(with_site > without_site, "{with_site} <= {without_site}"); + let e = finish(&mut r, 1_000).0.unwrap_err(); + assert_eq!((e.kind, e.line, e.message.as_str()), (RunErrorKind::Thrown, 2, "Error: original")); + assert!(r.interp().finally_sites.is_empty()); +} + +/// A run that a host function stops inside a `finally` block leaves no saved throw site behind. +#[test] +fn a_run_stopped_in_a_finally_block_leaves_no_saved_throw_site() { + let mut r = realm(1_000_000); + let f: HostFunction = Rc::new(|_| Err(HostError::Abort("stopped".into()))); + r.define_host_object("host", vec![("abort".into(), HostItem::Function(f))]).unwrap(); + let e = r.run("try { throw new Error('original'); } finally { host.abort(); }").unwrap_err(); + assert_eq!((e.kind, e.message.as_str()), (RunErrorKind::Limit(LimitKind::Host), "stopped")); + assert!(r.interp().finally_sites.is_empty()); + assert_eq!(r.run("1").unwrap(), JsValue::Number(1.0)); + assert!(r.interp().finally_sites.is_empty()); +} + +/// A `finally` block that `break`, `continue` or `return` leaves drops the saved site of the +/// exception it ran for as it leaves, before the stack grows back over that site's slot. +#[test] +fn leaving_a_finally_block_early_drops_its_saved_throw_site() { + let mut r = realm(1_000_000); + let marked = Rc::new(Cell::new(false)); + let seen = marked.clone(); + let f: HostFunction = Rc::new(move |_| { + seen.set(true); + Ok(Data::Undefined) + }); + r.define_host_object("host", vec![("mark".into(), HostItem::Function(f))]).unwrap(); + for source in [ + "do { try { throw new Error('x'); } finally { break; } } while (false);\nhost.mark(); var a = [1, 2, 3];", + "for (var i = 0; i < 1; i++) { try { throw new Error('x'); } finally { continue; } }\nhost.mark(); var a = [1, 2, 3];", + "function f() { try { throw new Error('x'); } finally { return 1; } }\nf(); host.mark(); var a = [f, f, f];", + ] { + marked.set(false); + let mut saved = false; + r.start(source).unwrap(); + while r.advance(1).unwrap() == Progress::Paused { + saved |= !r.interp().finally_sites.is_empty(); + if marked.get() { + assert!(r.interp().finally_sites.is_empty(), "{source}"); + } + } + assert!(saved && marked.get(), "{source}"); + } +} + #[test] fn advancing_without_a_run_is_an_error() { let mut r = realm(1_000); diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index afb6cde63..cda117eca 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -235,9 +235,9 @@ pub(crate) struct Interp { /// Where the exceptions `finally` blocks are running for were raised, innermost last, each with /// the stack slot that holds the exception while the block runs. The block's end throws the /// exception on from that site. An entry whose slot is at or above the stack's length belongs to - /// a block that was left, and [`Self::drop_left_finally_sites`] removes it. [`Self::unwind`] - /// removes the entries at or above a handler's slot before it adds one, so the slots rise and - /// the list is never longer than the stack. + /// a block that was left, and [`Self::drop_left_finally_sites`] removes it at each `Jump`, each + /// return and each collection. [`Self::unwind`] removes the entries at or above a handler's slot + /// before it adds one, so the slots rise and the list is never longer than the stack. pub(crate) finally_sites: Vec<(ThrowSite, usize)>, /// The mark of the host error the last run ended with. pub(crate) error_mark: Option, @@ -885,7 +885,11 @@ impl Interp { Op::New(n) => self.op_new(n), Op::Binary(b) => self.op_binary(b), Op::Neg | Op::Plus | Op::BitNot | Op::Not | Op::Typeof | Op::ToNumber => self.op_unary(op), - Op::Jump(t) => self.jump_to(t), + Op::Jump(t) => { + // `break` and `continue` leave a `finally` block by popping its exception and jumping. + self.drop_left_finally_sites(self.stack.len()); + self.jump_to(t) + } Op::JumpIfFalse(t) | Op::JumpIfTrue(t) => self.op_branch(t, matches!(op, Op::JumpIfTrue(_)), false), Op::JumpIfFalseKeep(t) | Op::JumpIfTrueKeep(t) => self.op_branch(t, matches!(op, Op::JumpIfTrueKeep(_)), true), Op::Throw => { diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 18d50447f..9d7c94d0e 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -743,6 +743,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - Scripts: the Script Console panel's report line reads as `script.console` gives it, `time unavailable` included on a platform without a clock; counts of host calls, dropped lines and cut lines take the singular for one, in each language's own plural forms; and a run that is no longer available still gets its end line (`crates/engine/src/tests/script_console.rs`, `crates/ui-egui/src/panels/tests_script_console.rs`). - Scripts: an error `$.evalFile` throws for a file it cannot run is reported at that file's line and column only while no `catch` has taken it. Caught and thrown again later, in the script or in another file, it is reported where it was thrown again, and a new error whose message holds its text is reported where that error was thrown. VectorCraft's own rule. `crates/engine/src/tests/script_include.rs`, `crates/js/src/host_tests.rs`. - Scripts: an uncaught error that one or more `finally` blocks pass on unchanged, in the script or in a function it called, is reported at the line and column where it was thrown, in `script.run`'s report and the Script Console. An error a `finally` block throws is reported at that throw, and one its `return`, `break` or `continue` discards is not reported. VectorCraft's own rule. `crates/engine/src/tests/script_include.rs`, `crates/js/src/host_tests.rs`. + - Scripts: an error a `finally` block passes on is still reported where it was thrown when the block's allocations collect garbage under a small heap limit, and a `finally` block that `return`, `break` or `continue` leaves drops the saved location of its error as it leaves, so a collection no longer counts that code as live. VectorCraft's own rule. `crates/js/src/host_tests.rs`. - Scripts: a run reads its preprocessor lines before any of it runs: `#include` and `//@include` put another file in place of the line, looked for beside the including file, in the `#includepath` folders and in `JSINCLUDE`; `#script` names the run; `#target` and `#strict off` are taken, `#strict on` turns on strict write checks (below), `#targetengine` names the engine (below), and `#engine` is refused. `$.evalFile` runs a file with the same globals and returns its last value, and `$.fileName` and `$.includePath` give the file being run and the include folders. A script run from a file reads these files only in its folder and below, without `..` names or links that lead out, and an error in one is reported at its own file, line and column ([preprocessor directives](https://extendscript.docsforadobe.dev/extendscript-tools-features/preprocessor-directives/), [the `$` object](https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/); `crates/engine/src/tests/script_include.rs`). - Scripts: `#targetengine name` (and `//@targetengine name`) runs the script in a named engine that lasts for the session, and a later script that names it sees the globals and functions earlier ones left ([preprocessor directives](https://extendscript.docsforadobe.dev/extendscript-tools-features/preprocessor-directives/)); `$.engineName` gives the name ([the `$` object](https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/)). VectorCraft's own rules: a script without the line and `#targetengine main` get a new engine each run, the line comes once in the script itself before its code, a session keeps 8 named engines with 64 MiB of heap each, an error or a stop keeps the globals the run set while the documents go back, kept host functions act for the run that calls them, `script.engines` lists the engines and `script.engine.drop` drops one, and the web build has none. `crates/engine/src/tests/script_engines.rs`. - Scripts: `File` and `Folder` ([File object](https://extendscript.docsforadobe.dev/file-system-access/file-object/), [Folder object](https://extendscript.docsforadobe.dev/file-system-access/folder-object/)) name files and folders by platform or URI path, with `name`, `fsName`, `absoluteURI`, `relativeURI`, `parent`, `Folder.current` and `encode`/`decode`, and read them: `exists`, `length`, the dates, `getFiles` with a mask or a function, and `open('r')` with `read`, `readln`, `readch`, `seek`, `tell` and `close`, decoding by byte order mark, `encoding`, UTF-8 or CP1252. VectorCraft's own rules: a script reads only in its own folder and below (and inside the automation read roots when they are set); any other path and a network or device path are refused before anything is looked up, and a link that leads out is refused as well (`false` or `null`, with `error` set); a file is read whole up to 8 MiB and a run reads up to 32 MiB; writing, the file dialogs and `execute` are not available yet. `crates/engine/src/tests/script_io.rs`.