Bug 1278832 - Make sure heap check zeal mode traces the heap outside of a GC. r=terrence, a=lizzard
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 13 Jun 2016 11:39:10 +0100
changeset 339727 c61ac4fc1125899776b9dec46e824341ecbd238d
parent 339726 7cb944f2d37f09ddfcfcecbfd8cb89486b7039a3
child 339728 6fae46fe5feededc16062836879dbd872933edf0
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, lizzard
bugs1278832
milestone49.0a2
Bug 1278832 - Make sure heap check zeal mode traces the heap outside of a GC. r=terrence, a=lizzard
js/src/gc/GCInternals.h
js/src/gc/Nursery.cpp
js/src/gc/Verifier.cpp
js/src/jit-test/tests/gc/bug-1278832.js
js/src/jsgc.cpp
js/src/vm/TypeInference.cpp
--- a/js/src/gc/GCInternals.h
+++ b/js/src/gc/GCInternals.h
@@ -119,17 +119,17 @@ class MOZ_RAII AutoStopVerifyingBarriers
 struct MOZ_RAII AutoStopVerifyingBarriers
 {
     AutoStopVerifyingBarriers(JSRuntime*, bool) {}
 };
 #endif /* JS_GC_ZEAL */
 
 #ifdef JSGC_HASH_TABLE_CHECKS
 void CheckHashTablesAfterMovingGC(JSRuntime* rt);
