2007-08-02 Mark Rowe <mrowe@apple.com>
authorbdash <bdash@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2007 09:33:22 +0000 (09:33 +0000)
committerbdash <bdash@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Aug 2007 09:33:22 +0000 (09:33 +0000)
        Reviewed by Maciej.

        <rdar://problem/5352887> "Out of memory" error during repeated JS string concatenation leaks hundreds of MBs of RAM

        A call to fastRealloc was failing which lead to UString::expandCapacity leaking the buffer it was trying to reallocate.
        It also resulted in the underlying UString::rep having both a null baseString and buf field, which meant that attempting
        to access the contents of the string after the failed memory reallocation would crash.

        A third issue is that expandedSize size was calculating the new length in a way that led to an integer overflow occurring.
        Attempting to allocate a string more than 190,000,000 characters long would fail a the integer overflow would lead to a
        memory allocation of around 3.6GB being attempted rather than the expected 390MB.  Sizes that would lead to an overflow
        are now  returned as zero and callers are updated to treat this as though the memory allocation has failed.

        * kjs/array_object.cpp:
        (ArrayProtoFunc::callAsFunction): Check whether the append failed and raise an "Out of memory" exception if it did.
        * kjs/ustring.cpp:
        (KJS::allocChars): Wrapper around fastMalloc that takes a length in characters.  It will return 0 when asked to allocate a zero-length buffer.
        (KJS::reallocChars): Wrapper around fastRealloc that takes a length in characters.  It will return 0 when asked to allocate a zero-length buffer.
        (KJS::UString::expandedSize): Split the size calculation in two and guard against overflow during each step.
        (KJS::UString::expandCapacity): Don't leak r->buf if reallocation fails.  Instead free the memory and use the null representation.
        (KJS::UString::expandPreCapacity): If fastMalloc fails then use the null representation rather than crashing in memcpy.
        (KJS::UString::UString): If calls to expandCapacity, expandPreCapacity or fastMalloc fail then use the null representation rather than crashing in memcpy.
        (KJS::UString::append): Ditto.
        (KJS::UString::operator=): Ditto.
        * kjs/ustring.h: Change return type of expandedSize from int to size_t.

2007-08-02  Mark Rowe  <mrowe@apple.com>

        Reviewed by Maciej.

        <rdar://problem/5352887> "Out of memory" error during repeated JS string concatenation leaks hundreds of MBs of RAM

        Update test to check that accessing the string after the "Out of memory" exception was raised does not crash.

        * fast/js/resources/string-concatenate-outofmemory.js:
        * fast/js/string-concatenate-outofmemory-expected.txt:

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

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/array_object.cpp
JavaScriptCore/kjs/ustring.cpp
JavaScriptCore/kjs/ustring.h
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/string-concatenate-outofmemory.js
LayoutTests/fast/js/string-concatenate-outofmemory-expected.txt

index 8ff9016..c674144 100644 (file)
@@ -1,3 +1,31 @@
+2007-08-02  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Maciej.
+
+        <rdar://problem/5352887> "Out of memory" error during repeated JS string concatenation leaks hundreds of MBs of RAM
+
+        A call to fastRealloc was failing which lead to UString::expandCapacity leaking the buffer it was trying to reallocate.
+        It also resulted in the underlying UString::rep having both a null baseString and buf field, which meant that attempting
+        to access the contents of the string after the failed memory reallocation would crash.
+
+        A third issue is that expandedSize size was calculating the new length in a way that led to an integer overflow occurring.
+        Attempting to allocate a string more than 190,000,000 characters long would fail a the integer overflow would lead to a
+        memory allocation of around 3.6GB being attempted rather than the expected 390MB.  Sizes that would lead to an overflow
+        are now  returned as zero and callers are updated to treat this as though the memory allocation has failed.
+
+        * kjs/array_object.cpp:
+        (ArrayProtoFunc::callAsFunction): Check whether the append failed and raise an "Out of memory" exception if it did.
+        * kjs/ustring.cpp:
+        (KJS::allocChars): Wrapper around fastMalloc that takes a length in characters.  It will return 0 when asked to allocate a zero-length buffer.
+        (KJS::reallocChars): Wrapper around fastRealloc that takes a length in characters.  It will return 0 when asked to allocate a zero-length buffer.
+        (KJS::UString::expandedSize): Split the size calculation in two and guard against overflow during each step.
+        (KJS::UString::expandCapacity): Don't leak r->buf if reallocation fails.  Instead free the memory and use the null representation.
+        (KJS::UString::expandPreCapacity): If fastMalloc fails then use the null representation rather than crashing in memcpy.
+        (KJS::UString::UString): If calls to expandCapacity, expandPreCapacity or fastMalloc fail then use the null representation rather than crashing in memcpy.
+        (KJS::UString::append): Ditto.
+        (KJS::UString::operator=): Ditto.
+        * kjs/ustring.h: Change return type of expandedSize from int to size_t.
+
 2007-08-01  Darin Adler  <darin@apple.com>
 
         Reviewed by Kevin McCullough.
 2007-08-01  Darin Adler  <darin@apple.com>
 
         Reviewed by Kevin McCullough.
