tor-browser

commit 08292d61d3173d205f5dbad8e44e2a242b525d22
parent cd3ccc448017c1a69d18ba28c09ef7ce54c138a6
Author: Iain Ireland <iireland@mozilla.com>
Date:   Tue, 28 Oct 2025 16:28:29 +0000

Bug 1995939: Don't clobber nargs before pushing undefined arguments r=jandem

Technically the change in emitAllocateSpaceForApply isn't necessary, because we'll end up storing `undefined` in the padding we've just allocated. However, loadFunctionArgCount is a load (guaranteed to hit in L1) and a shift, and we'd be doing a shift anyway. One L1 load is probably cheaper than an extra iteration of the undefined loop.

Differential Revision: https://phabricator.services.mozilla.com/D270058

Diffstat:
A
js/src/jit-test/tests/basic/bug1995939-1.js
 | 
16
++++++++++++++++
A
js/src/jit-test/tests/basic/bug1995939-2.js
 | 
9
+++++++++
M
js/src/jit/CodeGenerator.cpp
 | 
20
++++++++++++++++++--

3 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/js/src/jit-test/tests/basic/bug1995939-1.js b/js/src/jit-test/tests/basic/bug1995939-1.js
@@ -0,0 +1,16 @@
+class C {
+  constructor(a,b) { this.b = b; }
+}
+class C2 {
+  constructor(a,b) { this.b = b; }
+}
+
+var flip = false;
+function foo() {
+  let c = flip ? C : C2;
+  flip = !flip;
+  return new c(...arguments);
+}
+for (var i = 0; i < 2000; i++) {
+  assertEq(foo().b, undefined);
+}
diff --git a/js/src/jit-test/tests/basic/bug1995939-2.js b/js/src/jit-test/tests/basic/bug1995939-2.js
@@ -0,0 +1,9 @@
+// |jit-test| --trial-inlining-warmup-threshold=10; --ion-offthread-compile=off
+
+d = function(a,b) {}
+class e extends d {}
+f = function() {}.bind()
+for (i=0; i<2000; ++i) {
+  g = f
+  Reflect.construct(e, [], g)
+}
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
@@ -6986,7 +6986,15 @@ void CodeGenerator::emitAllocateSpaceForApply(T* apply, Register calleeReg,
         }
         masm.lshiftPtr(Imm32(ValueShift), scratch);
         masm.subFromStackPtr(scratch);
-        masm.rshiftPtr(Imm32(ValueShift), scratch);
+
+        // We need callee->nargs in `scratch`. If we rounded it up
+        // above, we need to reload it. If we only shifted it, we can
+        // simply shift it back.
+        if (JitStackValueAlignment > 1) {
+          masm.loadFunctionArgCount(calleeReg, scratch);
+        } else {
+          masm.rshiftPtr(Imm32(ValueShift), scratch);
+        }
       }
 
       // Count from callee->nargs() down to argc, storing undefined values.
@@ -7104,7 +7112,15 @@ void CodeGenerator::emitAllocateSpaceForConstructAndPushNewTarget(
         }
         masm.lshiftPtr(Imm32(ValueShift), scratch);
         masm.subFromStackPtr(scratch);
-        masm.rshiftPtr(Imm32(ValueShift), scratch);
+
+        // We need callee->nargs in `scratch`. If we rounded it down
+        // above, we need to reload it. If we only shifted it, we can
+        // simply shift it back.
+        if (JitStackValueAlignment > 1) {
+          masm.loadFunctionArgCount(calleeReg, scratch);
+        } else {
+          masm.rshiftPtr(Imm32(ValueShift), scratch);
+        }
       }
 
       // Count from callee->nargs() down to argc, storing undefined values.