Title: [258188] releases/WebKitGTK/webkit-2.28
Revision
258188
Author
[email protected]
Date
2020-03-10 01:28:46 -0700 (Tue, 10 Mar 2020)

Log Message

Merge r258143 - Tail calls are broken on ARM_THUMB2 and MIPS
https://bugs.webkit.org/show_bug.cgi?id=197797

Reviewed by Yusuke Suzuki.

JSTests:

* stress/tail-call-with-spilled-registers.js: Added.

Source/_javascript_Core:

`prepareForTailCall` operation expects that header size + parameters
size is aligned with stack (alignment is 16-bytes for every architecture).
This means that headerSizeInBytes + argumentsIncludingThisInBytes needs
to be multiple of 16. This was not being preserved during getter IC code
for 32-bits. The code generated was taking in account only
headerSizeInRegisters (it is 4 on 32-bits) and argumentsIncludingThis
(that is always 1 for getters) and allocating 32-bytes when applying
operation `(headerSize + argumentsIncludingThis) * 8 - sizeof(CallerFrameAndPC)`.
This results in a stack frame with size of 40 bytes (after we push
`lr` and `sp`). Since `prepareForTailCall` expects frames to be
16-bytes aligned, it will then calculate the top of such frame
considering it is 48 bytes, cloberring values of previous frame and
causing unexpected behavior. This patch is fixing how this IC code
calculates the stack frame using `roundArgumentCountToAlignFrame(numberOfParameters)`
aligning with what we do on code without IC installed.
This was not a problem for getter and setter IC on 64-bits because
`roundArgumentCountToAlignFrame(1) == 1` and `roundArgumentCountToAlignFrame(2) == 3`
while it is `roundArgumentCountToAlignFrame(1) == 2` and
`roundArgumentCountToAlignFrame(2) == 2` for MIPS and ARMv7.

* bytecode/AccessCase.cpp:
(JSC::AccessCase::generateImpl):

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.28/JSTests/ChangeLog (258187 => 258188)


--- releases/WebKitGTK/webkit-2.28/JSTests/ChangeLog	2020-03-10 08:28:41 UTC (rev 258187)
+++ releases/WebKitGTK/webkit-2.28/JSTests/ChangeLog	2020-03-10 08:28:46 UTC (rev 258188)
@@ -1,3 +1,12 @@
+2020-03-09  Caio Lima  <[email protected]>
+
+        Tail calls are broken on ARM_THUMB2 and MIPS
+        https://bugs.webkit.org/show_bug.cgi?id=197797
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/tail-call-with-spilled-registers.js: Added.
+
 2020-02-26  Caio Lima  <[email protected]>
 
         [JSC][MIPS] Adding support to Checkpoints

Added: releases/WebKitGTK/webkit-2.28/JSTests/stress/tail-call-with-spilled-registers.js (0 => 258188)


--- releases/WebKitGTK/webkit-2.28/JSTests/stress/tail-call-with-spilled-registers.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.28/JSTests/stress/tail-call-with-spilled-registers.js	2020-03-10 08:28:46 UTC (rev 258188)
@@ -0,0 +1,50 @@
+//@ requireOptions("--useConcurrentJIT=false")
+
+"use strict";
+
+function assert(a, e) {
+  if (a !== e)
+    throw new Error('Expected: ' + e + ' but got: ' + a);
+}
+noInline(assert);
+
+function c3(v, b, c, d, e) {
+    return v + b + c + d + e;
+}
+noInline(c3);
+
+function c1(o) {
+    let ret = o.c2;
+    if (o.a)
+      assert(o.a, 126);
+    return o;
+}
+noInline(c1);
+
+function getter() {
+    let b = Math.random();
+    let c = Math.random();
+    let d = Math.random();
+    let e = Math.random();
+    return c3('test', b, c, d, e);
+}
+noInline(getter);
+
+let c = [];
+
+c[0] = {a: 126};
+c[0].foo = 0;
+c[0].c2 = 15;
+
+c[1] = {};
+c[1].bar = 99;
+
+c[2] = {};
+Object.defineProperty(c[2], 'c2', { get: getter });
+
+for (let i = 0; i < 10000; i++) {
+    if (numberOfDFGCompiles(c1) > 0)
+        c1(c[2]);
+    else
+        c1(c[i % 2]);
+}

