commit 04830ff9bcb739527f06a866510daddf3e5046fe
parent 1333863e9bb7e713105a9f9106089f1715bf66df
Author: Iain Ireland <iireland@mozilla.com>
Date: Wed, 15 Oct 2025 17:54:48 +0000
Bug 1993404: Adjust for arguments underflow in InvokeFunction r=spidermonkey-reviewers,jandem
Prior to bug 1991223, we only called the arguments rectifier in `CodeGenerator::emitApplyGeneric` if we were definitely calling JIT code. The slow path (a VM call to InvokeFunction) would never pad out missing arguments. After the patch, we push the missing arguments earlier. We do [check](https://searchfox.org/firefox-main/rev/b30a9b734819436853abf4eba6d768037514b3f6/js/src/jit/CodeGenerator.cpp#7120-7129) to make sure that we're calling a function with a JIT entry, but there are a [few more checks](https://searchfox.org/firefox-main/rev/b30a9b734819436853abf4eba6d768037514b3f6/js/src/jit/CodeGenerator.cpp#7473-7486) in emitApplyGeneric that might cause us to take the slow path, and those checks aren't duplicated. In particular, if newTarget is a proxy object, then CreateThisFromIon will return null, forcing us down the slow path. InvokeFunction expects newTarget at argv[argc], but since we pushed extra undefined values, it's actually at argv[callee->nargs()].
This patch updates InvokeFunction to check for this case. The alternative would be to pass `this` in to emitAllocateSpaceForConstructAndPushNewTarget and add a check [here](https://searchfox.org/firefox-main/rev/b30a9b734819436853abf4eba6d768037514b3f6/js/src/jit/CodeGenerator.cpp#7120-7129) to see if `this` is null. (We don't need to check whether the callee is a constructor because InvokeFunction already does that before looking at newTarget. We don't need to handle this problem in non-constructing contexts because then we won't be passing newTarget.) I went with the current approach instead because a) it moves the extra check into the slow path, and b) it will continue to work if we add new branches to the slow path in the future, without needing us to update emitAllocateSpaceForConstructAndPushNewTarget.
I believe we could technically remove the branchIfFunctionHasJitEntry check in emitAllocateSpaceForConstructAndPushNewTarget, but that lets us avoid doing extra work when calling non-scripted constructors, so I think it is still pulling its own weight. The cases that we're fixing here are significantly less common.
Differential Revision: https://phabricator.services.mozilla.com/D268633
Diffstat:
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/js/src/jit-test/tests/bug1993404.js b/js/src/jit-test/tests/bug1993404.js
@@ -0,0 +1,18 @@
+class C {
+ constructor(a,b,c) {}
+}
+class D extends C {
+ constructor(x) {
+ super(...x);
+ }
+}
+let P = new Proxy(D, {});
+
+function foo() {
+ return new P([1,2]);
+}
+
+with ({}) {}
+for (var i = 0; i < 2000; i++) {
+ foo();
+}
diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp
@@ -507,6 +507,25 @@ bool InvokeFunction(JSContext* cx, HandleObject obj, bool constructing,
RootedValue newTarget(cx, argvWithoutThis[argc]);
+ // The JIT ABI expects at least callee->nargs() arguments, with undefined
+ // values passed for missing formal arguments. These undefined values are
+ // passed before newTarget. We don't normally insert undefined values when
+ // calling native functions like this one, but by detecting and supporting
+ // that case here, it is easier for jit code to fall back to InvokeFunction
+ // as a slow path.
+ if (newTarget.isUndefined()) {
+ MOZ_RELEASE_ASSERT(obj->is<JSFunction>());
+ JSFunction* callee = &obj->as<JSFunction>();
+#ifdef DEBUG
+ MOZ_ASSERT(callee->nargs() > argc);
+ for (uint32_t i = argc; i < callee->nargs(); i++) {
+ MOZ_ASSERT(argvWithoutThis[i].isUndefined());
+ }
+#endif
+ newTarget = argvWithoutThis[callee->nargs()];
+ MOZ_ASSERT(newTarget.isObject());
+ }
+
// See CreateThisFromIon for why this can be NullValue.
if (thisv.isNull()) {
thisv.setMagic(JS_IS_CONSTRUCTING);
