[webkit-changes] [207861] trunk

Tue, 25 Oct 2016 18:15:06 -0700

Title: [207861] trunk
Revision
207861
Author
mark....@apple.com
Date
2016-10-25 18:15:49 -0700 (Tue, 25 Oct 2016)

Log Message

String.prototype.replace() should throw an OutOfMemoryError when using too much memory.
https://bugs.webkit.org/show_bug.cgi?id=163996
<rdar://problem/28263117>

Reviewed by Geoffrey Garen.

JSTests:

* stress/string-prototype-replace-should-throw-out-of-memory-error-when-using-too-much-memory.js: Added.

Source/_javascript_Core:

String.prototype.replace() uses a Vector internally for bookkeeping work.
Currently, if this vector gets too big, we just crash on allocation failure.
While this is correct behavior, it is not too friendly.

We now detect the imminent failure, and throw a OutOfMemoryError instead.

* runtime/StringPrototype.cpp:
(JSC::removeUsingRegExpSearch):
(JSC::replaceUsingRegExpSearch):
(JSC::operationStringProtoFuncReplaceRegExpEmptyStr):
(JSC::stringProtoFuncReplaceUsingRegExp):

Source/WTF:

* wtf/Vector.h:
(WTF::minCapacity>::tryConstructAndAppend):
(WTF::minCapacity>::tryConstructAndAppendSlowCase):
- Added try versions of constructAndAppend() so that we can handle the failure
  to allocate more gracefully.

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (207860 => 207861)


--- trunk/JSTests/ChangeLog	2016-10-26 01:15:36 UTC (rev 207860)
+++ trunk/JSTests/ChangeLog	2016-10-26 01:15:49 UTC (rev 207861)
@@ -1,5 +1,15 @@
 2016-10-25  Mark Lam  <mark....@apple.com>
 
+        String.prototype.replace() should throw an OutOfMemoryError when using too much memory.
+        https://bugs.webkit.org/show_bug.cgi?id=163996
+        <rdar://problem/28263117>
+
+        Reviewed by Geoffrey Garen.
+
+        * stress/string-prototype-replace-should-throw-out-of-memory-error-when-using-too-much-memory.js: Added.
+
+2016-10-25  Mark Lam  <mark....@apple.com>
+
         JSStringJoiner::joinedLength() should limit joined string lengths to INT_MAX.
         https://bugs.webkit.org/show_bug.cgi?id=163937
         <rdar://problem/28642990>

Added: trunk/JSTests/stress/string-prototype-replace-should-throw-out-of-memory-error-when-using-too-much-memory.js (0 => 207861)


--- trunk/JSTests/stress/string-prototype-replace-should-throw-out-of-memory-error-when-using-too-much-memory.js	                        (rev 0)
+++ trunk/JSTests/stress/string-prototype-replace-should-throw-out-of-memory-error-when-using-too-much-memory.js	2016-10-26 01:15:49 UTC (rev 207861)
@@ -0,0 +1,23 @@
+//@ runFTLNoCJIT
+//@ slow!
+//@largeHeap
+// This test should not crash or fail any assertions.
+
+function shouldEqual(testId, actual, expected) {
+    if (actual != expected) {
+        throw testId + ": ERROR: expect " + expected + ", actual " + actual;
+    }
+}
+
+var exception = undefined;
+
+s2 = 'x'.repeat(0x3fffffff);
+r0 = /((?=\S))/giy;
+
+try {
+    s2.replace(r0, '')
+} catch (e) {
+    exception = e;
+}
+
+shouldEqual(10000, exception, "Error: Out of memory");

Modified: trunk/Source/_javascript_Core/ChangeLog (207860 => 207861)


--- trunk/Source/_javascript_Core/ChangeLog	2016-10-26 01:15:36 UTC (rev 207860)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-10-26 01:15:49 UTC (rev 207861)
@@ -1,5 +1,25 @@
 2016-10-25  Mark Lam  <mark....@apple.com>
 
+        String.prototype.replace() should throw an OutOfMemoryError when using too much memory.
+        https://bugs.webkit.org/show_bug.cgi?id=163996
+        <rdar://problem/28263117>
+
+        Reviewed by Geoffrey Garen.
+
+        String.prototype.replace() uses a Vector internally for bookkeeping work.
+        Currently, if this vector gets too big, we just crash on allocation failure.
+        While this is correct behavior, it is not too friendly.
+
+        We now detect the imminent failure, and throw a OutOfMemoryError instead.
+
+        * runtime/StringPrototype.cpp:
+        (JSC::removeUsingRegExpSearch):
+        (JSC::replaceUsingRegExpSearch):
+        (JSC::operationStringProtoFuncReplaceRegExpEmptyStr):
+        (JSC::stringProtoFuncReplaceUsingRegExp):
+
+2016-10-25  Mark Lam  <mark....@apple.com>
+
         Rename the reject() helper function to something more meaningful.
         https://bugs.webkit.org/show_bug.cgi?id=163549

Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (207860 => 207861)


