Skip to content

Commit

Permalink
[JSC] Fix CachedCall's argument count if RegExp has named captures
Browse files Browse the repository at this point in the history
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: http://svn.webkit.org/repository/webkit/trunk@232092 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
[email protected] committed May 23, 2018
1 parent 9dfc74f commit b64e39d
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 118 deletions.
9 changes: 9 additions & 0 deletions JSTests/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
2018-05-22 Yusuke Suzuki <[email protected]>

[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 <[email protected]>

StringImpl utf8 conversion should not fail silently.
Expand Down
3 changes: 0 additions & 3 deletions JSTests/test262/expectations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
17 changes: 17 additions & 0 deletions Source/JavaScriptCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
2018-05-22 Yusuke Suzuki <[email protected]>

[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 <[email protected]>

StringImpl utf8 conversion should not fail silently.
Expand Down
163 changes: 48 additions & 115 deletions Source/JavaScriptCore/runtime/StringPrototype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b64e39d

Please sign in to comment.