RegExp operations should not take fast patch if lastIndex is not numeric.

https://bugs.webkit.org/show_bug.cgi?id=191731
<rdar://problem/46017305>

Reviewed by Saam Barati.

JSTests:

* stress/regress-191731.js: Added.

Source/JavaScriptCore:

This is because if lastIndex is an object with a valueOf() method, it can execute
arbitrary code which may have side effects, and side effects are not permitted by
the RegExp fast paths.

* builtins/RegExpPrototype.js:
(globalPrivate.hasObservableSideEffectsForRegExpMatch):
(overriddenName.string_appeared_here.search):
(globalPrivate.hasObservableSideEffectsForRegExpSplit):
(intrinsic.RegExpTestIntrinsic.test):
* builtins/StringPrototype.js:
(globalPrivate.hasObservableSideEffectsForStringReplace):

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

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 768c100..2957689 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,13 @@
+2018-11-15  Mark Lam  <mark.lam@apple.com>
+
+        RegExp operations should not take fast patch if lastIndex is not numeric.
+        https://bugs.webkit.org/show_bug.cgi?id=191731
+        <rdar://problem/46017305>
+
+        Reviewed by Saam Barati.
+
+        * stress/regress-191731.js: Added.
+
 2018-11-13  Saam Barati  <sbarati@apple.com>
 
         TypeProfileLog::processLogEntries should stash away any pending exceptions and re-apply them to the VM

diff --git a/JSTests/stress/regress-191731.js b/JSTests/stress/regress-191731.js
new file mode 100644 (file)
index 0000000..a93e66f
--- /dev/null
+++ b/JSTests/stress/regress-191731.js
@@ -0,0 +1,27 @@
+function assertEq(actual, expected) {
+    if (actual != expected)
+        throw ("Expected: " + expected + ", actual: " + actual);
+}
+
+function foo(arr, regexp, str) {
+    regexp[Symbol.match](str);
+    arr[1] = 3.54484805889626e-310;
+    return arr[0];
+}
+
+let arr = [1.1, 2.2, 3.3];
+let regexp = /a/y;
+
+for (let i = 0; i < 10000; i++)
+    foo(arr, regexp, "abcd");
+
+regexp.lastIndex = {
+    valueOf: () => {
+        arr[0] = arr;
+        return 0;
+    }
+};
+let result = foo(arr, regexp, "abcd");
+
+assertEq(arr[1], "3.54484805889626e-310");
+assertEq(result, ",3.54484805889626e-310,3.3");

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 0b588b0..e6dbe45 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,23 @@
+2018-11-15  Mark Lam  <mark.lam@apple.com>
+
+        RegExp operations should not take fast patch if lastIndex is not numeric.
+        https://bugs.webkit.org/show_bug.cgi?id=191731
+        <rdar://problem/46017305>
+
+        Reviewed by Saam Barati.
+
+        This is because if lastIndex is an object with a valueOf() method, it can execute
+        arbitrary code which may have side effects, and side effects are not permitted by
+        the RegExp fast paths.
+
+        * builtins/RegExpPrototype.js:
+        (globalPrivate.hasObservableSideEffectsForRegExpMatch):
+        (overriddenName.string_appeared_here.search):
+        (globalPrivate.hasObservableSideEffectsForRegExpSplit):
+        (intrinsic.RegExpTestIntrinsic.test):
+        * builtins/StringPrototype.js:
+        (globalPrivate.hasObservableSideEffectsForStringReplace):
+
 2018-11-15  Keith Rollin  <krollin@apple.com>
 
         Delete old .xcfilelist files

diff --git a/Source/JavaScriptCore/builtins/RegExpPrototype.js b/Source/JavaScriptCore/builtins/RegExpPrototype.js
index 2860ee1..4854c56 100644 (file)
--- a/Source/JavaScriptCore/builtins/RegExpPrototype.js
+++ b/Source/JavaScriptCore/builtins/RegExpPrototype.js
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -67,6 +67,9 @@ function hasObservableSideEffectsForRegExpMatch(regexp)
 {
     "use strict";
 
+    if (!@isRegExpObject(regexp))
+        return true;
+
     // This is accessed by the RegExpExec internal function.
     let regexpExec = @tryGetById(regexp, "exec");
     if (regexpExec !== @regExpBuiltinExec)
@@ -79,7 +82,7 @@ function hasObservableSideEffectsForRegExpMatch(regexp)
     if (regexpUnicode !== @regExpProtoUnicodeGetter)
         return true;
 
-    return !@isRegExpObject(regexp);
+    return typeof regexp.lastIndex !== "number";
 }
 
 @globalPrivate
@@ -315,7 +318,9 @@ function search(strArg)
     let regexp = this;
 
     // Check for observable side effects and call the fast path if there aren't any.
-    if (@isRegExpObject(regexp) && @tryGetById(regexp, "exec") === @regExpBuiltinExec)
+    if (@isRegExpObject(regexp)
+        && @tryGetById(regexp, "exec") === @regExpBuiltinExec
+        && typeof regexp.lastIndex === "number")
         return @regExpSearchFast.@call(regexp, strArg);
 
     // 1. Let rx be the this value.
@@ -358,6 +363,9 @@ function hasObservableSideEffectsForRegExpSplit(regexp)
 {
     "use strict";
 
+    if (!@isRegExpObject(regexp))
+        return true;
+
     // This is accessed by the RegExpExec internal function.
     let regexpExec = @tryGetById(regexp, "exec");
     if (regexpExec !== @regExpBuiltinExec)
@@ -389,8 +397,8 @@ function hasObservableSideEffectsForRegExpSplit(regexp)
     let regexpSource = @tryGetById(regexp, "source");
     if (regexpSource !== @regExpProtoSourceGetter)
         return true;
-    
-    return !@isRegExpObject(regexp);
+
+    return typeof regexp.lastIndex !== "number";
 }
 
 // ES 21.2.5.11 RegExp.prototype[@@split](string, limit)
@@ -536,7 +544,9 @@ function test(strArg)
     let regexp = this;
 
     // Check for observable side effects and call the fast path if there aren't any.
-    if (@isRegExpObject(regexp) && @tryGetById(regexp, "exec") === @regExpBuiltinExec)
+    if (@isRegExpObject(regexp)
+        && @tryGetById(regexp, "exec") === @regExpBuiltinExec
+        && typeof regexp.lastIndex === "number")
         return @regExpTestFast.@call(regexp, strArg);
 
     // 1. Let R be the this value.

diff --git a/Source/JavaScriptCore/builtins/StringPrototype.js b/Source/JavaScriptCore/builtins/StringPrototype.js
index db0c401..4028f80 100644 (file)
--- a/Source/JavaScriptCore/builtins/StringPrototype.js
+++ b/Source/JavaScriptCore/builtins/StringPrototype.js
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2015 Andy VanWagoner <andy@vanwagoner.family>.
  * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com>
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -195,6 +195,9 @@ function hasObservableSideEffectsForStringReplace(regexp, replacer)
 {
     "use strict";
 
+    if (!@isRegExpObject(regexp))
+        return true;
+
     if (replacer !== @regExpPrototypeSymbolReplace)
         return true;
     
@@ -210,7 +213,7 @@ function hasObservableSideEffectsForStringReplace(regexp, replacer)
     if (regexpUnicode !== @regExpProtoUnicodeGetter)
         return true;
 
-    return !@isRegExpObject(regexp);
+    return typeof regexp.lastIndex !== "number";
 }
 
 @intrinsic=StringPrototypeReplaceIntrinsic