We don't model regexp effects properly
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Apr 2018 01:17:06 +0000 (01:17 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Apr 2018 01:17:06 +0000 (01:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185059
<rdar://problem/39736150>

Reviewed by Filip Pizlo.

JSTests:

* stress/regexp-exec-test-effectful-last-index.js: Added.
(assert):
(foo):
(i.regexLastIndex.toString):
(bar):

Source/JavaScriptCore:

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231145 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/regexp-exec-test-effectful-last-index.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGClobberize.h

index d4fb07a..9fe2b28 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-28  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 Filip Pizlo.
+
+        * stress/regexp-exec-test-effectful-last-index.js: Added.
+        (assert):
+        (foo):
+        (i.regexLastIndex.toString):
+        (bar):
+
 2018-04-28  Rick Waldron  <waldron.rick@gmail.com>
 
         Token misspelled "tocken" in error message string
diff --git a/JSTests/stress/regexp-exec-test-effectful-last-index.js b/JSTests/stress/regexp-exec-test-effectful-last-index.js
new file mode 100644 (file)
index 0000000..ef68e55
--- /dev/null
@@ -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 40f58c7..e736a31 100644 (file)
@@ -1,3 +1,19 @@
+2018-04-28  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 Filip Pizlo.
+
+        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-28  Rick Waldron  <waldron.rick@gmail.com>
 
         Token misspelled "tocken" in error message string
index 6f85734..c3afd0c 100644 (file)
@@ -2010,11 +2010,9 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
     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<AbstractStateType>::executeEffects(unsigned clobberLimi
         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 24fd941..dd68a59 100644 (file)
@@ -1513,19 +1513,19 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
 
     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:
+        read(RegExpState);
+        read(RegExpObject_lastIndex);
+        write(RegExpState);
+        write(RegExpObject_lastIndex);
+        return;
+
     case RegExpExecNonGlobalOrSticky:
     case RegExpMatchFastGlobal:
         read(RegExpState);