Unreviewed, rolling out r231197.
authorryanhaddad@apple.com <ryanhaddad@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 May 2018 16:58:48 +0000 (16:58 +0000)
committerryanhaddad@apple.com <ryanhaddad@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 May 2018 16:58:48 +0000 (16:58 +0000)
The test added with this change crashes on the 32-bit JSC bot.

Reverted changeset:

"Correctly detect string overflow when using the 'Function'
constructor"
https://bugs.webkit.org/show_bug.cgi?id=184883
https://trac.webkit.org/changeset/231197

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231310 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js [deleted file]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/FunctionConstructor.cpp
Source/JavaScriptCore/runtime/JSONObject.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringBuilder.cpp
Source/WTF/wtf/text/StringBuilder.h
Source/WTF/wtf/text/StringBuilderJSON.cpp

index 23aede9..2427143 100644 (file)
@@ -1,3 +1,16 @@
+2018-05-03  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r231197.
+
+        The test added with this change crashes on the 32-bit JSC bot.
+
+        Reverted changeset:
+
+        "Correctly detect string overflow when using the 'Function'
+        constructor"
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        https://trac.webkit.org/changeset/231197
+
 2018-05-02  Filip Pizlo  <fpizlo@apple.com>
 
         JSC should know how to cache custom getter accesses on the prototype chain
diff --git a/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js b/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js
deleted file mode 100644 (file)
index 265cead..0000000
+++ /dev/null
@@ -1,13 +0,0 @@
-var hugeString = "x";
-for (i = 0; i < 25; ++i) {
-    hugeString += hugeString;
-}
-
-var weird = 'Â\8f';
-try {
-    var f = new Function(hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
-      hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
-      hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
-      () => 42,
-      "return 42;");
-} catch (e) {}
index 2470668..1797b2e 100644 (file)
@@ -1,3 +1,16 @@
+2018-05-03  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r231197.
+
+        The test added with this change crashes on the 32-bit JSC bot.
+
+        Reverted changeset:
+
+        "Correctly detect string overflow when using the 'Function'
+        constructor"
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        https://trac.webkit.org/changeset/231197
+
 2018-05-03  Dominik Infuehr  <dinfuehr@igalia.com>
 
         Disable usage of fused multiply-add instructions for JSC with compiler flag
