Changeset 16797 in webkit for trunk/JavaScriptCore

Timestamp:

Oct 4, 2006, 6:07:49 PM (19 years ago)

Author:

ggaren

Message:

Patch by Darin and me, reviewed by Maciej.

Fixed <rdar://problem/4518397> REGRESSION(?): Oft-seen but unrepro crash

in JavaScript garbage collection (KJS::Collector::collect())

<rdar://problem/4752492> Crash in KJS::collect

The issue here was allocating one garbage-collected object in the midst
of allocating a second garbage-collected object. In such a case, the
zeroIfFree word lies.

• kjs/collector.cpp: (KJS::Collector::allocate): (KJS::Collector::collect):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• kjs/collector.cpp (modified) (3 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2006-10-04  Geoffrey Garen  <[email protected]>
+
+        Patch by Darin and me, reviewed by Maciej.
+
+        Fixed <rdar://problem/4518397> REGRESSION(?): Oft-seen but unrepro crash 
+              in JavaScript garbage collection (KJS::Collector::collect())
+               <rdar://problem/4752492> Crash in KJS::collect
+              
+        The issue here was allocating one garbage-collected object in the midst 
+        of allocating a second garbage-collected object. In such a case, the
+        zeroIfFree word lies.
+
+        * kjs/collector.cpp:
+        (KJS::Collector::allocate):
+        (KJS::Collector::collect):
+
 2006-10-04  Kevin McCullough  <[email protected]>
 

trunk/JavaScriptCore/kjs/collector.cpp

@@ -119 +119 @@
   size_t numLiveObjectsAtLastCollect = heap.numLiveObjectsAtLastCollect;
   size_t numNewObjects = numLiveObjects - numLiveObjectsAtLastCollect;
+
   if (numNewObjects >= ALLOCATIONS_PER_COLLECTION && numNewObjects >= numLiveObjectsAtLastCollect) {
     collect();
@@ -493 +494 @@
           imp->m_marked = false;
         } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
+          // special case for allocated but uninitialized object
+          // (We don't need this check earlier because nothing prior this point assumes the object has a valid vptr.)
+          if (cell->u.freeCell.zeroIfFree == 0)
+            continue;
+
           imp->~JSCell();
           --usedCells;
@@ -505 +511 @@
     } else {
       size_t minimumCellsToProcess = usedCells;
-      for (size_t i = 0; i < minimumCellsToProcess; i++) {
+      for (size_t i = 0; (i < minimumCellsToProcess) & (i < CELLS_PER_BLOCK); i++) {
         CollectorCell *cell = curBlock->cells + i;
         if (cell->u.freeCell.zeroIfFree == 0) {