Modified: releases/WebKitGTK/webkit-2.28/Source/_javascript_Core/ChangeLog (258187 => 258188)


--- releases/WebKitGTK/webkit-2.28/Source/_javascript_Core/ChangeLog	2020-03-10 08:28:41 UTC (rev 258187)
+++ releases/WebKitGTK/webkit-2.28/Source/_javascript_Core/ChangeLog	2020-03-10 08:28:46 UTC (rev 258188)
@@ -1,3 +1,33 @@
+2020-03-09  Caio Lima  <[email protected]>
+
+        Tail calls are broken on ARM_THUMB2 and MIPS
+        https://bugs.webkit.org/show_bug.cgi?id=197797
+
+        Reviewed by Yusuke Suzuki.
+
+        `prepareForTailCall` operation expects that header size + parameters
+        size is aligned with stack (alignment is 16-bytes for every architecture).
+        This means that headerSizeInBytes + argumentsIncludingThisInBytes needs
+        to be multiple of 16. This was not being preserved during getter IC code
+        for 32-bits. The code generated was taking in account only
+        headerSizeInRegisters (it is 4 on 32-bits) and argumentsIncludingThis
+        (that is always 1 for getters) and allocating 32-bytes when applying
+        operation `(headerSize + argumentsIncludingThis) * 8 - sizeof(CallerFrameAndPC)`.
+        This results in a stack frame with size of 40 bytes (after we push
+        `lr` and `sp`). Since `prepareForTailCall` expects frames to be
+        16-bytes aligned, it will then calculate the top of such frame
+        considering it is 48 bytes, cloberring values of previous frame and
+        causing unexpected behavior. This patch is fixing how this IC code
+        calculates the stack frame using `roundArgumentCountToAlignFrame(numberOfParameters)`
+        aligning with what we do on code without IC installed.
+        This was not a problem for getter and setter IC on 64-bits because
+        `roundArgumentCountToAlignFrame(1) == 1` and `roundArgumentCountToAlignFrame(2) == 3`
+        while it is `roundArgumentCountToAlignFrame(1) == 2` and
+        `roundArgumentCountToAlignFrame(2) == 2` for MIPS and ARMv7.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::generateImpl):
+
 2020-03-05  Paulo Matos  <[email protected]>
 
         [JSCOnly] 32-bits warning on memset of JSValue

Modified: releases/WebKitGTK/webkit-2.28/Source/_javascript_Core/bytecode/AccessCase.cpp (258187 => 258188)


--- releases/WebKitGTK/webkit-2.28/Source/_javascript_Core/bytecode/AccessCase.cpp	2020-03-10 08:28:41 UTC (rev 258187)
+++ releases/WebKitGTK/webkit-2.28/Source/_javascript_Core/bytecode/AccessCase.cpp	2020-03-10 08:28:46 UTC (rev 258188)
@@ -1581,7 +1581,8 @@
             CCallHelpers::Jump returnUndefined = jit.branchTestPtr(
                 CCallHelpers::Zero, loadedValueGPR);
 
-            unsigned numberOfRegsForCall = CallFrame::headerSizeInRegisters + numberOfParameters;
+            unsigned numberOfRegsForCall = CallFrame::headerSizeInRegisters + roundArgumentCountToAlignFrame(numberOfParameters);
+            ASSERT(!(numberOfRegsForCall % stackAlignmentRegisters()));
             unsigned numberOfBytesForCall = numberOfRegsForCall * sizeof(Register) - sizeof(CallerFrameAndPC);
 
             unsigned alignedNumberOfBytesForCall =
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to