Bug 824321 - "Assertion failure: !IsThingPoisoned(thing)," r=terrence

--HG--
extra : rebase_source : e1662451f8f90af0b1f25073e7f919cebdf5d410

js/src/gc/Heap.h

@@ -988,6 +988,36 @@ Cell::isAligned() const
 }
 #endif
 
+inline bool
+InFreeList(ArenaHeader *aheader, void *thing)
+{
+    if (!aheader->hasFreeThings())
+        return false;
+
+    FreeSpan firstSpan(aheader->getFirstFreeSpan());
+    uintptr_t addr = reinterpret_cast<uintptr_t>(thing);
+
+    for (const FreeSpan *span = &firstSpan;;) {
+        /* If the thing comes before the current span, it's not free. */
+        if (addr < span->first)
+            return false;
+
+        /*
+         * If we find it inside the span, it's dead. We use here "<=" and not
+         * "<" even for the last span as we know that thing is inside the
+         * arena. Thus, for the last span thing < span->end.
+         */
+        if (addr <= span->last)
+            return true;
+
+        /*
+         * The last possible empty span is an the end of the arena. Here
+         * span->end < thing < thingsEnd and so we must have more spans.
+         */
+        span = span->nextSpan();
+    }
+}
+
 } /* namespace gc */
 } /* namespace js */
 

js/src/gc/Marking.cpp

@@ -144,7 +144,14 @@ CheckMarkedThing(JSTracer *trc, T *thing)
                  thing->compartment()->isGCMarkingGray() ||
                  thing->compartment() == rt->atomsCompartment);
 
-    JS_ASSERT(!IsThingPoisoned(thing));
+    /*
+     * Try to assert that the thing is allocated.  This is complicated by the
+     * fact that allocated things may still contain the poison pattern if that
+     * part has not been overwritten, and that the free span list head in the
+     * ArenaHeader may not be synced with the real one in ArenaLists.
+     */
+    JS_ASSERT_IF(IsThingPoisoned(thing) && rt->isHeapBusy(),
+                 !InFreeList(thing->arenaHeader(), thing));
 }
 
 static GCMarker *

js/src/gc/RootMarking.cpp

@@ -87,35 +87,6 @@ MarkExactStackRoots(JSTracer *trc)
 }
 #endif /* JSGC_USE_EXACT_ROOTING */
 
-static inline bool
-InFreeList(ArenaHeader *aheader, uintptr_t addr)
-{
-    if (!aheader->hasFreeThings())
-        return false;
-
-    FreeSpan firstSpan(aheader->getFirstFreeSpan());
-
-    for (const FreeSpan *span = &firstSpan;;) {
-        /* If the thing comes fore the current span, it's not free. */
-        if (addr < span->first)
-            return false;
-
-        /*
-         * If we find it inside the span, it's dead. We use here "<=" and not
-         * "<" even for the last span as we know that thing is inside the
-         * arena. Thus for the last span thing < span->end.
-         */
-        if (addr <= span->last)
-            return true;
-
-        /*
-         * The last possible empty span is an the end of the arena. Here
-         * span->end < thing < thingsEnd and so we must have more spans.
-         */
-        span = span->nextSpan();
-    }
-}
-
 enum ConservativeGCTest
 {
     CGCT_VALID,
@@ -240,7 +211,7 @@ MarkIfGCThingWord(JSTracer *trc, uintptr_t w)
      * this point we no longer have the mark bits from the previous GC run and
      * we must account for newly allocated things.
      */
-    if (InFreeList(aheader, uintptr_t(thing)))
+    if (InFreeList(aheader, thing))
         return CGCT_NOTLIVE;
 
     JSGCTraceKind traceKind = MapAllocToTraceKind(thingKind);

js/src/jit-test/tests/gc/bug-824321.js

@@ -0,0 +1,3 @@
+x = "\udada\udada";
+gc();
+

js/src/jsgc.cpp

@@ -267,7 +267,7 @@ ArenaHeader::checkSynchronizedWithFreeList() const
      * list in the compartment can mutate at any moment. We cannot do any
      * checks in this case.
      */
-    if (!compartment->rt->isHeapBusy())
+    if (IsBackgroundFinalized(getAllocKind()) && !compartment->rt->isHeapBusy())
         return;
 
     FreeSpan firstSpan = FreeSpan::decodeOffsets(arenaAddress(), firstFreeSpanOffsets);

js/src/jsgcinlines.h

@@ -485,19 +485,6 @@ class GCCompartmentGroupIter {
  * in the partially initialized thing.
  */
 
-template<typename T>
-static inline void
-UnpoisonThing(T *thing)
-{
-#ifdef DEBUG
-    /* Change the contents of memory slightly so that IsThingPoisoned returns false. */
-    JS_STATIC_ASSERT(sizeof(T) >= sizeof(FreeSpan) + sizeof(uint8_t));
-    uint8_t *p =
-        reinterpret_cast<uint8_t *>(reinterpret_cast<FreeSpan *>(thing) + 1);
-    *p = 0;
-#endif
-}
-
 template <typename T>
 inline T *
 NewGCThing(JSContext *cx, js::gc::AllocKind kind, size_t thingSize)
@@ -534,8 +521,6 @@ NewGCThing(JSContext *cx, js::gc::AllocKind kind, size_t thingSize)
         comp->gcNursery.insertPointer(t);
 #endif
 
-    if (t)
-        UnpoisonThing(t);
     return t;
 }
 
@@ -567,8 +552,6 @@ TryNewGCThing(JSContext *cx, js::gc::AllocKind kind, size_t thingSize)
         comp->gcNursery.insertPointer(t);
 #endif
 
-    if (t)
-        UnpoisonThing(t);
     return t;
 }