index bbf04e7..d2b8893 100644 (file)
@@ -546,6 +546,11 @@ JSValue* ArrayProtoFunc::callAsFunction(ExecState* exec, JSObject* thisObj, cons
     for (unsigned int k = 0; k < length; k++) {
         if (k >= 1)
             str += separator;
     for (unsigned int k = 0; k < length; k++) {
         if (k >= 1)
             str += separator;
+        if (str.isNull()) {
+            JSObject *error = Error::create(exec, GeneralError, "Out of memory");
+            exec->setException(error);
+            break;
+        }
 
         JSValue* element = thisObj->get(exec, k);
         if (element->isUndefinedOrNull())
 
         JSValue* element = thisObj->get(exec, k);
         if (element->isUndefinedOrNull())
@@ -565,6 +570,11 @@ JSValue* ArrayProtoFunc::callAsFunction(ExecState* exec, JSObject* thisObj, cons
         if (id == ToString || id == Join || fallback)
             str += element->toString(exec);
 
         if (id == ToString || id == Join || fallback)
             str += element->toString(exec);
 
+        if (str.isNull()) {
+            JSObject *error = Error::create(exec, GeneralError, "Out of memory");
+            exec->setException(error);
+        }
+
         if (exec->hadException())
             break;
     }
         if (exec->hadException())
             break;
     }
index e9ca36a..083e0d9 100644 (file)
@@ -54,6 +54,20 @@ namespace KJS {
 extern const double NaN;
 extern const double Inf;
 
 extern const double NaN;
 extern const double Inf;
 
+static inline UChar* allocChars(size_t length)
+{
+    if (!length)
+        return 0;
+    return static_cast<UChar*>(fastMalloc(sizeof(UChar) * length));
+}
+
+static inline UChar* reallocChars(void* buffer, size_t length)
+{
+    if (!length)
+        return 0;
+    return static_cast<UChar*>(fastRealloc(buffer, sizeof(UChar) * length));
+}
+
 // we'd rather not do shared substring append for small strings, since
 // this runs too much risk of a tiny initial string holding down a
 // huge buffer. This is also tuned to match the extra cost size, so we
 // we'd rather not do shared substring append for small strings, since
 // this runs too much risk of a tiny initial string holding down a
 // huge buffer. This is also tuned to match the extra cost size, so we
@@ -347,10 +361,18 @@ unsigned UString::Rep::computeHash(const char *s)
 }
 
 // put these early so they can be inlined
 }
 
 // put these early so they can be inlined
-inline int UString::expandedSize(int size, int otherSize) const
+inline size_t UString::expandedSize(size_t size, size_t otherSize) const
 {
 {
-  int s = (size * 11 / 10) + 1 + otherSize;
-  return s;
+    // Do the size calculation in two parts, being careful to avoid overflow
+    static const size_t maximumAllowableSize = SIZE_T_MAX / sizeof(UChar);
+    if (size > maximumAllowableSize)
+        return 0;
+
+    size_t expandedSize = ((size + 10) / 10 * 11) + 1;
+    if (maximumAllowableSize - expandedSize < otherSize)
+        return 0;
+
+    return expandedSize + otherSize;
 }
 
 inline int UString::usedCapacity() const
 }
 
 inline int UString::usedCapacity() const
@@ -368,8 +390,14 @@ void UString::expandCapacity(int requiredLength)
   Rep* r = m_rep->baseString;
 
   if (requiredLength > r->capacity) {
   Rep* r = m_rep->baseString;
 
   if (requiredLength > r->capacity) {
-    int newCapacity = expandedSize(requiredLength, r->preCapacity);
-    r->buf = static_cast<UChar *>(fastRealloc(r->buf, newCapacity * sizeof(UChar)));
+    size_t newCapacity = expandedSize(requiredLength, r->preCapacity);
+    UChar* oldBuf = r->buf;
+    r->buf = reallocChars(r->buf, newCapacity);
+    if (!r->buf) {
+        r->buf = oldBuf;
+        m_rep = &Rep::null;
+        return;
+    }
     r->capacity = newCapacity - r->preCapacity;
   }
   if (requiredLength > r->usedCapacity) {
     r->capacity = newCapacity - r->preCapacity;
   }
   if (requiredLength > r->usedCapacity) {
@@ -382,10 +410,14 @@ void UString::expandPreCapacity(int requiredPreCap)
   Rep* r = m_rep->baseString;
 
   if (requiredPreCap > r->preCapacity) {
   Rep* r = m_rep->baseString;
 
   if (requiredPreCap > r->preCapacity) {
-    int newCapacity = expandedSize(requiredPreCap, r->capacity);
+    size_t newCapacity = expandedSize(requiredPreCap, r->capacity);
     int delta = newCapacity - r->capacity - r->preCapacity;
 
     int delta = newCapacity - r->capacity - r->preCapacity;
 
-    UChar *newBuf = static_cast<UChar *>(fastMalloc(newCapacity * sizeof(UChar)));
+    UChar* newBuf = allocChars(newCapacity);
+    if (!newBuf) {
+        m_rep = &Rep::null;
+        return;
+    }
     memcpy(newBuf + delta, r->buf, (r->capacity + r->preCapacity) * sizeof(UChar));
     fastFree(r->buf);
     r->buf = newBuf;
     memcpy(newBuf + delta, r->buf, (r->capacity + r->preCapacity) * sizeof(UChar));
     fastFree(r->buf);
     r->buf = newBuf;
@@ -408,10 +440,14 @@ UString::UString(const char *c)
     m_rep = &Rep::empty;
     return;
   }
     m_rep = &Rep::empty;
     return;
   }
-  UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * length));
-  for (size_t i = 0; i < length; i++)
-    d[i].uc = c[i];
-  m_rep = Rep::create(d, static_cast<int>(length));
+  UChar *d = allocChars(length);
+  if (!d)
+      m_rep = &Rep::null;
+  else {
+      for (size_t i = 0; i < length; i++)
+          d[i].uc = c[i];
+      m_rep = Rep::create(d, static_cast<int>(length));
+  }
 }
 
 UString::UString(const UChar *c, int length)
 }
 
 UString::UString(const UChar *c, int length)
