Make mock functions return an object when used as a constructor

[robobun]

Fuzzilli found this crash:

ASSERTION FAILED: cell->isObjectSlow()
JavaScriptCore/JSObject.h(1051) : JSObject *JSC::asObject(JSCell *)

Minimal reproduction:

const { spyOn } = Bun.jest();
Date.foo = "hello";
const spy = spyOn(Date, "foo");
Reflect.construct(spy, []); // boom

What was wrong

JSMockFunction is a JSC::InternalFunction, and it passed jsMockFunctionCall as both its call function and its construct function:

: Base(vm, structure, jsMockFunctionCall, jsMockFunctionCall)

jsMockFunctionCall returns whatever the mock implementation returns, which is very often not an object (undefined for a fresh mock(), the spied-on value for spyOn, whatever mockReturnValue was given).

A native [[Construct]] must always return an object. JSC::Interpreter::executeConstruct (reached from Reflect.construct, JSC::construct(), proxy construct traps) ends with:

return asObject(JSValue::decode(result));

There is no type check there, so a primitive return was an assertion failure in debug and a non-object cell reinterpreted as a JSObject* in release. On the released build you could see it directly:

typeof Reflect.construct(Bun.jest().mock(() => "a string"), []); // "string"

The bytecode new path did not assert, but it was wrong too: new mock() returned the primitive instead of the new instance, so new (mock(() => 42))() evaluated to 42.

The fix

JSMockFunction gets its own construct function. It derives this from new.target (via InternalFunction::createSubclassStructure, the same helper JSC's own constructors use), runs the mock with that as the receiver, and returns the implementation's result only when it is an object. Otherwise it returns the newly created object.

That is what an ordinary JS constructor does, and it matches jest, whose mocks are plain JS functions:

expression	before	after
Reflect.construct(mock(), [])	assert / UB	new object
new (mock())()	undefined	new object
new (mock(() => 42))()	42	new object
new (mock(() => ({x:1})))()	{x:1}	{x:1}
new (mock(function(){this.x=1}))()	undefined	{x:1}

mock.contexts now records the constructed instance for new calls rather than new.target, which is also what jest records.

I checked the other InternalFunction subclasses in the tree for the same "call function doubles as the construct function" shape. JSBufferListConstructor and JSStringDecoderConstructor do reuse one function for both, but theirs always return an object, and v8::shim::ObjectTemplate's is ASSERT_NOT_REACHED. None of the separate constructX host functions return a primitive, so this was the only instance.

Tests

Added a used as a constructor block to test/js/bun/test/mock-fn.test.js, covering every primitive an implementation can return (including undefined, symbols and bigints, which are the cells that tripped the original assertion), the object passthrough case, this binding, mockReturnValue, and Reflect.construct with an explicit newTarget. That file is written to run under jest and vitest too, so the assertions are the cross-runtime behavior.

10 of the 11 new assertions fail on the current release build and all pass with this change. The original Fuzzilli sample now exits cleanly, and a 2000-iteration construct + Bun.gc(true) stress loop runs clean under the debug ASAN build.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8aabf85c-d486-4c98-8036-3add8099059d

📥 Commits

Reviewing files that changed from the base of the PR and between 1498d7b and 3ff97f5.

📒 Files selected for processing (2)

• src/jsc/bindings/JSMockFunction.cpp
• test/js/bun/test/mock-fn.test.js

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 12:31 AM PT - Jul 3rd, 2026}

@robobun, your commit 3ff97f5 is building: #68179

[github-actions]

No related open issues found.

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. fix(bun:test): return an object when a mock function is constructed with new #31386 - Fixes the exact same bug (mock function [[Construct]] returning non-objects) with the same approach: a dedicated jsMockFunctionConstruct handler and mock.contexts updates
2. Fix non-object returns from native construct handlers #31955 - Superset fix that includes the identical JSMockFunction construct handler alongside fixes for other native construct handlers (JSFFIFunction, node:buffer)

🤖 Generated with Claude Code

[robobun]

Confirming the bot above: there is no linked issue. This came out of the Fuzzilli crash corpus rather than a bug report, so there is no issue number to reference. That is also why the regression test went into test/js/bun/test/mock-fn.test.js next to the rest of the mock coverage instead of test/regression/issue/.

