Changeset 259029 in webkit


Ignore:
Timestamp:
Mar 25, 2020 7:24:29 PM (4 years ago)
Author:
Alexey Shvayka
Message:

RegExp.prototype[@@replace] relies on globals and doesn't perform ToLength
​https://bugs.webkit.org/show_bug.cgi?id=173867

Reviewed by Ross Kirsling.

JSTests:

  • test262/expectations.yaml: Mark 4 test cases as passing.

Source/JavaScriptCore:

This change:

a) Adds "lastIndex" ToLength coercion [1], which is observable, unlike ToLength coercion

of RegExpExec result [2] that we omit, just like the one in @@split [3].

b) Removes lastPosition checks/updates, as there are none in the spec, and it was

equivalent to checking nextSourcePosition.

c) Removes reliance of @@replace on globals and also replaces @stringSubstrInternal

built-in with @stringSubstringInternal, as the former is Annex B and accepts size
as 2nd paramter, which is not very handy because ECMA-262 usually says "substring
of S consisting of the code units at indices X (inclusive) through Y (exclusive)".

[1]: ​https://tc39.es/ecma262/#sec-regexp.prototype-@@replace (step 11.c.iii.2.a)
[2]: ​https://tc39.es/ecma262/#sec-regexp.prototype-@@replace (step 14.a)
[3]: ​https://tc39.es/ecma262/#sec-regexp.prototype-@@split (step 19.d.iv.6)

  • builtins/BuiltinNames.h:
  • builtins/RegExpPrototype.js:

(getSubstitution):
(Symbol.replace):
(Symbol.split):

  • builtins/StringPrototype.js:

(globalPrivate.repeatCharactersSlowPath):

  • bytecode/LinkTimeConstant.h:
  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

  • runtime/StringPrototype.cpp:

(JSC::stringIndexOfImpl):
(JSC::stringProtoFuncIndexOf):
(JSC::builtinStringIndexOfInternal):
(JSC::stringProtoFuncSubstr):
(JSC::stringSubstringImpl):
(JSC::stringProtoFuncSubstring):
(JSC::builtinStringSubstringInternal):
(JSC::stringProtoFuncSubstrImpl): Deleted.
(JSC::builtinStringSubstrInternal): Deleted.

  • runtime/StringPrototype.h:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r259026 r259029  
     12020-03-25  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        RegExp.prototype[@@replace] relies on globals and doesn't perform ToLength
     4        https://bugs.webkit.org/show_bug.cgi?id=173867
     5
     6        Reviewed by Ross Kirsling.
     7
     8        * test262/expectations.yaml: Mark 4 test cases as passing.
     9
    1102020-03-25  Alexey Shvayka  <shvaikalesh@gmail.com>
    211

  • trunk/JSTests/test262/expectations.yaml

    r259026 r259029  
    16551655  default: 'Test262Error: Expected SameValue(«�», «null») to be true'
    16561656  strict mode: 'Test262Error: Expected SameValue(«�», «null») to be true'
    1657 test/built-ins/RegExp/prototype/Symbol.replace/coerce-lastindex.js:
    1658   default: 'Test262Error: Expected SameValue(«18014398509481984», «9007199254740992») to be true'
    1659   strict mode: 'Test262Error: Expected SameValue(«18014398509481984», «9007199254740992») to be true'
    1660 test/built-ins/RegExp/prototype/Symbol.replace/poisoned-stdlib.js:
    1661   default: 'TypeError: undefined is not a function'
    1662   strict mode: 'TypeError: undefined is not a function'
    16631657test/built-ins/RegExp/prototype/Symbol.search/set-lastindex-init-samevalue.js:
    16641658  default: 'Test262Error: Expected SameValue(«0», «0») to be true'

  • trunk/Source/JavaScriptCore/ChangeLog

    r259026 r259029  
     12020-03-25  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        RegExp.prototype[@@replace] relies on globals and doesn't perform ToLength
     4        https://bugs.webkit.org/show_bug.cgi?id=173867
     5
     6        Reviewed by Ross Kirsling.
     7
     8        This change:
     9
     10        a) Adds "lastIndex" ToLength coercion [1], which is observable, unlike ToLength coercion
     11           of RegExpExec result [2] that we omit, just like the one in @@split [3].
     12
     13        b) Removes `lastPosition` checks/updates, as there are none in the spec, and it was
     14           equivalent to checking `nextSourcePosition`.
     15
     16        c) Removes reliance of @@replace on globals and also replaces @stringSubstrInternal
     17           built-in with @stringSubstringInternal, as the former is Annex B and accepts size
     18           as 2nd paramter, which is not very handy because ECMA-262 usually says "substring
     19           of S consisting of the code units at indices X (inclusive) through Y (exclusive)".
     20
     21        [1]: https://tc39.es/ecma262/#sec-regexp.prototype-@@replace (step 11.c.iii.2.a)
     22        [2]: https://tc39.es/ecma262/#sec-regexp.prototype-@@replace (step 14.a)
     23        [3]: https://tc39.es/ecma262/#sec-regexp.prototype-@@split (step 19.d.iv.6)
     24
     25        * builtins/BuiltinNames.h:
     26        * builtins/RegExpPrototype.js:
     27        (getSubstitution):
     28        (Symbol.replace):
     29        (Symbol.split):
     30        * builtins/StringPrototype.js:
     31        (globalPrivate.repeatCharactersSlowPath):
     32        * bytecode/LinkTimeConstant.h:
     33        * runtime/JSGlobalObject.cpp:
     34        (JSC::JSGlobalObject::init):
     35        * runtime/StringPrototype.cpp:
     36        (JSC::stringIndexOfImpl):
     37        (JSC::stringProtoFuncIndexOf):
     38        (JSC::builtinStringIndexOfInternal):
     39        (JSC::stringProtoFuncSubstr):
     40        (JSC::stringSubstringImpl):
     41        (JSC::stringProtoFuncSubstring):
     42        (JSC::builtinStringSubstringInternal):
     43        (JSC::stringProtoFuncSubstrImpl): Deleted.
     44        (JSC::builtinStringSubstrInternal): Deleted.
     45        * runtime/StringPrototype.h:
     46
    1472020-03-25  Alexey Shvayka  <shvaikalesh@gmail.com>
    248

  • trunk/Source/JavaScriptCore/builtins/BuiltinNames.h

    r258801 r259029  
    160160    macro(regExpStringIteratorDone) \
    161161    macro(stringIncludesInternal) \
     162    macro(stringIndexOfInternal) \
    162163    macro(stringSplitFast) \
    163     macro(stringSubstrInternal) \
     164    macro(stringSubstringInternal) \
    164165    macro(makeBoundFunction) \
    165166    macro(hasOwnLengthProperty) \

  • trunk/Source/JavaScriptCore/builtins/RegExpPrototype.js

    r258968 r259029  
    192192        var lastStart = 0;
    193193
    194         for (var start = 0; start = replacement.indexOf("$", lastStart), start !== -1; lastStart = start) {
     194        for (var start = 0; start = @stringIndexOfInternal.@call(replacement, "$", lastStart), start !== -1; lastStart = start) {
    195195            if (start - lastStart > 0)
    196                 result = result + replacement.substring(lastStart, start);
     196                result = result + @stringSubstringInternal.@call(replacement, lastStart, start);
    197197            start++;
    198             var ch = replacement.charAt(start);
    199             if (ch === "")
     198            if (start >= replacementLength)
    200199                result = result + "$";
    201200            else {
     201                var ch = replacement[start];
    202202                switch (ch)
    203203                {
    … …  
    212212                case "`":
    213213                    if (position > 0)
    214                         result = result + str.substring(0, position);
     214                        result = result + @stringSubstringInternal.@call(str, 0, position);
    215215                    start++;
    216216                    break;
    217217                case "'":
    218218                    if (tailPos < stringLength)
    219                         result = result + str.substring(tailPos);
     219                        result = result + @stringSubstringInternal.@call(str, tailPos);
    220220                    start++;
    221221                    break;
    … …  
    223223                    if (namedCaptures !== @undefined) {
    224224                        var groupNameStartIndex = start + 1;
    225                         var groupNameEndIndex = replacement.indexOf(">", groupNameStartIndex);
     225                        var groupNameEndIndex = @stringIndexOfInternal.@call(replacement, ">", groupNameStartIndex);
    226226                        if (groupNameEndIndex !== -1) {
    227                             var groupName = replacement.substring(groupNameStartIndex, groupNameEndIndex);
     227                            var groupName = @stringSubstringInternal.@call(replacement, groupNameStartIndex, groupNameEndIndex);
    228228                            var capture = namedCaptures[groupName];
    229229                            if (capture !== @undefined)
    … …  
    239239                    break;
    240240                default:
    241                     var chCode = ch.charCodeAt(0);
     241                    var chCode = ch.@charCodeAt(0);
    242242                    if (chCode >= 0x30 && chCode <= 0x39) {
    243243                        var originalStart = start - 1;
    … …  
    246246                        var n = chCode - 0x30;
    247247                        if (n > m) {
    248                             result = result + replacement.substring(originalStart, start);
     248                            result = result + @stringSubstringInternal.@call(replacement, originalStart, start);
    249249                            break;
    250250                        }
    251251
    252252                        if (start < replacementLength) {
    253                             var nextChCode = replacement.charCodeAt(start);
     253                            var nextChCode = replacement.@charCodeAt(start);
    254254                            if (nextChCode >= 0x30 && nextChCode <= 0x39) {
    255255                                var nn = 10 * n + nextChCode - 0x30;
    … …  
    262262
    263263                        if (n == 0) {
    264                             result = result + replacement.substring(originalStart, start);
     264                            result = result + @stringSubstringInternal.@call(replacement, originalStart, start);
    265265                            break;
    266266                        }
    … …  
    276276        }
    277277
    278         return result + replacement.substring(lastStart);
     278        return result + @stringSubstringInternal.@call(replacement, lastStart);
    279279    }
    280280
    … …  
    314314                var matchStr = @toString(result[0]);
    315315
    316                 if (!matchStr.length)
    317                     regexp.lastIndex = @advanceStringIndex(str, regexp.lastIndex, unicode);
     316                if (!matchStr.length) {
     317                    var thisIndex = @toLength(regexp.lastIndex);
     318                    regexp.lastIndex = @advanceStringIndex(str, thisIndex, unicode);
     319                }
    318320            }
    319321        }
    … …  
    322324    var accumulatedResult = "";
    323325    var nextSourcePosition = 0;
    324     var lastPosition = 0;
    325326
    326327    for (var i = 0, resultListLength = resultList.length; i < resultListLength; ++i) {
    … …  
    347348
    348349        if (functionalReplace) {
    349             var replacerArgs = [ matched ].concat(captures);
     350            var replacerArgs = [ matched ];
     351            for (var j = 0; j < captures.length; j++)
     352                replacerArgs.@push(captures[j]);
     353
    350354            replacerArgs.@push(position);
    351355            replacerArgs.@push(str);
    … …  
    363367        }
    364368
    365         if (position >= nextSourcePosition && position >= lastPosition) {
    366             accumulatedResult = accumulatedResult + str.substring(nextSourcePosition, position) + replacement;
     369        if (position >= nextSourcePosition) {
     370            accumulatedResult = accumulatedResult + @stringSubstringInternal.@call(str, nextSourcePosition, position) + replacement;
    367371            nextSourcePosition = position + matchLength;
    368             lastPosition = position;
    369372        }
    370373    }
    … …  
    373376        return  accumulatedResult;
    374377
    375     return accumulatedResult + str.substring(nextSourcePosition);
     378    return accumulatedResult + @stringSubstringInternal.@call(str, nextSourcePosition);
    376379}
    377380
    … …  
    563566            else {
    564567                // 1. Let T be a String value equal to the substring of S consisting of the elements at indices p (inclusive) through q (exclusive).
    565                 var subStr = @stringSubstrInternal.@call(str, position, matchPosition - position);
     568                var subStr = @stringSubstringInternal.@call(str, position, matchPosition);
    566569                // 2. Perform ! CreateDataProperty(A, ! ToString(lengthA), T).
    567570                // 3. Let lengthA be lengthA + 1.
    … …  
    598601    }
    599602    // 20. Let T be a String value equal to the substring of S consisting of the elements at indices p (inclusive) through size (exclusive).
    600     var remainingStr = @stringSubstrInternal.@call(str, position, size);
     603    var remainingStr = @stringSubstringInternal.@call(str, position, size);
    601604    // 21. Perform ! CreateDataProperty(A, ! ToString(lengthA), T).
    602605    @putByValDirect(result, result.length, remainingStr);

  • trunk/Source/JavaScriptCore/builtins/StringPrototype.js

    r257681 r259029  
    114114    }
    115115    if (remainingCharacters)
    116         result += @stringSubstrInternal.@call(string, 0, remainingCharacters);
     116        result += @stringSubstringInternal.@call(string, 0, remainingCharacters);
    117117    return result;
    118118}

  • trunk/Source/JavaScriptCore/bytecode/LinkTimeConstant.h

    r258801 r259029  
    9191    v(regExpTestFast, nullptr) \
    9292    v(stringIncludesInternal, nullptr) \
     93    v(stringIndexOfInternal, nullptr) \
    9394    v(stringSplitFast, nullptr) \
    94     v(stringSubstrInternal, nullptr) \
     95    v(stringSubstringInternal, nullptr) \
    9596    v(makeBoundFunction, nullptr) \
    9697    v(hasOwnLengthProperty, nullptr) \

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r258801 r259029  
    11431143            init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 1, String(), builtinStringIncludesInternal));
    11441144        });
     1145    m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::stringIndexOfInternal)].initLater([] (const Initializer<JSCell>& init) {
     1146            init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 1, String(), builtinStringIndexOfInternal));
     1147        });
    11451148    m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::stringSplitFast)].initLater([] (const Initializer<JSCell>& init) {
    11461149            init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 2, String(), stringProtoFuncSplitFast));
    11471150        });
    1148     m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::stringSubstrInternal)].initLater([] (const Initializer<JSCell>& init) {
    1149             init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 2, String(), builtinStringSubstrInternal));
     1151    m_linkTimeConstants[static_cast<unsigned>(LinkTimeConstant::stringSubstringInternal)].initLater([] (const Initializer<JSCell>& init) {
     1152            init.set(JSFunction::create(init.vm, jsCast<JSGlobalObject*>(init.owner), 2, String(), builtinStringSubstringInternal));
    11501153        });
    11511154

  • trunk/Source/JavaScriptCore/runtime/StringPrototype.cpp

    r258443 r259029  
    10771077}
    10781078
    1079 EncodedJSValue JSC_HOST_CALL stringProtoFuncIndexOf(JSGlobalObject* globalObject, CallFrame* callFrame)
     1079static EncodedJSValue stringIndexOfImpl(JSGlobalObject* globalObject, CallFrame* callFrame)
    10801080{
    10811081    VM& vm = globalObject->vm();
    … …  
    11221122        return JSValue::encode(jsNumber(-1));
    11231123    return JSValue::encode(jsNumber(result));
     1124}
     1125
     1126EncodedJSValue JSC_HOST_CALL stringProtoFuncIndexOf(JSGlobalObject* globalObject, CallFrame* callFrame)
     1127{
     1128    return stringIndexOfImpl(globalObject, callFrame);
     1129}
     1130
     1131EncodedJSValue JSC_HOST_CALL builtinStringIndexOfInternal(JSGlobalObject* globalObject, CallFrame* callFrame)
     1132{
     1133    ASSERT(callFrame->thisValue().isString());
     1134    ASSERT(callFrame->argument(0).isString());
     1135    ASSERT(callFrame->argument(1).isNumber() || callFrame->argument(1).isUndefined());
     1136    return stringIndexOfImpl(globalObject, callFrame);
    11241137}
    11251138
    … …  
    13661379}
    13671380
    1368 static EncodedJSValue stringProtoFuncSubstrImpl(JSGlobalObject* globalObject, CallFrame* callFrame)
     1381EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstr(JSGlobalObject* globalObject, CallFrame* callFrame)
    13691382{
    13701383    VM& vm = globalObject->vm();
    … …  
    14101423}
    14111424
    1412 EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstr(JSGlobalObject* globalObject, CallFrame* callFrame)
    1413 {
    1414     return stringProtoFuncSubstrImpl(globalObject, callFrame);
    1415 }
    1416 
    1417 EncodedJSValue JSC_HOST_CALL builtinStringSubstrInternal(JSGlobalObject* globalObject, CallFrame* callFrame)
    1418 {
    1419     // @substrInternal should not have any observable side effects (e.g. it should not call
    1420     // GetMethod(..., @@toPrimitive) on the thisValue).
    1421 
    1422     // It is ok to use the default stringProtoFuncSubstr as the implementation of
    1423     // @substrInternal because @substrInternal will only be called by builtins, which will
    1424     // guarantee that we only pass it a string thisValue. As a result, stringProtoFuncSubstr
    1425     // will not need to call toString() on the thisValue, and there will be no observable
    1426     // side-effects.
    1427     ASSERT(callFrame->thisValue().isString());
    1428     return stringProtoFuncSubstrImpl(globalObject, callFrame);
    1429 }
    1430 
    1431 EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstring(JSGlobalObject* globalObject, CallFrame* callFrame)
     1425static EncodedJSValue stringSubstringImpl(JSGlobalObject* globalObject, CallFrame* callFrame)
    14321426{
    14331427    VM& vm = globalObject->vm();
    … …  
    14711465    unsigned substringLength = static_cast<unsigned>(end) - substringStart;
    14721466    RELEASE_AND_RETURN(scope, JSValue::encode(jsSubstring(globalObject, jsString, substringStart, substringLength)));
     1467}
     1468
     1469EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstring(JSGlobalObject* globalObject, CallFrame* callFrame)
     1470{
     1471    return stringSubstringImpl(globalObject, callFrame);
     1472}
     1473
     1474EncodedJSValue JSC_HOST_CALL builtinStringSubstringInternal(JSGlobalObject* globalObject, CallFrame* callFrame)
     1475{
     1476    ASSERT(callFrame->thisValue().isString());
     1477    ASSERT(callFrame->argument(0).isNumber());
     1478    ASSERT(callFrame->argument(1).isNumber() || callFrame->argument(1).isUndefined());
     1479    return stringSubstringImpl(globalObject, callFrame);   
    14731480}
    14741481

  • trunk/Source/JavaScriptCore/runtime/StringPrototype.h

    r253432 r259029  
    6161EncodedJSValue JSC_HOST_CALL stringProtoFuncSplitFast(JSGlobalObject*, CallFrame*);
    6262
    63 EncodedJSValue JSC_HOST_CALL builtinStringSubstrInternal(JSGlobalObject*, CallFrame*);
     63EncodedJSValue JSC_HOST_CALL builtinStringSubstringInternal(JSGlobalObject*, CallFrame*);
    6464EncodedJSValue JSC_HOST_CALL builtinStringIncludesInternal(JSGlobalObject*, CallFrame*);
     65EncodedJSValue JSC_HOST_CALL builtinStringIndexOfInternal(JSGlobalObject*, CallFrame*);
    6566
    6667} // namespace JSC
Note: See TracChangeset for help on using the changeset viewer.