index 1b72af5..031fe32 100644 (file)
@@ -125,24 +125,19 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(
         program = makeString("{", prefix, functionName.string(), "() {\n", body, "\n}}");
     } else {
         StringBuilder builder;
-        bool success = true;
-        success &= builder.tryAppend('{');
-        success &= builder.tryAppend(prefix);
-        success &= builder.tryAppend(functionName.string());
-        success &= builder.tryAppend('(');
+        builder.append('{');
+        builder.append(prefix);
+        builder.append(functionName.string());
+        builder.append('(');
         StringBuilder parameterBuilder;
         auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
-        success &= parameterBuilder.tryAppend(viewWithString.view);
+        parameterBuilder.append(viewWithString.view);
         for (size_t i = 1; i < args.size() - 1; i++) {
-            success &= parameterBuilder.tryAppendLiteral(", ");
+            parameterBuilder.appendLiteral(", ");
             auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(exec);
             RETURN_IF_EXCEPTION(scope, nullptr);
-            success &= parameterBuilder.tryAppend(viewWithString.view);
-        }
-        if (!success) {
-            throwOutOfMemoryError(exec, scope);
-            return nullptr;
+            parameterBuilder.append(viewWithString.view);
         }
 
         {
@@ -158,18 +153,14 @@ JSObject* constructFunctionSkippingEvalEnabledCheck(
             }
         }
 
-        success &= builder.tryAppend(parameterBuilder);
-        success &= builder.tryAppendLiteral(") {\n");
+        builder.append(parameterBuilder);
+        builder.appendLiteral(") {\n");
         auto body = args.at(args.size() - 1).toWTFString(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
         checkBody(body);
         RETURN_IF_EXCEPTION(scope, nullptr);
-        success &= builder.tryAppend(body);
-        success &= builder.tryAppendLiteral("\n}}");
-        if (!success) {
-            throwOutOfMemoryError(exec, scope);
-            return nullptr;
-        }
+        builder.append(body);
+        builder.appendLiteral("\n}}");
         program = builder.toString();
     }
 
index 7e9b3d6..b3a7808 100644 (file)
@@ -357,7 +357,7 @@ Stringifier::StringifyResult Stringifier::appendStringifiedValue(StringBuilder&
     if (value.isString()) {
         const String& string = asString(value)->value(m_exec);
         RETURN_IF_EXCEPTION(scope, StringifyFailed);
-        if (builder.tryAppendQuotedJSONString(string))
+        if (builder.appendQuotedJSONString(string))
             return StringifySucceeded;
         throwOutOfMemoryError(m_exec, scope);
         return StringifyFailed;
index 9454561..85bbd26 100644 (file)
@@ -1,3 +1,16 @@
+2018-05-03  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r231197.
+
+        The test added with this change crashes on the 32-bit JSC bot.
+
+        Reverted changeset:
+
+        "Correctly detect string overflow when using the 'Function'
+        constructor"
+        https://bugs.webkit.org/show_bug.cgi?id=184883
+        https://trac.webkit.org/changeset/231197
+
 2018-05-03  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r231223 and r231288.
index d79d4c5..85b0c21 100644 (file)
@@ -126,17 +126,10 @@ void StringBuilder::allocateBuffer(const UChar* currentCharacters, unsigned requ
 // from either m_string or m_buffer, neither will be reassigned until the copy has completed).
 void StringBuilder::allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength)
 {
-    if (!tryAllocateBufferUpConvert(currentCharacters, requiredLength))
-        CRASH();
-}
-bool StringBuilder::tryAllocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength)
-{
     ASSERT(m_is8Bit);
     ASSERT(requiredLength >= m_length);
     // Copy the existing data into a new buffer, set result to point to the end of the existing data.
-    auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters16);
-    if (!buffer)
-        return false;
+    auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters16);
     for (unsigned i = 0; i < m_length; ++i)
         m_bufferCharacters16[i] = currentCharacters[i];
     
@@ -146,7 +139,6 @@ bool StringBuilder::tryAllocateBufferUpConvert(const LChar* currentCharacters, u
     m_buffer = WTFMove(buffer);
     m_string = String();
     ASSERT(m_buffer->length() == requiredLength);
-    return true;
 }
 
 template <>
@@ -209,7 +201,6 @@ void StringBuilder::reserveCapacity(unsigned newCapacity)
 
 // Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
 // return a pointer to the newly allocated storage.
-// Returns nullptr if the size of the new builder would have overflowed
 template <typename CharType>
 ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
 {
@@ -218,7 +209,7 @@ ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
     // Calculate the new size of the builder after appending.
     unsigned requiredLength = length + m_length;
     if (requiredLength < length)
-        return nullptr;
+        CRASH();
 
     if ((m_buffer) && (requiredLength <= m_buffer->length())) {
         // If the buffer is valid it must be at least as long as the current builder contents!
@@ -257,14 +248,8 @@ CharType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
 
 void StringBuilder::append(const UChar* characters, unsigned length)
 {
-    if (!tryAppend(characters, length))
-        CRASH();
-}
-
-bool StringBuilder::tryAppend(const UChar* characters, unsigned length)
-{
     if (!length)
-        return true;
+        return;
 
     ASSERT(characters);
 
@@ -272,54 +257,40 @@ bool StringBuilder::tryAppend(const UChar* characters, unsigned length)
         if (length == 1 && !(*characters & ~0xff)) {
             // Append as 8 bit character
             LChar lChar = static_cast<LChar>(*characters);
-            return tryAppend(&lChar, 1);
+            append(&lChar, 1);
+            return;
         }
 
         // Calculate the new size of the builder after appending.
         unsigned requiredLength = length + m_length;
         if (requiredLength < length)
-            return false;
+            CRASH();
         
         if (m_buffer) {
             // If the buffer is valid it must be at least as long as the current builder contents!
             ASSERT(m_buffer->length() >= m_length);
-            if (!tryAllocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength)))
-                return false;
+            
+            allocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength));
         } else {
             ASSERT(m_string.length() == m_length);
-            if (!tryAllocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength)))
-                return false;
+            allocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength));
         }
 
         memcpy(m_bufferCharacters16 + m_length, characters, static_cast<size_t>(length) * sizeof(UChar));
         m_length = requiredLength;
