Repository navigation
Probe the native stack before instanceof calls a target's own @@hasInstance method - #4184
Conversation
Gate: no regression attributable to this changePaired, alternating order, Stage 1:
|
| row | median | 95% CI | sign | verdict |
|---|---|---|---|---|
InstanceofMixed (runs the discriminator on every instanceof) |
+3.63% | [−8.50, +8.78] | 4/6 | no change |
ForInProtoChain |
+3.34% | [+1.88, +6.89] | 6/6 | SLOWER |
| the other 9 rows | −3.19% … +4.99% | every CI includes zero | no change |
ForInProtoChain is for…in plus hasOwnProperty and contains no instanceof, so it cannot execute this change. The subject row's interval was too wide to decide either way, so both went to stage 2 along with a control.
Stage 2: 12 rounds
| row | median | 95% CI | sign | verdict |
|---|---|---|---|---|
InstanceofMixed |
+0.68% | [−1.23, +11.21] | 6/12 | no change |
ForInProtoChain |
+1.94% | [−1.49, +2.76] | 7/12 | no change |
InOperatorMixed (control) |
−0.91% | [−1.68, +0.94] | 4/12 | no change |
InstanceofMixed reads slower in only half the rounds. The median is +0.68%, and the wide upper bound comes from a few outlier rounds rather than a shift. The stage-1 ForInProtoChain flag does not reproduce. The discriminator (an isinst plus a few field loads and a ReferenceEquals on the ordinary path) is below what this box can resolve.
A possible follow-up would repay it outright, and it is deliberately not in this PR: when the handler is the intrinsic, call target.OrdinaryHasInstance(this) directly, which is BindFunction's shape, and skip the dispatcher call and its params array.
🤖 Generated with Claude Code
…stance 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, on either lane. Both ended the process with
a native stack overflow.
The call is now behind EnsureNativeStackHeadroom for every method except
%Function.prototype[@@hasInstance]%, which is what an ordinary function
inherits and so what nearly every instanceof finds; that one is still
called unprobed, since its whole behaviour is OrdinaryHasInstance. This
is the same placement and the same intrinsic check BindFunction's walk
uses for the bound form of the same step (sebastienros#4172), so the check moves to
FunctionPrototype where both call it.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
sebastienros#4187's counted-eval rows noted @@hasInstance as the lane's documented gap. With the operator probing before any method but the intrinsic, that route is a catchable RangeError on this lane too, pinned by the @@hasInstance rows above, so the remark points there instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
a61ea85 to
375db42
Compare
…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>
…ons from killing the host (#4221) * Backport #4172 to 4.x: Ask every bound link for its own @@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> * Backport #4184 to 4.x: Probe the native stack before instanceof 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 (#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 #4187's counted-eval rows, which arrive with the #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> * Backport #4187 to 4.x: Bound eval recursion with a catchable 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. - #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-#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> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
JsValue.InstanceofOperatorstep 3 calls whateverGetMethod(target, @@hasInstance)finds. A method that asksinstanceofof its own target is therefore a recursion that script controls, one native frame per level, with no call expression in it:The operator made that call without probing the native stack and relied on the callee to probe for itself. That does not hold on two routes:
MaxExecutionStackCountlane. A script function does not probe on entry there, so the route above ends the process on net10.0, net8.0 and net472 (Stack overflow./Process is terminated due to StackOverflowException). With the defaultStackOverflowGuardlane it was already a catchableRangeError.evalas the method, on both lanes.Object.defineProperty(C, Symbol.hasInstance, { value: eval })with the source's instanceof C're-enters the operator without entering any function, so even the default lane ended the process.The call is now made behind
EnsureNativeStackHeadroomfor every method except%Function.prototype[@@hasInstance]%. Every ordinary function inherits that one, so nearly everyinstanceoffinds it, and it is still called unprobed: its whole behaviour isOrdinaryHasInstance. This is the same placement and the same intrinsic check that #4172 gaveBindFunction's walk over the bound form of the same step, so the check moves toFunctionPrototype.IsIntrinsicHasInstance, where both call it.Behaviour notes
RangeError: Maximum call stack size exceededand the engine recovers. That covers a class's static method, adefineProperty'd method, a plain-object target,Function.prototype.callas the method, a proxy apply trap, andeval. Ordinaryinstanceofanswers are unchanged.MaxExecutionStackCount = 100000, a legitimate 3000-deep call recursion that runs a non-recursive custom@@hasInstanceat every level used to return 3000, because the lane hops to a fresh thread. It now raisesRangeError, because the probe fires on native-stack exhaustion before the hop. The existing forward probes from Walk the prototype chain in a loop instead of one native frame per link #4078 (trapless proxy) and Ask each bound target for its own Symbol.hasInstance in instanceof #4172 (bound link) already behave this way on the analogous shapes. Plain deep recursion and ordinary deepinstanceofstill return 3000.@@hasInstancegetter that recurses. That is the documented accessor route (StackOverflowGuardTests.UnboundedRecursions, THREAT_MODEL TM-07) and is out of scope here.instanceofgains only the discriminator: oneisinst Function, a few field loads to the realm's cached intrinsic, and aReferenceEquals. There is no probe on it. A paired measurement ofForInGuardBenchmark.InstanceofMixedfollows before merge.Tests
Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs: six new depth rows covering the routes above on both lanes. On the unfixed engine the test host dies on all three TFMs (net10/net8Stack overflow., exit0xC00000FD; net472Process is terminated due to StackOverflowException.). With the fix all 47 rows of the class pass on each TFM.Full suites, Release, whole projects with no
--timeout, 0 failures:test262 was not run locally, because nothing spec-visible changes: the probe fires only on stack exhaustion and the discriminator changes no answer. CI runs it.
Found separately and not in this PR: plain
evalrecursion (var s = 'eval(s)'; eval(s)) also ends the host on the default lane. It is being fixed in its own PR.Follow-up to #4172. Base:
upstream/mainat the time of push.🤖 Generated with Claude Code