@@ -456,7 +492,7 @@ UString::UString(const UString &a, const UString &b)
     // - however, if b qualifies for prepend and is longer than a, we'd rather prepend
     UString x(a);
     x.expandCapacity(aOffset + length);
     // - however, if b qualifies for prepend and is longer than a, we'd rather prepend
     UString x(a);
     x.expandCapacity(aOffset + length);
-    if (a.data()) {
+    if (a.data() && x.data()) {
         memcpy(const_cast<UChar *>(a.data() + aSize), b.data(), bSize * sizeof(UChar));
         m_rep = Rep::create(a.m_rep, 0, length);
     } else
         memcpy(const_cast<UChar *>(a.data() + aSize), b.data(), bSize * sizeof(UChar));
         m_rep = Rep::create(a.m_rep, 0, length);
     } else
@@ -467,22 +503,23 @@ UString::UString(const UString &a, const UString &b)
     //   string does more harm than good
     UString y(b);
     y.expandPreCapacity(-bOffset + aSize);
     //   string does more harm than good
     UString y(b);
     y.expandPreCapacity(-bOffset + aSize);
-    if (b.data()) {
+    if (b.data() && y.data()) {
         memcpy(const_cast<UChar *>(b.data() - aSize), a.data(), aSize * sizeof(UChar));
         m_rep = Rep::create(b.m_rep, -aSize, length);
     } else
         m_rep = &Rep::null;
   } else {
     // a does not qualify for append, and b does not qualify for prepend, gotta make a whole new string
         memcpy(const_cast<UChar *>(b.data() - aSize), a.data(), aSize * sizeof(UChar));
         m_rep = Rep::create(b.m_rep, -aSize, length);
     } else
         m_rep = &Rep::null;
   } else {
     // a does not qualify for append, and b does not qualify for prepend, gotta make a whole new string
-    int newCapacity = expandedSize(length, 0);
-    UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * newCapacity));
-    if (d) {
+    size_t newCapacity = expandedSize(length, 0);
+    UChar* d = allocChars(newCapacity);
+    if (!d)
+        m_rep = &Rep::null;
+    else {
         memcpy(d, a.data(), aSize * sizeof(UChar));
         memcpy(d + aSize, b.data(), bSize * sizeof(UChar));
         m_rep = Rep::create(d, length);
         m_rep->capacity = newCapacity;
         memcpy(d, a.data(), aSize * sizeof(UChar));
         memcpy(d + aSize, b.data(), bSize * sizeof(UChar));
         m_rep = Rep::create(d, length);
         m_rep->capacity = newCapacity;
-    } else
-        m_rep = &Rep::null;
+    }
   }
 }
 
   }
 }
 
