Title: [102728] trunk/Source/_javascript_Core
Revision
102728
Author
[email protected]
Date
2011-12-13 18:55:02 -0800 (Tue, 13 Dec 2011)

Log Message

<rdar://problem/10577239> GC Crash introduced in r102545

Reviewed by Gavin Barraclough.
        
MarkedArgumentBuffer was still marking items in forwards order, even though
the argument order has been reversed.
        
I fixed this bug, and replaced address calculation code with some helper
functions -- mallocBase() and slotFor() -- so it stays fixed everywhere.

* runtime/ArgList.cpp:
(JSC::MarkedArgumentBuffer::markLists):
(JSC::MarkedArgumentBuffer::slowAppend):
* runtime/ArgList.h:
(JSC::MarkedArgumentBuffer::~MarkedArgumentBuffer):
(JSC::MarkedArgumentBuffer::at):
(JSC::MarkedArgumentBuffer::append):
(JSC::MarkedArgumentBuffer::last):
(JSC::MarkedArgumentBuffer::slotFor):
(JSC::MarkedArgumentBuffer::mallocBase):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (102727 => 102728)


--- trunk/Source/_javascript_Core/ChangeLog	2011-12-14 02:32:43 UTC (rev 102727)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-12-14 02:55:02 UTC (rev 102728)
@@ -1,3 +1,26 @@
+2011-12-13  Geoffrey Garen  <[email protected]>
+
+        <rdar://problem/10577239> GC Crash introduced in r102545
+
+        Reviewed by Gavin Barraclough.
+        
+        MarkedArgumentBuffer was still marking items in forwards order, even though
+        the argument order has been reversed.
+        
+        I fixed this bug, and replaced address calculation code with some helper
+        functions -- mallocBase() and slotFor() -- so it stays fixed everywhere.
+
+        * runtime/ArgList.cpp:
+        (JSC::MarkedArgumentBuffer::markLists):
+        (JSC::MarkedArgumentBuffer::slowAppend):
+        * runtime/ArgList.h:
+        (JSC::MarkedArgumentBuffer::~MarkedArgumentBuffer):
+        (JSC::MarkedArgumentBuffer::at):
+        (JSC::MarkedArgumentBuffer::append):
+        (JSC::MarkedArgumentBuffer::last):
+        (JSC::MarkedArgumentBuffer::slotFor):
+        (JSC::MarkedArgumentBuffer::mallocBase):
+
 2011-12-13  Filip Pizlo  <[email protected]>
 
         DFG OSR exit for UInt32ToNumber should roll forward, not roll backward

Modified: trunk/Source/_javascript_Core/runtime/ArgList.cpp (102727 => 102728)


--- trunk/Source/_javascript_Core/runtime/ArgList.cpp	2011-12-14 02:32:43 UTC (rev 102727)
+++ trunk/Source/_javascript_Core/runtime/ArgList.cpp	2011-12-14 02:55:02 UTC (rev 102728)
@@ -46,7 +46,8 @@
     ListSet::iterator end = markSet.end();
     for (ListSet::iterator it = markSet.begin(); it != end; ++it) {
         MarkedArgumentBuffer* list = *it;
-        heapRootVisitor.visit(reinterpret_cast<JSValue*>(list->m_buffer), list->m_size);
+        for (int i = 0; i < list->m_size; ++i)
+            heapRootVisitor.visit(reinterpret_cast<JSValue*>(&list->slotFor(i)));
     }
 }
 
@@ -57,13 +58,13 @@
     for (int i = 0; i < m_capacity; ++i)
         newBuffer[-i] = m_buffer[-i];
 
-    if (m_capacity != static_cast<int>(inlineCapacity))
-        delete [] &m_buffer[-(m_capacity - 1)];
+    if (EncodedJSValue* base = mallocBase())
+        delete [] base;
 
     m_buffer = newBuffer;
     m_capacity = newCapacity;
 
-    m_buffer[-m_size] = JSValue::encode(v);
+    slotFor(m_size) = JSValue::encode(v);
     ++m_size;
 
     if (m_markSet)
@@ -75,7 +76,7 @@
     // our Vector's inline capacity, though, our values move to the 
     // heap, where they do need explicit marking.
     for (int i = 0; i < m_size; ++i) {
-        Heap* heap = Heap::heap(JSValue::decode(m_buffer[-i]));
+        Heap* heap = Heap::heap(JSValue::decode(slotFor(i)));
         if (!heap)
             continue;
 

Modified: trunk/Source/_javascript_Core/runtime/ArgList.h (102727 => 102728)


--- trunk/Source/_javascript_Core/runtime/ArgList.h	2011-12-14 02:32:43 UTC (rev 102727)
+++ trunk/Source/_javascript_Core/runtime/ArgList.h	2011-12-14 02:55:02 UTC (rev 102728)
@@ -58,8 +58,8 @@
             if (m_markSet)
                 m_markSet->remove(this);
 
-            if (m_capacity != static_cast<int>(inlineCapacity))
-                delete [] &m_buffer[-(m_capacity - 1)];
+            if (EncodedJSValue* base = mallocBase())
+                delete [] base;
         }
 
         size_t size() const { return m_size; }
@@ -70,7 +70,7 @@
             if (i >= m_size)
                 return jsUndefined();
 
-            return JSValue::decode(m_buffer[-i]);
+            return JSValue::decode(slotFor(i));
         }
 
         void clear()
@@ -83,7 +83,7 @@
             if (m_size >= m_capacity)
                 return slowAppend(v);
 
-            m_buffer[-m_size] = JSValue::encode(v);
+            slotFor(m_size) = JSValue::encode(v);
             ++m_size;
         }
 
@@ -96,7 +96,7 @@
         JSValue last() 
         {
             ASSERT(m_size);
-            return JSValue::decode(m_buffer[-(m_size - 1)]);
+            return JSValue::decode(slotFor(m_size - 1));
         }
         
         static void markLists(HeapRootVisitor&, ListSet&);
@@ -104,6 +104,18 @@
     private:
         void slowAppend(JSValue);
         
+        EncodedJSValue& slotFor(int item) const
+        {
+            return m_buffer[-item];
+        }
+        
+        EncodedJSValue* mallocBase()
+        {
+            if (m_capacity == static_cast<int>(inlineCapacity))
+                return 0;
+            return &slotFor(m_capacity - 1);
+        }
+        
         int m_size;
         int m_capacity;
         EncodedJSValue m_inlineBuffer[inlineCapacity];
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to