Attachment 357811 Details for Bug 192853 – Patch for landing

[patch] Patch for landing

bug-192853-20181220162748.patch (text/plain), 19.75 KB, created by Tadeu Zagallo on 2018-12-20 07:28:55 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2018-12-20 07:28:55 PST

Size: 19.75 KB

patch

obsolete

>Subversion Revision: 239373
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index f54952cb4fbe04e67129de96c27f609542524cae..215e2456db0ef3bd51d77ba0ae2dcde5f89a29f8 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,40 @@
>+2018-12-19  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Consistently use MaxLength for all WTF strings
>+        https://bugs.webkit.org/show_bug.cgi?id=192853
>+        <rdar://problem/45726906>
>+
>+        Reviewed by Mark Lam.
>+
>+        MaxLength was introduced to be INT_MAX for WTF::String and StringImpl,
>+        but all the assertions were still done using UINT_MAX. Change it to
>+        use MaxLength for all checks.
>+
>+        * wtf/text/StringImpl.cpp:
>+        (WTF::StringImpl::createUninitializedInternalNonEmpty):
>+        (WTF::StringImpl::reallocateInternal):
>+        (WTF::StringImpl::create):
>+        (WTF::StringImpl::convertToLowercaseWithoutLocale):
>+        (WTF::StringImpl::convertToUppercaseWithoutLocale):
>+        (WTF::StringImpl::convertToLowercaseWithLocale):
>+        (WTF::StringImpl::convertToUppercaseWithLocale):
>+        (WTF::StringImpl::foldCase):
>+        (WTF::StringImpl::find):
>+        (WTF::StringImpl::replace):
>+        (WTF::StringImpl::utf8ForCharacters):
>+        (WTF::StringImpl::tryGetUtf8ForRange const):
>+        * wtf/text/StringImpl.h:
>+        (WTF::lengthOfNullTerminatedString):
>+        (WTF::StringImpl::tryCreateUninitialized):
>+        (WTF::StringImpl::adopt):
>+        (WTF::StringImpl::maxInternalLength):
>+        * wtf/text/WTFString.cpp:
>+        (WTF::String::append):
>+        (WTF::String::insert):
>+        (WTF::String::fromUTF8):
>+        * wtf/text/WTFString.h:
>+        (WTF::String::reverseFind const):
>+
> 2018-12-17  Chris Fleizach  <cfleizach@apple.com>
> 
>         Some builds are broken after r239262
>diff --git a/Source/WTF/wtf/text/StringImpl.cpp b/Source/WTF/wtf/text/StringImpl.cpp
>index 0fc01ace39bda94791bf7813930a3412ba7a9f99..60f3fd32b64f6eb275cec883a663eeac470debc2 100644
>--- a/Source/WTF/wtf/text/StringImpl.cpp
>+++ b/Source/WTF/wtf/text/StringImpl.cpp
>@@ -193,7 +193,7 @@ template<typename CharacterType> inline Ref<StringImpl> StringImpl::createUninit
>     // Allocate a single buffer large enough to contain the StringImpl
>     // struct as well as the data which it contains. This removes one
>     // heap allocation from this call.
>-    if (length > ((std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) / sizeof(CharacterType)))
>+    if (length > maxInternalLength<CharacterType>())
>         CRASH();
>     StringImpl* string = static_cast<StringImpl*>(fastMalloc(allocationSize<CharacterType>(length)));
> 
>@@ -222,7 +222,7 @@ template<typename CharacterType> inline Expected<Ref<StringImpl>, UTF8Conversion
>     }
> 
>     // Same as createUninitialized() except here we use fastRealloc.
>-    if (length > ((std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) / sizeof(CharacterType)))
>+    if (length > maxInternalLength<CharacterType>())
>         return makeUnexpected(UTF8ConversionError::OutOfMemory);
> 
>     originalString->~StringImpl();
>@@ -307,7 +307,7 @@ Ref<StringImpl> StringImpl::create(const LChar* string)
>     if (!string)
>         return *empty();
>     size_t length = strlen(reinterpret_cast<const char*>(string));
>-    if (length > std::numeric_limits<unsigned>::max())
>+    if (length > MaxLength)
>         CRASH();
>     return create(string, length);
> }
>@@ -377,7 +377,7 @@ Ref<StringImpl> StringImpl::convertToLowercaseWithoutLocale()
>         return newImpl;
>     }
> 
>-    if (m_length > static_cast<unsigned>(std::numeric_limits<int32_t>::max()))
>+    if (m_length > MaxLength)
>         CRASH();
>     int32_t length = m_length;
> 
>@@ -429,7 +429,7 @@ Ref<StringImpl> StringImpl::convertToUppercaseWithoutLocale()
>     // few actual calls to upper() are no-ops, so it wouldn't be worth
>     // the extra time for pre-scanning.
> 
>-    if (m_length > static_cast<unsigned>(std::numeric_limits<int32_t>::max()))
>+    if (m_length > MaxLength)
>         CRASH();
>     int32_t length = m_length;
> 
>@@ -541,7 +541,7 @@ Ref<StringImpl> StringImpl::convertToLowercaseWithLocale(const AtomicString& loc
>     // this last part into a shared function that takes a locale string, since this is
>     // just like the end of that function.
> 
>-    if (m_length > static_cast<unsigned>(std::numeric_limits<int32_t>::max()))
>+    if (m_length > MaxLength)
>         CRASH();
>     int length = m_length;
> 
>@@ -572,7 +572,7 @@ Ref<StringImpl> StringImpl::convertToUppercaseWithLocale(const AtomicString& loc
>     if (!needsTurkishCasingRules(localeIdentifier) || find('i') == notFound)
>         return convertToUppercaseWithoutLocale();
> 
>-    if (m_length > static_cast<unsigned>(std::numeric_limits<int32_t>::max()))
>+    if (m_length > MaxLength)
>         CRASH();
>     int length = m_length;
> 
>@@ -657,7 +657,7 @@ SlowPath:
>         }
>     }
> 
>-    if (m_length > static_cast<unsigned>(std::numeric_limits<int32_t>::max()))
>+    if (m_length > MaxLength)
>         CRASH();
> 
>     auto upconvertedCharacters = StringView(*this).upconvertedCharacters();
>@@ -935,7 +935,7 @@ size_t StringImpl::find(const LChar* matchString, unsigned index)
>     if (!matchString)
>         return notFound;
>     size_t matchStringLength = strlen(reinterpret_cast<const char*>(matchString));
>-    if (matchStringLength > std::numeric_limits<unsigned>::max())
>+    if (matchStringLength > MaxLength)
>         CRASH();
>     unsigned matchLength = matchStringLength;
>     if (!matchLength)
>@@ -1307,7 +1307,7 @@ Ref<StringImpl> StringImpl::replace(unsigned position, unsigned lengthToReplace,
>     if (!lengthToReplace && !lengthToInsert)
>         return *this;
> 
>-    if ((length() - lengthToReplace) >= (std::numeric_limits<unsigned>::max() - lengthToInsert))
>+    if ((length() - lengthToReplace) >= (MaxLength - lengthToInsert))
>         CRASH();
> 
>     if (is8Bit() && (!string || string->is8Bit())) {
>@@ -1364,12 +1364,12 @@ Ref<StringImpl> StringImpl::replace(UChar pattern, const LChar* replacement, uns
>     if (!matchCount)
>         return *this;
> 
>-    if (repStrLength && matchCount > std::numeric_limits<unsigned>::max() / repStrLength)
>+    if (repStrLength && matchCount > MaxLength / repStrLength)
>         CRASH();
> 
>     unsigned replaceSize = matchCount * repStrLength;
>     unsigned newSize = m_length - matchCount;
>-    if (newSize >= (std::numeric_limits<unsigned>::max() - replaceSize))
>+    if (newSize >= (MaxLength - replaceSize))
>         CRASH();
> 
>     newSize += replaceSize;
>@@ -1440,12 +1440,12 @@ Ref<StringImpl> StringImpl::replace(UChar pattern, const UChar* replacement, uns
>     if (!matchCount)
>         return *this;
> 
>-    if (repStrLength && matchCount > std::numeric_limits<unsigned>::max() / repStrLength)
>+    if (repStrLength && matchCount > MaxLength / repStrLength)
>         CRASH();
> 
>     unsigned replaceSize = matchCount * repStrLength;
>     unsigned newSize = m_length - matchCount;
>-    if (newSize >= (std::numeric_limits<unsigned>::max() - replaceSize))
>+    if (newSize >= (MaxLength - replaceSize))
>         CRASH();
> 
>     newSize += replaceSize;
>@@ -1525,10 +1525,10 @@ Ref<StringImpl> StringImpl::replace(StringImpl* pattern, StringImpl* replacement
>         return *this;
>     
>     unsigned newSize = m_length - matchCount * patternLength;
>-    if (repStrLength && matchCount > std::numeric_limits<unsigned>::max() / repStrLength)
>+    if (repStrLength && matchCount > MaxLength / repStrLength)
>         CRASH();
> 
>-    if (newSize > (std::numeric_limits<unsigned>::max() - matchCount * repStrLength))
>+    if (newSize > (MaxLength - matchCount * repStrLength))
>         CRASH();
> 
>     newSize += matchCount * repStrLength;
>@@ -1804,7 +1804,7 @@ Expected<CString, UTF8ConversionError> StringImpl::utf8ForCharacters(const LChar
> {
>     if (!length)
>         return CString("", 0);
>-    if (length > std::numeric_limits<unsigned>::max() / 3)
>+    if (length > MaxLength / 3)
>         return makeUnexpected(UTF8ConversionError::OutOfMemory);
>     Vector<char, 1024> bufferVector(length * 3);
>     char* buffer = bufferVector.data();
>@@ -1818,7 +1818,7 @@ Expected<CString, UTF8ConversionError> StringImpl::utf8ForCharacters(const UChar
> {
>     if (!length)
>         return CString("", 0);
>-    if (length > std::numeric_limits<unsigned>::max() / 3)
>+    if (length > MaxLength / 3)
>         return makeUnexpected(UTF8ConversionError::OutOfMemory);
>     Vector<char, 1024> bufferVector(length * 3);
>     char* buffer = bufferVector.data();
>@@ -1846,7 +1846,7 @@ Expected<CString, UTF8ConversionError> StringImpl::tryGetUtf8ForRange(unsigned o
>     //  * We could allocate a CStringBuffer with an appropriate size to
>     //    have a good chance of being able to write the string into the
>     //    buffer without reallocing (say, 1.5 x length).
>-    if (length > std::numeric_limits<unsigned>::max() / 3)
>+    if (length > MaxLength / 3)
>         return makeUnexpected(UTF8ConversionError::OutOfMemory);
>     Vector<char, 1024> bufferVector(length * 3);
> 
>diff --git a/Source/WTF/wtf/text/StringImpl.h b/Source/WTF/wtf/text/StringImpl.h
>index 14a66ed1b3380aff5f47f5e23c1cbe517dbc2558..bd01f2c268599dd1100ff1572c51aaad606f3d9c 100644
>--- a/Source/WTF/wtf/text/StringImpl.h
>+++ b/Source/WTF/wtf/text/StringImpl.h
>@@ -379,7 +379,7 @@ public:
>     // its own copy of the string.
>     Ref<StringImpl> isolatedCopy() const;
> 
>-    WTF_EXPORT_PRIVATE Ref<StringImpl> substring(unsigned position, unsigned length = std::numeric_limits<unsigned>::max());
>+    WTF_EXPORT_PRIVATE Ref<StringImpl> substring(unsigned position, unsigned length = MaxLength);
> 
>     UChar at(unsigned) const;
>     UChar operator[](unsigned i) const { return at(i); }
>@@ -437,8 +437,8 @@ public:
>     WTF_EXPORT_PRIVATE size_t findIgnoringASCIICase(const StringImpl*) const;
>     WTF_EXPORT_PRIVATE size_t findIgnoringASCIICase(const StringImpl*, unsigned startOffset) const;
> 
>-    WTF_EXPORT_PRIVATE size_t reverseFind(UChar, unsigned index = std::numeric_limits<unsigned>::max());
>-    WTF_EXPORT_PRIVATE size_t reverseFind(StringImpl*, unsigned index = std::numeric_limits<unsigned>::max());
>+    WTF_EXPORT_PRIVATE size_t reverseFind(UChar, unsigned index = MaxLength);
>+    WTF_EXPORT_PRIVATE size_t reverseFind(StringImpl*, unsigned index = MaxLength);
> 
>     WTF_EXPORT_PRIVATE bool startsWith(const StringImpl*) const;
>     WTF_EXPORT_PRIVATE bool startsWith(const StringImpl&) const;
>@@ -495,6 +495,7 @@ protected:
> 
> private:
>     template<typename> static size_t allocationSize(Checked<size_t> tailElementCount);
>+    template<typename> static size_t maxInternalLength();
>     template<typename> static size_t tailOffset();
> 
>     bool requiresCopy() const;
>@@ -569,10 +570,10 @@ template<unsigned length> bool equalLettersIgnoringASCIICase(const StringImpl*,
> size_t find(const LChar*, unsigned length, CodeUnitMatchFunction, unsigned index = 0);
> size_t find(const UChar*, unsigned length, CodeUnitMatchFunction, unsigned index = 0);
> 
>-template<typename CharacterType> size_t reverseFindLineTerminator(const CharacterType*, unsigned length, unsigned index = std::numeric_limits<unsigned>::max());
>-template<typename CharacterType> size_t reverseFind(const CharacterType*, unsigned length, CharacterType matchCharacter, unsigned index = std::numeric_limits<unsigned>::max());
>-size_t reverseFind(const UChar*, unsigned length, LChar matchCharacter, unsigned index = std::numeric_limits<unsigned>::max());
>-size_t reverseFind(const LChar*, unsigned length, UChar matchCharacter, unsigned index = std::numeric_limits<unsigned>::max());
>+template<typename CharacterType> size_t reverseFindLineTerminator(const CharacterType*, unsigned length, unsigned index = StringImpl::MaxLength);
>+template<typename CharacterType> size_t reverseFind(const CharacterType*, unsigned length, CharacterType matchCharacter, unsigned index = StringImpl::MaxLength);
>+size_t reverseFind(const UChar*, unsigned length, LChar matchCharacter, unsigned index = StringImpl::MaxLength);
>+size_t reverseFind(const LChar*, unsigned length, UChar matchCharacter, unsigned index = StringImpl::MaxLength);
> 
> template<size_t inlineCapacity> bool equalIgnoringNullity(const Vector<UChar, inlineCapacity>&, StringImpl*);
> 
>@@ -755,7 +756,7 @@ template<typename CharacterType> inline unsigned lengthOfNullTerminatedString(co
>     while (string[length])
>         ++length;
> 
>-    RELEASE_ASSERT(length < std::numeric_limits<unsigned>::max());
>+    RELEASE_ASSERT(length < StringImpl::MaxLength);
>     return static_cast<unsigned>(length);
> }
> 
>@@ -954,7 +955,7 @@ template<typename CharacterType> ALWAYS_INLINE RefPtr<StringImpl> StringImpl::tr
>         return empty();
>     }
> 
>-    if (length > ((std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) / sizeof(CharacterType))) {
>+    if (length > maxInternalLength<CharacterType>()) {
>         output = nullptr;
>         return nullptr;
>     }
>@@ -973,7 +974,7 @@ inline Ref<StringImpl> StringImpl::adopt(Vector<CharacterType, inlineCapacity, O
> {
>     if (size_t size = vector.size()) {
>         ASSERT(vector.data());
>-        if (size > std::numeric_limits<unsigned>::max())
>+        if (size > MaxLength)
>             CRASH();
>         return adoptRef(*new StringImpl(vector.releaseBuffer(), size));
>     }
>@@ -1112,6 +1113,13 @@ template<typename T> inline size_t StringImpl::allocationSize(Checked<size_t> ta
>     return (tailOffset<T>() + tailElementCount * sizeof(T)).unsafeGet();
> }
> 
>+template<typename CharacterType>
>+inline size_t StringImpl::maxInternalLength()
>+{
>+    // In order to not overflow the unsigned length, the check for (std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) is needed when sizeof(CharacterType) == 2.
>+    return std::min(static_cast<size_t>(MaxLength), (std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) / sizeof(CharacterType));
>+}
>+
> template<typename T> inline size_t StringImpl::tailOffset()
> {
> #if COMPILER(MSVC)
>diff --git a/Source/WTF/wtf/text/WTFString.cpp b/Source/WTF/wtf/text/WTFString.cpp
>index c96cc991ef884f28fe9e174a318e00bd6b217f74..248fc35e1ce2ca12170840436a851bf2542c0613 100644
>--- a/Source/WTF/wtf/text/WTFString.cpp
>+++ b/Source/WTF/wtf/text/WTFString.cpp
>@@ -99,7 +99,7 @@ void String::append(const String& otherString)
> 
>     auto length = m_impl->length();
>     auto otherLength = otherString.m_impl->length();
>-    if (otherLength > std::numeric_limits<unsigned>::max() - length)
>+    if (otherLength > MaxLength - length)
>         CRASH();
> 
>     if (m_impl->is8Bit() && otherString.m_impl->is8Bit()) {
>@@ -129,7 +129,7 @@ void String::append(LChar character)
>         append(static_cast<UChar>(character));
>         return;
>     }
>-    if (m_impl->length() >= std::numeric_limits<unsigned>::max())
>+    if (m_impl->length() >= MaxLength)
>         CRASH();
>     LChar* data;
>     auto newImpl = StringImpl::createUninitialized(m_impl->length() + 1, data);
>@@ -150,7 +150,7 @@ void String::append(UChar character)
>         append(static_cast<LChar>(character));
>         return;
>     }
>-    if (m_impl->length() >= std::numeric_limits<unsigned>::max())
>+    if (m_impl->length() >= MaxLength)
>         CRASH();
>     UChar* data;
>     auto newImpl = StringImpl::createUninitialized(m_impl->length() + 1, data);
>@@ -183,7 +183,7 @@ void String::insert(const String& string, unsigned position)
>         return;
>     }
> 
>-    if (lengthToInsert > std::numeric_limits<unsigned>::max() - length())
>+    if (lengthToInsert > MaxLength - length())
>         CRASH();
> 
>     if (is8Bit() && string.is8Bit()) {
>@@ -222,7 +222,7 @@ void String::append(const LChar* charactersToAppend, unsigned lengthToAppend)
>     unsigned strLength = m_impl->length();
> 
>     if (m_impl->is8Bit()) {
>-        if (lengthToAppend > std::numeric_limits<unsigned>::max() - strLength)
>+        if (lengthToAppend > MaxLength - strLength)
>             CRASH();
>         LChar* data;
>         auto newImpl = StringImpl::createUninitialized(strLength + lengthToAppend, data);
>@@ -232,7 +232,7 @@ void String::append(const LChar* charactersToAppend, unsigned lengthToAppend)
>         return;
>     }
> 
>-    if (lengthToAppend > std::numeric_limits<unsigned>::max() - strLength)
>+    if (lengthToAppend > MaxLength - strLength)
>         CRASH();
>     UChar* data;
>     auto newImpl = StringImpl::createUninitialized(length() + lengthToAppend, data);
>@@ -258,7 +258,7 @@ void String::append(const UChar* charactersToAppend, unsigned lengthToAppend)
>     unsigned strLength = m_impl->length();
>     
>     ASSERT(charactersToAppend);
>-    if (lengthToAppend > std::numeric_limits<unsigned>::max() - strLength)
>+    if (lengthToAppend > MaxLength - strLength)
>         CRASH();
>     UChar* data;
>     auto newImpl = StringImpl::createUninitialized(strLength + lengthToAppend, data);
>@@ -885,7 +885,7 @@ String String::make16BitFrom8BitSource(const LChar* source, size_t length)
> 
> String String::fromUTF8(const LChar* stringStart, size_t length)
> {
>-    if (length > std::numeric_limits<unsigned>::max())
>+    if (length > MaxLength)
>         CRASH();
> 
>     if (!stringStart)
>diff --git a/Source/WTF/wtf/text/WTFString.h b/Source/WTF/wtf/text/WTFString.h
>index 60bb16eeb04ab9862eddf3229d79491b5af35759..821f059149a69263052882f370fb3ea3821b1dfb 100644
>--- a/Source/WTF/wtf/text/WTFString.h
>+++ b/Source/WTF/wtf/text/WTFString.h
>@@ -194,8 +194,8 @@ public:
>     size_t find(const LChar* string, unsigned start = 0) const { return m_impl ? m_impl->find(string, start) : notFound; }
> 
>     // Find the last instance of a single character or string.
>-    size_t reverseFind(UChar character, unsigned start = std::numeric_limits<unsigned>::max()) const { return m_impl ? m_impl->reverseFind(character, start) : notFound; }
>-    size_t reverseFind(const String& string, unsigned start = std::numeric_limits<unsigned>::max()) const { return m_impl ? m_impl->reverseFind(string.impl(), start) : notFound; }
>+    size_t reverseFind(UChar character, unsigned start = MaxLength) const { return m_impl ? m_impl->reverseFind(character, start) : notFound; }
>+    size_t reverseFind(const String& string, unsigned start = MaxLength) const { return m_impl ? m_impl->reverseFind(string.impl(), start) : notFound; }
> 
>     WTF_EXPORT_PRIVATE Vector<UChar> charactersWithNullTermination() const;
> 
>@@ -237,8 +237,8 @@ public:
>     WTF_EXPORT_PRIVATE void truncate(unsigned length);
>     WTF_EXPORT_PRIVATE void remove(unsigned position, unsigned length = 1);
> 
>-    WTF_EXPORT_PRIVATE String substring(unsigned position, unsigned length = std::numeric_limits<unsigned>::max()) const;
>-    WTF_EXPORT_PRIVATE String substringSharingImpl(unsigned position, unsigned length = std::numeric_limits<unsigned>::max()) const;
>+    WTF_EXPORT_PRIVATE String substring(unsigned position, unsigned length = MaxLength) const;
>+    WTF_EXPORT_PRIVATE String substringSharingImpl(unsigned position, unsigned length = MaxLength) const;
>     String left(unsigned length) const { return substring(0, length); }
>     String right(unsigned length) const { return substring(this->length() - length, length); }
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index dbd5b3c9aa08d93ba5f21dfcc232013e8b813ae2..70b8d94e82d393506f03a73b3b23fc641509d037 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-12-19  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        WTF::String and StringImpl overflow MaxLength
>+        https://bugs.webkit.org/show_bug.cgi?id=192853
>+        <rdar://problem/45726906>
>+
>+        Reviewed by Mark Lam.
>+
>+        * stress/string-16bit-repeat-overflow.js: Added.
>+        (catch):
>+
> 2018-12-18  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         Error message for `-x ** y` contains a typo.
>diff --git a/JSTests/stress/string-16bit-repeat-overflow.js b/JSTests/stress/string-16bit-repeat-overflow.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..bc724fbf62bc86786d7b0ed775a53c1077e5ee79
>--- /dev/null
>+++ b/JSTests/stress/string-16bit-repeat-overflow.js
>@@ -0,0 +1,9 @@
>+var exception;
>+try {
>+    print('\ud000'.repeat(2**30));
>+} catch (e) {
>+    exception = e;
>+}
>+
>+if (exception != "Error: Out of memory")
>+    throw "FAILED";

Actions: View | Formatted Diff | Diff

Attachments on bug 192853: 357678 | 357741 | 357811