[JSC] Fix CachedCall's argument count if RegExp has named captures

https://bugs.webkit.org/show_bug.cgi?id=185587

Reviewed by Mark Lam.

JSTests:

* test262/expectations.yaml:

Source/JavaScriptCore:

If the given RegExp has named captures, the argument count of CachedCall in String#replace
should be increased by one. This causes crash with assertion in test262. This patch corrects
the argument count.

This patch also unifies source.is8Bit()/!source.is8Bit() code since they are now completely
the same.

* runtime/StringPrototype.cpp:
(JSC::replaceUsingRegExpSearch):

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

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 4f4d857..e61325f 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,12 @@
+2018-05-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Fix CachedCall's argument count if RegExp has named captures
+        https://bugs.webkit.org/show_bug.cgi?id=185587
+
+        Reviewed by Mark Lam.
+
+        * test262/expectations.yaml:
+
 2018-05-22  Mark Lam  <mark.lam@apple.com>
 
         StringImpl utf8 conversion should not fail silently.

diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml
index ee0b380..2ae0564 100644 (file)
--- a/JSTests/test262/expectations.yaml
+++ b/JSTests/test262/expectations.yaml
@@ -1303,9 +1303,6 @@ test/built-ins/RegExp/lookBehind/variable-length.js:
 test/built-ins/RegExp/lookBehind/word-boundary.js:
   default: 'SyntaxError: Invalid regular expression: invalid group specifier name'
   strict mode: 'SyntaxError: Invalid regular expression: invalid group specifier name'
-test/built-ins/RegExp/named-groups/functional-replace-global.js:
-  default: "TypeError: undefined is not an object (evaluating 'groups.fst')"
-  strict mode: "TypeError: undefined is not an object (evaluating 'groups.fst')"
 test/built-ins/RegExp/named-groups/groups-object-subclass-sans.js:
   default: 'Test262Error: Expected SameValue(«b», «$<a>») to be true'
   strict mode: 'Test262Error: Expected SameValue(«b», «$<a>») to be true'

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 20474cd..d904320 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2018-05-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Fix CachedCall's argument count if RegExp has named captures
+        https://bugs.webkit.org/show_bug.cgi?id=185587
+
+        Reviewed by Mark Lam.
+
+        If the given RegExp has named captures, the argument count of CachedCall in String#replace
+        should be increased by one. This causes crash with assertion in test262. This patch corrects
+        the argument count.
+
+        This patch also unifies source.is8Bit()/!source.is8Bit() code since they are now completely
+        the same.
+
+        * runtime/StringPrototype.cpp:
+        (JSC::replaceUsingRegExpSearch):
+
 2018-05-22  Mark Lam  <mark.lam@apple.com>
 
         StringImpl utf8 conversion should not fail silently.

diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp
index 33eacb2..b50b174 100644 (file)
--- a/Source/JavaScriptCore/runtime/StringPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp
@@ -565,140 +565,73 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch(
     if (global && callType == CallType::JS) {
         // regExp->numSubpatterns() + 1 for pattern args, + 2 for match start and string
         int argCount = regExp->numSubpatterns() + 1 + 2;
+        if (hasNamedCaptures)
+            ++argCount;
         JSFunction* func = jsCast<JSFunction*>(replaceValue);
         CachedCall cachedCall(exec, func, argCount);
         RETURN_IF_EXCEPTION(scope, nullptr);
-        if (source.is8Bit()) {
-            while (true) {
-                int* ovector;
-                MatchResult result = regExpConstructor->performMatch(vm, regExp, string, source, startPosition, &ovector);
-                if (!result)
-                    break;
-
-                if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
-                    OUT_OF_MEMORY(exec, scope);
-
-                unsigned i = 0;
-                cachedCall.clearArguments();
-
-                JSObject* groups = nullptr;
-
-                if (hasNamedCaptures) {
-                    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
-                    groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
-                }
-
-                for (; i < regExp->numSubpatterns() + 1; ++i) {
-                    int matchStart = ovector[i * 2];
-                    int matchLen = ovector[i * 2 + 1] - matchStart;
-
-                    JSValue patternValue;
-
-                    if (matchStart < 0)
-                        patternValue = jsUndefined();
-                    else
-                        patternValue = jsSubstring(&vm, source, matchStart, matchLen);
-
-                    cachedCall.appendArgument(patternValue);
-
-                    if (i && hasNamedCaptures) {
-                        String groupName = regExp->getCaptureGroupName(i);
-                        if (!groupName.isEmpty())
-                            groups->putDirect(vm, Identifier::fromString(&vm, groupName), patternValue);
-                    }
-                }
-
-                cachedCall.appendArgument(jsNumber(result.start));
-                cachedCall.appendArgument(string);
-                if (hasNamedCaptures)
-                    cachedCall.appendArgument(groups);
+        while (true) {
+            int* ovector;
+            MatchResult result = regExpConstructor->performMatch(vm, regExp, string, source, startPosition, &ovector);
+            if (!result)
+                break;
 
-                cachedCall.setThis(jsUndefined());
-                if (UNLIKELY(cachedCall.hasOverflowedArguments())) {
-                    throwOutOfMemoryError(exec, scope);
-                    return nullptr;
-                }
+            if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
+                OUT_OF_MEMORY(exec, scope);
 
-                JSValue jsResult = cachedCall.call();
-                RETURN_IF_EXCEPTION(scope, nullptr);
-                replacements.append(jsResult.toWTFString(exec));
-                RETURN_IF_EXCEPTION(scope, nullptr);
+            cachedCall.clearArguments();
 
-                lastIndex = result.end;
-                startPosition = lastIndex;
+            JSObject* groups = nullptr;
 
-                // special case of empty match
-                if (result.empty()) {
-                    startPosition++;
-                    if (startPosition > sourceLen)
-                        break;
-                }
+            if (hasNamedCaptures) {
+                JSGlobalObject* globalObject = exec->lexicalGlobalObject();
+                groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
             }
-        } else {
-            while (true) {
-                int* ovector;
-                MatchResult result = regExpConstructor->performMatch(vm, regExp, string, source, startPosition, &ovector);
-                if (!result)
-                    break;
 
-                if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
-                    OUT_OF_MEMORY(exec, scope);
+            for (unsigned i = 0; i < regExp->numSubpatterns() + 1; ++i) {
+                int matchStart = ovector[i * 2];
+                int matchLen = ovector[i * 2 + 1] - matchStart;
 
-                unsigned i = 0;
-                cachedCall.clearArguments();
+                JSValue patternValue;
 
-                JSObject* groups = nullptr;
-
-                if (hasNamedCaptures) {
-                    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
-                    groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
-                }
-
-                for (; i < regExp->numSubpatterns() + 1; ++i) {
-                    int matchStart = ovector[i * 2];
-                    int matchLen = ovector[i * 2 + 1] - matchStart;
-
-                    JSValue patternValue;
-
-                    if (matchStart < 0)
-                        patternValue = jsUndefined();
-                    else
-                        patternValue = jsSubstring(&vm, source, matchStart, matchLen);
+                if (matchStart < 0)
+                    patternValue = jsUndefined();
+                else
+                    patternValue = jsSubstring(&vm, source, matchStart, matchLen);
 
-                    cachedCall.appendArgument(patternValue);
+                cachedCall.appendArgument(patternValue);
 
-                    if (i && hasNamedCaptures) {
-                        String groupName = regExp->getCaptureGroupName(i);
-                        if (!groupName.isEmpty())
-                            groups->putDirect(vm, Identifier::fromString(&vm, groupName), patternValue);
-                    }
+                if (i && hasNamedCaptures) {
+                    String groupName = regExp->getCaptureGroupName(i);
+                    if (!groupName.isEmpty())
+                        groups->putDirect(vm, Identifier::fromString(&vm, groupName), patternValue);
                 }
+            }
 
-                cachedCall.appendArgument(jsNumber(result.start));
-                cachedCall.appendArgument(string);
-                if (hasNamedCaptures)
-                    cachedCall.appendArgument(groups);
+            cachedCall.appendArgument(jsNumber(result.start));
+            cachedCall.appendArgument(string);
+            if (hasNamedCaptures)
+                cachedCall.appendArgument(groups);
 
-                cachedCall.setThis(jsUndefined());
-                if (UNLIKELY(cachedCall.hasOverflowedArguments())) {
-                    throwOutOfMemoryError(exec, scope);
-                    return nullptr;
-                }
+            cachedCall.setThis(jsUndefined());
+            if (UNLIKELY(cachedCall.hasOverflowedArguments())) {
+                throwOutOfMemoryError(exec, scope);
+                return nullptr;
+            }
 
-                JSValue jsResult = cachedCall.call();
-                RETURN_IF_EXCEPTION(scope, nullptr);
-                replacements.append(jsResult.toWTFString(exec));
-                RETURN_IF_EXCEPTION(scope, nullptr);
+            JSValue jsResult = cachedCall.call();
+            RETURN_IF_EXCEPTION(scope, nullptr);
+            replacements.append(jsResult.toWTFString(exec));
+            RETURN_IF_EXCEPTION(scope, nullptr);
 
-                lastIndex = result.end;
-                startPosition = lastIndex;
+            lastIndex = result.end;
+            startPosition = lastIndex;
 
-                // special case of empty match
-                if (result.empty()) {
-                    startPosition++;
-                    if (startPosition > sourceLen)
-                        break;
-                }
+            // special case of empty match
+            if (result.empty()) {
+                startPosition++;
+                if (startPosition > sourceLen)
+                    break;
             }
         }
     } else {