Changeset 51933 in webkit for trunk/JavaScriptCore

Timestamp:

Dec 9, 2009, 5:44:20 PM (15 years ago)

Author:

[email protected]

Message:

JavaScriptCore: ​https://bugs.webkit.org/show_bug.cgi?id=32228
Make destruction of ropes non-recursive to prevent stack exhaustion.
Also, pass a UString& into initializeFiber rather than a Ustring::Rep*,
since the Rep is not being ref counted this could result in usage of a
Rep with refcount zero (where the Rep comes from a temporary UString
returned from a function).

Reviewed by Oliver Hunt.

• runtime/JSString.cpp:

(JSC::JSString::Rope::destructNonRecursive):
(JSC::JSString::Rope::~Rope):

• runtime/JSString.h:

(JSC::JSString::Rope::initializeFiber):

• runtime/Operations.h:

(JSC::concatenateStrings):

LayoutTests: ​https://bugs.webkit.org/show_bug.cgi?id=32228
Reenabling tests.

Reviewed by Oliver Hunt.

• platform/win/Skipped:

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSString.cpp (modified) (1 diff)
• runtime/JSString.h (modified) (2 diffs)
• runtime/Operations.h (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2009-12-09  Gavin Barraclough  <[email protected]>
+
+        Reviewed by Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=32228
+        Make destruction of ropes non-recursive to prevent stack exhaustion.
+        Also, pass a UString& into initializeFiber rather than a Ustring::Rep*,
+        since the Rep is not being ref counted this could result in usage of a
+        Rep with refcount zero (where the Rep comes from a temporary UString
+        returned from a function).
+
+        * runtime/JSString.cpp:
+        (JSC::JSString::Rope::destructNonRecursive):
+        (JSC::JSString::Rope::~Rope):
+        * runtime/JSString.h:
+        (JSC::JSString::Rope::initializeFiber):
+        * runtime/Operations.h:
+        (JSC::concatenateStrings):
+
 2009-12-09  Zoltan Herczeg  <[email protected]>
 

trunk/JavaScriptCore/runtime/JSString.cpp

@@ -32 +32 @@
 namespace JSC {
 
+void JSString::Rope::destructNonRecursive()
+{
+    Vector<Rope*, 32> workQueue;
+    Rope* rope = this;
+
+    while (true) {
+        unsigned length = rope->ropeLength();
+        for (unsigned i = 0; i < length; ++i) {
+            Fiber& fiber = rope->fibers(i);
+            if (fiber.isString())
+                fiber.string()->deref();
+            else {
+                Rope* nextRope = fiber.rope();
+                if (nextRope->hasOneRef())
+                    workQueue.append(nextRope);
+                else
+                    nextRope->deref();
+            }
+        }
+        if (rope != this)
+            fastFree(rope);
+
+        if (workQueue.isEmpty())
+            return;
+
+        rope = workQueue.last();
+        workQueue.removeLast();
+    }
+}
+
 JSString::Rope::~Rope()
 {
-    for (unsigned i = 0; i < m_ropeLength; ++i) {
-        Fiber& fiber = m_fibers[i];
-        if (fiber.isRope())
-            fiber.rope()->deref();
-        else
-            fiber.string()->deref();
-        fiber = Fiber(reinterpret_cast<UString::Rep*>(0xfeedbeee));
-    }
+    destructNonRecursive();
 }
 

trunk/JavaScriptCore/runtime/JSString.h

@@ -96 +96 @@
 
             ~Rope();
-
-            void initializeFiber(unsigned index, UString::Rep* string)
+            void destructNonRecursive();
+
+            void initializeFiber(unsigned index, const UString& string)
             {
-                string->ref();
-                m_fibers[index] = Fiber(string);
-                m_stringLength += string->len;
+                UString::Rep* rep = string.rep();
+                rep->ref();
+                m_fibers[index] = Fiber(rep);
+                m_stringLength += rep->len;
             }
             void initializeFiber(unsigned index, Rope* rope)
@@ -114 +116 @@
                     initializeFiber(index, jsString->rope());
                 else
-                    initializeFiber(index, jsString->string().rep());
+                    initializeFiber(index, jsString->string());
             }
 

trunk/JavaScriptCore/runtime/Operations.h

@@ -318 +318 @@
                 rope->initializeFiber(i, asString(v));
             else
-                rope->initializeFiber(i, v.toString(callFrame).rep());
+                rope->initializeFiber(i, v.toString(callFrame));
         }