Skip to content

Fix crash when constructing a mock function that returns a primitive #31094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 2 commits into from
Closed

Fix crash when constructing a mock function that returns a primitive #31094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a25957
Fix crash when constructing a mock function that returns a primitive
robobun May 19, 2026
b393c56
Populate mock.instances on constructor calls
robobun May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 51 additions & 7 deletions src/jsc/bindings/JSMockFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ inline To tryJSDynamicCast(JSC::WriteBarrier<WriteBarrierT>& from)
}

JSC_DECLARE_HOST_FUNCTION(jsMockFunctionCall);
JSC_DECLARE_HOST_FUNCTION(jsMockFunctionConstruct);
JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_protoImpl);
JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_mock);
JSC_DECLARE_HOST_FUNCTION(jsMockFunctionGetter_mockGetLastCall);
Expand Down Expand Up @@ -462,7 +463,7 @@ class JSMockFunction : public JSC::InternalFunction {
}

JSMockFunction(JSC::VM& vm, JSC::Structure* structure, CallbackKind wrapKind)
: Base(vm, structure, jsMockFunctionCall, jsMockFunctionCall)
: Base(vm, structure, jsMockFunctionCall, jsMockFunctionConstruct)
{
initMock();
}
Expand Down Expand Up @@ -826,7 +827,7 @@ static JSValue createMockResult(JSC::VM& vm, Zig::GlobalObject* globalObject, co
return result;
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
static EncodedJSValue jsMockFunctionCallOrConstruct(JSGlobalObject* lexicalGlobalObject, CallFrame* callframe, bool isConstructCall)
{
Zig::GlobalObject* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
auto& vm = JSC::getVM(globalObject);
Expand All @@ -839,6 +840,39 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje

JSC::ArgList args = JSC::ArgList(callframe);
JSValue thisValue = callframe->thisValue();

if (isConstructCall) {
JSValue newTarget = callframe->newTarget();
JSObject* prototype = globalObject->objectPrototype();
if (newTarget && newTarget.isObject()) {
JSValue prototypeValue = asObject(newTarget)->get(globalObject, vm.propertyNames->prototype);
RETURN_IF_EXCEPTION(scope, {});
if (prototypeValue.isObject())
prototype = asObject(prototypeValue);
}
thisValue = JSC::constructEmptyObject(globalObject, prototype);
RETURN_IF_EXCEPTION(scope, {});

if (auto* instances = fn->instances.get()) {
instances->push(globalObject, thisValue);
RETURN_IF_EXCEPTION(scope, {});
} else {
JSC::ObjectInitializationScope object(vm);
instances = JSC::JSArray::tryCreateUninitializedRestricted(
object,
globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous),
1);
instances->initializeIndex(object, 0, thisValue);
fn->instances.set(vm, fn, instances);
}
}
Comment on lines +844 to +855

@claude claude Bot May 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟣 Pre-existing nit (not blocking): fn->instances is still never populated, so fn.mock.instances will remain [] after new fn() even though Jest records the constructed this there. Since this PR now synthesizes the correct thisValue for the construct path, it would be a natural place to also push it onto fn->instances (mirroring how calls/contexts/results are pushed) — but that can also land separately.

Extended reasoning...

What's missing

JSMockFunction declares mutable JSC::WriteBarrier<JSC::JSArray> instances;, clears it in clear(), GC-visits it, lazily initializes an empty array in getInstances(), and exposes it on the mock object at offset 2 as the instances property. However, nothing in jsMockFunctionCallOrConstruct (or anywhere else) ever pushes to it. The function pushes to fn->calls, fn->contexts, fn->invocationCallOrder, and fn->returnValues on every invocation, but fn->instances is dead storage that always reads back as [].

Why it's relevant to this PR

In Jest, mockFn.mock.instances is documented to record the this that was bound for each invocation — for a constructor call, that's the freshly-allocated instance. Before this PR, new fn() on a mock that returned a primitive would assert in debug builds and return a primitive in release builds, so the fact that mock.instances stayed empty was largely unobservable (you couldn't really use mocks as constructors anyway). After this PR, new fn() works correctly and the construct path explicitly allocates the right thisValue via constructEmptyObject(globalObject, prototype) — so users who can now construct mocks may reasonably reach for fn.mock.instances and find it empty.

Step-by-step

  1. const fn = jest.fn(function () { this.x = 1; });
  2. const inst = new fn(); → enters jsMockFunctionConstruct → jsMockFunctionCallOrConstruct(..., true).
  3. isConstructCall is true, so thisValue = constructEmptyObject(globalObject, prototype) — this is the instance.
  4. The function pushes argumentsArray to fn->calls, thisValue to fn->contexts, the invocation id to fn->invocationCallOrder, and the result record to fn->returnValues. fn->instances is never touched.
  5. encodeReturn(returnValue) returns thisValue (the impl returned undefined, a non-object), so inst is the constructed object — correct.
  6. fn.mock.instances → getInstances() → lazy-creates and returns an empty array. Expected (Jest): [inst]. Actual: [].

Why nothing prevents it

There's simply no write site. Grepping the file shows instances only at the declaration, clear(), getInstances(), the visitor, and the mock-object structure setup. The existing test suite only asserts fn.mock.instances is empty after mockClear/mockReset, never that it's populated, so no test catches this.

Impact

