Attachment 359220 Details for Bug 193438 – Patch

[patch] Patch

bug-193438-20190115162627.patch (text/plain), 5.42 KB, created by Yusuke Suzuki on 2019-01-15 16:26:28 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-01-15 16:26:28 PST

Size: 5.42 KB

patch

obsolete

>Subversion Revision: 240000
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 3e57f55dc915ebb4fa52941378ea919206d816de..87a46a06223ae29cd75db3ac7e214f77541543e2 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] CSE breaks the previous proven AI's AbstractValue
>+        https://bugs.webkit.org/show_bug.cgi?id=193438
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        GetByVal(Array::String) emits Check(String) before that. But AI can broaden type constraint in the second run.
>+        After the first run removes Check(String), it would happen that AI starts saying the tyep of 1st child is not String.
>+        To claim that it *is* a String type, we should use KnownStringUse here.
>+
>+        * dfg/DFGFixupPhase.cpp:
>+        (JSC::DFG::FixupPhase::fixupNode): StringCharAt and GetByVal(Array::String) share the underlying compiler code. We should
>+        change StringUse => KnownStringUse for StringCharAt too. And StringCharAt and StringCharCodeAt potentially have the same
>+        problem. This patch fixes it too.
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
>+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
>+
> 2019-01-15  Tomas Popela  <tpopela@redhat.com>
> 
>         Unreviewed: Fix the -Wformat compiler warnings
>diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>index 05112da4d6f5161403173eccd23dfc2f7091079b..e779deb4d48c11e1947e5c40dc69e1d0eec1c584 100644
>--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
>@@ -782,7 +782,7 @@ class FixupPhase : public Phase {
>             // Currently we have no good way of refining these.
>             ASSERT(node->arrayMode() == ArrayMode(Array::String, Array::Read));
>             blessArrayOperation(node->child1(), node->child2(), node->child3());
>-            fixEdge<KnownCellUse>(node->child1());
>+            fixEdge<KnownStringUse>(node->child1());
>             fixEdge<Int32Use>(node->child2());
>             break;
>         }
>@@ -900,6 +900,10 @@ class FixupPhase : public Phase {
>                 break;
>             case Array::ForceExit:
>                 break;
>+            case Array::String:
>+                fixEdge<KnownStringUse>(m_graph.varArgChild(node, 0));
>+                fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
>+                break;
>             default:
>                 fixEdge<KnownCellUse>(m_graph.varArgChild(node, 0));
>                 fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 73836a8aad901e4324a7eea8e40e1ad9368d5137..c6348ba0479b32c76163b6818aed8f2b4d64157b 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -6590,7 +6590,7 @@ class LowerDFGToB3 {
>     
>     void compileStringCharAt()
>     {
>-        LValue base = lowCell(m_graph.child(m_node, 0));
>+        LValue base = lowString(m_graph.child(m_node, 0));
>         LValue index = lowInt32(m_graph.child(m_node, 1));
>         LValue storage = lowStorage(m_graph.child(m_node, 2));
>             
>@@ -6704,7 +6704,7 @@ class LowerDFGToB3 {
>         LBasicBlock is16Bit = m_out.newBlock();
>         LBasicBlock continuation = m_out.newBlock();
> 
>-        LValue base = lowCell(m_node->child1());
>+        LValue base = lowString(m_node->child1());
>         LValue index = lowInt32(m_node->child2());
>         LValue storage = lowStorage(m_node->child3());
>         
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 54c47fac29ba7b7ae6aee5dd2bee0be449b2e143..53ef7eaa8b415e17fad03f58e2317fcb46eb689d 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] CSE breaks the previous proven AI's AbstractValue
>+        https://bugs.webkit.org/show_bug.cgi?id=193438
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Under the heavy load (like, compiling WebKit), AI in this code can broaden type information after the 1st run.
>+        Then, GetByVal(String) crashed.
>+
>+        * stress/type-for-get-by-val-can-be-widen-after-ai.js: Added.
>+        (Hello):
>+        (foo):
>+
> 2019-01-15  Tomas Popela  <tpopela@redhat.com>
> 
>         Unreviewed, skip JIT tests if it's not enabled
>diff --git a/JSTests/stress/type-for-get-by-val-can-be-widen-after-ai.js b/JSTests/stress/type-for-get-by-val-can-be-widen-after-ai.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..94818b7f9d7b8c560226e82c0e82bcfac7f53b37
>--- /dev/null
>+++ b/JSTests/stress/type-for-get-by-val-can-be-widen-after-ai.js
>@@ -0,0 +1,25 @@
>+//@ runDefault("--jitPolicyScale=0")
>+// Run with for i in {1..1000}; do echo $i && VM=/path/to/WebKit/WebKitBuild/Debug/ && DYLD_FRAMEWORK_PATH=$VM $VM/jsc --useDollarVM=1 --jitPolicyScale=0 type-for-get-by-val-can-be-widen-after-ai.js ; done
>+
>+function Hello(y) {
>+  this.y = y;
>+  this.x = foo(this.y);
>+}
>+function foo(z) {
>+  try {
>+    for (var i = 0; i < 1; i++) {
>+      z[i];
>+    }
>+  } catch {
>+  }
>+}
>+new Hello('a');
>+new Hello('a');
>+for (let i = 0; i < 100; ++i) {
>+  new Hello();
>+}
>+
>+// Busy loop to let the crash reporter have a chance to capture the crash log for the Compiler thread.
>+for (let i = 0; i < 1000000; ++i) {
>+    $vm.ftlTrue();
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 193438: 359220 | 359226 | 359227