Title: [102220] trunk/Source/_javascript_Core
Revision
102220
Author
fpi...@apple.com
Date
2011-12-06 21:25:49 -0800 (Tue, 06 Dec 2011)

Log Message

Zapping a block that is Marked leads to dead objects being mistaken for live ones
https://bugs.webkit.org/show_bug.cgi?id=73982

Reviewed by Geoff Garen.
        
Changed the zapping code to ignore blocks that are Marked or Zapped. Additionally,
the code asserts that:
        
- If we zap a Marked or Zapped block then the free list is empty, because this
  can only happen if the block was never free-listed.
          
- Zapping can only happen for Marked, Zapped, or FreeListed blocks, since Allocated
  blocks are those that cannot be referred to by SizeClass::currentBlock (since
  SizeClass::currentBlock only refers to blocks that are candidates for allocation,
  and Allocated blocks are those who have been exhausted by allocation and will not
  be allocated from again), and New blocks cannot be referred to by anything except
  during a brief window inside the allocation slow-path.

* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::zapFreeList):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (102219 => 102220)


--- trunk/Source/_javascript_Core/ChangeLog	2011-12-07 05:01:38 UTC (rev 102219)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-12-07 05:25:49 UTC (rev 102220)
@@ -1,5 +1,28 @@
 2011-12-06  Filip Pizlo  <fpi...@apple.com>
 
+        Zapping a block that is Marked leads to dead objects being mistaken for live ones
+        https://bugs.webkit.org/show_bug.cgi?id=73982
+
+        Reviewed by Geoff Garen.
+        
+        Changed the zapping code to ignore blocks that are Marked or Zapped. Additionally,
+        the code asserts that:
+        
+        - If we zap a Marked or Zapped block then the free list is empty, because this
+          can only happen if the block was never free-listed.
+          
+        - Zapping can only happen for Marked, Zapped, or FreeListed blocks, since Allocated
+          blocks are those that cannot be referred to by SizeClass::currentBlock (since
+          SizeClass::currentBlock only refers to blocks that are candidates for allocation,
+          and Allocated blocks are those who have been exhausted by allocation and will not
+          be allocated from again), and New blocks cannot be referred to by anything except
+          during a brief window inside the allocation slow-path.
+
+        * heap/MarkedBlock.cpp:
+        (JSC::MarkedBlock::zapFreeList):
+
+2011-12-06  Filip Pizlo  <fpi...@apple.com>
+
         DFG 32_64 call linking does not handle non-cell callees correctly
         https://bugs.webkit.org/show_bug.cgi?id=73965
 

Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.cpp (102219 => 102220)


--- trunk/Source/_javascript_Core/heap/MarkedBlock.cpp	2011-12-07 05:01:38 UTC (rev 102219)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.cpp	2011-12-07 05:25:49 UTC (rev 102220)
@@ -141,16 +141,47 @@
 {
     HEAP_LOG_BLOCK_STATE_TRANSITION(this);
 
+    if (m_state == Marked) {
+        // If the block is in the Marked state then we know that:
+        // 1) It was not used for allocation during the previous allocation cycle.
+        // 2) It may have dead objects, and we only know them to be dead by the
+        //    fact that their mark bits are unset.
+        // Hence if the block is Marked we need to leave it Marked.
+        
+        ASSERT(!firstFreeCell);
+        
+        return;
+    }
+    
+    if (m_state == Zapped) {
+        // If the block is in the Zapped state then we know that someone already
+        // zapped it for us. This could not have happened during a GC, but might
+        // be the result of someone having done a GC scan to perform some operation
+        // over all live objects (or all live blocks). It also means that somebody
+        // had allocated in this block since the last GC, swept all dead objects
+        // onto the free list, left the block in the FreeListed state, then the heap
+        // scan happened, and canonicalized the block, leading to all dead objects
+        // being zapped. Therefore, it is safe for us to simply do nothing, since
+        // dead objects will have 0 in their vtables and live objects will have
+        // non-zero vtables, which is consistent with the block being zapped.
+        
+        ASSERT(!firstFreeCell);
+        
+        return;
+    }
+    
+    ASSERT(m_state == FreeListed);
+    
     // Roll back to a coherent state for Heap introspection. Cells newly
     // allocated from our free list are not currently marked, so we need another
     // way to tell what's live vs dead. We use zapping for that.
-
+    
     FreeCell* next;
     for (FreeCell* current = firstFreeCell; current; current = next) {
         next = current->next;
         reinterpret_cast<JSCell*>(current)->zap();
     }
-
+    
     m_state = Zapped;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to