@@ -656,7 +693,9 @@ UString UString::spliceSubstringsWithSeparators(const Range* substringRanges, in
   for (int i = 0; i < separatorCount; i++)
     totalLength += separators[i].size();
 
   for (int i = 0; i < separatorCount; i++)
     totalLength += separators[i].size();
 
-  UChar* buffer = static_cast<UChar*>(fastMalloc(totalLength * sizeof(UChar)));
+  UChar* buffer = allocChars(totalLength);
+  if (!buffer)
+      return null();
 
   int maxCount = max(rangeCount, separatorCount);
   int bufferPos = 0;
 
   int maxCount = max(rangeCount, separatorCount);
   int bufferPos = 0;
@@ -690,22 +729,30 @@ UString &UString::append(const UString &t)
   } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
     // this is direct and has refcount of 1 (so we can just alter it directly)
     expandCapacity(thisOffset + length);
   } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
     // this is direct and has refcount of 1 (so we can just alter it directly)
     expandCapacity(thisOffset + length);
-    memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
-    m_rep->len = length;
-    m_rep->_hash = 0;
+    if (data()) {
+        memcpy(const_cast<UChar*>(data() + thisSize), t.data(), tSize * sizeof(UChar));
+        m_rep->len = length;
+        m_rep->_hash = 0;
+    }
   } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
     // this reaches the end of the buffer - extend it if it's long enough to append to
     expandCapacity(thisOffset + length);
   } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
     // this reaches the end of the buffer - extend it if it's long enough to append to
     expandCapacity(thisOffset + length);
-    memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
-    m_rep = Rep::create(m_rep, 0, length);
+    if (data()) {
+        memcpy(const_cast<UChar*>(data() + thisSize), t.data(), tSize * sizeof(UChar));
+        m_rep = Rep::create(m_rep, 0, length);
+    }
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
-    int newCapacity = expandedSize(length, 0);
-    UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * newCapacity));
-    memcpy(d, data(), thisSize * sizeof(UChar));
-    memcpy(const_cast<UChar *>(d + thisSize), t.data(), tSize * sizeof(UChar));
-    m_rep = Rep::create(d, length);
-    m_rep->capacity = newCapacity;
+    size_t newCapacity = expandedSize(length, 0);
+    UChar* d = allocChars(newCapacity);
+    if (!d)
+        m_rep = &Rep::null;
+    else {
+        memcpy(d, data(), thisSize * sizeof(UChar));
+        memcpy(const_cast<UChar*>(d + thisSize), t.data(), tSize * sizeof(UChar));
+        m_rep = Rep::create(d, length);
+        m_rep->capacity = newCapacity;
+    }
   }
 
   return *this;
   }
 
   return *this;
