Currently our gray marking asserts cover HeapSlot, which used for JSObject properties and elements. We should extend the asserts to cover HeapPtr and GCPtr too.

This patch changes the way we do the gray marking asserts which happen on heap writes. Currently we check whether the source of edge is black and the target gray. This means you have to know the source, which works for HeapSlot but not for the other types of barriered pointer. The patch changes this to only check whether the target is gray and disables the check for a couple of areas where we expect to write gray pointers. It turned out there were only three, all GC-releated. Now we can do this check for the other pointer types too. I had to remove an assert that we don't checking whether a pointer is gray during GC because it turned out that happened quite a lot. I removed one (ScriptSourceObject::finalize) but it looked like this was going to be simpler just to take the assert out.

Comment on attachment 8954085 [details] [diff] [review] bug1440739-improve-gray-asserts Review of attachment 8954085 [details] [diff] [review]: ----------------------------------------------------------------- Oh, wow, this is smart. Though it makes me wonder if it would be nice to have some sort of unified state indicator for GC, just to have the state machine in one place and stop the proliferation of trivial RAII region markers. Its components might need to be store in different places. Something encompassing gcState, performingGC, gcSweeping, incrementalState, invocationKind, isIncremental, isFull, isCompacting, grayBitsValid, majorGCTriggerReason, fullGCForAtomsRequested_, mode (gcMode), grayBufferState, etc., and maybe stuff like keepAtoms and incrementalAllowed. Not in a single variable, and probably scattered across a number of memory locations. Anyway, not for this bug. ::: js/src/gc/GC.cpp @@ +9082,5 @@ > // of cells that will be marked black by the next GC slice in an incremental > // GC. For performance reasons we don't do this in CellIsMarkedGrayIfKnown. > > // TODO: I'd like to AssertHeapIsIdle() here, but this ends up getting > + // called while during GC and while iterating the heap for memory reporting. s/while during/during/

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a805b66dfcc Improve gray marking assertions to cover more types of pointer r=sfink