Diff
Modified: trunk/JSTests/ChangeLog (206280 => 206281)
--- trunk/JSTests/ChangeLog 2016-09-22 22:34:02 UTC (rev 206280)
+++ trunk/JSTests/ChangeLog 2016-09-22 23:41:56 UTC (rev 206281)
@@ -1,3 +1,14 @@
+2016-09-22 Mark Lam <mark....@apple.com>
+
+ Array.prototype.join should do overflow checks on string joins.
+ https://bugs.webkit.org/show_bug.cgi?id=162459
+
+ Reviewed by Saam Barati.
+
+ * stress/array-join-on-strings-need-overflow-checks.js: Added.
+ (assert):
+ (catch):
+
2016-09-22 Joseph Pecoraro <pecor...@apple.com>
test262: Function length should be number of parameters before parameters with default values
Added: trunk/JSTests/stress/array-join-on-strings-need-overflow-checks.js (0 => 206281)
--- trunk/JSTests/stress/array-join-on-strings-need-overflow-checks.js (rev 0)
+++ trunk/JSTests/stress/array-join-on-strings-need-overflow-checks.js 2016-09-22 23:41:56 UTC (rev 206281)
@@ -0,0 +1,33 @@
+function assert(x, y) {
+ if (x != y)
+ throw(" Expect: " + y + ", actual: " + x);
+}
+
+s1 = "";
+for (var k = 0; k < 2000; ++k)
+ s1 += "z";
+var expectedLength = 2000;
+assert(s1.length, 2000);
+
+s2 = 'x';
+expectedLength = 1;
+assert(s2.length, expectedLength);
+
+for (var i = 0; i < 22; ++i) {
+ expectedLength += expectedLength;
+ s2 += s2;
+ assert(s2.length, expectedLength);
+}
+
+var caughtException;
+try {
+ expectedLength = ((s1.length - 1) * s2.length) + 1;
+ result = Array.prototype.join.apply(s1, [s2]);
+ assert(result.length, expectedLength);
+} catch (e) {
+ caughtException = e;
+}
+
+if (!caughtException)
+ throw("Array.prototype.join should have thrown an exception when string length overflows");
+assert(caughtException, "Error: Out of memory");
Modified: trunk/Source/_javascript_Core/ChangeLog (206280 => 206281)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-22 22:34:02 UTC (rev 206280)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-09-22 23:41:56 UTC (rev 206281)
@@ -1,3 +1,21 @@
+2016-09-22 Mark Lam <mark....@apple.com>
+
+ Array.prototype.join should do overflow checks on string joins.
+ https://bugs.webkit.org/show_bug.cgi?id=162459
+
+ Reviewed by Saam Barati.
+
+ Change the 2 JSRopeString::create() functions that do joins to be private, and
+ force all clients of it to go through the jsString() utility functions that do
+ overflow checks before creating the ropes.
+
+ * dfg/DFGOperations.cpp:
+ * runtime/ArrayPrototype.cpp:
+ (JSC::slowJoin):
+ * runtime/JSString.h:
+ * runtime/Operations.h:
+ (JSC::jsString):
+
2016-09-22 Filip Pizlo <fpi...@apple.com>
Fences on x86 should be a lot cheaper
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (206280 => 206281)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2016-09-22 22:34:02 UTC (rev 206280)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2016-09-22 23:41:56 UTC (rev 206281)
@@ -52,6 +52,7 @@
#include "JSMap.h"
#include "JSSet.h"
#include "ObjectConstructor.h"
+#include "Operations.h"
#include "RegExpObject.h"
#include "Repatch.h"
#include "ScopedArguments.h"
@@ -1500,14 +1501,8 @@
{
VM& vm = exec->vm();
NativeCallFrameTracer tracer(&vm, exec);
- auto scope = DECLARE_THROW_SCOPE(vm);
- if (sumOverflows<int32_t>(left->length(), right->length())) {
- throwOutOfMemoryError(exec, scope);
- return nullptr;
- }
-
- return JSRopeString::create(vm, left, right);
+ return jsString(exec, left, right);
}
JSCell* JIT_OPERATION operationMakeRope3(ExecState* exec, JSString* a, JSString* b, JSString* c)
@@ -1514,14 +1509,8 @@
{
VM& vm = exec->vm();
NativeCallFrameTracer tracer(&vm, exec);
- auto scope = DECLARE_THROW_SCOPE(vm);
- if (sumOverflows<int32_t>(a->length(), b->length(), c->length())) {
- throwOutOfMemoryError(exec, scope);
- return nullptr;
- }
-
- return JSRopeString::create(vm, a, b, c);
+ return jsString(exec, a, b, c);
}
JSCell* JIT_OPERATION operationStrCat2(ExecState* exec, EncodedJSValue a, EncodedJSValue b)
@@ -1535,12 +1524,8 @@
JSString* str2 = JSValue::decode(b).toString(exec);
ASSERT(!scope.exception());
- if (sumOverflows<int32_t>(str1->length(), str2->length())) {
- throwOutOfMemoryError(exec, scope);
- return nullptr;
- }
-
- return JSRopeString::create(vm, str1, str2);
+ scope.release();
+ return jsString(exec, str1, str2);
}
JSCell* JIT_OPERATION operationStrCat3(ExecState* exec, EncodedJSValue a, EncodedJSValue b, EncodedJSValue c)
@@ -1556,12 +1541,8 @@
JSString* str3 = JSValue::decode(c).toString(exec);
ASSERT(!scope.exception());
- if (sumOverflows<int32_t>(str1->length(), str2->length(), str3->length())) {
- throwOutOfMemoryError(exec, scope);
- return nullptr;
- }
-
- return JSRopeString::create(vm, str1, str2, str3);
+ scope.release();
+ return jsString(exec, str1, str2, str3);
}
char* JIT_OPERATION operationFindSwitchImmTargetForDouble(
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (206280 => 206281)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2016-09-22 22:34:02 UTC (rev 206280)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2016-09-22 23:41:56 UTC (rev 206281)
@@ -41,6 +41,7 @@
#include "Lookup.h"
#include "ObjectConstructor.h"
#include "ObjectPrototype.h"
+#include "Operations.h"
#include "StringRecursionChecker.h"
#include <algorithm>
#include <wtf/Assertions.h>
@@ -546,7 +547,9 @@
// a. Let S be the String value produced by concatenating R and sep.
// d. Let R be a String value produced by concatenating S and next.
- r = JSRopeString::create(vm, r, separator, next);
+ r = jsString(&exec, r, separator, next);
+ if (UNLIKELY(scope.exception()))
+ return JSValue();
}
// 10. Return R.
return r;
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (206280 => 206281)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2016-09-22 22:34:02 UTC (rev 206280)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2016-09-22 23:41:56 UTC (rev 206281)
@@ -219,7 +219,7 @@
String& string() { ASSERT(!isRope()); return m_value; }
StringView unsafeView(ExecState&) const;
- friend JSValue jsString(ExecState*, JSString*, JSString*);
+ friend JSString* jsString(ExecState*, JSString*, JSString*);
friend JSString* jsSubstring(ExecState*, JSString*, unsigned offset, unsigned length);
};
@@ -277,6 +277,7 @@
void finishCreation(VM& vm, JSString* s1, JSString* s2)
{
Base::finishCreation(vm);
+ ASSERT(!sumOverflows<int32_t>(s1->length(), s2->length()));
m_length = s1->length() + s2->length();
setIs8Bit(s1->is8Bit() && s2->is8Bit());
setIsSubstring(false);
@@ -288,6 +289,7 @@
void finishCreation(VM& vm, JSString* s1, JSString* s2, JSString* s3)
{
Base::finishCreation(vm);
+ ASSERT(!sumOverflows<int32_t>(s1->length(), s2->length(), s3->length()));
m_length = s1->length() + s2->length() + s3->length();
setIs8Bit(s1->is8Bit() && s2->is8Bit() && s3->is8Bit());
setIsSubstring(false);
@@ -357,19 +359,6 @@
}
public:
- static JSString* create(VM& vm, JSString* s1, JSString* s2)
- {
- JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
- newString->finishCreation(vm, s1, s2);
- return newString;
- }
- static JSString* create(VM& vm, JSString* s1, JSString* s2, JSString* s3)
- {
- JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
- newString->finishCreation(vm, s1, s2, s3);
- return newString;
- }
-
static JSString* create(VM& vm, ExecState* exec, JSString* base, unsigned offset, unsigned length)
{
JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
@@ -391,6 +380,19 @@
static const unsigned s_maxInternalRopeLength = 3;
private:
+ static JSString* create(VM& vm, JSString* s1, JSString* s2)
+ {
+ JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
+ newString->finishCreation(vm, s1, s2);
+ return newString;
+ }
+ static JSString* create(VM& vm, JSString* s1, JSString* s2, JSString* s3)
+ {
+ JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm);
+ newString->finishCreation(vm, s1, s2, s3);
+ return newString;
+ }
+
friend JSValue jsStringFromRegisterArray(ExecState*, Register*, unsigned);
friend JSValue jsStringFromArguments(ExecState*, JSValue);
@@ -449,6 +451,11 @@
uintptr_t number;
WriteBarrierBase<JSString> string;
} u[s_maxInternalRopeLength];
+
+
+ friend JSString* jsString(ExecState*, JSString*, JSString*);
+ friend JSString* jsString(ExecState*, JSString*, JSString*, JSString*);
+ friend JSString* jsString(ExecState*, const String&, const String&, const String&);
};
class JSString::SafeView {
Modified: trunk/Source/_javascript_Core/runtime/Operations.h (206280 => 206281)
--- trunk/Source/_javascript_Core/runtime/Operations.h 2016-09-22 22:34:02 UTC (rev 206280)
+++ trunk/Source/_javascript_Core/runtime/Operations.h 2016-09-22 23:41:56 UTC (rev 206281)
@@ -37,7 +37,7 @@
bool jsIsFunctionType(JSValue);
size_t normalizePrototypeChain(CallFrame*, Structure*);
-ALWAYS_INLINE JSValue jsString(ExecState* exec, JSString* s1, JSString* s2)
+ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2)
{
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -48,14 +48,44 @@
int32_t length2 = s2->length();
if (!length2)
return s1;
- if (sumOverflows<int32_t>(length1, length2))
- return throwOutOfMemoryError(exec, scope);
+ if (sumOverflows<int32_t>(length1, length2)) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
return JSRopeString::create(vm, s1, s2);
}
-ALWAYS_INLINE JSValue jsString(ExecState* exec, const String& u1, const String& u2, const String& u3)
+ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2, JSString* s3)
{
+ VM& vm = exec->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ int32_t length1 = s1->length();
+ if (!length1) {
+ scope.release();
+ return jsString(exec, s2, s3);
+ }
+ int32_t length2 = s2->length();
+ if (!length2) {
+ scope.release();
+ return jsString(exec, s1, s3);
+ }
+ int32_t length3 = s3->length();
+ if (!length3) {
+ scope.release();
+ return jsString(exec, s1, s2);
+ }
+
+ if (sumOverflows<int32_t>(length1, length2, length3)) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
+ return JSRopeString::create(vm, s1, s2, s3);
+}
+
+ALWAYS_INLINE JSString* jsString(ExecState* exec, const String& u1, const String& u2, const String& u3)
+{
VM* vm = &exec->vm();
auto scope = DECLARE_THROW_SCOPE(*vm);
@@ -63,20 +93,30 @@
int32_t length2 = u2.length();
int32_t length3 = u3.length();
- if (length1 < 0 || length2 < 0 || length3 < 0)
- return throwOutOfMemoryError(exec, scope);
+ if (length1 < 0 || length2 < 0 || length3 < 0) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
- if (!length1)
+ if (!length1) {
+ scope.release();
return jsString(exec, jsString(vm, u2), jsString(vm, u3));
- if (!length2)
+ }
+ if (!length2) {
+ scope.release();
return jsString(exec, jsString(vm, u1), jsString(vm, u3));
- if (!length3)
+ }
+ if (!length3) {
+ scope.release();
return jsString(exec, jsString(vm, u1), jsString(vm, u2));
+ }
- if (sumOverflows<int32_t>(length1, length2, length3))
- return throwOutOfMemoryError(exec, scope);
+ if (sumOverflows<int32_t>(length1, length2, length3)) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
- return JSRopeString::create(exec->vm(), jsString(vm, u1), jsString(vm, u2), jsString(vm, u3));
+ return JSRopeString::create(*vm, jsString(vm, u1), jsString(vm, u2), jsString(vm, u3));
}
ALWAYS_INLINE JSValue jsStringFromRegisterArray(ExecState* exec, Register* strings, unsigned count)
