Diff
Modified: branches/safari-606-branch/JSTests/ChangeLog (237314 => 237315)
--- branches/safari-606-branch/JSTests/ChangeLog 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/JSTests/ChangeLog 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,5 +1,64 @@
2018-10-21 Babak Shafiei <[email protected]>
+ Cherry-pick r236804. rdar://problem/45285687
+
+ Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.
+ https://bugs.webkit.org/show_bug.cgi?id=190187
+ <rdar://problem/42512909>
+
+ Reviewed by Michael Saboff.
+
+ JSTests:
+
+ * stress/regress-190187.js: Added.
+
+ Source/_javascript_Core:
+
+ Allowing different max string lengths at each level opens up opportunities for
+ bugs to creep in. With 2 different max length values, it is more difficult to
+ keep the story straight on how we do overflow / bounds checks at each place in
+ the code. It's also difficult to tell if a seemingly valid check at the WTF level
+ will have bad ramifications at the JSC level. Also, it's also not meaningful to
+ support a max length > INT_MAX. To eliminate this class of bugs, we'll
+ standardize on a MaxLength of INT_MAX at all levels.
+
+ We'll also standardize the way we do length overflow checks on using
+ CheckedArithmetic, and add some asserts to document the assumptions of the code.
+
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ - Fix OOM error handling which crashed a test after the new MaxLength was applied.
+ * runtime/JSString.h:
+ (JSC::JSString::finishCreation):
+ (JSC::JSString::createHasOtherOwner):
+ (JSC::JSString::setLength):
+ * runtime/JSStringInlines.h:
+ (JSC::jsMakeNontrivialString):
+ * runtime/Operations.h:
+ (JSC::jsString):
+
+ Source/WTF:
+
+ * wtf/text/StringConcatenate.h:
+ (WTF::tryMakeStringFromAdapters):
+ (WTF::sumWithOverflow): Deleted.
+ * wtf/text/StringImpl.h:
+ * wtf/text/WTFString.h:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236804 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-10-03 Mark Lam <[email protected]>
+
+ Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.
+ https://bugs.webkit.org/show_bug.cgi?id=190187
+ <rdar://problem/42512909>
+
+ Reviewed by Michael Saboff.
+
+ * stress/regress-190187.js: Added.
+
+2018-10-21 Babak Shafiei <[email protected]>
+
Cherry-pick r234127. rdar://problem/45285391
[INTL] Language tags are not canonicalized
Added: branches/safari-606-branch/JSTests/stress/regress-190187.js (0 => 237315)
--- branches/safari-606-branch/JSTests/stress/regress-190187.js (rev 0)
+++ branches/safari-606-branch/JSTests/stress/regress-190187.js 2018-10-22 07:19:26 UTC (rev 237315)
@@ -0,0 +1,18 @@
+//@ runDefault
+//@ skip if $memoryLimited or $buildType == "debug"
+//@ slow!
+
+try {
+ var v1 = "AAAAAAAAAAA";
+ for(var i = 0; i < 27; i++)
+ v1 = v1 + v1;
+ var v2;
+ var v3 = RegExp.prototype.toString.call({source:v1,flags:v1});
+ v3 += v1;
+ v2 += v3.localeCompare(v1);
+} catch (e) {
+ exception = e;
+}
+
+if (exception != "Error: Out of memory")
+ throw "FAILED";
Modified: branches/safari-606-branch/Source/_javascript_Core/ChangeLog (237314 => 237315)
--- branches/safari-606-branch/Source/_javascript_Core/ChangeLog 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/_javascript_Core/ChangeLog 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,5 +1,85 @@
2018-10-21 Babak Shafiei <[email protected]>
+ Cherry-pick r236804. rdar://problem/45285687
+
+ Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.
+ https://bugs.webkit.org/show_bug.cgi?id=190187
+ <rdar://problem/42512909>
+
+ Reviewed by Michael Saboff.
+
+ JSTests:
+
+ * stress/regress-190187.js: Added.
+
+ Source/_javascript_Core:
+
+ Allowing different max string lengths at each level opens up opportunities for
+ bugs to creep in. With 2 different max length values, it is more difficult to
+ keep the story straight on how we do overflow / bounds checks at each place in
+ the code. It's also difficult to tell if a seemingly valid check at the WTF level
+ will have bad ramifications at the JSC level. Also, it's also not meaningful to
+ support a max length > INT_MAX. To eliminate this class of bugs, we'll
+ standardize on a MaxLength of INT_MAX at all levels.
+
+ We'll also standardize the way we do length overflow checks on using
+ CheckedArithmetic, and add some asserts to document the assumptions of the code.
+
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ - Fix OOM error handling which crashed a test after the new MaxLength was applied.
+ * runtime/JSString.h:
+ (JSC::JSString::finishCreation):
+ (JSC::JSString::createHasOtherOwner):
+ (JSC::JSString::setLength):
+ * runtime/JSStringInlines.h:
+ (JSC::jsMakeNontrivialString):
+ * runtime/Operations.h:
+ (JSC::jsString):
+
+ Source/WTF:
+
+ * wtf/text/StringConcatenate.h:
+ (WTF::tryMakeStringFromAdapters):
+ (WTF::sumWithOverflow): Deleted.
+ * wtf/text/StringImpl.h:
+ * wtf/text/WTFString.h:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236804 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-10-03 Mark Lam <[email protected]>
+
+ Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.
+ https://bugs.webkit.org/show_bug.cgi?id=190187
+ <rdar://problem/42512909>
+
+ Reviewed by Michael Saboff.
+
+ Allowing different max string lengths at each level opens up opportunities for
+ bugs to creep in. With 2 different max length values, it is more difficult to
+ keep the story straight on how we do overflow / bounds checks at each place in
+ the code. It's also difficult to tell if a seemingly valid check at the WTF level
+ will have bad ramifications at the JSC level. Also, it's also not meaningful to
+ support a max length > INT_MAX. To eliminate this class of bugs, we'll
+ standardize on a MaxLength of INT_MAX at all levels.
+
+ We'll also standardize the way we do length overflow checks on using
+ CheckedArithmetic, and add some asserts to document the assumptions of the code.
+
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ - Fix OOM error handling which crashed a test after the new MaxLength was applied.
+ * runtime/JSString.h:
+ (JSC::JSString::finishCreation):
+ (JSC::JSString::createHasOtherOwner):
+ (JSC::JSString::setLength):
+ * runtime/JSStringInlines.h:
+ (JSC::jsMakeNontrivialString):
+ * runtime/Operations.h:
+ (JSC::jsString):
+
+2018-10-21 Babak Shafiei <[email protected]>
+
Cherry-pick r234127. rdar://problem/45285391
[INTL] Language tags are not canonicalized
Modified: branches/safari-606-branch/Source/_javascript_Core/runtime/FunctionConstructor.cpp (237314 => 237315)
--- branches/safari-606-branch/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2018-10-22 07:19:26 UTC (rev 237315)
@@ -143,7 +143,11 @@
{
// The spec mandates that the parameters parse as a valid parameter list
// independent of the function body.
- String program = makeString("(", prefix, "(", parameterBuilder.toString(), "){\n\n})");
+ String program = tryMakeString("(", prefix, "(", parameterBuilder.toString(), "){\n\n})");
+ if (UNLIKELY(!program)) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
SourceCode source = makeSource(program, sourceOrigin, sourceURL, position);
JSValue exception;
checkSyntax(exec, source, &exception);
Modified: branches/safari-606-branch/Source/_javascript_Core/runtime/JSString.h (237314 => 237315)
--- branches/safari-606-branch/Source/_javascript_Core/runtime/JSString.h 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/_javascript_Core/runtime/JSString.h 2018-10-22 07:19:26 UTC (rev 237315)
@@ -95,8 +95,12 @@
return &vm.stringSpace;
}
- static const unsigned MaxLength = std::numeric_limits<int32_t>::max();
-
+ // We employ overflow checks in many places with the assumption that MaxLength
+ // is INT_MAX. Hence, it cannot be changed into another length value without
+ // breaking all the bounds and overflow checks that assume this.
+ static constexpr unsigned MaxLength = std::numeric_limits<int32_t>::max();
+ static_assert(MaxLength == String::MaxLength, "");
+
private:
JSString(VM& vm, Ref<StringImpl>&& value)
: JSCell(vm, vm.stringStructure.get())
@@ -109,7 +113,7 @@
{
}
- void finishCreation(VM& vm, size_t length)
+ void finishCreation(VM& vm, unsigned length)
{
ASSERT(!m_value.isNull());
Base::finishCreation(vm);
@@ -117,7 +121,7 @@
setIs8Bit(m_value.impl()->is8Bit());
}
- void finishCreation(VM& vm, size_t length, size_t cost)
+ void finishCreation(VM& vm, unsigned length, size_t cost)
{
ASSERT(!m_value.isNull());
Base::finishCreation(vm);
@@ -145,7 +149,7 @@
}
static JSString* createHasOtherOwner(VM& vm, Ref<StringImpl>&& value)
{
- size_t length = value->length();
+ unsigned length = value->length();
JSString* newString = new (NotNull, allocateCell<JSString>(vm.heap)) JSString(vm, WTFMove(value));
newString->finishCreation(vm, length);
return newString;
@@ -209,6 +213,7 @@
ALWAYS_INLINE void setLength(unsigned length)
{
+ ASSERT(length <= MaxLength);
m_length = length;
}
@@ -256,10 +261,14 @@
return false;
if (m_index == JSRopeString::s_maxInternalRopeLength)
expand();
- if (static_cast<int32_t>(m_jsString->length() + jsString->length()) < 0) {
+
+ static_assert(JSString::MaxLength == std::numeric_limits<int32_t>::max(), "");
+ auto sum = checkedSum<int32_t>(m_jsString->length(), jsString->length());
+ if (sum.hasOverflowed()) {
this->overflowed();
return false;
}
+ ASSERT(static_cast<unsigned>(sum.unsafeGet()) <= MaxLength);
m_jsString->append(m_vm, m_index++, jsString);
return true;
}
Modified: branches/safari-606-branch/Source/_javascript_Core/runtime/JSStringInlines.h (237314 => 237315)
--- branches/safari-606-branch/Source/_javascript_Core/runtime/JSStringInlines.h 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/_javascript_Core/runtime/JSStringInlines.h 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -50,6 +50,7 @@
String result = tryMakeString(std::forward<StringType>(string), std::forward<StringTypes>(strings)...);
if (UNLIKELY(!result))
return throwOutOfMemoryError(exec, scope);
+ ASSERT(result.length() <= JSString::MaxLength);
return jsNontrivialString(exec, WTFMove(result));
}
Modified: branches/safari-606-branch/Source/_javascript_Core/runtime/Operations.h (237314 => 237315)
--- branches/safari-606-branch/Source/_javascript_Core/runtime/Operations.h 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/_javascript_Core/runtime/Operations.h 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999-2000 Harri Porten ([email protected])
- * Copyright (C) 2002-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2002-2018 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -42,12 +42,13 @@
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- int32_t length1 = s1->length();
+ unsigned length1 = s1->length();
if (!length1)
return s2;
- int32_t length2 = s2->length();
+ unsigned length2 = s2->length();
if (!length2)
return s1;
+ static_assert(JSString::MaxLength == std::numeric_limits<int32_t>::max(), "");
if (sumOverflows<int32_t>(length1, length2)) {
throwOutOfMemoryError(exec, scope);
return nullptr;
@@ -61,22 +62,23 @@
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- int32_t length1 = s1->length();
+ unsigned length1 = s1->length();
if (!length1) {
scope.release();
return jsString(exec, s2, s3);
}
- int32_t length2 = s2->length();
+ unsigned length2 = s2->length();
if (!length2) {
scope.release();
return jsString(exec, s1, s3);
}
- int32_t length3 = s3->length();
+ unsigned length3 = s3->length();
if (!length3) {
scope.release();
return jsString(exec, s1, s2);
}
+ static_assert(JSString::MaxLength == std::numeric_limits<int32_t>::max(), "");
if (sumOverflows<int32_t>(length1, length2, length3)) {
throwOutOfMemoryError(exec, scope);
return nullptr;
@@ -89,15 +91,13 @@
VM* vm = &exec->vm();
auto scope = DECLARE_THROW_SCOPE(*vm);
- int32_t length1 = u1.length();
- int32_t length2 = u2.length();
- int32_t length3 = u3.length();
+ unsigned length1 = u1.length();
+ unsigned length2 = u2.length();
+ unsigned length3 = u3.length();
+ ASSERT(length1 <= JSString::MaxLength);
+ ASSERT(length2 <= JSString::MaxLength);
+ ASSERT(length3 <= JSString::MaxLength);
- if (length1 < 0 || length2 < 0 || length3 < 0) {
- throwOutOfMemoryError(exec, scope);
- return nullptr;
- }
-
if (!length1) {
scope.release();
return jsString(exec, jsString(vm, u2), jsString(vm, u3));
@@ -111,6 +111,7 @@
return jsString(exec, jsString(vm, u1), jsString(vm, u2));
}
+ static_assert(JSString::MaxLength == std::numeric_limits<int32_t>::max(), "");
if (sumOverflows<int32_t>(length1, length2, length3)) {
throwOutOfMemoryError(exec, scope);
return nullptr;
Modified: branches/safari-606-branch/Source/WTF/ChangeLog (237314 => 237315)
--- branches/safari-606-branch/Source/WTF/ChangeLog 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/WTF/ChangeLog 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,3 +1,66 @@
+2018-10-21 Babak Shafiei <[email protected]>
+
+ Cherry-pick r236804. rdar://problem/45285687
+
+ Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.
+ https://bugs.webkit.org/show_bug.cgi?id=190187
+ <rdar://problem/42512909>
+
+ Reviewed by Michael Saboff.
+
+ JSTests:
+
+ * stress/regress-190187.js: Added.
+
+ Source/_javascript_Core:
+
+ Allowing different max string lengths at each level opens up opportunities for
+ bugs to creep in. With 2 different max length values, it is more difficult to
+ keep the story straight on how we do overflow / bounds checks at each place in
+ the code. It's also difficult to tell if a seemingly valid check at the WTF level
+ will have bad ramifications at the JSC level. Also, it's also not meaningful to
+ support a max length > INT_MAX. To eliminate this class of bugs, we'll
+ standardize on a MaxLength of INT_MAX at all levels.
+
+ We'll also standardize the way we do length overflow checks on using
+ CheckedArithmetic, and add some asserts to document the assumptions of the code.
+
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ - Fix OOM error handling which crashed a test after the new MaxLength was applied.
+ * runtime/JSString.h:
+ (JSC::JSString::finishCreation):
+ (JSC::JSString::createHasOtherOwner):
+ (JSC::JSString::setLength):
+ * runtime/JSStringInlines.h:
+ (JSC::jsMakeNontrivialString):
+ * runtime/Operations.h:
+ (JSC::jsString):
+
+ Source/WTF:
+
+ * wtf/text/StringConcatenate.h:
+ (WTF::tryMakeStringFromAdapters):
+ (WTF::sumWithOverflow): Deleted.
+ * wtf/text/StringImpl.h:
+ * wtf/text/WTFString.h:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236804 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-10-03 Mark Lam <[email protected]>
+
+ Make string MaxLength for all WTF and JS strings consistently equal to INT_MAX.
+ https://bugs.webkit.org/show_bug.cgi?id=190187
+ <rdar://problem/42512909>
+
+ Reviewed by Michael Saboff.
+
+ * wtf/text/StringConcatenate.h:
+ (WTF::tryMakeStringFromAdapters):
+ (WTF::sumWithOverflow): Deleted.
+ * wtf/text/StringImpl.h:
+ * wtf/text/WTFString.h:
+
2018-09-06 Babak Shafiei <[email protected]>
Cherry-pick r235721. rdar://problem/44212406
Modified: branches/safari-606-branch/Source/WTF/wtf/text/StringConcatenate.h (237314 => 237315)
--- branches/safari-606-branch/Source/WTF/wtf/text/StringConcatenate.h 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/WTF/wtf/text/StringConcatenate.h 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,6 +27,7 @@
#define StringConcatenate_h
#include <string.h>
+#include <wtf/CheckedArithmetic.h>
#ifndef AtomicString_h
#include <wtf/text/AtomicString.h>
@@ -141,9 +142,7 @@
while (m_characters[length])
++length;
- if (length > std::numeric_limits<unsigned>::max()) // FIXME this is silly https://bugs.webkit.org/show_bug.cgi?id=165790
- CRASH();
-
+ RELEASE_ASSERT(length <= String::MaxLength);
m_length = length;
}
@@ -259,24 +258,6 @@
}
};
-inline void sumWithOverflow(bool& overflow, unsigned& total, unsigned addend)
-{
- unsigned oldTotal = total;
- total = oldTotal + addend;
- if (total < oldTotal)
- overflow = true;
-}
-
-template<typename... Unsigned>
-inline void sumWithOverflow(bool& overflow, unsigned& total, unsigned addend, Unsigned ...addends)
-{
- unsigned oldTotal = total;
- total = oldTotal + addend;
- if (total < oldTotal)
- overflow = true;
- sumWithOverflow(overflow, total, addends...);
-}
-
template<typename Adapter>
inline bool are8Bit(Adapter adapter)
{
@@ -305,12 +286,13 @@
template<typename StringTypeAdapter, typename... StringTypeAdapters>
String tryMakeStringFromAdapters(StringTypeAdapter adapter, StringTypeAdapters ...adapters)
{
- bool overflow = false;
- unsigned length = adapter.length();
- sumWithOverflow(overflow, length, adapters.length()...);
- if (overflow)
+ static_assert(String::MaxLength == std::numeric_limits<int32_t>::max(), "");
+ auto sum = checkedSum<int32_t>(adapter.length(), adapters.length()...);
+ if (sum.hasOverflowed())
return String();
+ unsigned length = sum.unsafeGet();
+ ASSERT(length <= String::MaxLength);
if (are8Bit(adapter, adapters...)) {
LChar* buffer;
RefPtr<StringImpl> resultImpl = StringImpl::tryCreateUninitialized(length, buffer);
Modified: branches/safari-606-branch/Source/WTF/wtf/text/StringImpl.h (237314 => 237315)
--- branches/safari-606-branch/Source/WTF/wtf/text/StringImpl.h 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/WTF/wtf/text/StringImpl.h 2018-10-22 07:19:26 UTC (rev 237315)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999 Lars Knoll ([email protected])
- * Copyright (C) 2005-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2018 Apple Inc. All rights reserved.
* Copyright (C) 2009 Google Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
@@ -130,6 +130,9 @@
class StringImplShape {
WTF_MAKE_NONCOPYABLE(StringImplShape);
+public:
+ static constexpr unsigned MaxLength = std::numeric_limits<int32_t>::max();
+
protected:
StringImplShape(unsigned refCount, unsigned length, const LChar*, unsigned hashAndFlags);
StringImplShape(unsigned refCount, unsigned length, const UChar*, unsigned hashAndFlags);
@@ -179,6 +182,8 @@
public:
enum BufferOwnership { BufferInternal, BufferOwned, BufferSubstring };
+ static constexpr unsigned MaxLength = StringImplShape::MaxLength;
+
// The bottom 6 bits in the hash are flags.
static constexpr const unsigned s_flagCount = 6;
private:
Modified: branches/safari-606-branch/Source/WTF/wtf/text/WTFString.h (237314 => 237315)
--- branches/safari-606-branch/Source/WTF/wtf/text/WTFString.h 2018-10-22 07:19:20 UTC (rev 237314)
+++ branches/safari-606-branch/Source/WTF/wtf/text/WTFString.h 2018-10-22 07:19:26 UTC (rev 237315)
@@ -366,6 +366,8 @@
// This is useful for clearing String-based caches.
void clearImplIfNotShared();
+ static constexpr unsigned MaxLength = StringImpl::MaxLength;
+
private:
template<typename CharacterType> void removeInternal(const CharacterType*, unsigned, unsigned);
template<typename CharacterType> void appendInternal(CharacterType);
