Fix an interiorness bug, and some debugging gunk
authorbenjamin@smedbergs.us
Tue, 11 Dec 2007 17:52:11 -0500
changeset 20 458babdaa518471a7a2e4b09f514955c86b7c60f
parent 19 9440555b77389c85b8100c0e8215f3b460613e2d
child 22 0ca31a2285356f7802d7a2ecda77ba7591f833cc
push id1
push userbsmedberg@mozilla.com
push dateMon, 21 Apr 2008 01:54:18 +0000
Fix an interiorness bug, and some debugging gunk
assert-non-interiorness
debug-print-finalizers
enable-traces
series
new file mode 100644
--- /dev/null
+++ b/assert-non-interiorness
@@ -0,0 +1,79 @@
+diff --git a/MMgc/GC.cpp b/MMgc/GC.cpp
+--- a/MMgc/GC.cpp
++++ b/MMgc/GC.cpp
+@@ -2567,7 +2567,7 @@ bail:
+ 				}
+ 				else
+ 				{
+-					item = FindBeginning((void *) val);
++					item = GetRealPointer(FindBeginning((void *) val));
+ 				}
+ #else
+ 				// back up to real beginning
+@@ -3184,5 +3184,16 @@ bail:
+ 	}
+ #endif
+ 
+-
++#ifdef _DEBUG
++	GCWorkItem::GCWorkItem(const void *p, uint32 s, bool isGCItem)
++		: ptr(p)
++		, _size(s | uint32(isGCItem))
++	{
++		// If a WI is a GC item, it should always point to the
++		// "real" item, not an interior pointer!
++		if (IsGCItem()) {
++			GCAssert(GC::GetGC(p)->FindBeginning(p) == p);
++		}
++	}
++#endif
+ }
+diff --git a/MMgc/GC.h b/MMgc/GC.h
+--- a/MMgc/GC.h
++++ b/MMgc/GC.h
+@@ -978,10 +978,17 @@ namespace MMgc
+ 		/** @access Requires(request) */
+ 		void *FindBeginning(const void *gcItem)
+ 		{
++			GCAcquireSpinlock lock(pageMapLock);
++			return FindBeginningAlreadyLocked(gcItem);
++		}
++
++		/** @access Requires(pageMapLock) */
++		void *FindBeginningAlreadyLocked(const void *gcItem)
++		{
+ 			GCAssert(gcItem != NULL);
+-			GCAssert(GetPageMapValue((uintptr)gcItem) != 0);
++			GCAssert(GetPageMapValueAlreadyLocked((uintptr)gcItem) != 0);
+ 			void *realItem = NULL;
+-			int bits = GetPageMapValue((uintptr)gcItem);
++			int bits = GetPageMapValueAlreadyLocked((uintptr)gcItem);
+ 			switch(bits)
+ 			{
+ 			case kGCAllocPage:
+@@ -994,7 +1001,7 @@ namespace MMgc
+ 				while(bits == kGCLargeAllocPageRest)
+ 				{
+ 					gcItem = (void*) ((uintptr)gcItem - GCHeap::kBlockSize);
+-					bits = GetPageMapValue((uintptr)gcItem);
++					bits = GetPageMapValueAlreadyLocked((uintptr)gcItem);
+ 				}
+ 				realItem = GCLargeAlloc::FindBeginning(gcItem);
+ 				break;
+diff --git a/MMgc/GCTypes.h b/MMgc/GCTypes.h
+--- a/MMgc/GCTypes.h
++++ b/MMgc/GCTypes.h
+@@ -119,7 +119,12 @@ namespace MMgc
+ 	{
+ 	public:
+ 		GCWorkItem() : ptr(0), _size(0) {}
+-		GCWorkItem(const void *p, uint32 s, bool isGCItem) : ptr(p), _size(s | uint32(isGCItem)) {}
++
++#ifdef _DEBUG
++		GCWorkItem(const void *p, uint32 s, bool isGCItem);
++#else
++		GCWorkItem(const void *p, uint32 s, bool isGCItem) : ptr(p), _size(s | uint32(isGCItem)) { }
++#endif
+ 		uint32 GetSize() const { return _size & ~1; }
+ 		int IsGCItem() const { return _size & 1; }
+ 		const void *ptr;
new file mode 100644
--- /dev/null
+++ b/debug-print-finalizers
@@ -0,0 +1,48 @@
+diff --git a/MMgc/GCAlloc.cpp b/MMgc/GCAlloc.cpp
+--- a/MMgc/GCAlloc.cpp
++++ b/MMgc/GCAlloc.cpp
+@@ -42,6 +42,9 @@
+ 
+ #include "MMgc.h"
+ 
++#include <typeinfo>
++#include <stdio.h>
++
+ namespace MMgc
+ {
+ 	GCAlloc::GCAlloc(GC* _gc, int _itemSize, bool _containsPointers, bool _isRC, int _sizeClassIndex) : 
+@@ -435,6 +438,10 @@ start:
+ 					{     
+ 						GCFinalizable *obj = (GCFinalizedObject*)GetUserPointer(item);
+ 						GCAssert(*(int*)obj != 0);
++
++						fprintf(stderr, "Finalizing class %s at %p\n",
++							typeid(*obj).name(), obj);
++
+ 						obj->~GCFinalizable();
+ 
+ 						bits[i] &= ~(kFinalize<<(j*4));
+diff --git a/MMgc/GCLargeAlloc.cpp b/MMgc/GCLargeAlloc.cpp
+--- a/MMgc/GCLargeAlloc.cpp
++++ b/MMgc/GCLargeAlloc.cpp
+@@ -42,6 +42,9 @@
+ #include <string.h>
+ 
+ #include "MMgc.h"
++
++#include <typeinfo>
++#include <stdio.h>
+ 
+ namespace MMgc
+ {
+@@ -132,6 +135,10 @@ namespace MMgc
+ 				if (NeedsFinalize(b)) {
+ 					GCFinalizable *obj = (GCFinalizable *) item;
+ 					obj = (GCFinalizable *) GetUserPointer(obj);
++
++					fprintf(stderr, "Finalizing class %s at %p\n",
++						typeid(*obj).name(), obj);
++
+ 					obj->~GCFinalizable();
+ #if defined(_DEBUG) && defined(MMGC_DRC)
+ 					if((b->flags & kRCObject) != 0) {
new file mode 100644
--- /dev/null
+++ b/enable-traces
@@ -0,0 +1,12 @@
+diff --git a/MMgc/GCMemoryProfiler.cpp b/MMgc/GCMemoryProfiler.cpp
+--- a/MMgc/GCMemoryProfiler.cpp
++++ b/MMgc/GCMemoryProfiler.cpp
+@@ -50,7 +50,7 @@ namespace MMgc
+ 	GCThreadLocal<void*> memtype;
+ 
+ 	// Turn this to see GC stack traces.
+-	const bool enableTraces = false;
++	const bool enableTraces = true;
+ 
+ 	// this is how many stack frames we'll attempt to lookup, we may not get this many and 
+ 	// we may leave some out
--- a/series
+++ b/series
@@ -4,8 +4,11 @@ gcstack-access
 workitems-notgc-noassert
 gc-graph #+graphviz
 alloc-backtrace #+graphviz
 configure-with-threadsafe-mmgc #+threadsafe
 mmgc-threadsafe #+threadsafe
 mmgc-threadsafe-gctests #+threadsafe
 mmgc-graphviz #+jorendorff-graphviz
 mmgc-bit-checks #+threadsafe
+enable-traces
+assert-non-interiorness
+debug-print-finalizers