Skip to content

Backport #4172, #4184, #4187 to 4.x: stop instanceof and eval recursions from killing the host - #4221

Merged
lahma merged 3 commits into
sebastienros:4.xfrom
lahma:backport/4x-stackguard
Oct 5, 2026
Merged

lahma merged 3 commits into
sebastienros:4.xfrom
lahma:backport/4x-stackguard

Conversation

@lahma

@lahma lahma commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

This backports three fixes from main to 4.x. Each one stops a recursion that script controls from ending the host process, or from running on new threads forever. Each commit is a cherry-pick of the main PR, adapted to 4.x. No public API changes and no default changes. The public API Verify snapshots are byte-identical.

4.x commit main PR (commit) What it fixes
7b0649bc5 #4172 (5a7534069) instanceof over a bound function now asks every bound link for its own @@hasInstance, as OrdinaryHasInstance step 2 requires. Before, it jumped to the innermost target, so a bound class's static [Symbol.hasInstance] was never called, no lookup was observable, and a bound proxy target was refused as "not callable". The walk is still a loop. A link that inherits %Function.prototype[@@hasInstance]% is one step of the loop. Any other method is called behind a native stack probe.
df8395454 #4184 (e880b428a) JsValue.InstanceofOperator probes the native stack before it calls a target's own @@hasInstance, unless that method is the intrinsic. Without the probe, class C { static [Symbol.hasInstance](v) { return v instanceof C; } }, Function.prototype.call used as the method, or eval used as the method ended the process.
49ccd5e23 #4187 (553cc7c45) PerformEval probes before it parses. On the MaxExecutionStackCount lane, a direct eval(…) or eval?.(…) now counts as a call. Before, var s = 'eval(s)'; eval(s) and its indirect, optional-call and callback forms ended the process. On the count lane, the direct and optional forms hopped to a new thread at every exhaustion and never threw.

Adapted for 4.x

How the process-death tests are isolated

Main's tests run each body on a small dedicated thread, and this PR mirrors that. That thread only makes the overflow quick and deterministic: a native stack overflow still kills the whole test host. To keep one dying row from hiding the others, the "unfixed" evidence below runs each theory row in its own process. It uses -preEnumerateTheories -id <row id> against the test executable, and records the exit code and the repeated top frame. A regression in the committed suite would still show as a dead test host (TestPipelineException), the same as on main.

Evidence (Windows x64, Release)

Unfixed means: the new tests, run against the engine files as they were before the commit that ports each fix. The earlier backports in this PR are applied.

Test net10.0 unfixed net472 unfixed fixed (both TFMs)
#4172 FunctionTests.Instanceof* (4 new) 4 fail: own @@hasInstance not called ("false"…), link reads "2,1,0" not observed, bound proxy refused, cross-realm bottom method skipped same 4 fail pass (FunctionTests 76/76)
#4172 BoundFunctionChainWalkTests (4 new rows) 2 fail ("false" where RangeError expected, both lanes); the 2 deep-chain Answers rows pass (they pin that the walk stays a loop) same 7/7
#4184 AHasInstanceMethodThatAsksInstanceofOfItsOwnTarget… (6 rows) 4 kill the host (class static, count lane; built-in, count lane; eval, both lanes). Stack overflow. with JsValue.InstanceofOperator → ScriptFunction.CallOnce repeated ×916. 2 pass (guard-only lane, where the callee probes) same 4 kill the host (Process is terminated due to StackOverflowException.) 6/6
#4187 ARecursionThroughEvaluatedSource… (7 rows) 4 kill the host (direct, indirect, optional-call, callback; exit 0xC00000FD, JintCallExpression.HandleEval ×531 for direct). 3 pass (eval as @@hasInstance, already bounded by #4184; both Function constructor rows, never broken) same 4 kill the host 7/7
#4187 OnTheExecutionStackCountLane…StopsAtTheCount (10 rows) 4 fail at the 2-minute wedge ceiling, "the lane is hopping threads without bound" (direct and optional-call, guard on and off). 6 pass same 4 fail 10/10
#4187 OnTheExecutionStackCountLaneAFiniteRecursionThroughEval… (2 rows) 2 pass (they pin that the fix did not turn the lane's hop into a throw) 2 pass 2/2

With all three commits, HostNativeRecursionGuardTests, BoundFunctionChainWalkTests, HostStackOverflowGuardTests and CallStackTests together pass 82/82 on both net10.0 and net472.

Skipped

None. All three source PRs apply to 4.x.

Test totals (all three commits, dotnet test -c Release, no --timeout)

Project net10.0 net472
Jint.Tests 7659 passed, 4 skipped, 0 failed 7574 passed, 4 skipped, 0 failed
Jint.Tests.PublicInterface 1932 passed, 9 skipped, 0 failed 1924 passed, 9 skipped, 0 failed
Jint.Tests.Test262 (full suite) 102509 passed, 175 skipped, 0 failed n/a (net10.0 only)

