Title: [258395] releases/WebKitGTK/webkit-2.26
Revision
258395
Author
ape...@igalia.com
Date
2020-03-13 07:51:35 -0700 (Fri, 13 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.26/JSTests/ChangeLog (258394 => 258395)


--- releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog	2020-03-13 12:01:23 UTC (rev 258394)
+++ releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog	2020-03-13 14:51:35 UTC (rev 258395)
@@ -1,3 +1,12 @@
+2020-03-09  Caio Lima  <ticaiol...@gmail.com>
+
+        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.
+
 2019-10-01  Saam Barati  <sbar...@apple.com>
 
         ObjectAllocationSinkingPhase shouldn't insert hints for allocations which are no longer valid

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


--- releases/WebKitGTK/webkit-2.26/JSTests/stress/tail-call-with-spilled-registers.js	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.26/JSTests/stress/tail-call-with-spilled-registers.js	2020-03-13 14:51:35 UTC (rev 258395)
@@ -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.26/Source/_javascript_Core/ChangeLog (258394 => 258395)


--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog	2020-03-13 12:01:23 UTC (rev 258394)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog	2020-03-13 14:51:35 UTC (rev 258395)
@@ -1,3 +1,33 @@
+2020-03-09  Caio Lima  <ticaiol...@gmail.com>
+
+        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-01-22  Adrian Perez de Castro  <ape...@igalia.com>
 
         Unreviewed. Fix non-unified build

Modified: releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/bytecode/AccessCase.cpp (258394 => 258395)


--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/bytecode/AccessCase.cpp	2020-03-13 12:01:23 UTC (rev 258394)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/bytecode/AccessCase.cpp	2020-03-13 14:51:35 UTC (rev 258395)
@@ -890,7 +890,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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to