Skip to content

Enable the stack-overflow guard by default - #3057

Merged
lahma merged 1 commit into
sebros/default-clr-array-copy-498from
sebros/default-stack-guard
Aug 21, 2026
Merged

lahma merged 1 commit into
sebros/default-clr-array-copy-498from
sebros/default-stack-guard

Conversation

@sebastienros

Copy link
Copy Markdown
Owner

Summary

  • enable Options.Constraints.StackOverflowGuard by default for every new Engine
  • retain options.Constraints.StackOverflowGuard = false as the explicit compatibility opt-out for trusted, independently bounded scripts
  • preserve the distinction between the native-stack backstop and MaxRecursionDepth, which counts repeated occurrences of one function definition
  • update the README and threat model with the breaking default, migration guidance, residual risks, and hardened configuration

Stack

This PR targets sebros/default-clr-array-copy-498 (#3056), then stacks through #3054 -> #3052 -> #3051 -> #3046 -> #3045 -> #3037 -> #3036 -> #3035 -> #3030.

Integration

The reviewed stack-guard commit was transplanted onto the live #3056 tip 39b68c1a398d8cea2e65026f5c202d7417311f77. The only conflict was in .github/THREAT_MODEL.md; resolution retained both the cumulative parser-limit guidance from the stack and the default-on stack guard guidance. No prior security behavior was weakened.

A fresh security specialist review against origin/sebros/default-clr-array-copy-498 specifically checked recursion-path coverage, thread/async state, exception flattening, cleanup/retry, MaxRecursionDepth interaction, and hot-path behavior. It reported no security findings.

Validation

  • Release multi-target Jint/Jint.csproj build: clean, 0 warnings
  • Release Jint.Benchmark build: clean, 0 warnings
  • full Jint.Tests.PublicInterface: 1,737 passed, 9 configuration-specific skips
  • full Jint.Tests.PublicInterface with host-contract verification: 1,741 passed, 5 inverse-configuration skips
  • full Jint.Tests under UTC: 5,796 passed, 4 existing skips
  • host-verified focused stack/recursion/proxy/module/result/array/concurrency core tests under UTC: 280 passed
  • focused stack-guard suites: 37 passed
  • git diff --check: clean

The mandated NuGet proxy currently mirrors Meziantou.Analyzer through 3.0.141 while the repository pins 3.0.156, and does not mirror Test262Harness.Console 1.1.2. Validation used a temporary local analyzer downgrade that was restored exactly; test262 generation could not run through the mandated feed.

Performance

Default-job BenchmarkDotNet RecursionBenchmark, Apple M4 Pro / .NET 10, compared directly with #3056:

Row #3056 Default guard Delta
Fib 270.931 ms 275.915 ms +1.8%
Tak 8.202 ms 8.419 ms +2.6%
DeepSum 133.758 ms 135.700 ms +1.5%

Allocations were identical in all rows. These recursion-heavy deltas agree with the documented roughly 1.5-3% range; no claim is made below normal run-to-run noise.

@lahma lahma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as the incremental diff against sebros/default-clr-array-copy-498. This is the cleanest of the default flips: a one-property change with the documentation, threat model, and both test suites (default-on pin plus explicit opt-out pin) updated coherently, and the ScriptFunction/StackGuard comments rewritten to match. The cost story is honest — ~1.5–3% on the recursion-heavy rows, shallow calls in the noise, allocations identical — and I agree the old ">1% blocks the default" verdict deserved to be revisited for this particular trade: a couple of percent on deep recursion against silent process termination is the right default for an engine whose README leads with running untrusted script. The opt-out is retained and pinned by AnExplicitOptOutDoesNotProbeAtAll. Approving.

Two small notes: the quoted numbers are a default-job run on an M4 Pro rather than a JINT_BENCH_MODE=gate run on the gating machine — they agree with the previously measured 1.7–2.3% so I don't doubt them, but the re-roll after the #3035 fix is a good moment to attach a gate-mode confirmation. And this is breaking-default number four; it belongs in the same major-release migration story as #3051/#3054/#3056.

@lahma
lahma force-pushed the sebros/default-stack-guard branch from 1eeebe2 to 4957406 Compare August 21, 2026 19:07
@lahma
lahma merged commit 2dac468 into main Aug 21, 2026
5 checks passed
@lahma
lahma deleted the sebros/default-stack-guard branch August 21, 2026 19:59
lahma added a commit to lahma/jint that referenced this pull request Aug 23, 2026
…lly has

Main has moved 181 commits past v4.16.0 and nothing records what an embedder
has to react to. Most of it is invisible to a compiler: every entry below still
compiles exactly as it did in 4.16 and behaves differently at run time, and six
of them flip a default.

`docs/v5-migration.md` is the artefact those entries go in. It is deliberately
not a second README: a table per change, before/after code where it helps, and
the rationale left in the pull request it cites.

Seeded from git history, every claim checked against the tree rather than
against the pull request title:

  * sebastienros#3054 `Interop.AllowWrite` true -> false. A projected CLR write is now
    silently ignored in sloppy mode and a TypeError in strict mode.
  * sebastienros#3056 `Interop.ArrayConversion` LiveView -> Copy. `Array.isArray` flips to
    true, `push`/`length` stop throwing, and CLR-side mutations after the
    crossing stop being visible.
  * sebastienros#3057 `Constraints.StackOverflowGuard` false -> true. RangeError instead of
    a process that ends with no exception in the log.
  * sebastienros#3058 `AgentCanSuspend` true -> false. `Atomics.wait` is a TypeError before
    a waiter is registered; `waitAsync` is untouched.
  * sebastienros#3052 namespace type discovery loses its implicit fallback to
    `Assembly.GetCallingAssembly()` / `GetExecutingAssembly()` /
    `Type.GetType(name)` and becomes the `AllowedAssemblies` allow-list.
  * sebastienros#3051 host exception, module-load and CLR-resolution messages are redacted
    from script; `ExposeDetailedErrors()` restores all three.
  * sebastienros#3035 concurrent `Engine` use fails fast, and an engine stays reserved for
    the lifetime of a returned async Task.
  * sebastienros#3036 `LimitMemory` charges allocations across async continuations, so a
    budget can now trip where one synchronous segment never reached it.
  * sebastienros#3252 every `*Async` entry reports the operation's failures through the
    task and only a usage error out of the call.
  * sebastienros#3248 an array-like `length` above 2^32-1 stops answering differently per
    target framework.
  * sebastienros#3037, sebastienros#3045, sebastienros#3046 add parser, module-graph and result bounds that all
    default to unlimited, so they change nothing until configured; sebastienros#3059 and
    sebastienros#3060 add the diagnostics and the hardened profile on top.

Two entries the prompt for this work had slightly differently, both checked and
written as the tree has them: the stack-overflow guard raises a catchable
`RangeError`, not a `RecursionDepthOverflowException`, and `ArrayOperations` is
internal, so sebastienros#3248 removes no public member — its break is what a script sees.

Sections 2, 3 and 6 (removed API, renamed API, AOT) are explicit empty
placeholders. Nothing public has been removed or renamed since v4.16.0, and a
parallel task is measuring the AOT state.

The target-framework section is written and marked pending: `Jint.csproj` still
lists net462, and the net472 change is not on main yet.

`Jint/AGENTS.md` gains the rule that makes the guide stay current — a change to
anything in its public-contract table is a row in the guide, in the same pull
request, including a change that breaks nothing at compile time. The root
`AGENTS.md` is untouched; it is at 23 KB against a 24 KiB budget.

README's "Branches and releases" described `main` alone and did not mention the
`3.x` branch at all. It now has a row per live branch, in the same wording
sebastienros#3291 gives the 4.x branch's own copy, plus a pointer to the guide.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma added a commit that referenced this pull request Aug 23, 2026
…lly has (#3294)

Main has moved 181 commits past v4.16.0 and nothing records what an embedder
has to react to. Most of it is invisible to a compiler: every entry below still
compiles exactly as it did in 4.16 and behaves differently at run time, and six
of them flip a default.

`docs/v5-migration.md` is the artefact those entries go in. It is deliberately
not a second README: a table per change, before/after code where it helps, and
the rationale left in the pull request it cites.

Seeded from git history, every claim checked against the tree rather than
against the pull request title:

  * #3054 `Interop.AllowWrite` true -> false. A projected CLR write is now
    silently ignored in sloppy mode and a TypeError in strict mode.
  * #3056 `Interop.ArrayConversion` LiveView -> Copy. `Array.isArray` flips to
    true, `push`/`length` stop throwing, and CLR-side mutations after the
    crossing stop being visible.
  * #3057 `Constraints.StackOverflowGuard` false -> true. RangeError instead of
    a process that ends with no exception in the log.
  * #3058 `AgentCanSuspend` true -> false. `Atomics.wait` is a TypeError before
    a waiter is registered; `waitAsync` is untouched.
  * #3052 namespace type discovery loses its implicit fallback to
    `Assembly.GetCallingAssembly()` / `GetExecutingAssembly()` /
    `Type.GetType(name)` and becomes the `AllowedAssemblies` allow-list.
  * #3051 host exception, module-load and CLR-resolution messages are redacted
    from script; `ExposeDetailedErrors()` restores all three.
  * #3035 concurrent `Engine` use fails fast, and an engine stays reserved for
    the lifetime of a returned async Task.
  * #3036 `LimitMemory` charges allocations across async continuations, so a
    budget can now trip where one synchronous segment never reached it.
  * #3252 every `*Async` entry reports the operation's failures through the
    task and only a usage error out of the call.
  * #3248 an array-like `length` above 2^32-1 stops answering differently per
    target framework.
  * #3037, #3045, #3046 add parser, module-graph and result bounds that all
    default to unlimited, so they change nothing until configured; #3059 and
    #3060 add the diagnostics and the hardened profile on top.

Two entries the prompt for this work had slightly differently, both checked and
written as the tree has them: the stack-overflow guard raises a catchable
`RangeError`, not a `RecursionDepthOverflowException`, and `ArrayOperations` is
internal, so #3248 removes no public member — its break is what a script sees.

Sections 2, 3 and 6 (removed API, renamed API, AOT) are explicit empty
placeholders. Nothing public has been removed or renamed since v4.16.0, and a
parallel task is measuring the AOT state.

The target-framework section is written and marked pending: `Jint.csproj` still
lists net462, and the net472 change is not on main yet.

`Jint/AGENTS.md` gains the rule that makes the guide stay current — a change to
anything in its public-contract table is a row in the guide, in the same pull
request, including a change that breaks nothing at compile time. The root
`AGENTS.md` is untouched; it is at 23 KB against a 24 KiB budget.

README's "Branches and releases" described `main` alone and did not mention the
`3.x` branch at all. It now has a row per live branch, in the same wording
#3291 gives the 4.x branch's own copy, plus a pointer to the guide.


Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants