Attachment 340269 Details for Bug 185587 – Patch

[patch] Patch

bug-185587-20180514033518.patch (text/plain), 11.00 KB, created by Yusuke Suzuki on 2018-05-13 11:35:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-13 11:35:19 PDT

Size: 11.00 KB

patch

obsolete

>Subversion Revision: 231741
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 9236b46544dfd53012c6077d2bb79c3f681035a6..d37a4668a88cde4c73eab9af167e91f44442aaa9 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-05-13  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 NOBODY (OOPS!).
>+
>+        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-12  Filip Pizlo  <fpizlo@apple.com>
> 
>         CachedCall::call() should be faster
>diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp
>index 33eacb2a0be2b7a172e4c8b48bf4fd7cf88c5e40..b50b174cf90e37a49e1276d4cbe2c0e10629fca7 100644
>--- 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 {
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 018348aa491e5a2a015dbe53ca51df7383ba6d6e..2ad88e365f68a85bac2e267c0cdc4676386ac72d 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-05-13  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 NOBODY (OOPS!).
>+
>+        * test262/expectations.yaml:
>+
> 2018-05-13  Andy VanWagoner  <andy@vanwagoner.family>
> 
>         [INTL] Improve spec & test262 compliance for Intl APIs
>diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml
>index ff3bd3862614a21aed5e9f7790a39ce8bf086ac6..cd764501848b119505d4dbc2bd953f7c6cfea8ac 100644
>--- a/JSTests/test262/expectations.yaml
>+++ b/JSTests/test262/expectations.yaml
>@@ -1309,9 +1309,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'

Actions: View | Formatted Diff | Diff

Attachments on bug 185587: 340269