Harden JSStringJoiner
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Mar 2013 22:15:54 +0000 (22:15 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Mar 2013 22:15:54 +0000 (22:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=112093

Reviewed by Filip Pizlo.

Harden JSStringJoiner, make it use our CheckedArithmetic
class to simplify everything.

* runtime/JSStringJoiner.cpp:
(JSC::JSStringJoiner::build):
* runtime/JSStringJoiner.h:
(JSStringJoiner):
(JSC::JSStringJoiner::JSStringJoiner):
(JSC::JSStringJoiner::append):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSStringJoiner.cpp
Source/JavaScriptCore/runtime/JSStringJoiner.h

index 295abc6..1f9f5fe 100644 (file)
@@ -1,3 +1,20 @@
+2013-03-11  Oliver Hunt  <oliver@apple.com>
+
+        Harden JSStringJoiner
+        https://bugs.webkit.org/show_bug.cgi?id=112093
+
+        Reviewed by Filip Pizlo.
+
+        Harden JSStringJoiner, make it use our CheckedArithmetic
+        class to simplify everything.
+
+        * runtime/JSStringJoiner.cpp:
+        (JSC::JSStringJoiner::build):
+        * runtime/JSStringJoiner.h:
+        (JSStringJoiner):
+        (JSC::JSStringJoiner::JSStringJoiner):
+        (JSC::JSStringJoiner::append):
+
 2013-03-12  Filip Pizlo  <fpizlo@apple.com>
 
         DFG generic array access cases should not be guarded by CheckStructure even of the profiling tells us that it could be
index 5b1f28d..a23fa02 100644 (file)
@@ -102,20 +102,24 @@ JSValue JSStringJoiner::build(ExecState* exec)
     if (!m_strings.size())
         return jsEmptyString(exec);
 
-    size_t separatorLength = m_separator.length();
+    Checked<size_t, RecordOverflow> separatorLength = m_separator.length();
     // FIXME: add special cases of joinStrings() for (separatorLength == 0) and (separatorLength == 1).
     ASSERT(m_strings.size() > 0);
-    size_t totalSeparactorsLength = separatorLength * (m_strings.size() - 1);
-    size_t outputStringSize = totalSeparactorsLength + m_cumulatedStringsLength;
+    Checked<size_t, RecordOverflow> totalSeparactorsLength = separatorLength * (m_strings.size() - 1);
+    Checked<size_t, RecordOverflow> outputStringSize = totalSeparactorsLength + m_accumulatedStringsLength;
 
+    size_t finalSize;
+    if (outputStringSize.safeGet(finalSize) == CheckedState::DidOverflow)
+        return throwOutOfMemoryError(exec);
+        
     if (!outputStringSize)
         return jsEmptyString(exec);
 
     RefPtr<StringImpl> outputStringImpl;
     if (m_is8Bits)
-        outputStringImpl = joinStrings<LChar>(m_strings, m_separator, outputStringSize);
+        outputStringImpl = joinStrings<LChar>(m_strings, m_separator, finalSize);
     else
-        outputStringImpl = joinStrings<UChar>(m_strings, m_separator, outputStringSize);
+        outputStringImpl = joinStrings<UChar>(m_strings, m_separator, finalSize);
 
     if (!outputStringImpl)
         return throwOutOfMemoryError(exec);
index 6340cbf..4074888 100644 (file)
@@ -46,14 +46,13 @@ private:
     String m_separator;
     Vector<String> m_strings;
 
-    unsigned m_cumulatedStringsLength;
+    Checked<unsigned, RecordOverflow> m_accumulatedStringsLength;
     bool m_isValid;
     bool m_is8Bits;
 };
 
 inline JSStringJoiner::JSStringJoiner(const String& separator, size_t stringCount)
     : m_separator(separator)
-    , m_cumulatedStringsLength(0)
     , m_isValid(true)
     , m_is8Bits(m_separator.is8Bit())
 {
@@ -66,9 +65,9 @@ inline void JSStringJoiner::append(const String& str)
     if (!m_isValid)
         return;
 
-    m_strings.uncheckedAppend(str);
+    m_strings.append(str);
     if (!str.isNull()) {
-        m_cumulatedStringsLength += str.length();
+        m_accumulatedStringsLength += str.length();
         m_is8Bits = m_is8Bits && str.is8Bit();
     }
 }