Bug 1203273 - Add more diagnostics for gray buffer marking crashes on OSX. r=sfink, a=RyanVM
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 30 May 2018 14:18:18 -0400
changeset 473522 57f7d2058ce2ef621dcf298b6415bbb829a1cb56
parent 473521 ffdd779eb0b91165288c9cee981cf74009bd965f
child 473523 dbfb05cdda81d130050354ed1b108f685f0bd30f
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, RyanVM
bugs1203273
milestone61.0
Bug 1203273 - Add more diagnostics for gray buffer marking crashes on OSX. r=sfink, a=RyanVM
js/src/gc/RootMarking.cpp
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -467,38 +467,29 @@ class BufferGrayRootsTracer final : publ
 bool
 js::IsBufferGrayRootsTracer(JSTracer* trc)
 {
     return trc->isCallbackTracer() &&
            trc->asCallbackTracer()->getTracerKind() == JS::CallbackTracer::TracerKind::GrayBuffering;
 }
 #endif
 
-// A canary value used to check the gray buffer contents are valid.
-static Cell* const GrayBufferCanary = reinterpret_cast<Cell*>(0x47726179); // "Gray"
-
 void
 js::gc::GCRuntime::bufferGrayRoots()
 {
     // Precondition: the state has been reset to "unused" after the last GC
     //               and the zone's buffers have been cleared.
     MOZ_ASSERT(grayBufferState == GrayBufferState::Unused);
     for (GCZonesIter zone(rt); !zone.done(); zone.next())
         MOZ_ASSERT(zone->gcGrayRoots().empty());
 
     BufferGrayRootsTracer grayBufferer(rt);
     if (JSTraceDataOp op = grayRootTracer.op)
         (*op)(&grayBufferer, grayRootTracer.data);
 
-    // Push a canary value onto the end of the list.
-    for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
-        if (!zone->gcGrayRoots().empty() && !zone->gcGrayRoots().append(GrayBufferCanary))
-            grayBufferer.setFailed();
-    }
-
     // Propagate the failure flag from the marker to the runtime.
     if (grayBufferer.failed()) {
       grayBufferState = GrayBufferState::Failed;
       resetBufferedGrayRoots();
     } else {
       grayBufferState = GrayBufferState::Okay;
     }
 }
@@ -534,23 +525,31 @@ GCRuntime::markBufferedGrayRoots(JS::Zon
 {
     MOZ_ASSERT(grayBufferState == GrayBufferState::Okay);
     MOZ_ASSERT(zone->isGCMarkingGray() || zone->isGCCompacting());
 
     auto& roots = zone->gcGrayRoots();
     if (roots.empty())
         return;
 
-    // Check for canary value but don't remove it.
-    MOZ_RELEASE_ASSERT(roots.length() > 1);
-    MOZ_RELEASE_ASSERT(roots.back() == GrayBufferCanary);
+    for (size_t i = 0; i < roots.length(); i++) {
+        Cell* cell = roots[i];
 
-    for (size_t i = 0; i < roots.length() - 1; i++) {
-        Cell* cell = roots[i];
+        // Bug 1203273: Check for bad pointers on OSX and output diagnostics.
+#if defined(XP_DARWIN) && defined(MOZ_DIAGNOSTIC_ASSERT_ENABLED)
+        auto addr = uintptr_t(cell);
+        if (addr < ChunkSize || addr % CellAlignBytes != 0) {
+            MOZ_CRASH_UNSAFE_PRINTF(
+                "Bad GC thing pointer in gray root buffer: %p at index %zu of %zu, address %p",
+                cell, i, roots.length(), &roots[i]);
+        }
+#else
         MOZ_ASSERT(IsCellPointerValid(cell));
+#endif
+
         TraceManuallyBarrieredGenericPointerEdge(&marker, &cell, "buffered gray root");
     }
 }
 
 void
 GCRuntime::resetBufferedGrayRoots() const
 {
     MOZ_ASSERT(grayBufferState != GrayBufferState::Okay,