Changeset 34974 in webkit for trunk/JavaScriptCore

Timestamp:

Jul 2, 2008, 11:48:01 PM (17 years ago)

Author:

[email protected]

Message:

2008-07-02 Geoffrey Garen <​[email protected]>

Reviewed by Cameron Zwarich.

Fixed ​https://bugs.webkit.org/show_bug.cgi?id=19862
REGRESSION (r34907): Gmail crashes in JavaScriptCore code while editing drafts

I was never able to reproduce this issue, but Cameron could, and he says
that this patch fixes it.

The crash seems tied to a timer or event handler callback. In such a case,
the sole reference to the global object may be in the current call frame,
so we can't depend on the global object to mark the call frame area in
the register file.

The new GC marking rule is: the global object is not responsible for
marking the whole register file -- it's just responsible for the globals
section it's tied to. The heap is responsible for marking the call frame area.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• VM/RegisterFile.h (modified) (1 diff)
• kjs/JSGlobalData.cpp (modified) (1 diff)
• kjs/JSGlobalObject.cpp (modified) (1 diff)
• kjs/collector.cpp (modified) (2 diffs)
• kjs/collector.h (modified) (2 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-07-02  Geoffrey Garen  <[email protected]>
+
+        Reviewed by Cameron Zwarich.
+        
+        Fixed https://bugs.webkit.org/show_bug.cgi?id=19862
+        REGRESSION (r34907): Gmail crashes in JavaScriptCore code while editing drafts
+        
+        I was never able to reproduce this issue, but Cameron could, and he says
+        that this patch fixes it.
+        
+        The crash seems tied to a timer or event handler callback. In such a case,
+        the sole reference to the global object may be in the current call frame,
+        so we can't depend on the global object to mark the call frame area in
+        the register file.
+        
+        The new GC marking rule is: the global object is not responsible for
+        marking the whole register file -- it's just responsible for the globals
+        section it's tied to. The heap is responsible for marking the call frame area.
+
 2008-07-02  Mark Rowe  <[email protected]>
 

trunk/JavaScriptCore/VM/RegisterFile.h

@@ -165 +165 @@
 
         Register* lastGlobal() { return m_base - m_numGlobals; }
-
-        void mark(Heap* heap)
-        {
-            heap->markConservatively(lastGlobal(), m_base + m_size);
-        }
+        
+        void markGlobals(Heap* heap) { heap->markConservatively(lastGlobal(), m_base); }
+        void markCallFrames(Heap* heap) { heap->markConservatively(m_base, m_base + m_size); }
 
     private:

trunk/JavaScriptCore/kjs/JSGlobalData.cpp

@@ -59 +59 @@
 JSGlobalData::JSGlobalData(bool isShared)
     : machine(new Machine)
-    , heap(new Heap(isShared))
+    , heap(new Heap(machine, isShared))
 #if USE(MULTIPLE_THREADS)
     , arrayTable(new HashTable(KJS::arrayTable))

trunk/JavaScriptCore/kjs/JSGlobalObject.cpp

@@ -351 +351 @@
     RegisterFile& registerFile = globalData()->machine->registerFile();
     if (registerFile.globalObject() == this)
-        registerFile.mark(globalData()->heap);
+        registerFile.markGlobals(globalData()->heap);
 
     markIfNeeded(d()->globalExec->exception());

trunk/JavaScriptCore/kjs/collector.cpp

@@ -93 +93 @@
 static void freeHeap(CollectorHeap*);
 
-Heap::Heap(bool isShared)
+Heap::Heap(Machine* machine, bool isShared)
     : m_markListSet(0)
     , m_isShared(isShared)
+    , m_machine(machine)
 {
     memset(&primaryHeap, 0, sizeof(CollectorHeap));
@@ -945 +946 @@
     if (m_markListSet && m_markListSet->size())
         ArgList::markLists(*m_markListSet);
+    m_machine->registerFile().markCallFrames(this);
 
     JAVASCRIPTCORE_GC_MARKED();

trunk/JavaScriptCore/kjs/collector.h

@@ -112 +112 @@
 
         friend class JSGlobalData;
-        Heap(bool isShared);
+        Heap(Machine*, bool isShared);
         ~Heap();
 
@@ -133 +133 @@
 
         bool m_isShared;
+        
+        Machine* m_machine;
     };