🤖 Generated with Claude Code

lahma and others added 3 commits October 5, 2026 12:39
…@hasInstance in instanceof

OrdinaryHasInstance step 2 answers a bound function with
? InstanceofOperator(O, BC) for its [[BoundTargetFunction]], which does
GetMethod(BC, @@hasInstance) and calls whatever it finds before falling
back to OrdinaryHasInstance. BindFunction instead walked straight to the
innermost non-bound target and ran OrdinaryHasInstance on it, so a bound
target's own @@hasInstance (a class's static method, a link given one with
defineProperty) was never called, no link's lookup was observable, and a
proxy target was refused as "not callable".

The walk stays a loop, so a deep chain still answers: a link whose
GetMethod finds %Function.prototype[@@hasInstance]% (any realm's) is a
step of the loop. Any other method is called behind a native stack probe.

Adapted for 4.x:
- BindFunction derives from ObjectInstance here and 4.x spells [[Call]]
  presence IsCallable rather than HasCall; the loop uses IsCallable.
- StackOverflowGuard is opt-in on 4.x and the probe is gated on it like
  every other native-stack probe, so the new MaxExecutionStackCount row
  asks for the guard too (main's default has it on).
- bind writes "bound " + name eagerly on 4.x, so the new 200,000-link
  chains delete each link's own name, as the existing BoundChain does.
- Tests transcribed from NUnit to xUnit v3.

(cherry picked from commit 5a75340)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…anceof calls a target's own @@hasInstance method

InstanceofOperator step 3 calls whatever GetMethod(target, @@hasInstance)
finds, so a method that asks instanceof of its own target - a class's
static [Symbol.hasInstance](v) { return v instanceof C; } - is a
recursion script controls, one native frame per level, with no call
expression in it. JsValue.InstanceofOperator made that call with no probe
and relied on the callee to probe for itself, which does not hold on two
routes: on the MaxExecutionStackCount lane a script function does not
probe on entry, and eval as the method re-enters the operator without
entering any function at all. Both ended the process with a native stack
overflow.

The call is now behind EnsureNativeStackHeadroom for every method except
%Function.prototype[@@hasInstance]%, the same placement and intrinsic
check BindFunction's walk uses (sebastienros#4172), so the check moves to
FunctionPrototype where both call it.

Adapted for 4.x:
- StackOverflowGuard is opt-in on 4.x and EnsureNativeStackHeadroom is
  gated on it, so the fix bounds engines that ask for the guard (alone or
  with MaxExecutionStackCount); a default 4.x engine is unchanged. The
  InstanceofOperator remarks say so, and every test engine sets the guard,
  the MaxExecutionStackCount rows included.
- Main's second commit only edits a remark on sebastienros#4187's counted-eval rows,
  which arrive with the sebastienros#4187 backport after this one; that remark is
  taken there in its final form.
- Tests transcribed from NUnit to xUnit v3.

(cherry picked from commit e880b42)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ble RangeError instead of a native stack overflow or an endless thread hop

Evaluating eval's source re-enters the interpreter without entering a
function, so no function-entry probe saw it: `var s = 'eval(s)'; eval(s)`,
`(0, eval)(s)`, `eval?.(s)` and `[s].forEach(eval)` ended the host with a
native stack overflow on net10.0 and net472 even with StackOverflowGuard on.

PerformEval now probes before it parses, gated like the ScriptFunction
entries (off on the MaxExecutionStackCount lane). On that lane a direct
eval and eval?.() were the one call a call expression dispatches without a
call-stack frame, so a recursion of them never grew the count and hopped to
a fresh thread at every exhaustion without ever throwing; they now count as
the call they are, through a counter rather than a frame because a frame
is observable in error.stack, the debugger and MaxRecursionDepth.

Adapted for 4.x:
- StackOverflowGuard is opt-in on 4.x, so the PerformEval probe bounds
  engines that ask for it; a default 4.x engine is unchanged. The
  StackOverflowGuard summary keeps 4.x's wording and default ("false") and
  gains only main's "or into eval source".
- The direct-eval count on the MaxExecutionStackCount lane does not depend
  on the guard, so the counted rows run with the guard on (main's default
  configuration) and off (4.x's default).
- Jint/Constraints/AGENTS.md does not exist on 4.x; that hunk is dropped.
- sebastienros#4184 landed first here, so the eval-as-@@hasInstance row is already
  bounded by the operator's probe, and the counted rows' remark is taken in
  its post-sebastienros#4184 form.
- Tests transcribed from NUnit to xUnit v3; TestBudgets.WedgeCeiling is a
  local two-minute constant, as in HostModuleGraphDepthTests.

(cherry picked from commit 553cc7c)

Co-Authored-By: Claude Opus 5.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.

1 participant