Diff
Modified: trunk/JSTests/ChangeLog (208698 => 208699)
--- trunk/JSTests/ChangeLog 2016-11-14 19:26:20 UTC (rev 208698)
+++ trunk/JSTests/ChangeLog 2016-11-14 19:42:41 UTC (rev 208699)
@@ -1,5 +1,15 @@
2016-11-14 Mark Lam <mark....@apple.com>
+ Some of JSStringView::SafeView methods are not idiomatically safe for JSString to StringView conversions.
+ https://bugs.webkit.org/show_bug.cgi?id=164701
+ <rdar://problem/27462104>
+
+ Reviewed by Darin Adler.
+
+ * stress/string-prototype-charCodeAt-on-too-long-rope.js: Added.
+
+2016-11-14 Mark Lam <mark....@apple.com>
+
RegExpObject::exec/match should handle errors gracefully.
https://bugs.webkit.org/show_bug.cgi?id=155145
<rdar://problem/27435934>
Added: trunk/JSTests/stress/string-prototype-charCodeAt-on-too-long-rope.js (0 => 208699)
--- trunk/JSTests/stress/string-prototype-charCodeAt-on-too-long-rope.js (rev 0)
+++ trunk/JSTests/stress/string-prototype-charCodeAt-on-too-long-rope.js 2016-11-14 19:42:41 UTC (rev 208699)
@@ -0,0 +1,32 @@
+//@ if $buildType == "debug" then runFTLNoCJIT("--maxSingleAllocationSize=20000000") else skip end
+
+function shouldEqual(actual, expected) {
+ if (actual != expected) {
+ throw "ERROR: expect " + expected + ", actual " + actual;
+ }
+}
+
+s0 = "";
+s1 = "NaNxxxxx";
+
+try {
+ for (var count = 0; count < 27; count++) {
+ var oldS1 = s1;
+ s1 += s1;
+ s0 = oldS1;
+ }
+} catch (e) { }
+
+try {
+ s1 += s0;
+} catch (e) { }
+
+var exception;
+try {
+ for (var v of s1) { }
+} catch (e) {
+ exception = e;
+}
+
+shouldEqual(exception, "Error: Out of memory");
+
Modified: trunk/Source/_javascript_Core/ChangeLog (208698 => 208699)
--- trunk/Source/_javascript_Core/ChangeLog 2016-11-14 19:26:20 UTC (rev 208698)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-11-14 19:42:41 UTC (rev 208699)
@@ -1,5 +1,43 @@
2016-11-14 Mark Lam <mark....@apple.com>
+ Some of JSStringView::SafeView methods are not idiomatically safe for JSString to StringView conversions.
+ https://bugs.webkit.org/show_bug.cgi?id=164701
+ <rdar://problem/27462104>
+
+ Reviewed by Darin Adler.
+
+ The characters8(), characters16(), and operator[] in JSString::SafeView converts
+ the underlying JSString to a StringView via get(), and then uses the StringView
+ without first checking if an exception was thrown during the conversion. This is
+ unsafe because the conversion may have failed.
+
+ Instead, we should remove these 3 convenience methods, and make the caller
+ explicitly call get() and do the appropriate exception checks before using the
+ StringView.
+
+ * runtime/JSGlobalObjectFunctions.cpp:
+ (JSC::toStringView):
+ (JSC::encode):
+ (JSC::decode):
+ (JSC::globalFuncParseInt):
+ (JSC::globalFuncEscape):
+ (JSC::globalFuncUnescape):
+ (JSC::toSafeView): Deleted.
+ * runtime/JSONObject.cpp:
+ (JSC::JSONProtoFuncParse):
+ * runtime/JSString.h:
+ (JSC::JSString::SafeView::length):
+ (JSC::JSString::SafeView::characters8): Deleted.
+ (JSC::JSString::SafeView::characters16): Deleted.
+ (JSC::JSString::SafeView::operator[]): Deleted.
+ * runtime/StringPrototype.cpp:
+ (JSC::stringProtoFuncRepeatCharacter):
+ (JSC::stringProtoFuncCharAt):
+ (JSC::stringProtoFuncCharCodeAt):
+ (JSC::stringProtoFuncNormalize):
+
+2016-11-14 Mark Lam <mark....@apple.com>
+
RegExpObject::exec/match should handle errors gracefully.
https://bugs.webkit.org/show_bug.cgi?id=155145
<rdar://problem/27435934>
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp (208698 => 208699)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2016-11-14 19:26:20 UTC (rev 208698)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2016-11-14 19:42:41 UTC (rev 208699)
@@ -57,13 +57,17 @@
static const char* const ObjectProtoCalledOnNullOrUndefinedError = "Object.prototype.__proto__ called on null or undefined";
template<typename CallbackWhenNoException>
-static ALWAYS_INLINE typename std::result_of<CallbackWhenNoException(JSString::SafeView&)>::type toSafeView(ExecState* exec, JSValue value, CallbackWhenNoException callback)
+static ALWAYS_INLINE typename std::result_of<CallbackWhenNoException(StringView)>::type toStringView(ExecState* exec, JSValue value, CallbackWhenNoException callback)
{
+ VM& vm = exec->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
JSString* string = value.toStringOrNull(exec);
if (UNLIKELY(!string))
return { };
JSString::SafeView view = string->view(exec);
- return callback(view);
+ StringView stringView = view.get();
+ RETURN_IF_EXCEPTION(scope, { });
+ return callback(stringView);
}
template<unsigned charactersCount>
@@ -158,7 +162,7 @@
static JSValue encode(ExecState* exec, const Bitmap<256>& doNotEscape)
{
- return toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
+ return toStringView(exec, exec->argument(0), [&] (StringView view) {
if (view.is8Bit())
return encode(exec, doNotEscape, view.characters8(), view.length());
return encode(exec, doNotEscape, view.characters16(), view.length());
@@ -236,7 +240,7 @@
static JSValue decode(ExecState* exec, const Bitmap<256>& doNotUnescape, bool strict)
{
- return toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
+ return toStringView(exec, exec->argument(0), [&] (StringView view) {
if (view.is8Bit())
return decode(exec, view.characters8(), view.length(), doNotUnescape, strict);
return decode(exec, view.characters16(), view.length(), doNotUnescape, strict);
@@ -707,8 +711,8 @@
}
// If ToString throws, we shouldn't call ToInt32.
- return toSafeView(exec, value, [&] (JSString::SafeView& view) {
- return JSValue::encode(jsNumber(parseInt(view.get(), radixValue.toInt32(exec))));
+ return toStringView(exec, value, [&] (StringView view) {
+ return JSValue::encode(jsNumber(parseInt(view, radixValue.toInt32(exec))));
});
}
@@ -765,7 +769,7 @@
"*+-./@_"
);
- return JSValue::encode(toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
+ return JSValue::encode(toStringView(exec, exec->argument(0), [&] (StringView view) {
JSStringBuilder builder;
if (view.is8Bit()) {
const LChar* c = view.characters8();
@@ -804,7 +808,7 @@
EncodedJSValue JSC_HOST_CALL globalFuncUnescape(ExecState* exec)
{
- return JSValue::encode(toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
+ return JSValue::encode(toStringView(exec, exec->argument(0), [&] (StringView view) {
StringBuilder builder;
int k = 0;
int len = view.length();
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (208698 => 208699)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2016-11-14 19:26:20 UTC (rev 208698)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2016-11-14 19:42:41 UTC (rev 208699)
@@ -763,16 +763,18 @@
return throwVMError(exec, scope, createError(exec, ASCIILiteral("JSON.parse requires at least one parameter")));
JSString::SafeView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = source.get();
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
JSValue unfiltered;
LocalScope localScope(vm);
- if (source.is8Bit()) {
- LiteralParser<LChar> jsonParser(exec, source.characters8(), source.length(), StrictJSON);
+ if (view.is8Bit()) {
+ LiteralParser<LChar> jsonParser(exec, view.characters8(), view.length(), StrictJSON);
unfiltered = jsonParser.tryLiteralParse();
if (!unfiltered)
return throwVMError(exec, scope, createSyntaxError(exec, jsonParser.getErrorMessage()));
} else {
- LiteralParser<UChar> jsonParser(exec, source.characters16(), source.length(), StrictJSON);
+ LiteralParser<UChar> jsonParser(exec, view.characters16(), view.length(), StrictJSON);
unfiltered = jsonParser.tryLiteralParse();
if (!unfiltered)
return throwVMError(exec, scope, createSyntaxError(exec, jsonParser.getErrorMessage()));
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (208698 => 208699)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2016-11-14 19:26:20 UTC (rev 208698)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2016-11-14 19:42:41 UTC (rev 208699)
@@ -482,9 +482,6 @@
bool is8Bit() const { return m_string->is8Bit(); }
unsigned length() const { return m_string->length(); }
- const LChar* characters8() const { return get().characters8(); }
- const UChar* characters16() const { return get().characters16(); }
- UChar operator[](unsigned index) const { return get()[index]; }
private:
ExecState& m_state;
Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (208698 => 208699)
--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2016-11-14 19:26:20 UTC (rev 208698)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2016-11-14 19:42:41 UTC (rev 208699)
@@ -791,6 +791,9 @@
EncodedJSValue JSC_HOST_CALL stringProtoFuncRepeatCharacter(ExecState* exec)
{
+ VM& vm = exec->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
// For a string which length is single, instead of creating ropes,
// allocating a sequential buffer and fill with the repeated string for efficiency.
ASSERT(exec->argumentCount() == 2);
@@ -802,18 +805,18 @@
JSValue repeatCountValue = exec->uncheckedArgument(1);
RELEASE_ASSERT(repeatCountValue.isNumber());
int32_t repeatCount;
- {
- VM& vm = exec->vm();
- auto scope = DECLARE_THROW_SCOPE(vm);
- double value = repeatCountValue.asNumber();
- if (value > JSString::MaxLength)
- return JSValue::encode(throwOutOfMemoryError(exec, scope));
- repeatCount = static_cast<int32_t>(value);
- }
+ double value = repeatCountValue.asNumber();
+ if (value > JSString::MaxLength)
+ return JSValue::encode(throwOutOfMemoryError(exec, scope));
+ repeatCount = static_cast<int32_t>(value);
ASSERT(repeatCount >= 0);
ASSERT(!repeatCountValue.isDouble() || repeatCountValue.asDouble() == repeatCount);
- UChar character = string->view(exec)[0];
+ JSString::SafeView safeView = string->view(exec);
+ StringView view = safeView.get();
+ ASSERT(view.length() == 1 && !scope.exception());
+ UChar character = view[0];
+ scope.release();
if (!(character & ~0xff))
return JSValue::encode(repeatCharacter(*exec, static_cast<LChar>(character), repeatCount));
return JSValue::encode(repeatCharacter(*exec, character, repeatCount));
@@ -904,16 +907,19 @@
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec, scope);
JSString::SafeView string = thisValue.toString(exec)->view(exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = string.get();
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
JSValue a0 = exec->argument(0);
if (a0.isUInt32()) {
uint32_t i = a0.asUInt32();
- if (i < string.length())
- return JSValue::encode(jsSingleCharacterString(exec, string[i]));
+ if (i < view.length())
+ return JSValue::encode(jsSingleCharacterString(exec, view[i]));
return JSValue::encode(jsEmptyString(exec));
}
double dpos = a0.toInteger(exec);
- if (dpos >= 0 && dpos < string.length())
- return JSValue::encode(jsSingleCharacterString(exec, string[static_cast<unsigned>(dpos)]));
+ if (dpos >= 0 && dpos < view.length())
+ return JSValue::encode(jsSingleCharacterString(exec, view[static_cast<unsigned>(dpos)]));
return JSValue::encode(jsEmptyString(exec));
}
@@ -925,17 +931,21 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec, scope);
- JSString::SafeView string = thisValue.toString(exec)->view(exec);
+ JSString* jsString = thisValue.toString(exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ JSString::SafeView string = jsString->view(exec);
+ StringView view = string.get();
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
JSValue a0 = exec->argument(0);
if (a0.isUInt32()) {
uint32_t i = a0.asUInt32();
- if (i < string.length())
- return JSValue::encode(jsNumber(string[i]));
+ if (i < view.length())
+ return JSValue::encode(jsNumber(view[i]));
return JSValue::encode(jsNaN());
}
double dpos = a0.toInteger(exec);
- if (dpos >= 0 && dpos < string.length())
- return JSValue::encode(jsNumber(string[static_cast<int>(dpos)]));
+ if (dpos >= 0 && dpos < view.length())
+ return JSValue::encode(jsNumber(view[static_cast<int>(dpos)]));
return JSValue::encode(jsNaN());
}
@@ -2008,6 +2018,8 @@
return throwVMTypeError(exec, scope);
JSString::SafeView source = thisValue.toString(exec)->view(exec);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = source.get();
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
UNormalizationMode form = UNORM_NFC;
// Verify that the argument is provided and is not undefined.
@@ -2027,7 +2039,7 @@
return throwVMError(exec, scope, createRangeError(exec, ASCIILiteral("argument does not match any normalization form")));
}
- return JSValue::encode(normalize(exec, source.get().upconvertedCharacters(), source.length(), form));
+ return JSValue::encode(normalize(exec, view.upconvertedCharacters(), view.length(), form));
}
} // namespace JSC