Low — it's a Jest-compat gap, not a crash or correctness issue in the PR's stated scope (a Fuzzilli-found assertion fix). Users porting Jest tests that assert on mock.instances (e.g. expect(fn.mock.instances[0]).toBe(inst)) will see failures.

Suggested fix (optional, can be a follow-up)

In jsMockFunctionCallOrConstruct, alongside the contexts push, also push thisValue to fn->instances (Jest pushes this for every call, not just constructs — non-construct calls record whatever this was, and plain fn() records undefined):

JSC::JSArray* instances = fn->instances.get();
if (instances) {
    instances->push(globalObject, thisValue);
    RETURN_IF_EXCEPTION(scope, {});
} else {
    JSC::ObjectInitializationScope object(vm);
    instances = JSC::JSArray::tryCreateUninitializedRestricted(
        object,
        globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous),
        1);
    instances->initializeIndex(object, 0, thisValue);
    fn->instances.set(vm, fn, instances);
}

Severity

Pre-existing / nit. This gap predates the PR entirely — instances was never populated for any kind of call. The PR is a targeted crash fix and shouldn't be blocked on this; it's flagged only because the PR is restructuring exactly the construct path that now has the right value to record.


auto encodeReturn = [&](JSValue value) -> EncodedJSValue {
if (isConstructCall && !value.isObject())
return JSValue::encode(thisValue);
return JSValue::encode(value);
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

JSC::JSArray* argumentsArray = nullptr;
{
JSC::ObjectInitializationScope object(vm);
Expand Down Expand Up @@ -954,22 +988,22 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje
fn->returnValues.set(vm, fn, returnValuesArray);
}

return JSValue::encode(returnValue);
return encodeReturn(returnValue);
}
case JSMockImplementation::Kind::ReturnValue: {
JSValue returnValue = impl->underlyingValue.get();
setReturnValue(createMockResult(vm, globalObject, "return"_s, returnValue));
return JSValue::encode(returnValue);
return encodeReturn(returnValue);
}
case JSMockImplementation::Kind::ReturnThis: {
setReturnValue(createMockResult(vm, globalObject, "return"_s, thisValue));
return JSValue::encode(thisValue);
return encodeReturn(thisValue);
}
case JSMockImplementation::Kind::RejectedValue: {
JSValue rejectedPromise = JSC::JSPromise::rejectedPromise(globalObject, impl->underlyingValue.get());
RETURN_IF_EXCEPTION(scope, {});
setReturnValue(createMockResult(vm, globalObject, "return"_s, rejectedPromise));
return JSValue::encode(rejectedPromise);
return encodeReturn(rejectedPromise);
}
default: {
RELEASE_ASSERT_NOT_REACHED();
Expand All @@ -978,7 +1012,17 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje
}

setReturnValue(createMockResult(vm, globalObject, "return"_s, jsUndefined()));
return JSValue::encode(jsUndefined());
return encodeReturn(jsUndefined());
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
return jsMockFunctionCallOrConstruct(lexicalGlobalObject, callframe, false);
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionConstruct, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
return jsMockFunctionCallOrConstruct(lexicalGlobalObject, callframe, true);
}

void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
Expand Down
63 changes: 63 additions & 0 deletions test/js/bun/test/mock-fn.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,69 @@ describe("mock()", () => {

expect(bar()()).toBe(true);
});

describe("as constructor", () => {
test("returns an object when the implementation returns a primitive", () => {
const fn = jest.fn(function () {
this.x = 1;
return "primitive";
});
expect(fn.call({})).toBe("primitive");
const instance = new fn();
expect(typeof instance).toBe("object");
expect(instance.x).toBe(1);
expect(typeof Reflect.construct(fn, [])).toBe("object");
});

test("records instances and results", () => {
const fn = jest.fn(function () {
this.x = 1;
return "primitive";
});
const instance = new fn();
expect(fn.mock.instances).toHaveLength(1);
expect(fn.mock.instances[0]).toBe(instance);
expect(fn.mock.contexts[0]).toBe(instance);
expect(fn.mock.results[0]).toEqual({ type: "return", value: "primitive" });
});

test("returns the implementation's return value when it is an object", () => {
const obj = { custom: true };
const fn = jest.fn(() => obj);
expect(new fn()).toBe(obj);
expect(Reflect.construct(fn, [])).toBe(obj);
});

test("works with no implementation", () => {
const fn = jest.fn();
expect(typeof new fn()).toBe("object");
expect(typeof Reflect.construct(fn, [])).toBe("object");
});

test("works with mockReturnValue", () => {
const fn = jest.fn().mockReturnValue(42);
expect(fn()).toBe(42);
expect(typeof new fn()).toBe("object");
expect(typeof Reflect.construct(fn, [])).toBe("object");
});

test("sets the prototype from new.target", () => {
class Base {}
const fn = jest.fn(() => undefined);
expect(Reflect.construct(fn, [], Base)).toBeInstanceOf(Base);
});

if (isBun) {
test("no crash when implementation returns a non-object cell", () => {
for (const impl of [Symbol, BigInt, () => "str", () => 1n]) {
const fn = jest.fn(impl);
expect(typeof Reflect.construct(fn, [1])).toBe("object");
expect(typeof new fn(1)).toBe("object");
}
Bun.gc(true);
});
}
});
});

describe("spyOn", () => {
Expand Down
Loading