@@ -728,26 +775,34 @@ UString &UString::append(const char *t)
     // this is direct and has refcount of 1 (so we can just alter it directly)
     expandCapacity(thisOffset + length);
     UChar *d = const_cast<UChar *>(data());
     // this is direct and has refcount of 1 (so we can just alter it directly)
     expandCapacity(thisOffset + length);
     UChar *d = const_cast<UChar *>(data());
-    for (int i = 0; i < tSize; ++i)
-      d[thisSize+i] = t[i];
-    m_rep->len = length;
-    m_rep->_hash = 0;
+    if (d) {
+        for (int i = 0; i < tSize; ++i)
+            d[thisSize + i] = t[i];
+        m_rep->len = length;
+        m_rep->_hash = 0;
+    }
   } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
     // this string reaches the end of the buffer - extend it
     expandCapacity(thisOffset + length);
     UChar *d = const_cast<UChar *>(data());
   } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
     // this string reaches the end of the buffer - extend it
     expandCapacity(thisOffset + length);
     UChar *d = const_cast<UChar *>(data());
-    for (int i = 0; i < tSize; ++i)
-      d[thisSize+i] = t[i];
-    m_rep = Rep::create(m_rep, 0, length);
+    if (d) {
+        for (int i = 0; i < tSize; ++i)
+            d[thisSize + i] = t[i];
+        m_rep = Rep::create(m_rep, 0, length);
+    }
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
-    int newCapacity = expandedSize(length, 0);
-    UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * newCapacity));
-    memcpy(d, data(), thisSize * sizeof(UChar));
-    for (int i = 0; i < tSize; ++i)
-      d[thisSize+i] = t[i];
-    m_rep = Rep::create(d, length);
-    m_rep->capacity = newCapacity;
+    size_t newCapacity = expandedSize(length, 0);
+    UChar* d = allocChars(newCapacity);
+    if (!d)
+        m_rep = &Rep::null;
+    else {
+        memcpy(d, data(), thisSize * sizeof(UChar));
+        for (int i = 0; i < tSize; ++i)
+            d[thisSize + i] = t[i];
+        m_rep = Rep::create(d, length);
+        m_rep->capacity = newCapacity;
+    }
   }
 
   return *this;
   }
 
   return *this;
@@ -761,32 +816,44 @@ UString &UString::append(unsigned short c)
   // possible cases:
   if (length == 0) {
     // this is empty - must make a new m_rep because we don't want to pollute the shared empty one 
   // possible cases:
   if (length == 0) {
     // this is empty - must make a new m_rep because we don't want to pollute the shared empty one 
-    int newCapacity = expandedSize(1, 0);
-    UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * newCapacity));
-    d[0] = c;
-    m_rep = Rep::create(d, 1);
-    m_rep->capacity = newCapacity;
+    size_t newCapacity = expandedSize(1, 0);
+    UChar* d = allocChars(newCapacity);
+    if (!d)
+        m_rep = &Rep::null;
+    else {
+        d[0] = c;
+        m_rep = Rep::create(d, 1);
+        m_rep->capacity = newCapacity;
+    }
   } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
     // this is direct and has refcount of 1 (so we can just alter it directly)
     expandCapacity(thisOffset + length + 1);
     UChar *d = const_cast<UChar *>(data());
   } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
     // this is direct and has refcount of 1 (so we can just alter it directly)
     expandCapacity(thisOffset + length + 1);
     UChar *d = const_cast<UChar *>(data());