-    } else {
-        UChar* dest = appendUninitialized<UChar>(length);
-        if (!dest)
-            return false;
-        memcpy(dest, characters, static_cast<size_t>(length) * sizeof(UChar));
-    }
+    } else
+        memcpy(appendUninitialized<UChar>(length), characters, static_cast<size_t>(length) * sizeof(UChar));
     ASSERT(m_buffer->length() >= m_length);
-    return true;
 }
 
 void StringBuilder::append(const LChar* characters, unsigned length)
 {
-    if (!tryAppend(characters, length))
-        CRASH();
-}
-
-bool StringBuilder::tryAppend(const LChar* characters, unsigned length)
-{
     if (!length)
-        return true;
-
+        return;
     ASSERT(characters);
 
     if (m_is8Bit) {
         LChar* dest = appendUninitialized<LChar>(length);
-        if (!dest)
-            return false;
         if (length > 8)
             memcpy(dest, characters, static_cast<size_t>(length) * sizeof(LChar));
         else {
@@ -329,13 +300,10 @@ bool StringBuilder::tryAppend(const LChar* characters, unsigned length)
         }
     } else {
         UChar* dest = appendUninitialized<UChar>(length);
-        if (!dest)
-            return false;
         const LChar* end = characters + length;
         while (characters < end)
             *(dest++) = *(characters++);
     }
-    return true;
 }
 
 #if USE(CF)
index 7e974c1..8e4c720 100644 (file)
@@ -49,12 +49,9 @@ public:
     StringBuilder& operator=(StringBuilder&&) = default;
 
     WTF_EXPORT_PRIVATE void append(const UChar*, unsigned);
-    WTF_EXPORT_PRIVATE bool tryAppend(const UChar*, unsigned);
     WTF_EXPORT_PRIVATE void append(const LChar*, unsigned);
-    WTF_EXPORT_PRIVATE bool tryAppend(const LChar*, unsigned);
 
     ALWAYS_INLINE void append(const char* characters, unsigned length) { append(reinterpret_cast<const LChar*>(characters), length); }
-    ALWAYS_INLINE bool tryAppend(const char* characters, unsigned length) { return tryAppend(reinterpret_cast<const LChar*>(characters), length); }
 
     void append(const AtomicString& atomicString)
     {
@@ -63,13 +60,8 @@ public:
 
     void append(const String& string)
     {
-        if (!tryAppend(string))
-            CRASH();
-    }
-    bool tryAppend(const String& string)
-    {
         if (!string.length())
-            return true;
+            return;
 
         // If we're appending to an empty string, and there is not a buffer (reserveCapacity has not been called)
         // then just retain the string.
@@ -77,50 +69,40 @@ public:
             m_string = string;
             m_length = string.length();
             m_is8Bit = m_string.is8Bit();
-            return true;
+            return;
         }
 
         if (string.is8Bit())
-            return tryAppend(string.characters8(), string.length());
+            append(string.characters8(), string.length());
         else
-            return tryAppend(string.characters16(), string.length());
+            append(string.characters16(), string.length());
     }
 
     void append(const StringBuilder& other)
     {
-        if (!tryAppend(other))
-            CRASH();
-    }
-    bool tryAppend(const StringBuilder& other)
-    {
         if (!other.m_length)
-            return true;
+            return;
 
         // If we're appending to an empty string, and there is not a buffer (reserveCapacity has not been called)
         // then just retain the string.
         if (!m_length && !m_buffer && !other.m_string.isNull()) {
             m_string = other.m_string;
             m_length = other.m_length;
-            return true;
+            return;
         }
 
         if (other.is8Bit())
-            return tryAppend(other.characters8(), other.m_length);
+            append(other.characters8(), other.m_length);
         else
-            return tryAppend(other.characters16(), other.m_length);
+            append(other.characters16(), other.m_length);
     }
 
     void append(StringView stringView)
     {
-        if (!tryAppend(stringView))
-            CRASH();
-    }
-    bool tryAppend(StringView stringView)
-    {
         if (stringView.is8Bit())
-            return tryAppend(stringView.characters8(), stringView.length());
+            append(stringView.characters8(), stringView.length());
         else
-            return tryAppend(stringView.characters16(), stringView.length());
+            append(stringView.characters16(), stringView.length());
     }
 
 #if USE(CF)
