RegExpObject's collectMatches should not be using JSArray::push to fill in its match...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2018 05:32:47 +0000 (05:32 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2018 05:32:47 +0000 (05:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191730
<rdar://problem/46048517>

Reviewed by Saam Barati.

JSTests:

* stress/regress-187006.js: Removed.
  - this test is invalid because its sole purpose is to test for the non-spec
    compliant behavior that we just fixed.

* stress/regress-191730.js: Added.

Source/JavaScriptCore:

According to the spec https://www.ecma-international.org/ecma-262/9.0/index.html#sec-regexp.prototype-@@match,
the RegExp match results are filled in using the spec's CreateDataProperty()
function which does not consult the prototype for setters.  JSArray:push()
consults the prototype for setters.  We should be using putDirectIndex() instead.

* runtime/RegExpObjectInlines.h:
(JSC::collectMatches):

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

JSTests/ChangeLog
JSTests/stress/regress-187006.js [deleted file]
JSTests/stress/regress-191730.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/RegExpObjectInlines.h

index 2957689..a8224a1 100644 (file)
@@ -1,5 +1,19 @@
 2018-11-15  Mark Lam  <mark.lam@apple.com>
 
+        RegExpObject's collectMatches should not be using JSArray::push to fill in its match results.
+        https://bugs.webkit.org/show_bug.cgi?id=191730
+        <rdar://problem/46048517>
+
+        Reviewed by Saam Barati.
+
+        * stress/regress-187006.js: Removed.
+          - this test is invalid because its sole purpose is to test for the non-spec
+            compliant behavior that we just fixed.
+
+        * stress/regress-191730.js: Added.
+
+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>
diff --git a/JSTests/stress/regress-187006.js b/JSTests/stress/regress-187006.js
deleted file mode 100644 (file)
index 91098d8..0000000
+++ /dev/null
@@ -1,16 +0,0 @@
-Object.defineProperty(Array.prototype, '0', {
-    get() { },
-    set() { throw new Error(); }
-});
-
-var __v_7772 = "GGCCGGGTAAAGTGGCTCACGCCTGTAATCCCAGCACTTTACCCCCCGAGGCGGGCGGA";
-var exception;
-
-try {
-    __v_7772.match(/[cgt]gggtaaa|tttaccc[acg]/ig);
-} catch (e) {
-    exception = e;
-}
-
-if (exception != "Error")
-    throw "FAILED";
diff --git a/JSTests/stress/regress-191730.js b/JSTests/stress/regress-191730.js
new file mode 100644 (file)
index 0000000..b6ab1c9
--- /dev/null
@@ -0,0 +1,25 @@
+function assertEq(actual, expected) {
+    if (actual != expected)
+        throw ("Expected: " + expected + ", actual: " + actual);
+}
+
+var otherGlobal = $vm.createGlobalObject();
+
+Array.prototype.__defineSetter__(7, () => {
+    arr[0] = { };
+});
+
+let arr = new otherGlobal.Array(1.1, 2.2, 3.3);
+
+function foo(arr, regexp, str){
+    var result = regexp[Symbol.match](str);
+    arr[1] = 3.54484805889626e-310;
+    return arr[0];
+}
+
+let regexp = /a/g;
+for (let i = 0; i < 10000; i++)
+    foo(arr, regexp, "aaaa");
+
+let r = foo(arr, regexp, "aaaaaaaa");
+assertEq(arr[1], "3.54484805889626e-310");
index e6dbe45..ec6f3bf 100644 (file)
@@ -1,5 +1,21 @@
 2018-11-15  Mark Lam  <mark.lam@apple.com>
 
+        RegExpObject's collectMatches should not be using JSArray::push to fill in its match results.
+        https://bugs.webkit.org/show_bug.cgi?id=191730
+        <rdar://problem/46048517>
+
+        Reviewed by Saam Barati.
+
+        According to the spec https://www.ecma-international.org/ecma-262/9.0/index.html#sec-regexp.prototype-@@match,
+        the RegExp match results are filled in using the spec's CreateDataProperty()
+        function which does not consult the prototype for setters.  JSArray:push()
+        consults the prototype for setters.  We should be using putDirectIndex() instead.
+
+        * runtime/RegExpObjectInlines.h:
+        (JSC::collectMatches):
+
+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>
index c510714..09aa17f 100644 (file)
@@ -1,6 +1,6 @@
 /*
  *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
- *  Copyright (C) 2003, 2007, 2008, 2012, 2016 Apple Inc. All Rights Reserved.
+ *  Copyright (C) 2003-2018 Apple Inc. All Rights Reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -160,10 +160,11 @@ JSValue collectMatches(VM& vm, ExecState* exec, JSString* string, const String&
     RETURN_IF_EXCEPTION(scope, { });
 
     bool hasException = false;
+    unsigned arrayIndex = 0;
     auto iterate = [&] () {
         size_t end = result.end;
         size_t length = end - result.start;
-        array->push(exec, JSRopeString::createSubstringOfResolved(vm, string, result.start, length));
+        array->putDirectIndex(exec, arrayIndex++, JSRopeString::createSubstringOfResolved(vm, string, result.start, length));
         if (UNLIKELY(scope.exception())) {
             hasException = true;
             return;