One scope boundary worth calling out, since a reviewer will probably wonder: mock functions still have no own .prototype, so new mockFn() instanceof mockFn is false and class C extends mockFn {} still throws. Those are separate jest-compat gaps that predate this change and are not what crashed. This PR only fixes the [[Construct]] contract violation (a native construct function must return an object), and the newly created object gets Object.prototype when new.target has no object .prototype, which is the same thing JSC's own constructors do.

[robobun]

The bot is right, and #31955 is the one that should land. Closing this as a duplicate.

#31955 fixes the same JSMockFunction [[Construct]] bug with effectively the same patch, and it also covers two more instances of the bug class that this PR missed. I checked both against current main and they are still broken today:

$ bun-debug -e 'const {isUtf8} = require("node:buffer"); Reflect.construct(isUtf8, [new Uint8Array([1])]);'
ASSERTION FAILED: isCell()
JavaScriptCore/JSCJSValue.h(1043) : JSCell *JSC::JSValue::asCell() const

$ bun-debug /tmp/ffi.js   # Reflect.construct(ccCompiledFn, [1, 2])
call: 3
ASSERTION FAILED: isCell()
JavaScriptCore/JSCJSValue.h(1043) : JSCell *JSC::JSValue::asCell() const

Same root cause, different spelling: JSFFIFunction::create passes the FFI call function as its nativeConstructor to vm.getHostFunction, and NodeBufferModule passes isUtf8 / isAscii / resolveObjectURL as their own nativeConstructor to JSFunction::create. Both then return a primitive out of a native construct handler, which is the same contract violation.

Worth being straight about why I missed them: when I swept for sibling sites I only grepped the Base(vm, structure, call, construct) constructor shape, which catches InternalFunction subclasses and nothing else. The FFI and buffer cases go through vm.getHostFunction(...) and JSFunction::create(..., nativeConstructor), so they never showed up. The "this was the only instance" line in the PR description above is wrong, and #31955 has the correct picture.

#31955's tests are broader than this PR's too (it also covers a throwing implementation, constructing a spy, and mock.instances). #31386 is an earlier, narrower version of the same fix, so #31955 subsumes that one as well. Both are currently MERGEABLE and only blocked on review.

Nothing here is worth salvaging over #31955. Closing.

[claude]

No bugs found and the fix looks correct, but this is native JSC [[Construct]] code with a small observable behavior change to mock.contexts, so worth a maintainer's eyes.

Extended reasoning...

Overview

This PR fixes a Fuzzilli-found assertion failure / release-build UB where JSMockFunction reused its call host function as its construct host function. Since a native [[Construct]] must return an object and the call path returns whatever the mock implementation returns (often a primitive), Reflect.construct(mock, []) hit asObject() on a non-object. The fix adds a dedicated jsMockFunctionConstruct that allocates the receiver via InternalFunction::createSubclassStructure, runs the mock with it as this, and returns the implementation's result only when it is an object — standard JS constructor semantics. The old body is factored into invokeMockFunction(..., thisValue) so the call path is byte-identical modulo thisValue sourcing. Comprehensive tests are added covering every primitive return type, object passthrough, this binding, mockReturnValue, and Reflect.construct with an explicit newTarget.

Security risks

None. This is test-runner-only surface (jest.fn / mock()), not reachable from untrusted network input. The change removes a reachable UB path rather than adding one.

Level of scrutiny

Moderate. The diff is small (~25 net new C++ lines) and follows the established createSubclassStructure + RETURN_IF_EXCEPTION pattern used elsewhere in src/jsc/bindings/. However, it is native JSC bindings code where GC and exception-scope mistakes are memory-safety bugs, and it intentionally changes observable behavior: mock.contexts now records the constructed instance for new calls instead of new.target. That aligns with jest, but it is still a user-visible semantic change to a public API that a maintainer should sign off on.

Other factors

The exception handling looks correct (RETURN_IF_EXCEPTION after createSubclassStructure and after invokeMockFunction), asObject(callframe->newTarget()) is safe since newTarget is always an object in a construct call, and result.isObject() correctly excludes null. The PR author already audited other InternalFunction subclasses for the same bug shape. Test coverage is thorough and the file is designed to run under jest/vitest too, which cross-checks the semantics. I'm deferring only because native [[Construct]] implementations plus a behavior change warrant a human maintainer's confirmation, not because I found anything wrong.