Skip to content

Ask each bound target for its own Symbol.hasInstance in instanceof - #4172

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix/instanceof-bound-chain
Sep 25, 2026
Merged

lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix/instanceof-bound-chain

Conversation

@lahma

@lahma lahma commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Defect

instanceof with a bound function on the right skipped OrdinaryHasInstance step 2, which is ? InstanceofOperator(O, BC) for the bound function's [[BoundTargetFunction]] BC. InstanceofOperator does GetMethod(BC, @@hasInstance) and calls whatever it finds; only when nothing is found does it check IsCallable and run OrdinaryHasInstance(BC, O). BindFunction.OrdinaryHasInstance walked straight to the innermost non-bound target and ran OrdinaryHasInstance on it instead, so:

  • a bound target's own @@hasInstance was never called. That covers a class with a static [Symbol.hasInstance], bound once, and a link in the middle of a chain given one with defineProperty. ({}) instanceof b1.bind(null) where b1 answers true gave false;
  • a proxy target was refused. ({}) instanceof new Proxy(function(){}, {}).bind(null) threw TypeError: Right-hand side of 'instanceof' is not callable where the spec gives false;
  • no link's @@hasInstance lookup could be observed, whether through a getter or a proxy get trap.

test262 has one bound-function case, built-ins/Function/prototype/Symbol.hasInstance/this-val-bound-target.js. It binds a plain function once and never gives the target a method of its own. That is the one shape where the shortcut and the spec agree, which is why the conformance suite never caught this.

Fix

The chain is still walked in a loop, not recursed over. That is what #4165 did for IsConstructor and GetFunctionRealm, and it keeps a 200,000-link chain from ending the process. At each link the loop now does InstanceofOperator step by step:

  • GetMethod(BC, @@hasInstance) runs for every link, in the spec's order.
  • If that finds %Function.prototype[@@hasInstance]% (any realm's), the loop takes the next step itself instead of calling it. That intrinsic only does OrdinaryHasInstance(this, V). FunctionPrototype.IsHasInstanceFunction spots it by identity. It reads the property once and caches it, and the property is non-writable and non-configurable, so the cached value can never go stale.
  • Any other method is called with BC as this, and its result goes through ToBoolean. A native stack probe sits in front of that call, because this is where script can re-enter. On the MaxExecutionStackCount lane, script functions do not probe for themselves, and with the probe removed the new lane test ends the test host with Stack overflow. in BindFunction.OrdinaryHasInstance → JsValue.InstanceofOperator → BindFunction.OrdinaryHasInstance.
  • If nothing is found, IsCallable is checked, and then the loop goes on to the next link, or ends with OrdinaryHasInstance once the target is no longer bound.

The ordinary (non-bound) instanceof path is unchanged: JsValue.InstanceofOperator, FunctionPrototype.HasInstance and JsValue.OrdinaryHasInstance are untouched. Only instanceof against a bound function does more work than before: one GetMethod and one identity check per link, which the spec requires.

A bound CLR TypeReference now answers through its own @@hasInstance, like the unbound one already did. Before, it used the narrower TypeReference.OrdinaryHasInstance.

Tests

Jint.Tests/Runtime/FunctionTests.cs: four new tests. All four fail on unfixed main:

Test Unfixed Fixed
InstanceofThroughABoundFunctionAsksItsTargetForItsOwnHasInstance false,false,false,false,not called true,true,false,true,true,true,1
InstanceofThroughABoundProxyAsksTheProxy JavaScriptException: Right-hand side of 'instanceof' is not callable false,true,true,Symbol.hasInstance|prototype,TypeError
InstanceofThroughABoundChainReadsEveryLinksHasInstanceInOrder 2:true 2,1,0:true
InstanceofWalksADeepBoundChainOverAnotherRealmsFunctionToAMethodAtTheBottom (200,000 links, 1 MB thread) true,false,true true,true,false

The expected strings match Node 24. The cross-realm deep test also covers the intrinsic check. With the check disabled, it fails with RangeError: Maximum call stack size exceeded.

Jint.Tests.PublicInterface/BoundFunctionChainWalkTests.cs covers depth:

  • InstanceofOverADeepChainOfLinksWithTheirOwnHasInstanceRaisesACatchableError, on the default lane and on the MaxExecutionStackCount lane. Unfixed, both return "false" and fail. Fixed, both raise a catchable RangeError: Maximum call stack size exceeded and the engine still evaluates afterwards.
  • InstanceofOverADeepBoundChainAnswers, with the guard on and off. This is a no-regression pin, so it also passes unfixed: 200,000 links answer true,false without recursing.

RunRoutes now takes an Action<Options>, so the lane can be configured.

Verification

Base: upstream/main at b630bf4.

All in Release, on this exact tree:

Suite Total Passed Skipped Failed
Jint.Tests, net10.0 12,708 12,703 5 0
Jint.Tests, net472 8,756 8,750 6 0
Jint.Tests.PublicInterface, net10.0 3,802 3,781 21 0
Jint.Tests.PublicInterface, net472 3,009 2,988 21 0
Jint.Tests.Test262 (full), net10.0 102,740 102,637 103 0

No benchmark was run. The change only reaches instanceof whose right-hand side is a bound function.

🤖 Generated with Claude Code

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, since that intrinsic is only OrdinaryHasInstance(this, V).
Any other method is called, behind a native stack probe, because that
call is where script re-enters, and on the MaxExecutionStackCount lane
script functions do not probe for themselves.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lahma
lahma merged commit 5a75340 into sebastienros:main Sep 25, 2026
9 checks passed
@lahma
lahma deleted the fix/instanceof-bound-chain branch September 25, 2026 18:10
lahma added a commit to lahma/jint that referenced this pull request Sep 26, 2026
…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>
lahma added a commit that referenced this pull request Sep 26, 2026
…stance method (#4184)

* 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, 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 (#4172), so the check moves to
FunctionPrototype where both call it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* Say what bounds the @@hasInstance route on the count lane now

#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>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
lahma added a commit to lahma/jint that referenced this pull request Oct 5, 2026
…@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>
lahma added a commit to lahma/jint that referenced this pull request Oct 5, 2026
…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>
lahma added a commit that referenced this pull request Oct 5, 2026
…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>
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