-    d[length] = c;
-    m_rep->len = length + 1;
-    m_rep->_hash = 0;
+    if (d) {
+        d[length] = c;
+        m_rep->len = length + 1;
+        m_rep->_hash = 0;
+    }
   } else if (thisOffset + length == usedCapacity() && length >= minShareSize) {
     // this reaches the end of the string - extend it and share
     expandCapacity(thisOffset + length + 1);
     UChar *d = const_cast<UChar *>(data());
   } else if (thisOffset + length == usedCapacity() && length >= minShareSize) {
     // this reaches the end of the string - extend it and share
     expandCapacity(thisOffset + length + 1);
     UChar *d = const_cast<UChar *>(data());
-    d[length] = c;
-    m_rep = Rep::create(m_rep, 0, length + 1);
+    if (d) {
+        d[length] = c;
+        m_rep = Rep::create(m_rep, 0, length + 1);
+    }
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
-    int newCapacity = expandedSize((length + 1), 0);
-    UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * newCapacity));
-    memcpy(d, data(), length * sizeof(UChar));
-    d[length] = c;
-    m_rep = Rep::create(d, length + 1);
-    m_rep->capacity = newCapacity;
+    size_t newCapacity = expandedSize(length + 1, 0);
+    UChar* d = allocChars(newCapacity);
+    if (!d)
+        m_rep = &Rep::null;
+    else {
+        memcpy(d, data(), length * sizeof(UChar));
+        d[length] = c;
+        m_rep = Rep::create(d, length + 1);
+        m_rep->capacity = newCapacity;
+    }
   }
 
   return *this;
   }
 
   return *this;
@@ -843,7 +910,11 @@ UString &UString::operator=(const char *c)
     m_rep->_hash = 0;
     m_rep->len = l;
   } else {
     m_rep->_hash = 0;
     m_rep->len = l;
   } else {
-    d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * l));
+    d = allocChars(l);
+    if (!d) {
+        m_rep = &Rep::null;
+        return *this;
+    }
     m_rep = Rep::create(d, l);
   }
   for (int i = 0; i < l; i++)
     m_rep = Rep::create(d, l);
   }
   for (int i = 0; i < l; i++)
@@ -1139,9 +1210,13 @@ void UString::copyForWriting()
 {
   if (m_rep->rc > 1 || !m_rep->baseIsSelf()) {
     int l = size();
 {
   if (m_rep->rc > 1 || !m_rep->baseIsSelf()) {
     int l = size();
-    UChar *n = static_cast<UChar *>(fastMalloc(sizeof(UChar) * l));
-    memcpy(n, data(), l * sizeof(UChar));
-    m_rep = Rep::create(n, l);
+    UChar *n = allocChars(l);
+    if (!n)
+        m_rep = &Rep::null;
+    else {
+        memcpy(n, data(), l * sizeof(UChar));
+        m_rep = Rep::create(n, l);
+    }
   }
 }
 
   }
 }
 
index 7f42d68..123e329 100644 (file)
@@ -443,7 +443,7 @@ namespace KJS {
     size_t cost() const;
 
   private:
     size_t cost() const;
 
   private:
-    int expandedSize(int size, int otherSize) const;
+    size_t expandedSize(size_t size, size_t otherSize) const;
     int usedCapacity() const;
     int usedPreCapacity() const;
     void expandCapacity(int requiredLength);
     int usedCapacity() const;
     int usedPreCapacity() const;
     void expandCapacity(int requiredLength);
index 747c385..edc1b68 100644 (file)
@@ -1,3 +1,14 @@
+2007-08-02  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Maciej.
+
+        <rdar://problem/5352887> "Out of memory" error during repeated JS string concatenation leaks hundreds of MBs of RAM
+
+        Update test to check that accessing the string after the "Out of memory" exception was raised does not crash.
+
+        * fast/js/resources/string-concatenate-outofmemory.js:
+        * fast/js/string-concatenate-outofmemory-expected.txt:
+
 2007-07-31  Oliver Hunt  <oliver@apple.com>
 
         fast/encoding/char-encoding.html no longer needs to be in the Leopard skiplist
 2007-07-31  Oliver Hunt  <oliver@apple.com>
 
         fast/encoding/char-encoding.html no longer needs to be in the Leopard skiplist
@@ -20449,7 +20460,7 @@ tract()
 
 2006-10-20  Anders Carlsson  <acarlsson@apple.com>
 
 
 2006-10-20  Anders Carlsson  <acarlsson@apple.com>
 
-        Reviewed by GΓΈff.
+        Reviewed by Geoff.
 
         Add test case for timer crash.
         
 
         Add test case for timer crash.
         
index a3b4e7e..46a75e0 100644 (file)
@@ -2,6 +2,50 @@ description(
 'This test checks if repeated string concatenation causes an exception (and not a crash). From WebKit Bug <a href="http://bugs.webkit.org/show_bug.cgi?id=11131">Repeated string concatenation results in OOM crash</a>.'
 );
 
 'This test checks if repeated string concatenation causes an exception (and not a crash). From WebKit Bug <a href="http://bugs.webkit.org/show_bug.cgi?id=11131">Repeated string concatenation results in OOM crash</a>.'
 );
 