--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2016-10-26 01:15:36 UTC (rev 207860)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2016-10-26 01:15:49 UTC (rev 207861)
@@ -433,8 +433,15 @@
     return jsString(exec, WTFMove(impl));
 }
 
+#define OUT_OF_MEMORY(exec__, scope__) \
+    do { \
+        throwOutOfMemoryError(exec__, scope__); \
+        return encodedJSValue(); \
+    } while (false)
+
 static ALWAYS_INLINE EncodedJSValue removeUsingRegExpSearch(VM& vm, ExecState* exec, JSString* string, const String& source, RegExp* regExp)
 {
+    auto scope = DECLARE_THROW_SCOPE(vm);
     SuperSamplerScope superSamplerScope(false);
     
     size_t lastIndex = 0;
@@ -449,9 +456,10 @@
         if (!result)
             break;
 
-        if (lastIndex < result.start)
-            sourceRanges.constructAndAppend(lastIndex, result.start - lastIndex);
-
+        if (lastIndex < result.start) {
+            if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
+                OUT_OF_MEMORY(exec, scope);
+        }
         lastIndex = result.end;
         startPosition = lastIndex;
 
@@ -466,9 +474,11 @@
     if (!lastIndex)
         return JSValue::encode(string);
 
-    if (static_cast<unsigned>(lastIndex) < sourceLen)
-        sourceRanges.constructAndAppend(lastIndex, sourceLen - lastIndex);
-
+    if (static_cast<unsigned>(lastIndex) < sourceLen) {
+        if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, sourceLen - lastIndex)))
+            OUT_OF_MEMORY(exec, scope);
+    }
+    scope.release();
     return JSValue::encode(jsSpliceSubstrings(exec, string, source, sourceRanges.data(), sourceRanges.size()));
 }
 
@@ -490,8 +500,10 @@
         regExpObject->setLastIndex(exec, 0);
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
 
-        if (callType == CallType::None && !replacementString.length())
+        if (callType == CallType::None && !replacementString.length()) {
+            scope.release();
             return removeUsingRegExpSearch(vm, exec, string, source, regExp);
+        }
     }
 
     // FIXME: This is wrong because we may be called directly from the FTL.
@@ -518,7 +530,8 @@
                 if (!result)
                     break;
 
-                sourceRanges.constructAndAppend(lastIndex, result.start - lastIndex);
+                if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
+                    OUT_OF_MEMORY(exec, scope);
 
                 unsigned i = 0;
                 for (; i < regExp->numSubpatterns() + 1; ++i) {
@@ -556,7 +569,8 @@
                 if (!result)
                     break;
 
-                sourceRanges.constructAndAppend(lastIndex, result.start - lastIndex);
+                if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
+                    OUT_OF_MEMORY(exec, scope);
 
                 unsigned i = 0;
                 for (; i < regExp->numSubpatterns() + 1; ++i) {
@@ -596,7 +610,8 @@
                 break;
 
             if (callType != CallType::None) {
-                sourceRanges.constructAndAppend(lastIndex, result.start - lastIndex);
+                if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
+                    OUT_OF_MEMORY(exec, scope);
 
                 MarkedArgumentBuffer args;
 
@@ -620,7 +635,8 @@
             } else {
                 int replLen = replacementString.length();
                 if (lastIndex < result.start || replLen) {
-                    sourceRanges.constructAndAppend(lastIndex, result.start - lastIndex);
+                    if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, result.start - lastIndex)))
+                        OUT_OF_MEMORY(exec, scope);
 
                     if (replLen)
                         replacements.append(substituteBackreferences(replacementString, source, ovector, regExp));
@@ -644,9 +660,10 @@
     if (!lastIndex && replacements.isEmpty())
         return JSValue::encode(string);
 
-    if (static_cast<unsigned>(lastIndex) < sourceLen)
-        sourceRanges.constructAndAppend(lastIndex, sourceLen - lastIndex);
-
+    if (static_cast<unsigned>(lastIndex) < sourceLen) {
+        if (UNLIKELY(!sourceRanges.tryConstructAndAppend(lastIndex, sourceLen - lastIndex)))
+            OUT_OF_MEMORY(exec, scope);
+    }
     return JSValue::encode(jsSpliceSubstringsWithSeparators(exec, string, source, sourceRanges.data(), sourceRanges.size(), replacements.data(), replacements.size()));
 }
 
