Attachment 338940 Details for Bug 185059 – patch

[patch] patch

b-backup.diff (text/plain), 5.42 KB, created by Saam Barati on 2018-04-26 17:23:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-04-26 17:23:35 PDT

Size: 5.42 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 231082)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-04-26  Saam Barati  <sbarati@apple.com>
>+
>+        We don't model regexp effects properly
>+        https://bugs.webkit.org/show_bug.cgi?id=185059
>+        <rdar://problem/39736150>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/regexp-exec-test-effectful-last-index.js: Added.
>+        (assert):
>+        (foo):
>+        (i.regexLastIndex.toString):
>+        (bar):
>+
> 2018-04-25  Robin Morisset  <rmorisset@apple.com>
> 
>         In FTLLowerDFGToB3.cpp::compileCreateRest, always use a contiguous array as the indexing type when under isWatchingHavingABadTimeWatchpoint
>Index: JSTests/stress/regexp-exec-test-effectful-last-index.js
>===================================================================
>--- JSTests/stress/regexp-exec-test-effectful-last-index.js	(nonexistent)
>+++ JSTests/stress/regexp-exec-test-effectful-last-index.js	(working copy)
>@@ -0,0 +1,50 @@
>+function assert(b) {
>+    if (!b)
>+        throw new Error;
>+}
>+
>+let outer = 42;
>+
>+function foo(r, s) {
>+    let y = outer;
>+    r.test(s);
>+    return y + outer;
>+}
>+noInline(foo);
>+
>+for (let i = 0; i < 10000; ++i) {
>+    let r = /foo/g;
>+    regexLastIndex = {};
>+    regexLastIndex.toString = function() {
>+        outer = 1;
>+        return "1";
>+    };
>+
>+    r.lastIndex = regexLastIndex;
>+    let result = foo(r, "bar");
>+    assert(result === 43);
>+
>+    outer = 42;
>+}
>+
>+function bar(r, s) {
>+    let y = outer;
>+    r.exec(s);
>+    return y + outer;
>+}
>+noInline(bar);
>+
>+for (let i = 0; i < 10000; ++i) {
>+    let r = /foo/g;
>+    regexLastIndex = {};
>+    regexLastIndex.toString = function() {
>+        outer = 1;
>+        return "1";
>+    };
>+
>+    r.lastIndex = regexLastIndex;
>+    let result = bar(r, "bar");
>+    assert(result === 43);
>+
>+    outer = 42;
>+}
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231078)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2018-04-26  Saam Barati  <sbarati@apple.com>
>+
>+        We don't model regexp effects properly
>+        https://bugs.webkit.org/show_bug.cgi?id=185059
>+        <rdar://problem/39736150>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        RegExp exec/test can do arbitrary effects when toNumbering the lastIndex if
>+        the regexp is global.
>+
>+        * dfg/DFGAbstractInterpreterInlines.h:
>+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
>+        * dfg/DFGClobberize.h:
>+        (JSC::DFG::clobberize):
>+
> 2018-04-26  Mark Lam  <mark.lam@apple.com>
> 
>         Gardening: Windows build fix.
>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(revision 231078)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(working copy)
>@@ -2010,11 +2010,9 @@ bool AbstractInterpreter<AbstractStateTy
>     case RegExpExec:
>     case RegExpExecNonGlobalOrSticky:
>         if (node->op() == RegExpExec) {
>-            if (node->child2().useKind() == RegExpObjectUse
>-                && node->child3().useKind() == StringUse) {
>-                // This doesn't clobber the world since there are no conversions to perform.
>-            } else
>-                clobberWorld(node->origin.semantic, clobberLimit);
>+            // Even if we've proven known input types as RegExpObject and String,
>+            // accessing lastIndex is effectful if it's a global regexp.
>+            clobberWorld(node->origin.semantic, clobberLimit);
>         }
> 
>         if (JSValue globalObjectValue = forNode(node->child1()).m_value) {
>@@ -2034,11 +2032,9 @@ bool AbstractInterpreter<AbstractStateTy
>         break;
> 
>     case RegExpTest:
>-        if (node->child2().useKind() == RegExpObjectUse
>-            && node->child3().useKind() == StringUse) {
>-            // This doesn't clobber the world since there are no conversions to perform.
>-        } else
>-            clobberWorld(node->origin.semantic, clobberLimit);
>+        // Even if we've proven known input types as RegExpObject and String,
>+        // accessing lastIndex is effectful if it's a global regexp.
>+        clobberWorld(node->origin.semantic, clobberLimit);
>         forNode(node).setType(SpecBoolean);
>         break;
> 
>Index: Source/JavaScriptCore/dfg/DFGClobberize.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGClobberize.h	(revision 231078)
>+++ Source/JavaScriptCore/dfg/DFGClobberize.h	(working copy)
>@@ -1513,19 +1513,13 @@ void clobberize(Graph& graph, Node* node
> 
>     case RegExpExec:
>     case RegExpTest:
>-    case RegExpMatchFast:
>-        if (node->child2().useKind() == RegExpObjectUse
>-            && node->child3().useKind() == StringUse) {
>-            read(RegExpState);
>-            read(RegExpObject_lastIndex);
>-            write(RegExpState);
>-            write(RegExpObject_lastIndex);
>-            return;
>-        }
>+        // Even if we've proven known input types as RegExpObject and String,
>+        // accessing lastIndex is effectful if it's a global regexp.
>         read(World);
>         write(Heap);
>         return;
> 
>+    case RegExpMatchFast:
>     case RegExpExecNonGlobalOrSticky:
>     case RegExpMatchFastGlobal:
>         read(RegExpState);

Flags:

fpizlo: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 185059: 338940 | 338947 | 339044 | 339083