-shouldThrow('s = "a"; while (1) { s += s; }', '"Error: Out of memory"');
+shouldThrow('s = "a"; while (1) { s += s; }', '"Error: Out of memory"'); // Expand at end of string
+shouldThrow('s = "a"; while (1) { s += ("a" + s); }', '"Error: Out of memory"'); // Expand at beginning of string
+shouldThrow('s = "a"; while (1) { s = [s, s].join(); }', '"Error: Out of memory"'); // Expand using UString::append.
+
+debug('');
+debug(
+'We also verify that the the string is stil functional after the out of memory exception is raised.  In <a href="rdar://problem/5352887">rdar://problem/5352887</a>, accessing the string after the exception would crash.'
+);
+
+function ensureStringIsUsable(testName, stringName, str) {
+    str[str.length - 1];
+    try { [str, str].join(str); } catch (o) { }
+    "a" + str;
+    str + "a";
+    debug('PASS: String ' + stringName + ' was functional after ' + testName + ' raised out of memory exception.');
+}
+
+var s = "a";
+try {
+    while (1)
+        s += s; // This will expand the string at the end using UString::expandCapacity
+} catch (o) {
+    ensureStringIsUsable('expandCapacity', 's', s);
+}
+
+s = "a";
+var t = "";
+try {
+    while (1) {
+        t = "a" + s;
+        s += t; // This will expand the string at the beginning using UString::expandPreCapacity
+    }
+} catch (o) {
+    // Ensure both strings involved are unharmed
+    ensureStringIsUsable('expandPreCapacity', 's', s);
+    ensureStringIsUsable('expandPreCapacity', 't', t);
+}
+
+s = "a";
+try {
+    while (1)
+        s = [s, s].join(); // This will expand the string using UString::append.
+} catch (o) {
+    ensureStringIsUsable('append', 's', s);
+}
 
 var successfullyParsed = true;
 
 var successfullyParsed = true;
index 595555b..d05877a 100644 (file)
@@ -4,6 +4,14 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS s = "a"; while (1) { s += s; } threw exception Error: Out of memory.
 
 
 PASS s = "a"; while (1) { s += s; } threw exception Error: Out of memory.
+PASS s = "a"; while (1) { s += ("a" + s); } threw exception Error: Out of memory.
+PASS s = "a"; while (1) { s = [s, s].join(); } threw exception Error: Out of memory.
+
+We also verify that the the string is stil functional after the out of memory exception is raised.  In rdar://problem/5352887, accessing the string after the exception would crash.
+PASS: String s was functional after expandCapacity raised out of memory exception.
+PASS: String s was functional after expandPreCapacity raised out of memory exception.
+PASS: String t was functional after expandPreCapacity raised out of memory exception.
+PASS: String s was functional after append raised out of memory exception.
 PASS successfullyParsed is true
 
 TEST COMPLETE
 PASS successfullyParsed is true
 
 TEST COMPLETE