⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 202954 in webkit


Ignore:
Timestamp:
Jul 7, 2016, 8:13:11 PM (10 years ago)
Author:
commit-queue@webkit.org
Message:

Unexpected "Out of memory" error for "x".repeat(-1)
​https://bugs.webkit.org/show_bug.cgi?id=159529

Patch by Joseph Pecoraro <Joseph Pecoraro> on 2016-07-07
Reviewed by Benjamin Poulain.

Source/JavaScriptCore:

  • builtins/StringPrototype.js:

(globalPrivate.repeatSlowPath):
(repeat):
Move the @toInteger and range checking to the always path,
since the spec does say it should always happen. Also remove
the duplication of the fast path here.

  • runtime/StringPrototype.cpp:

(JSC::repeatCharacter):
Remove unused function.

(JSC::stringProtoFuncRepeatCharacter):
ASSERT if given a negative number. This is a private function
only used internally.

  • tests/stress/string-repeat-edge-cases.js:

(shouldThrow):
Update expected error message.

LayoutTests:

Extended test coverage for:

  • function properties
  • fast path with invalid counts
  • observable side effects for fast path which were wrong before
  • js/script-tests/string-repeat.js:
  • js/string-repeat-expected.txt:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r202953 r202954  
     12016-07-07  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Unexpected "Out of memory" error for "x".repeat(-1)
     4        https://bugs.webkit.org/show_bug.cgi?id=159529
     5
     6        Reviewed by Benjamin Poulain.
     7
     8        Extended test coverage for:
     9
     10          - function properties
     11          - fast path with invalid counts
     12          - observable side effects for fast path which were wrong before
     13
     14        * js/script-tests/string-repeat.js:
     15        * js/string-repeat-expected.txt:
     16
    1172016-07-07  Ryosuke Niwa  <rniwa@webkit.org>
    218

  • trunk/LayoutTests/js/script-tests/string-repeat.js

    r198838 r202954  
    1 description("This test checks the ES6 string functions repeat().");
     1description("This test checks String.prototype.repeat.");
     2
     3shouldBe('String.prototype.repeat.length', '1');
     4shouldBeEqualToString('String.prototype.repeat.name', 'repeat');
     5shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").configurable', 'true');
     6shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").enumerable', 'false');
     7shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").writable', 'true');
     8shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").get', 'undefined');
     9shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").set', 'undefined');
     10shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").value', 'String.prototype.repeat');
    211
    312shouldBe("'foo bar'.repeat(+0)", "''");
    … …  
    3241
    3342// Check range errors.
    34 shouldThrow("'foo bar'.repeat(-1)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity'");
    35 shouldThrow("'foo bar'.repeat(Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity'");
    36 shouldThrow("'foo bar'.repeat(-Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity'");
     43shouldThrow("'x'.repeat(-1)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
     44shouldThrow("'x'.repeat(Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
     45shouldThrow("'x'.repeat(-Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
     46shouldThrow("'foo bar'.repeat(-1)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
     47shouldThrow("'foo bar'.repeat(Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
     48shouldThrow("'foo bar'.repeat(-Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
    3749
    3850// Check out of memory errors.
    … …  
    4456shouldThrow("'foo bar'.repeat(0xFFFFFFFF + 1)", "'Error: Out of memory'");
    4557
    46 // Check side effects in repeat.
    47 var sideEffect = "";
    48 var stringRepeated = new String("foo bar");
    49 stringRepeated.toString = function() {
    50     sideEffect += "A";
    51     return this;
     58var sideEffect, stringRepeated, count;
     59function checkSideEffects(str) {
     60    // Check side effects in repeat.
     61    sideEffect = "";
     62    stringRepeated = new String(str);
     63    stringRepeated.toString = function() {
     64        sideEffect += "A";
     65        return this;
     66    }
     67    count = new Number(2);
     68    count.valueOf = function() {
     69        sideEffect += "B";
     70        return this;
     71    }
     72    // Calling stringRepeated.repeat implicitly calls stringRepeated.toString(),
     73    // and count.valueOf(), in that respective order.
     74    shouldBe("stringRepeated.repeat(count)", "'" + str + str + "'");
     75    shouldBe("sideEffect == 'AB'", "true");
     76
     77    // If stringRepeated.toString() throws an exception count.valueOf() is not called.
     78    stringRepeated.toString = function() {
     79        throw "error";
     80    }
     81    sideEffect = "";
     82    shouldThrow("stringRepeated.repeat(count)", "'error'");
     83    shouldBe("sideEffect == ''", "true");
     84
     85    // If count throws an exception stringRepeated.toString() was called.
     86    stringRepeated.toString = function() {
     87        sideEffect += "A";
     88        return this;
     89    }
     90    count.valueOf = function() {
     91        throw "error";
     92    }
     93    sideEffect = "";
     94    shouldThrow("stringRepeated.repeat(count)", "'error'");
     95    shouldBe("sideEffect == 'A'", "true");
    5296}
    53 var count = new Number(2);
    54 count.valueOf = function() {
    55     sideEffect += "B";
    56     return this;
    57 }
    58 // Calling stringRepeated.repeat implicitly calls stringRepeated.toString(),
    59 // and count.valueOf(), in that respective order.
    60 shouldBe("stringRepeated.repeat(count)", "'foo barfoo bar'");
    61 shouldBe("sideEffect == 'AB'", "true");
    6297
    63 // If stringRepeated throws an exception count.valueOf() is not called.
    64 stringRepeated.toString = function() {
    65     throw "error";
    66 }
    67 sideEffect = "";
    68 shouldThrow("stringRepeated.repeat(count)", "'error'");
    69 shouldBe("sideEffect == ''", "true");
     98// Fast path for single character string.
     99checkSideEffects("x");
    70100
    71 // If count throws an exception stringRepeated.toString() was called.
    72 stringRepeated.toString = function() {
    73     sideEffect += "A";
    74     return this;
    75 }
    76 count.valueOf = function() {
    77     throw "error";
    78 }
    79 sideEffect = "";
    80 shouldThrow("stringRepeated.repeat(count)", "'error'");
    81 shouldBe("sideEffect == 'A'", "true");
     101// Slow path for any other string.
     102checkSideEffects("foo bar");

  • trunk/LayoutTests/js/string-repeat-expected.txt

    r198838 r202954  
    1 This test checks the ES6 string functions repeat().
     1This test checks String.prototype.repeat.
    22
    33On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
    44
    55
     6PASS String.prototype.repeat.length is 1
     7PASS String.prototype.repeat.name is "repeat"
     8PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").configurable is true
     9PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").enumerable is false
     10PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").writable is true
     11PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").get is undefined
     12PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").set is undefined
     13PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").value is String.prototype.repeat
    614PASS 'foo bar'.repeat(+0) is ''
    715PASS 'foo bar'.repeat(-0) is ''
    … …  
    3139PASS ''.repeat(0xFFFFFFFF) is ''
    3240PASS ''.repeat(0xFFFFFFFF + 1) is ''
    33 PASS 'foo bar'.repeat(-1) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity.
    34 PASS 'foo bar'.repeat(Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity.
    35 PASS 'foo bar'.repeat(-Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity.
     41PASS 'x'.repeat(-1) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
     42PASS 'x'.repeat(Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
     43PASS 'x'.repeat(-Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
     44PASS 'foo bar'.repeat(-1) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
     45PASS 'foo bar'.repeat(Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
     46PASS 'foo bar'.repeat(-Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
    3647PASS 'f'.repeat(0xFFFFFFFF) threw exception Error: Out of memory.
    3748PASS 'f'.repeat(0xFFFFFFFF + 1) threw exception Error: Out of memory.
    … …  
    4051PASS 'foo bar'.repeat(0xFFFFFFFF) threw exception Error: Out of memory.
    4152PASS 'foo bar'.repeat(0xFFFFFFFF + 1) threw exception Error: Out of memory.
     53PASS stringRepeated.repeat(count) is 'xx'
     54PASS sideEffect == 'AB' is true
     55PASS stringRepeated.repeat(count) threw exception error.
     56PASS sideEffect == '' is true
     57PASS stringRepeated.repeat(count) threw exception error.
     58PASS sideEffect == 'A' is true
    4259PASS stringRepeated.repeat(count) is 'foo barfoo bar'
    4360PASS sideEffect == 'AB' is true

  • trunk/Source/JavaScriptCore/ChangeLog

    r202943 r202954  
     12016-07-07  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Unexpected "Out of memory" error for "x".repeat(-1)
     4        https://bugs.webkit.org/show_bug.cgi?id=159529
     5
     6        Reviewed by Benjamin Poulain.
     7
     8        * builtins/StringPrototype.js:
     9        (globalPrivate.repeatSlowPath):
     10        (repeat):
     11        Move the @toInteger and range checking to the always path,
     12        since the spec does say it should always happen. Also remove
     13        the duplication of the fast path here.
     14
     15        * runtime/StringPrototype.cpp:
     16        (JSC::repeatCharacter):
     17        Remove unused function.
     18
     19        (JSC::stringProtoFuncRepeatCharacter):
     20        ASSERT if given a negative number. This is a private function
     21        only used internally.
     22
     23        * tests/stress/string-repeat-edge-cases.js:
     24        (shouldThrow):
     25        Update expected error message.
     26
    1272016-07-07  Benjamin Poulain  <benjamin@webkit.org>
    228

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

    r202280 r202954  
    5252    "use strict";
    5353
    54     var repeatCount = @toInteger(count);
    55     if (repeatCount < 0 || repeatCount === @Infinity)
    56         throw new @RangeError("String.prototype.repeat argument must be greater than or equal to 0 and not be infinity");
    57 
    5854    // Return an empty string.
    59     if (repeatCount === 0 || string.length === 0)
     55    if (count === 0 || string.length === 0)
    6056        return "";
    6157
    6258    // Return the original string.
    63     if (repeatCount === 1)
     59    if (count === 1)
    6460        return string;
    6561
    66     if (string.length * repeatCount > @MAX_STRING_LENGTH)
     62    if (string.length * count > @MAX_STRING_LENGTH)
    6763        throw new @Error("Out of memory");
    6864
    69     if (string.length === 1) {
    70         // Here, |repeatCount| is always Int32.
    71         return @repeatCharacter(string, repeatCount);
    72     }
    73 
    74     // Bit operation onto |repeatCount| is safe because |repeatCount| should be within Int32 range,
     65    // Bit operation onto |count| is safe because |count| should be within Int32 range,
    7566    // Repeat log N times to generate the repeated string rope.
    7667    var result = "";
    7768    var operand = string;
    7869    while (true) {
    79         if (repeatCount & 1)
     70        if (count & 1)
    8071            result += operand;
    81         repeatCount >>= 1;
    82         if (!repeatCount)
     72        count >>= 1;
     73        if (!count)
    8374            return result;
    8475        operand += operand;
    … …  
    122113
    123114    var string = @toString(this);
     115    count = @toInteger(count);
     116
     117    if (count < 0 || count === @Infinity)
     118        throw new @RangeError("String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity");
     119
    124120    if (string.length === 1) {
    125121        var result = @repeatCharacter(string, count);

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

    r202916 r202954  
    750750
    751751template <typename CharacterType>
    752 static inline JSValue repeatCharacter(ExecState* exec, CharacterType character, unsigned repeatCount)
     752static inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
    753753{
    754754    CharacterType* buffer = nullptr;
    755755    auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
    756     if (!impl)
    757         return throwOutOfMemoryError(exec);
    758 
    759     std::fill_n(buffer, repeatCount, character);
    760 
    761     return jsString(exec, WTFMove(impl));
    762 }
    763 
    764 template <typename CharacterType>
    765 static inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
    766 {
    767     CharacterType* buffer = nullptr;
    768     auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
    769     if (!impl)
    770         return throwOutOfMemoryError(&exec), nullptr;
     756    if (!impl) {
     757        throwOutOfMemoryError(&exec);
     758        return nullptr;
     759    }
    771760
    772761    std::fill_n(buffer, repeatCount, character);
    … …  
    789778
    790779    int32_t repeatCount = exec->uncheckedArgument(1).asInt32();
     780    ASSERT(repeatCount >= 0);
     781
    791782    UChar character = string->view(exec)[0];
    792783    if (!(character & ~0xff))

  • trunk/Source/JavaScriptCore/tests/stress/string-repeat-edge-cases.js

    r198838 r202954  
    5050    shouldThrow(() => {
    5151        String.prototype.repeat.call("", Infinity);
    52     }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity`);
     52    }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity`);
    5353
    5454    shouldThrow(() => {
    5555        String.prototype.repeat.call("", -2000);
    56     }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity`);
     56    }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity`);
    5757}
Note: See TracChangeset for help on using the changeset viewer.