@@ -149,12 +131,6 @@ public:
         if (characters)
             append(characters, strlen(characters));
     }
-    bool tryAppend(const char* characters)
-    {
-        if (characters)
-            return tryAppend(characters, strlen(characters));
-        return true;
-    }
 
     void append(UChar c)
     {
@@ -174,23 +150,19 @@ public:
 
     void append(LChar c)
     {
-        if (!tryAppend(c))
-            CRASH();
-    }
-    bool tryAppend(LChar c)
-    {
         if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
             if (m_is8Bit)
                 m_bufferCharacters8[m_length++] = c;
             else
                 m_bufferCharacters16[m_length++] = c;
-            return true;
         } else
-            return tryAppend(&c, 1);
+            append(&c, 1);
     }
 
-    void append(char c) { append(static_cast<LChar>(c)); }
-    bool tryAppend(char c) { return tryAppend(static_cast<LChar>(c)); }
+    void append(char c)
+    {
+        append(static_cast<LChar>(c));
+    }
 
     void append(UChar32 c)
     {
@@ -202,13 +174,10 @@ public:
         append(U16_TRAIL(c));
     }
 
-    WTF_EXPORT_PRIVATE void appendQuotedJSONString(const String&);
-    WTF_EXPORT_PRIVATE bool tryAppendQuotedJSONString(const String&);
+    WTF_EXPORT_PRIVATE bool appendQuotedJSONString(const String&);
 
     template<unsigned characterCount>
     ALWAYS_INLINE void appendLiteral(const char (&characters)[characterCount]) { append(characters, characterCount - 1); }
-    template<unsigned characterCount>
-    ALWAYS_INLINE bool tryAppendLiteral(const char (&characters)[characterCount]) { return tryAppend(characters, characterCount - 1); }
 
     WTF_EXPORT_PRIVATE void appendNumber(int);
     WTF_EXPORT_PRIVATE void appendNumber(unsigned int);
@@ -329,7 +298,6 @@ private:
     void allocateBuffer(const LChar* currentCharacters, unsigned requiredLength);
     void allocateBuffer(const UChar* currentCharacters, unsigned requiredLength);
     void allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
-    bool tryAllocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
     template <typename CharType>
     void reallocateBuffer(unsigned requiredLength);
     template <typename CharType>
index 4c2766c..226295e 100644 (file)
@@ -74,13 +74,7 @@ ALWAYS_INLINE static void appendQuotedJSONStringInternal(OutputCharacterType*& o
     }
 }
 
-void StringBuilder::appendQuotedJSONString(const String& string)
-{
-    if (!tryAppendQuotedJSONString(string))
-        CRASH();
-}
-
-bool StringBuilder::tryAppendQuotedJSONString(const String& string)
+bool StringBuilder::appendQuotedJSONString(const String& string)
 {
     // Make sure we have enough buffer space to append this string without having
     // to worry about reallocating in the middle.