<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" /><style type="text/css"><!--
#msg dl { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fc0 solid; padding: 6px; }
#msg ul, pre { overflow: auto; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<title>[24820] trunk</title>
</head>
<body>
<div id="msg">
<dl>
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/24820">24820</a></dd>
<dt>Author</dt> <dd>bdash</dd>
<dt>Date</dt> <dd>2007-08-02 02:33:22 -0700 (Thu, 02 Aug 2007)</dd>
</dl>
<h3>Log Message</h3>
<pre>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-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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJavaScriptCoreChangeLog">trunk/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkJavaScriptCorekjsarray_objectcpp">trunk/JavaScriptCore/kjs/array_object.cpp</a></li>
<li><a href="#trunkJavaScriptCorekjsustringcpp">trunk/JavaScriptCore/kjs/ustring.cpp</a></li>
<li><a href="#trunkJavaScriptCorekjsustringh">trunk/JavaScriptCore/kjs/ustring.h</a></li>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastjsresourcesstringconcatenateoutofmemoryjs">trunk/LayoutTests/fast/js/resources/string-concatenate-outofmemory.js</a></li>
<li><a href="#trunkLayoutTestsfastjsstringconcatenateoutofmemoryexpectedtxt">trunk/LayoutTests/fast/js/string-concatenate-outofmemory-expected.txt</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/ChangeLog (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/ChangeLog 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/JavaScriptCore/ChangeLog 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+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.
+
</ins><span class="cx"> 2007-08-01 Darin Adler <darin@apple.com>
</span><span class="cx">
</span><span class="cx"> Reviewed by Kevin McCullough.
</span></span></pre></div>
<a id="trunkJavaScriptCorekjsarray_objectcpp"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/kjs/array_object.cpp (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/kjs/array_object.cpp 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/JavaScriptCore/kjs/array_object.cpp 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -546,6 +546,11 @@
</span><span class="cx"> for (unsigned int k = 0; k < length; k++) {
</span><span class="cx"> if (k >= 1)
</span><span class="cx"> str += separator;
</span><ins>+ if (str.isNull()) {
+ JSObject *error = Error::create(exec, GeneralError, "Out of memory");
+ exec->setException(error);
+ break;
+ }
</ins><span class="cx">
</span><span class="cx"> JSValue* element = thisObj->get(exec, k);
</span><span class="cx"> if (element->isUndefinedOrNull())
</span><span class="lines">@@ -565,6 +570,11 @@
</span><span class="cx"> if (id == ToString || id == Join || fallback)
</span><span class="cx"> str += element->toString(exec);
</span><span class="cx">
</span><ins>+ if (str.isNull()) {
+ JSObject *error = Error::create(exec, GeneralError, "Out of memory");
+ exec->setException(error);
+ }
+
</ins><span class="cx"> if (exec->hadException())
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkJavaScriptCorekjsustringcpp"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/kjs/ustring.cpp (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/kjs/ustring.cpp 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/JavaScriptCore/kjs/ustring.cpp 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -54,6 +54,20 @@
</span><span class="cx"> extern const double NaN;
</span><span class="cx"> extern const double Inf;
</span><span class="cx">
</span><ins>+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));
+}
+
</ins><span class="cx"> // we'd rather not do shared substring append for small strings, since
</span><span class="cx"> // this runs too much risk of a tiny initial string holding down a
</span><span class="cx"> // huge buffer. This is also tuned to match the extra cost size, so we
</span><span class="lines">@@ -347,10 +361,18 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // put these early so they can be inlined
</span><del>-inline int UString::expandedSize(int size, int otherSize) const
</del><ins>+inline size_t UString::expandedSize(size_t size, size_t otherSize) const
</ins><span class="cx"> {
</span><del>- int s = (size * 11 / 10) + 1 + otherSize;
- return s;
</del><ins>+ // 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;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline int UString::usedCapacity() const
</span><span class="lines">@@ -368,8 +390,14 @@
</span><span class="cx"> Rep* r = m_rep->baseString;
</span><span class="cx">
</span><span class="cx"> if (requiredLength > r->capacity) {
</span><del>- int newCapacity = expandedSize(requiredLength, r->preCapacity);
- r->buf = static_cast<UChar *>(fastRealloc(r->buf, newCapacity * sizeof(UChar)));
</del><ins>+ 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;
+ }
</ins><span class="cx"> r->capacity = newCapacity - r->preCapacity;
</span><span class="cx"> }
</span><span class="cx"> if (requiredLength > r->usedCapacity) {
</span><span class="lines">@@ -382,10 +410,14 @@
</span><span class="cx"> Rep* r = m_rep->baseString;
</span><span class="cx">
</span><span class="cx"> if (requiredPreCap > r->preCapacity) {
</span><del>- int newCapacity = expandedSize(requiredPreCap, r->capacity);
</del><ins>+ size_t newCapacity = expandedSize(requiredPreCap, r->capacity);
</ins><span class="cx"> int delta = newCapacity - r->capacity - r->preCapacity;
</span><span class="cx">
</span><del>- UChar *newBuf = static_cast<UChar *>(fastMalloc(newCapacity * sizeof(UChar)));
</del><ins>+ UChar* newBuf = allocChars(newCapacity);
+ if (!newBuf) {
+ m_rep = &Rep::null;
+ return;
+ }
</ins><span class="cx"> memcpy(newBuf + delta, r->buf, (r->capacity + r->preCapacity) * sizeof(UChar));
</span><span class="cx"> fastFree(r->buf);
</span><span class="cx"> r->buf = newBuf;
</span><span class="lines">@@ -408,10 +440,14 @@
</span><span class="cx"> m_rep = &Rep::empty;
</span><span class="cx"> return;
</span><span class="cx"> }
</span><del>- 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));
</del><ins>+ 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));
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> UString::UString(const UChar *c, int length)
</span><span class="lines">@@ -456,7 +492,7 @@
</span><span class="cx"> // - however, if b qualifies for prepend and is longer than a, we'd rather prepend
</span><span class="cx"> UString x(a);
</span><span class="cx"> x.expandCapacity(aOffset + length);
</span><del>- if (a.data()) {
</del><ins>+ if (a.data() && x.data()) {
</ins><span class="cx"> memcpy(const_cast<UChar *>(a.data() + aSize), b.data(), bSize * sizeof(UChar));
</span><span class="cx"> m_rep = Rep::create(a.m_rep, 0, length);
</span><span class="cx"> } else
</span><span class="lines">@@ -467,22 +503,23 @@
</span><span class="cx"> // string does more harm than good
</span><span class="cx"> UString y(b);
</span><span class="cx"> y.expandPreCapacity(-bOffset + aSize);
</span><del>- if (b.data()) {
</del><ins>+ if (b.data() && y.data()) {
</ins><span class="cx"> memcpy(const_cast<UChar *>(b.data() - aSize), a.data(), aSize * sizeof(UChar));
</span><span class="cx"> m_rep = Rep::create(b.m_rep, -aSize, length);
</span><span class="cx"> } else
</span><span class="cx"> m_rep = &Rep::null;
</span><span class="cx"> } else {
</span><span class="cx"> // a does not qualify for append, and b does not qualify for prepend, gotta make a whole new string
</span><del>- int newCapacity = expandedSize(length, 0);
- UChar *d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * newCapacity));
- if (d) {
</del><ins>+ size_t newCapacity = expandedSize(length, 0);
+ UChar* d = allocChars(newCapacity);
+ if (!d)
+ m_rep = &Rep::null;
+ else {
</ins><span class="cx"> memcpy(d, a.data(), aSize * sizeof(UChar));
</span><span class="cx"> memcpy(d + aSize, b.data(), bSize * sizeof(UChar));
</span><span class="cx"> m_rep = Rep::create(d, length);
</span><span class="cx"> m_rep->capacity = newCapacity;
</span><del>- } else
- m_rep = &Rep::null;
</del><ins>+ }
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -656,7 +693,9 @@
</span><span class="cx"> for (int i = 0; i < separatorCount; i++)
</span><span class="cx"> totalLength += separators[i].size();
</span><span class="cx">
</span><del>- UChar* buffer = static_cast<UChar*>(fastMalloc(totalLength * sizeof(UChar)));
</del><ins>+ UChar* buffer = allocChars(totalLength);
+ if (!buffer)
+ return null();
</ins><span class="cx">
</span><span class="cx"> int maxCount = max(rangeCount, separatorCount);
</span><span class="cx"> int bufferPos = 0;
</span><span class="lines">@@ -690,22 +729,30 @@
</span><span class="cx"> } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
</span><span class="cx"> // this is direct and has refcount of 1 (so we can just alter it directly)
</span><span class="cx"> expandCapacity(thisOffset + length);
</span><del>- memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
- m_rep->len = length;
- m_rep->_hash = 0;
</del><ins>+ if (data()) {
+ memcpy(const_cast<UChar*>(data() + thisSize), t.data(), tSize * sizeof(UChar));
+ m_rep->len = length;
+ m_rep->_hash = 0;
+ }
</ins><span class="cx"> } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
</span><span class="cx"> // this reaches the end of the buffer - extend it if it's long enough to append to
</span><span class="cx"> expandCapacity(thisOffset + length);
</span><del>- memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
- m_rep = Rep::create(m_rep, 0, length);
</del><ins>+ if (data()) {
+ memcpy(const_cast<UChar*>(data() + thisSize), t.data(), tSize * sizeof(UChar));
+ m_rep = Rep::create(m_rep, 0, length);
+ }
</ins><span class="cx"> } else {
</span><span class="cx"> // this is shared with someone using more capacity, gotta make a whole new string
</span><del>- 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;
</del><ins>+ 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;
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return *this;
</span><span class="lines">@@ -728,26 +775,34 @@
</span><span class="cx"> // this is direct and has refcount of 1 (so we can just alter it directly)
</span><span class="cx"> expandCapacity(thisOffset + length);
</span><span class="cx"> UChar *d = const_cast<UChar *>(data());
</span><del>- for (int i = 0; i < tSize; ++i)
- d[thisSize+i] = t[i];
- m_rep->len = length;
- m_rep->_hash = 0;
</del><ins>+ if (d) {
+ for (int i = 0; i < tSize; ++i)
+ d[thisSize + i] = t[i];
+ m_rep->len = length;
+ m_rep->_hash = 0;
+ }
</ins><span class="cx"> } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
</span><span class="cx"> // this string reaches the end of the buffer - extend it
</span><span class="cx"> expandCapacity(thisOffset + length);
</span><span class="cx"> UChar *d = const_cast<UChar *>(data());
</span><del>- for (int i = 0; i < tSize; ++i)
- d[thisSize+i] = t[i];
- m_rep = Rep::create(m_rep, 0, length);
</del><ins>+ if (d) {
+ for (int i = 0; i < tSize; ++i)
+ d[thisSize + i] = t[i];
+ m_rep = Rep::create(m_rep, 0, length);
+ }
</ins><span class="cx"> } else {
</span><span class="cx"> // this is shared with someone using more capacity, gotta make a whole new string
</span><del>- 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;
</del><ins>+ 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;
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return *this;
</span><span class="lines">@@ -761,32 +816,44 @@
</span><span class="cx"> // possible cases:
</span><span class="cx"> if (length == 0) {
</span><span class="cx"> // this is empty - must make a new m_rep because we don't want to pollute the shared empty one
</span><del>- 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;
</del><ins>+ 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;
+ }
</ins><span class="cx"> } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
</span><span class="cx"> // this is direct and has refcount of 1 (so we can just alter it directly)
</span><span class="cx"> expandCapacity(thisOffset + length + 1);
</span><span class="cx"> UChar *d = const_cast<UChar *>(data());
</span><del>- d[length] = c;
- m_rep->len = length + 1;
- m_rep->_hash = 0;
</del><ins>+ if (d) {
+ d[length] = c;
+ m_rep->len = length + 1;
+ m_rep->_hash = 0;
+ }
</ins><span class="cx"> } else if (thisOffset + length == usedCapacity() && length >= minShareSize) {
</span><span class="cx"> // this reaches the end of the string - extend it and share
</span><span class="cx"> expandCapacity(thisOffset + length + 1);
</span><span class="cx"> UChar *d = const_cast<UChar *>(data());
</span><del>- d[length] = c;
- m_rep = Rep::create(m_rep, 0, length + 1);
</del><ins>+ if (d) {
+ d[length] = c;
+ m_rep = Rep::create(m_rep, 0, length + 1);
+ }
</ins><span class="cx"> } else {
</span><span class="cx"> // this is shared with someone using more capacity, gotta make a whole new string
</span><del>- 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;
</del><ins>+ 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;
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> return *this;
</span><span class="lines">@@ -843,7 +910,11 @@
</span><span class="cx"> m_rep->_hash = 0;
</span><span class="cx"> m_rep->len = l;
</span><span class="cx"> } else {
</span><del>- d = static_cast<UChar *>(fastMalloc(sizeof(UChar) * l));
</del><ins>+ d = allocChars(l);
+ if (!d) {
+ m_rep = &Rep::null;
+ return *this;
+ }
</ins><span class="cx"> m_rep = Rep::create(d, l);
</span><span class="cx"> }
</span><span class="cx"> for (int i = 0; i < l; i++)
</span><span class="lines">@@ -1139,9 +1210,13 @@
</span><span class="cx"> {
</span><span class="cx"> if (m_rep->rc > 1 || !m_rep->baseIsSelf()) {
</span><span class="cx"> int l = size();
</span><del>- UChar *n = static_cast<UChar *>(fastMalloc(sizeof(UChar) * l));
- memcpy(n, data(), l * sizeof(UChar));
- m_rep = Rep::create(n, l);
</del><ins>+ UChar *n = allocChars(l);
+ if (!n)
+ m_rep = &Rep::null;
+ else {
+ memcpy(n, data(), l * sizeof(UChar));
+ m_rep = Rep::create(n, l);
+ }
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkJavaScriptCorekjsustringh"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/kjs/ustring.h (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/kjs/ustring.h 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/JavaScriptCore/kjs/ustring.h 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -443,7 +443,7 @@
</span><span class="cx"> size_t cost() const;
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- int expandedSize(int size, int otherSize) const;
</del><ins>+ size_t expandedSize(size_t size, size_t otherSize) const;
</ins><span class="cx"> int usedCapacity() const;
</span><span class="cx"> int usedPreCapacity() const;
</span><span class="cx"> void expandCapacity(int requiredLength);
</span></span></pre></div>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/LayoutTests/ChangeLog 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+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:
+
</ins><span class="cx"> 2007-07-31 Oliver Hunt <oliver@apple.com>
</span><span class="cx">
</span><span class="cx"> fast/encoding/char-encoding.html no longer needs to be in the Leopard skiplist
</span><span class="lines">@@ -20449,7 +20460,7 @@
</span><span class="cx">
</span><span class="cx"> 2006-10-20 Anders Carlsson <acarlsson@apple.com>
</span><span class="cx">
</span><del>- Reviewed by Gøff.
</del><ins>+ Reviewed by Geoff.
</ins><span class="cx">
</span><span class="cx"> Add test case for timer crash.
</span><span class="cx">
</span></span></pre></div>
<a id="trunkLayoutTestsfastjsresourcesstringconcatenateoutofmemoryjs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/js/resources/string-concatenate-outofmemory.js (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/resources/string-concatenate-outofmemory.js 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/LayoutTests/fast/js/resources/string-concatenate-outofmemory.js 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -2,6 +2,50 @@
</span><span class="cx"> '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>.'
</span><span class="cx"> );
</span><span class="cx">
</span><del>-shouldThrow('s = "a"; while (1) { s += s; }', '"Error: Out of memory"');
</del><ins>+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.
</ins><span class="cx">
</span><ins>+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);
+}
+
</ins><span class="cx"> var successfullyParsed = true;
</span></span></pre></div>
<a id="trunkLayoutTestsfastjsstringconcatenateoutofmemoryexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/js/string-concatenate-outofmemory-expected.txt (24819 => 24820)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/string-concatenate-outofmemory-expected.txt 2007-08-02 07:24:13 UTC (rev 24819)
+++ trunk/LayoutTests/fast/js/string-concatenate-outofmemory-expected.txt 2007-08-02 09:33:22 UTC (rev 24820)
</span><span class="lines">@@ -4,6 +4,14 @@
</span><span class="cx">
</span><span class="cx">
</span><span class="cx"> PASS s = "a"; while (1) { s += s; } threw exception Error: Out of memory.
</span><ins>+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.
</ins><span class="cx"> PASS successfullyParsed is true
</span><span class="cx">
</span><span class="cx"> TEST COMPLETE
</span></span></pre>
</div>
</div>
</body>
</html>