@@ -662,11 +679,13 @@
         // ES5.1 15.5.4.10 step 8.a.
         searchValue->setLastIndex(exec, 0);
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
+        scope.release();
         return removeUsingRegExpSearch(vm, exec, thisValue, thisValue->value(exec), regExp);
     }
 
     CallData callData;
     String replacementString = emptyString();
+    scope.release();
     return replaceUsingRegExpSearch(
         vm, exec, thisValue, searchValue, callData, CallType::None, replacementString, JSValue());
 }
@@ -695,6 +714,7 @@
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
     }
 
+    scope.release();
     return replaceUsingRegExpSearch(
         vm, exec, string, searchValue, callData, callType, replacementString, replaceValue);
 }
@@ -832,6 +852,7 @@
     if (!searchValue.inherits(RegExpObject::info()))
         return JSValue::encode(jsUndefined());
 
+    scope.release();
     return replaceUsingRegExpSearch(exec->vm(), exec, string, searchValue, exec->argument(1));
 }

Modified: trunk/Source/WTF/ChangeLog (207860 => 207861)


--- trunk/Source/WTF/ChangeLog	2016-10-26 01:15:36 UTC (rev 207860)
+++ trunk/Source/WTF/ChangeLog	2016-10-26 01:15:49 UTC (rev 207861)
@@ -1,3 +1,17 @@
+2016-10-25  Mark Lam  <mark....@apple.com>
+
+        String.prototype.replace() should throw an OutOfMemoryError when using too much memory.
+        https://bugs.webkit.org/show_bug.cgi?id=163996
+        <rdar://problem/28263117>
+
+        Reviewed by Geoffrey Garen.
+
+        * wtf/Vector.h:
+        (WTF::minCapacity>::tryConstructAndAppend):
+        (WTF::minCapacity>::tryConstructAndAppendSlowCase):
+        - Added try versions of constructAndAppend() so that we can handle the failure
+          to allocate more gracefully.
+
 2016-10-25  Konstantin Tokarev  <annu...@yandex.ru>
 
         Non-specialized version of deleteObject should not have template argument

Modified: trunk/Source/WTF/wtf/Vector.h (207860 => 207861)


--- trunk/Source/WTF/wtf/Vector.h	2016-10-26 01:15:36 UTC (rev 207860)
+++ trunk/Source/WTF/wtf/Vector.h	2016-10-26 01:15:49 UTC (rev 207861)
@@ -722,6 +722,7 @@
     void append(ValueType&& value) { append<ValueType>(std::forward<ValueType>(value)); }
     template<typename U> void append(U&&);
     template<typename... Args> void constructAndAppend(Args&&...);
+    template<typename... Args> bool tryConstructAndAppend(Args&&...);
 
     void uncheckedAppend(ValueType&& value) { uncheckedAppend<ValueType>(std::forward<ValueType>(value)); }
     template<typename U> void uncheckedAppend(U&&);
@@ -785,6 +786,7 @@
     template<typename U> U* expandCapacity(size_t newMinCapacity, U*); 
     template<typename U> void appendSlowCase(U&&);
     template<typename... Args> void constructAndAppendSlowCase(Args&&...);
+    template<typename... Args> bool tryConstructAndAppendSlowCase(Args&&...);
 
     void asanSetInitialBufferSizeTo(size_t);
     void asanSetBufferSizeToFullCapacity();
@@ -1225,6 +1227,19 @@
     constructAndAppendSlowCase(std::forward<Args>(args)...);
 }
 
+template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity> template<typename... Args>
+ALWAYS_INLINE bool Vector<T, inlineCapacity, OverflowHandler, minCapacity>::tryConstructAndAppend(Args&&... args)
+{
+    if (size() != capacity()) {
+        asanBufferSizeWillChangeTo(m_size + 1);
+        new (NotNull, end()) T(std::forward<Args>(args)...);
+        ++m_size;
+        return true;
+    }
+    
+    return tryConstructAndAppendSlowCase(std::forward<Args>(args)...);
+}
+
 template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity> template<typename U>
 void Vector<T, inlineCapacity, OverflowHandler, minCapacity>::appendSlowCase(U&& value)
 {
@@ -1252,6 +1267,21 @@
     ++m_size;
 }
 
+template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity> template<typename... Args>
+bool Vector<T, inlineCapacity, OverflowHandler, minCapacity>::tryConstructAndAppendSlowCase(Args&&... args)
+{
+    ASSERT(size() == capacity());
+    
+    if (UNLIKELY(!tryExpandCapacity(size() + 1)))
+        return false;
+    ASSERT(begin());
+    
+    asanBufferSizeWillChangeTo(m_size + 1);
+    new (NotNull, end()) T(std::forward<Args>(args)...);
+    ++m_size;
+    return true;
+}
+
 // This version of append saves a branch in the case where you know that the
 // vector's capacity is large enough for the append to succeed.
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to