Attachment 370650 Details for Bug 197797 – WIP - Patch

[patch] WIP - Patch

bug-197797-20190526201328.patch (text/plain), 3.75 KB, created by Caio Lima on 2019-05-26 11:13:30 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Caio Lima

Created: 2019-05-26 11:13:30 PDT

Size: 3.75 KB

patch

obsolete

>Subversion Revision: 245779
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index df5adc4cc725fa461b2ac8974f772558f11a5455..71241dee6fb0eaca005408b1ccb80110c16e1124 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-05-26  Caio Lima  <ticaiolima@gmail.com>
>+
>+        Tail calls are broken on ARM_THUMB2 and MIPS
>+        https://bugs.webkit.org/show_bug.cgi?id=197797
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        JSC's stack alignment was 16-bytes for every supported architecture.
>+        However, this is not preserved by function's prologue on ARMv7 and MIPS,
>+        because frame pointer and `lr` are pushed into stack during this operation,
>+        resulting in a frame that is not aligned.
>+        This is a problem when we are calculating top of the stack on tail calls,
>+        because it considers that frames are stack-aligned.
>+        Such disaligment may corrupt stack values when a tail call happens
>+        inside a getter execution.
>+        This Patch is changing the alignment of ARMv7 and MIPS to 8 bytes to
>+        avoid stack corruption.
>+
>+        * runtime/StackAlignment.h:
>+        (JSC::stackAlignmentBytes):
>+
> 2019-05-25  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         JITOperations getByVal should mark negative array indices as out-of-bounds
>diff --git a/Source/JavaScriptCore/runtime/StackAlignment.h b/Source/JavaScriptCore/runtime/StackAlignment.h
>index 51025c0173537f94f1a13241af79f72b402b0150..85af19baedd267f376ce6de3d0b18b67b179812b 100644
>--- a/Source/JavaScriptCore/runtime/StackAlignment.h
>+++ b/Source/JavaScriptCore/runtime/StackAlignment.h
>@@ -32,7 +32,14 @@
> namespace JSC {
> 
> // NB. Different platforms may have different requirements here. But 16 bytes is very common.
>-constexpr unsigned stackAlignmentBytes() { return 16; }
>+constexpr unsigned stackAlignmentBytes()
>+{
>+#if CPU(ARM_THUMB2) || CPU(MIPS)
>+    return 8;
>+#else
>+    return 16;
>+#endif
>+}
> 
> constexpr unsigned stackAlignmentRegisters()
> {
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 96d083c93a85dfc81c589526c3a439dbd8bd881a..d99d29e0fe269449d606c289c6f3927192cabc58 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+019-05-26  Caio Lima  <ticaiolima@gmail.com>
>+
>+        Tail calls are broken on ARM_THUMB2 and MIPS
>+        https://bugs.webkit.org/show_bug.cgi?id=197797
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/tail-call-with-spilled-registers.js: Added.
>+
> 2019-05-25  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         JITOperations getByVal should mark negative array indices as out-of-bounds
>diff --git a/JSTests/stress/tail-call-with-spilled-registers.js b/JSTests/stress/tail-call-with-spilled-registers.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..03fb71cababdea027249e1cf7d4859f2a99f0327
>--- /dev/null
>+++ b/JSTests/stress/tail-call-with-spilled-registers.js
>@@ -0,0 +1,51 @@
>+//@ run("--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]);
>+}
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 197797: 369606 | 369650 | 369895 | 369981 | 370019 | 370021 | 370650 | 370651 | 370661 | 370784 | 370785 | 371567 | 371580 | 392300 | 393034