Attachment 348562 Details for Bug 189173 – patch for landing

[patch] patch for landing

patch.diff (text/plain), 7.16 KB, created by Saam Barati on 2018-08-30 16:07:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-08-30 16:07:02 PDT

Size: 7.16 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 235523)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2018-08-30  Saam barati  <sbarati@apple.com>
>+
>+        convertToRegExpMatchFastGlobal must use KnownString as the child use kind
>+        https://bugs.webkit.org/show_bug.cgi?id=189173
>+        <rdar://problem/43501645>
>+
>+        Reviewed by Michael Saboff.
>+
>+        * stress/may-exit-should-be-false-regexp-constant-folding.js: Added.
>+        (foo):
>+        (bar):
>+
> 2018-08-30  Saam barati  <sbarati@apple.com>
> 
>         CSE DataViewGet* DFG nodes
>Index: JSTests/stress/may-exit-should-be-false-regexp-constant-folding.js
>===================================================================
>--- JSTests/stress/may-exit-should-be-false-regexp-constant-folding.js	(nonexistent)
>+++ JSTests/stress/may-exit-should-be-false-regexp-constant-folding.js	(working copy)
>@@ -0,0 +1,25 @@
>+//@ runDefault("--jitPolicyScale=0", "--useConcurrentJIT=0", "--validateGraphAtEachPhase=1")
>+
>+let re0 = /a/;
>+let str0 = 'b';
>+function foo() {
>+  /a/.exec('b');
>+  for (var i = 0; i < 6; i++) {
>+  }
>+  for (var i = 0; i < 3; i++) {
>+    re0.exec('a');
>+  }
>+  str0.match(/a/);
>+  for (var i = 0; i < 2; i++) {
>+    str0.match(/a/g);
>+  }
>+}
>+function bar() {
>+  for (var i = 0; i < 6; i++) {
>+    'a'.match(/b/);
>+  }
>+}
>+
>+foo();
>+bar();
>+foo();
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 235521)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,30 @@
>+2018-08-30  Saam barati  <sbarati@apple.com>
>+
>+        convertToRegExpMatchFastGlobal must use KnownString as the child use kind
>+        https://bugs.webkit.org/show_bug.cgi?id=189173
>+        <rdar://problem/43501645>
>+
>+        Reviewed by Michael Saboff.
>+
>+        We were crashing during validation because mayExit returned true
>+        at a point in the program when we weren't allowed to exit.
>+        
>+        The issue was is in StrengthReduction: we end up emitting code that
>+        had a StringUse on an edge after a node that did side effects and before
>+        an ExitOK/bytecode number transition. However, StrenghReduction did the
>+        right thing here and also emitted the type checks before the node with
>+        side effects. It just did bad bookkeeping. The node we convert to needs
>+        to use KnownStringUse instead of StringUse for the child edge.
>+
>+        * dfg/DFGNode.cpp:
>+        (JSC::DFG::Node::convertToRegExpExecNonGlobalOrStickyWithoutChecks):
>+        (JSC::DFG::Node::convertToRegExpMatchFastGlobalWithoutChecks):
>+        (JSC::DFG::Node::convertToRegExpExecNonGlobalOrSticky): Deleted.
>+        (JSC::DFG::Node::convertToRegExpMatchFastGlobal): Deleted.
>+        * dfg/DFGNode.h:
>+        * dfg/DFGStrengthReductionPhase.cpp:
>+        (JSC::DFG::StrengthReductionPhase::handleNode):
>+
> 2018-08-30  Saam barati  <sbarati@apple.com>
> 
>         InlineAccess should do StringLength
>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(revision 235520)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(working copy)
>@@ -2217,12 +2217,12 @@ bool AbstractInterpreter<AbstractStateTy
> 
>     case RegExpMatchFast:
>         ASSERT(node->child2().useKind() == RegExpObjectUse);
>-        ASSERT(node->child3().useKind() == StringUse);
>+        ASSERT(node->child3().useKind() == StringUse || node->child3().useKind() == KnownStringUse);
>         setTypeForNode(node, SpecOther | SpecArray);
>         break;
> 
>     case RegExpMatchFastGlobal:
>-        ASSERT(node->child2().useKind() == StringUse);
>+        ASSERT(node->child2().useKind() == StringUse || node->child2().useKind() == KnownStringUse);
>         setTypeForNode(node, SpecOther | SpecArray);
>         break;
>             
>Index: Source/JavaScriptCore/dfg/DFGNode.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGNode.cpp	(revision 235520)
>+++ Source/JavaScriptCore/dfg/DFGNode.cpp	(working copy)
>@@ -268,22 +268,22 @@ void Node::convertToCallDOM(Graph& graph
>         clearFlags(NodeMustGenerate);
> }
> 
>-void Node::convertToRegExpExecNonGlobalOrSticky(FrozenValue* regExp)
>+void Node::convertToRegExpExecNonGlobalOrStickyWithoutChecks(FrozenValue* regExp)
> {
>     ASSERT(op() == RegExpExec);
>     setOpAndDefaultFlags(RegExpExecNonGlobalOrSticky);
>     children.child1() = Edge(children.child1().node(), KnownCellUse);
>-    children.child2() = Edge(children.child3().node(), StringUse);
>+    children.child2() = Edge(children.child3().node(), KnownStringUse);
>     children.child3() = Edge();
>     m_opInfo = regExp;
> }
> 
>-void Node::convertToRegExpMatchFastGlobal(FrozenValue* regExp)
>+void Node::convertToRegExpMatchFastGlobalWithoutChecks(FrozenValue* regExp)
> {
>     ASSERT(op() == RegExpMatchFast);
>     setOpAndDefaultFlags(RegExpMatchFastGlobal);
>     children.child1() = Edge(children.child1().node(), KnownCellUse);
>-    children.child2() = Edge(children.child3().node(), StringUse);
>+    children.child2() = Edge(children.child3().node(), KnownStringUse);
>     children.child3() = Edge();
>     m_opInfo = regExp;
> }
>Index: Source/JavaScriptCore/dfg/DFGNode.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGNode.h	(revision 235520)
>+++ Source/JavaScriptCore/dfg/DFGNode.h	(working copy)
>@@ -757,8 +757,8 @@ public:
> 
>     void convertToCallDOM(Graph&);
> 
>-    void convertToRegExpExecNonGlobalOrSticky(FrozenValue* regExp);
>-    void convertToRegExpMatchFastGlobal(FrozenValue* regExp);
>+    void convertToRegExpExecNonGlobalOrStickyWithoutChecks(FrozenValue* regExp);
>+    void convertToRegExpMatchFastGlobalWithoutChecks(FrozenValue* regExp);
> 
>     void convertToSetRegExpObjectLastIndex()
>     {
>Index: Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp	(revision 235520)
>+++ Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp	(working copy)
>@@ -508,7 +508,7 @@ private:
>                         m_insertionSet.insertConstantForUse(
>                             m_nodeIndex, origin, jsNumber(0), UntypedUse));
>                     origin = origin.withInvalidExit();
>-                    m_node->convertToRegExpMatchFastGlobal(m_graph.freeze(regExp));
>+                    m_node->convertToRegExpMatchFastGlobalWithoutChecks(m_graph.freeze(regExp));
>                     m_node->origin = origin;
>                     m_changed = true;
>                     break;
>@@ -774,7 +774,7 @@ private:
>                 NodeOrigin origin = m_node->origin;
>                 m_insertionSet.insertNode(
>                     m_nodeIndex, SpecNone, Check, origin, m_node->children.justChecks());
>-                m_node->convertToRegExpExecNonGlobalOrSticky(m_graph.freeze(regExp));
>+                m_node->convertToRegExpExecNonGlobalOrStickyWithoutChecks(m_graph.freeze(regExp));
>                 m_changed = true;
>                 return true;
>             };

Actions: View | Formatted Diff | Diff

Attachments on bug 189173: 348547 | 348552 | 348562