-void CheckHeapAfterMovingGC(JSRuntime* rt, AutoLockForExclusiveAccess& lock);
+void CheckHeapAfterMovingGC(JSRuntime* rt);
 #endif
 
 struct MovingTracer : JS::CallbackTracer
 {
     explicit MovingTracer(JSRuntime* rt) : CallbackTracer(rt, TraceWeakMapKeysValues) {}
 
     void onObjectEdge(JSObject** objp) override;
     void onShapeEdge(Shape** shapep) override;
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -495,23 +495,16 @@ js::Nursery::collect(JSRuntime* rt, JS::
     // Make sure hashtables have been updated after the collection.
     TIME_START(checkHashTables);
 #ifdef JS_GC_ZEAL
     if (rt->hasZealMode(ZealMode::CheckHashTablesOnMinorGC))
         CheckHashTablesAfterMovingGC(rt);
 #endif
     TIME_END(checkHashTables);
 
-    TIME_START(checkHeap);
-#ifdef JS_GC_ZEAL
-    if (rt->hasZealMode(ZealMode::CheckHeapOnMovingGC))
-        CheckHeapAfterMovingGC(rt, session.lock);
-#endif
-    TIME_END(checkHeap);
-
     // Resize the nursery.
     TIME_START(resize);
     double promotionRate = mover.tenuredSize / double(allocationEnd() - start());
     if (promotionRate > 0.05)
         growAllocableSpace();
     else if (promotionRate < 0.01)
         shrinkAllocableSpace();
     TIME_END(resize);
@@ -552,17 +545,16 @@ js::Nursery::collect(JSRuntime* rt, JS::
         } PrintList[] = {
             {"canIon", TIME_TOTAL(cancelIonCompilations)},
             {"mkVals", TIME_TOTAL(traceValues)},
             {"mkClls", TIME_TOTAL(traceCells)},
             {"mkSlts", TIME_TOTAL(traceSlots)},
             {"mcWCll", TIME_TOTAL(traceWholeCells)},
             {"mkGnrc", TIME_TOTAL(traceGenericEntries)},
             {"ckTbls", TIME_TOTAL(checkHashTables)},
-            {"ckHeap", TIME_TOTAL(checkHeap)},
             {"mkRntm", TIME_TOTAL(markRuntime)},
             {"mkDbgr", TIME_TOTAL(markDebugger)},
             {"clrNOC", TIME_TOTAL(clearNewObjectCache)},
             {"collct", TIME_TOTAL(collectToFP)},
             {" tenCB", TIME_TOTAL(objectsTenuredCallback)},
             {"swpABO", TIME_TOTAL(sweepArrayBufferViewList)},
             {"updtIn", TIME_TOTAL(updateJitActivations)},
             {"frSlts", TIME_TOTAL(freeMallocedBuffers)},
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -521,17 +521,17 @@ CheckHeapTracer::check(AutoLockForExclus
                 failures, visited.count());
     }
     MOZ_RELEASE_ASSERT(failures == 0);
 
     return true;
 }
 
 void
-js::gc::CheckHeapAfterMovingGC(JSRuntime* rt, AutoLockForExclusiveAccess& lock)
+js::gc::CheckHeapAfterMovingGC(JSRuntime* rt)
 {
-    MOZ_ASSERT(rt->isHeapCollecting());
+    AutoTraceSession session(rt, JS::HeapState::Tracing);
     CheckHeapTracer tracer(rt);
-    if (!tracer.init() || !tracer.check(lock))
+    if (!tracer.init() || !tracer.check(session.lock))
         fprintf(stderr, "OOM checking heap\n");
 }
 
 #endif /* JSGC_HASH_TABLE_CHECKS */
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1278832.js
@@ -0,0 +1,12 @@
+function assertThrowsInstanceOf() {}
+gczeal(15)
+try {
+    gczeal(10, 2)
+} catch (Atomics) {}
+for (define of[__defineSetter__]) {
+    let nonCallable = [{}]
+    for (let value of nonCallable) assertThrowsInstanceOf(TypeError)
+    key = {
+        [Symbol]() {}
+    }
+}
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -5532,20 +5532,16 @@ GCRuntime::compactPhase(JS::gcreason::Re
 
     // Clear runtime caches that can contain cell pointers.
     rt->newObjectCache.purge();
     rt->nativeIterCache.purge();
 
 #ifdef DEBUG
     CheckHashTablesAfterMovingGC(rt);
 #endif
-#ifdef JS_GC_ZEAL
-    if (rt->hasZealMode(ZealMode::CheckHeapOnMovingGC))
-        CheckHeapAfterMovingGC(rt, lock);
-#endif
 
     return zonesToMaybeCompact.isEmpty() ? Finished : NotFinished;
 }
 
 void
 GCRuntime::endCompactPhase(JS::gcreason::Reason reason)
 {
     startedCompacting = false;
@@ -6304,16 +6300,23 @@ GCRuntime::collect(bool nonincrementalBy
          * repeat GCs that happen during shutdown (the gcShouldCleanUpEverything
          * case) until we can be sure that no additional garbage is created
          * (which typically happens if roots are dropped during finalizers).
          */
         repeat = (poked && cleanUpEverything) || wasReset || repeatForDeadZone;
     } while (repeat);
 
     agc.setCycleCount(cycleCount);
+
+#ifdef JS_GC_ZEAL
+    if (shouldCompact() && rt->hasZealMode(ZealMode::CheckHeapOnMovingGC)) {
+        gcstats::AutoPhase ap(rt->gc.stats, gcstats::PHASE_TRACE_HEAP);
+        CheckHeapAfterMovingGC(rt);
+    }
+#endif
 }
 
 js::AutoEnqueuePendingParseTasksAfterGC::~AutoEnqueuePendingParseTasksAfterGC()
 {
     if (!gc_.isIncrementalGCInProgress())
         EnqueuePendingParseTasksAfterGC(gc_.rt);
 }
 
@@ -6512,16 +6515,21 @@ GCRuntime::minorGCImpl(JS::gcreason::Rea
     minorGCTriggerReason = JS::gcreason::NO_REASON;
     TraceLoggerThread* logger = TraceLoggerForMainThread(rt);
     AutoTraceLog logMinorGC(logger, TraceLogger_MinorGC);
     nursery.collect(rt, reason, pretenureGroups);
     MOZ_ASSERT(nursery.isEmpty());
 
     blocksToFreeAfterMinorGC.freeAll();
 
+#ifdef JS_GC_ZEAL
+    if (rt->hasZealMode(ZealMode::CheckHeapOnMovingGC))
+        CheckHeapAfterMovingGC(rt);
+#endif
+
     AutoLockGC lock(rt);
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next())
         maybeAllocTriggerZoneGC(zone, lock);
 }
 
 // Alternate to the runtime-taking form that allows marking object groups as
 // needing pretenuring.
 void
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -4060,22 +4060,17 @@ ConstraintTypeSet::trace(Zone* zone, JST
 
 static inline void
 AssertGCStateForSweep(Zone* zone)
 {
     MOZ_ASSERT(zone->isGCSweepingOrCompacting());
 
     // IsAboutToBeFinalized doesn't work right on tenured objects when called
     // during a minor collection.
-    //
-    // We allow this when tracing the heap for CheckHeapOnMovingGC since that
-    // happens afterwards and is not part of minor collection.
-    DebugOnly<JSRuntime*> rt(zone->runtimeFromMainThread());
-    MOZ_ASSERT_IF(!rt->hasZealMode(ZealMode::CheckHeapOnMovingGC),
-                  !rt->isHeapMinorCollecting());
+    MOZ_ASSERT(!zone->runtimeFromMainThread()->isHeapMinorCollecting());
 }
 
 void
 ConstraintTypeSet::sweep(Zone* zone, AutoClearTypeInferenceStateOnOOM& oom)
 {
     AssertGCStateForSweep(zone);
 
     /*