Diff
Modified: trunk/LayoutTests/ChangeLog (202953 => 202954)
--- trunk/LayoutTests/ChangeLog 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/LayoutTests/ChangeLog 2016-07-08 03:13:11 UTC (rev 202954)
@@ -1,3 +1,19 @@
+2016-07-07 Joseph Pecoraro <[email protected]>
+
+ Unexpected "Out of memory" error for "x".repeat(-1)
+ https://bugs.webkit.org/show_bug.cgi?id=159529
+
+ Reviewed by Benjamin Poulain.
+
+ 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:
+
2016-07-07 Ryosuke Niwa <[email protected]>
Replace scoped flag in Event by composed flag
Modified: trunk/LayoutTests/js/script-tests/string-repeat.js (202953 => 202954)
--- trunk/LayoutTests/js/script-tests/string-repeat.js 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/LayoutTests/js/script-tests/string-repeat.js 2016-07-08 03:13:11 UTC (rev 202954)
@@ -1,5 +1,14 @@
-description("This test checks the ES6 string functions repeat().");
+description("This test checks String.prototype.repeat.");
+shouldBe('String.prototype.repeat.length', '1');
+shouldBeEqualToString('String.prototype.repeat.name', 'repeat');
+shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").configurable', 'true');
+shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").enumerable', 'false');
+shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").writable', 'true');
+shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").get', 'undefined');
+shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").set', 'undefined');
+shouldBe('Object.getOwnPropertyDescriptor(String.prototype, "repeat").value', 'String.prototype.repeat');
+
shouldBe("'foo bar'.repeat(+0)", "''");
shouldBe("'foo bar'.repeat(-0)", "''");
shouldBe("'foo bar'.repeat(1)", "'foo bar'");
@@ -31,9 +40,12 @@
shouldBe("''.repeat(0xFFFFFFFF + 1)", "''");
// Check range errors.
-shouldThrow("'foo bar'.repeat(-1)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity'");
-shouldThrow("'foo bar'.repeat(Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity'");
-shouldThrow("'foo bar'.repeat(-Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity'");
+shouldThrow("'x'.repeat(-1)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
+shouldThrow("'x'.repeat(Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
+shouldThrow("'x'.repeat(-Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
+shouldThrow("'foo bar'.repeat(-1)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
+shouldThrow("'foo bar'.repeat(Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
+shouldThrow("'foo bar'.repeat(-Infinity)", "'RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity'");
// Check out of memory errors.
shouldThrow("'f'.repeat(0xFFFFFFFF)", "'Error: Out of memory'");
@@ -43,39 +55,48 @@
shouldThrow("'foo bar'.repeat(0xFFFFFFFF)", "'Error: Out of memory'");
shouldThrow("'foo bar'.repeat(0xFFFFFFFF + 1)", "'Error: Out of memory'");
-// Check side effects in repeat.
-var sideEffect = "";
-var stringRepeated = new String("foo bar");
-stringRepeated.toString = function() {
- sideEffect += "A";
- return this;
-}
-var count = new Number(2);
-count.valueOf = function() {
- sideEffect += "B";
- return this;
-}
-// Calling stringRepeated.repeat implicitly calls stringRepeated.toString(),
-// and count.valueOf(), in that respective order.
-shouldBe("stringRepeated.repeat(count)", "'foo barfoo bar'");
-shouldBe("sideEffect == 'AB'", "true");
+var sideEffect, stringRepeated, count;
+function checkSideEffects(str) {
+ // Check side effects in repeat.
+ sideEffect = "";
+ stringRepeated = new String(str);
+ stringRepeated.toString = function() {
+ sideEffect += "A";
+ return this;
+ }
+ count = new Number(2);
+ count.valueOf = function() {
+ sideEffect += "B";
+ return this;
+ }
+ // Calling stringRepeated.repeat implicitly calls stringRepeated.toString(),
+ // and count.valueOf(), in that respective order.
+ shouldBe("stringRepeated.repeat(count)", "'" + str + str + "'");
+ shouldBe("sideEffect == 'AB'", "true");
-// If stringRepeated throws an exception count.valueOf() is not called.
-stringRepeated.toString = function() {
- throw "error";
+ // If stringRepeated.toString() throws an exception count.valueOf() is not called.
+ stringRepeated.toString = function() {
+ throw "error";
+ }
+ sideEffect = "";
+ shouldThrow("stringRepeated.repeat(count)", "'error'");
+ shouldBe("sideEffect == ''", "true");
+
+ // If count throws an exception stringRepeated.toString() was called.
+ stringRepeated.toString = function() {
+ sideEffect += "A";
+ return this;
+ }
+ count.valueOf = function() {
+ throw "error";
+ }
+ sideEffect = "";
+ shouldThrow("stringRepeated.repeat(count)", "'error'");
+ shouldBe("sideEffect == 'A'", "true");
}
-sideEffect = "";
-shouldThrow("stringRepeated.repeat(count)", "'error'");
-shouldBe("sideEffect == ''", "true");
-// If count throws an exception stringRepeated.toString() was called.
-stringRepeated.toString = function() {
- sideEffect += "A";
- return this;
-}
-count.valueOf = function() {
- throw "error";
-}
-sideEffect = "";
-shouldThrow("stringRepeated.repeat(count)", "'error'");
-shouldBe("sideEffect == 'A'", "true");
+// Fast path for single character string.
+checkSideEffects("x");
+
+// Slow path for any other string.
+checkSideEffects("foo bar");
Modified: trunk/LayoutTests/js/string-repeat-expected.txt (202953 => 202954)
--- trunk/LayoutTests/js/string-repeat-expected.txt 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/LayoutTests/js/string-repeat-expected.txt 2016-07-08 03:13:11 UTC (rev 202954)
@@ -1,8 +1,16 @@
-This test checks the ES6 string functions repeat().
+This test checks String.prototype.repeat.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS String.prototype.repeat.length is 1
+PASS String.prototype.repeat.name is "repeat"
+PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").configurable is true
+PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").enumerable is false
+PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").writable is true
+PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").get is undefined
+PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").set is undefined
+PASS Object.getOwnPropertyDescriptor(String.prototype, "repeat").value is String.prototype.repeat
PASS 'foo bar'.repeat(+0) is ''
PASS 'foo bar'.repeat(-0) is ''
PASS 'foo bar'.repeat(1) is 'foo bar'
@@ -30,9 +38,12 @@
PASS ''.repeat(1000) is ''
PASS ''.repeat(0xFFFFFFFF) is ''
PASS ''.repeat(0xFFFFFFFF + 1) is ''
-PASS 'foo bar'.repeat(-1) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity.
-PASS 'foo bar'.repeat(Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity.
-PASS 'foo bar'.repeat(-Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity.
+PASS 'x'.repeat(-1) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
+PASS 'x'.repeat(Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
+PASS 'x'.repeat(-Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
+PASS 'foo bar'.repeat(-1) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
+PASS 'foo bar'.repeat(Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
+PASS 'foo bar'.repeat(-Infinity) threw exception RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity.
PASS 'f'.repeat(0xFFFFFFFF) threw exception Error: Out of memory.
PASS 'f'.repeat(0xFFFFFFFF + 1) threw exception Error: Out of memory.
PASS 'foo'.repeat(0xFFFFFFFFF) threw exception Error: Out of memory.
@@ -39,6 +50,12 @@
PASS 'foo'.repeat(0xFFFFFFFFF + 1) threw exception Error: Out of memory.
PASS 'foo bar'.repeat(0xFFFFFFFF) threw exception Error: Out of memory.
PASS 'foo bar'.repeat(0xFFFFFFFF + 1) threw exception Error: Out of memory.
+PASS stringRepeated.repeat(count) is 'xx'
+PASS sideEffect == 'AB' is true
+PASS stringRepeated.repeat(count) threw exception error.
+PASS sideEffect == '' is true
+PASS stringRepeated.repeat(count) threw exception error.
+PASS sideEffect == 'A' is true
PASS stringRepeated.repeat(count) is 'foo barfoo bar'
PASS sideEffect == 'AB' is true
PASS stringRepeated.repeat(count) threw exception error.
Modified: trunk/Source/_javascript_Core/ChangeLog (202953 => 202954)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-08 03:13:11 UTC (rev 202954)
@@ -1,3 +1,29 @@
+2016-07-07 Joseph Pecoraro <[email protected]>
+
+ Unexpected "Out of memory" error for "x".repeat(-1)
+ https://bugs.webkit.org/show_bug.cgi?id=159529
+
+ Reviewed by Benjamin Poulain.
+
+ * 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.
+
2016-07-07 Benjamin Poulain <[email protected]>
[JSC] Array.prototype[Symbol.unscopables] should have the "includes" property
Modified: trunk/Source/_javascript_Core/builtins/StringPrototype.js (202953 => 202954)
--- trunk/Source/_javascript_Core/builtins/StringPrototype.js 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/Source/_javascript_Core/builtins/StringPrototype.js 2016-07-08 03:13:11 UTC (rev 202954)
@@ -51,35 +51,26 @@
{
"use strict";
- var repeatCount = @toInteger(count);
- if (repeatCount < 0 || repeatCount === @Infinity)
- throw new @RangeError("String.prototype.repeat argument must be greater than or equal to 0 and not be infinity");
-
// Return an empty string.
- if (repeatCount === 0 || string.length === 0)
+ if (count === 0 || string.length === 0)
return "";
// Return the original string.
- if (repeatCount === 1)
+ if (count === 1)
return string;
- if (string.length * repeatCount > @MAX_STRING_LENGTH)
+ if (string.length * count > @MAX_STRING_LENGTH)
throw new @Error("Out of memory");
- if (string.length === 1) {
- // Here, |repeatCount| is always Int32.
- return @repeatCharacter(string, repeatCount);
- }
-
- // Bit operation onto |repeatCount| is safe because |repeatCount| should be within Int32 range,
+ // Bit operation onto |count| is safe because |count| should be within Int32 range,
// Repeat log N times to generate the repeated string rope.
var result = "";
var operand = string;
while (true) {
- if (repeatCount & 1)
+ if (count & 1)
result += operand;
- repeatCount >>= 1;
- if (!repeatCount)
+ count >>= 1;
+ if (!count)
return result;
operand += operand;
}
@@ -121,6 +112,11 @@
}
var string = @toString(this);
+ count = @toInteger(count);
+
+ if (count < 0 || count === @Infinity)
+ throw new @RangeError("String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity");
+
if (string.length === 1) {
var result = @repeatCharacter(string, count);
if (result !== null)
Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (202953 => 202954)
--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2016-07-08 03:13:11 UTC (rev 202954)
@@ -749,25 +749,14 @@
}
template <typename CharacterType>
-static inline JSValue repeatCharacter(ExecState* exec, CharacterType character, unsigned repeatCount)
-{
- CharacterType* buffer = nullptr;
- auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
- if (!impl)
- return throwOutOfMemoryError(exec);
-
- std::fill_n(buffer, repeatCount, character);
-
- return jsString(exec, WTFMove(impl));
-}
-
-template <typename CharacterType>
static inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
{
CharacterType* buffer = nullptr;
auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
- if (!impl)
- return throwOutOfMemoryError(&exec), nullptr;
+ if (!impl) {
+ throwOutOfMemoryError(&exec);
+ return nullptr;
+ }
std::fill_n(buffer, repeatCount, character);
@@ -788,6 +777,8 @@
return JSValue::encode(jsNull());
int32_t repeatCount = exec->uncheckedArgument(1).asInt32();
+ ASSERT(repeatCount >= 0);
+
UChar character = string->view(exec)[0];
if (!(character & ~0xff))
return JSValue::encode(repeatCharacter(*exec, static_cast<LChar>(character), repeatCount));
Modified: trunk/Source/_javascript_Core/tests/stress/string-repeat-edge-cases.js (202953 => 202954)
--- trunk/Source/_javascript_Core/tests/stress/string-repeat-edge-cases.js 2016-07-08 01:57:44 UTC (rev 202953)
+++ trunk/Source/_javascript_Core/tests/stress/string-repeat-edge-cases.js 2016-07-08 03:13:11 UTC (rev 202954)
@@ -49,9 +49,9 @@
shouldBe(String.prototype.repeat.call("", 0xFFFFFFFFF), "");
shouldThrow(() => {
String.prototype.repeat.call("", Infinity);
- }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity`);
+ }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity`);
shouldThrow(() => {
String.prototype.repeat.call("", -2000);
- }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be infinity`);
+ }, `RangeError: String.prototype.repeat argument must be greater than or equal to 0 and not be Infinity`);
}
