Bug 1308039 - Convert AutoAssertOnGC to release assertion (r=jonco)
authorBill McCloskey <billm@mozilla.com>
Wed, 05 Oct 2016 16:04:31 -0700
changeset 423583 4c0e2f689239b80b26e32e87a12eb8866adc66b2
parent 423361 38892fd53242c6e452ed964e49410aaee1e935f2
child 423584 37e793632d13208ec59ab418866f24a73f70801f
push id31945
push users.kaspari@gmail.com
push dateTue, 11 Oct 2016 12:27:19 +0000
reviewersjonco
bugs1308039
milestone52.0a1
Bug 1308039 - Convert AutoAssertOnGC to release assertion (r=jonco)
js/public/GCAPI.h
js/src/builtin/SIMD.cpp
js/src/builtin/TypedObject.h
js/src/gc/Allocator.cpp
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
js/src/vm/Interpreter.cpp
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -479,39 +479,40 @@ IsGenerationalGCEnabled(JSRuntime* rt);
  * Returns the GC's "number". This does not correspond directly to the number
  * of GCs that have been run, but is guaranteed to be monotonically increasing
  * with GC activity.
  */
 extern JS_PUBLIC_API(size_t)
 GetGCNumber();
 
 /**
- * Assert if a GC occurs while this class is live. This class does not disable
- * the static rooting hazard analysis.
+ * Pass a subclass of this "abstract" class to callees to require that they
+ * never GC. Subclasses can use assertions or the hazard analysis to ensure no
+ * GC happens.
  */
-class JS_PUBLIC_API(AutoAssertOnGC)
+class JS_PUBLIC_API(AutoRequireNoGC)
 {
-#ifdef DEBUG
+  protected:
+    AutoRequireNoGC() {}
+    ~AutoRequireNoGC() {}
+};
+
+/**
+ * Release assert if a GC occurs while this class is live. This class does
+ * not disable the static rooting hazard analysis.
+ */
+class JS_PUBLIC_API(AutoAssertOnGC) : public AutoRequireNoGC
+{
     js::gc::GCRuntime* gc;
     size_t gcNumber;
 
   public:
     AutoAssertOnGC();
     explicit AutoAssertOnGC(JSContext* cx);
     ~AutoAssertOnGC();
-
-    static void VerifyIsSafeToGC(JSRuntime* rt);
-#else
-  public:
-    AutoAssertOnGC() {}
-    explicit AutoAssertOnGC(JSContext* cx) {}
-    ~AutoAssertOnGC() {}
-
-    static void VerifyIsSafeToGC(JSRuntime* rt) {}
-#endif
 };
 
 /**
  * Assert if an allocation of a GC thing occurs while this class is live. This
  * class does not disable the static rooting hazard analysis.
  */
 class JS_PUBLIC_API(AutoAssertNoAlloc)
 {
@@ -570,23 +571,34 @@ class JS_PUBLIC_API(AutoAssertGCCallback
  * Place AutoCheckCannotGC in scopes that you believe can never GC. These
  * annotations will be verified both dynamically via AutoAssertOnGC, and
  * statically with the rooting hazard analysis (implemented by making the
  * analysis consider AutoCheckCannotGC to be a GC pointer, and therefore
  * complain if it is live across a GC call.) It is useful when dealing with
  * internal pointers to GC things where the GC thing itself may not be present
  * for the static analysis: e.g. acquiring inline chars from a JSString* on the
  * heap.
+ *
+ * We only do the assertion checking in DEBUG builds.
  */
+#ifdef DEBUG
 class JS_PUBLIC_API(AutoCheckCannotGC) : public AutoAssertOnGC
 {
   public:
     AutoCheckCannotGC() : AutoAssertOnGC() {}
     explicit AutoCheckCannotGC(JSContext* cx) : AutoAssertOnGC(cx) {}
 } JS_HAZ_GC_INVALIDATED;
+#else
+class JS_PUBLIC_API(AutoCheckCannotGC) : public AutoRequireNoGC
+{
+  public:
+    AutoCheckCannotGC() {}
+    explicit AutoCheckCannotGC(JSContext* cx) {}
+} JS_HAZ_GC_INVALIDATED;
+#endif
 
 /**
  * Unsets the gray bit for anything reachable from |thing|. |kind| should not be
  * JS::TraceKind::Shape. |thing| should be non-null. The return value indicates
  * if anything was unmarked.
  */
 extern JS_FRIEND_API(bool)
 UnmarkGrayGCThingRecursively(GCCellPtr thing);
--- a/js/src/builtin/SIMD.cpp
+++ b/js/src/builtin/SIMD.cpp
@@ -185,17 +185,17 @@ template bool js::ToSimdConstant<Int16x8
 template bool js::ToSimdConstant<Int32x4>(JSContext* cx, HandleValue v, jit::SimdConstant* out);
 template bool js::ToSimdConstant<Float32x4>(JSContext* cx, HandleValue v, jit::SimdConstant* out);
 template bool js::ToSimdConstant<Bool8x16>(JSContext* cx, HandleValue v, jit::SimdConstant* out);
 template bool js::ToSimdConstant<Bool16x8>(JSContext* cx, HandleValue v, jit::SimdConstant* out);
 template bool js::ToSimdConstant<Bool32x4>(JSContext* cx, HandleValue v, jit::SimdConstant* out);
 
 template<typename Elem>
 static Elem
-TypedObjectMemory(HandleValue v, const JS::AutoAssertOnGC& nogc)
+TypedObjectMemory(HandleValue v, const JS::AutoRequireNoGC& nogc)
 {
     TypedObject& obj = v.toObject().as<TypedObject>();
     return reinterpret_cast<Elem>(obj.typedMem(nogc));
 }
 
 static const ClassOps SimdTypeDescrClassOps = {
     nullptr, /* addProperty */
     nullptr, /* delProperty */
--- a/js/src/builtin/TypedObject.h
+++ b/js/src/builtin/TypedObject.h
@@ -544,24 +544,24 @@ class TypedObject : public ShapedObject
     }
 
     TypeDescr& typeDescr() const {
         return group()->typeDescr();
     }
 
     int32_t offset() const;
     int32_t length() const;
-    uint8_t* typedMem(const JS::AutoAssertOnGC&) const { return typedMem(); }
+    uint8_t* typedMem(const JS::AutoRequireNoGC&) const { return typedMem(); }
     bool isAttached() const;
 
     int32_t size() const {
         return typeDescr().size();
     }
 
-    uint8_t* typedMem(size_t offset, const JS::AutoAssertOnGC& nogc) const {
+    uint8_t* typedMem(size_t offset, const JS::AutoRequireNoGC& nogc) const {
         // It seems a bit surprising that one might request an offset
         // == size(), but it can happen when taking the "address of" a
         // 0-sized value. (In other words, we maintain the invariant
         // that `offset + size <= size()` -- this is always checked in
         // the caller's side.)
         MOZ_ASSERT(offset <= (size_t) size());
         return typedMem(nogc) + offset;
     }
@@ -699,17 +699,17 @@ class InlineTypedObject : public TypedOb
 
     static gc::AllocKind allocKindForTypeDescriptor(TypeDescr* descr) {
         size_t nbytes = descr->size();
         MOZ_ASSERT(nbytes <= MaximumSize);
 
         return gc::GetGCObjectKindForBytes(nbytes + sizeof(TypedObject));
     }
 
-    uint8_t* inlineTypedMem(const JS::AutoAssertOnGC&) const {
+    uint8_t* inlineTypedMem(const JS::AutoRequireNoGC&) const {
         return inlineTypedMem();
     }
 
     uint8_t* inlineTypedMemForGC() const {
         return inlineTypedMem();
     }
 
     static void obj_trace(JSTracer* trace, JSObject* object);
--- a/js/src/gc/Allocator.cpp
+++ b/js/src/gc/Allocator.cpp
@@ -198,17 +198,17 @@ GCRuntime::checkAllocatorState(JSContext
                   kind == AllocKind::JITCODE ||
                   kind == AllocKind::SCOPE);
     MOZ_ASSERT(!rt->isHeapBusy());
     MOZ_ASSERT(isAllocAllowed());
 #endif
 
     // Crash if we perform a GC action when it is not safe.
     if (allowGC && !rt->mainThread.suppressGC)
-        JS::AutoAssertOnGC::VerifyIsSafeToGC(rt);
+        rt->gc.verifyIsSafeToGC();
 
     // For testing out of memory conditions
     if (js::oom::ShouldFailWithOOM()) {
         // If we are doing a fallible allocation, percolate up the OOM
         // instead of reporting it.
         if (allowGC)
             ReportOutOfMemory(cx);
         return false;
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -716,30 +716,35 @@ class GCRuntime
 
     bool isNurseryAllocAllowed() { return noNurseryAllocationCheck == 0; }
     void disallowNurseryAlloc() { ++noNurseryAllocationCheck; }
     void allowNurseryAlloc() {
         MOZ_ASSERT(!isNurseryAllocAllowed());
         --noNurseryAllocationCheck;
     }
 
+    bool isStrictProxyCheckingEnabled() { return disableStrictProxyCheckingCount == 0; }
+    void disableStrictProxyChecking() { ++disableStrictProxyCheckingCount; }
+    void enableStrictProxyChecking() {
+        MOZ_ASSERT(disableStrictProxyCheckingCount > 0);
+        --disableStrictProxyCheckingCount;
+    }
+#endif // DEBUG
+
     bool isInsideUnsafeRegion() { return inUnsafeRegion != 0; }
     void enterUnsafeRegion() { ++inUnsafeRegion; }
     void leaveUnsafeRegion() {
         MOZ_ASSERT(inUnsafeRegion > 0);
         --inUnsafeRegion;
     }
 
-    bool isStrictProxyCheckingEnabled() { return disableStrictProxyCheckingCount == 0; }
-    void disableStrictProxyChecking() { ++disableStrictProxyCheckingCount; }
-    void enableStrictProxyChecking() {
-        MOZ_ASSERT(disableStrictProxyCheckingCount > 0);
-        --disableStrictProxyCheckingCount;
+    void verifyIsSafeToGC() {
+        MOZ_DIAGNOSTIC_ASSERT(!isInsideUnsafeRegion(),
+                              "[AutoAssertOnGC] possible GC in GC-unsafe region");
     }
-#endif // DEBUG
 
     void setAlwaysPreserveCode() { alwaysPreserveCode = true; }
 
     bool isIncrementalGCAllowed() const { return incrementalAllowed; }
     void disallowIncrementalGC() { incrementalAllowed = false; }
 
     bool isIncrementalGCEnabled() const { return mode == JSGC_MODE_INCREMENTAL && incrementalAllowed; }
     bool isIncrementalGCInProgress() const { return state() != State::NotActive; }
@@ -1339,25 +1344,25 @@ class GCRuntime
      * collector.
      */
     CallbackVector<JSTraceDataOp> blackRootTracers;
     Callback<JSTraceDataOp> grayRootTracer;
 
     /* Always preserve JIT code during GCs, for testing. */
     bool alwaysPreserveCode;
 
-#ifdef DEBUG
     /*
      * Some regions of code are hard for the static rooting hazard analysis to
      * understand. In those cases, we trade the static analysis for a dynamic
      * analysis. When this is non-zero, we should assert if we trigger, or
      * might trigger, a GC.
      */
     int inUnsafeRegion;
 
+#ifdef DEBUG
     size_t noGCOrAllocationCheck;
     size_t noNurseryAllocationCheck;
 
     bool arenasEmptyAtShutdown;
 #endif
 
     /* Synchronize GC heap access between main thread and GCHelperState. */
     friend class js::AutoLockGC;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -859,18 +859,18 @@ GCRuntime::GCRuntime(JSRuntime* rt) :
     nextScheduled(0),
     deterministicOnly(false),
     incrementalLimit(0),
 #endif
     fullCompartmentChecks(false),
     mallocBytesUntilGC(0),
     mallocGCTriggered(false),
     alwaysPreserveCode(false),
+    inUnsafeRegion(0),
 #ifdef DEBUG
-    inUnsafeRegion(0),
     noGCOrAllocationCheck(0),
     noNurseryAllocationCheck(0),
     arenasEmptyAtShutdown(true),
 #endif
     allocTask(rt, emptyChunks_),
     decommitTask(rt),
     helperState(rt)
 {
@@ -6128,17 +6128,17 @@ GCRuntime::gcCycle(bool nonincrementalBy
     if (!isIncrementalGCInProgress())
         incMajorGcNumber();
 
     // It's ok if threads other than the main thread have suppressGC set, as
     // they are operating on zones which will not be collected from here.
     MOZ_ASSERT(!rt->mainThread.suppressGC);
 
     // Assert if this is a GC unsafe region.
-    JS::AutoAssertOnGC::VerifyIsSafeToGC(rt);
+    verifyIsSafeToGC();
 
     {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
 
         // Background finalization and decommit are finished by defininition
         // before we can start a new GC session.
         if (!isIncrementalGCInProgress()) {
             assertBackgroundSweepingFinished();
@@ -6969,17 +6969,16 @@ JS::GetGCNumber()
 {
     JSRuntime* rt = js::TlsPerThreadData.get()->runtimeFromMainThread();
     if (!rt)
         return 0;
     return rt->gc.gcNumber();
 }
 #endif
 
-#ifdef DEBUG
 JS::AutoAssertOnGC::AutoAssertOnGC()
   : gc(nullptr), gcNumber(0)
 {
     js::PerThreadData* data = js::TlsPerThreadData.get();
     if (data) {
         /*
          * GC's from off-thread will always assert, so off-thread is implicitly
          * AutoAssertOnGC. We still need to allow AutoAssertOnGC to be used in
@@ -7009,23 +7008,17 @@ JS::AutoAssertOnGC::~AutoAssertOnGC()
         /*
          * The following backstop assertion should never fire: if we bumped the
          * gcNumber, we should have asserted because inUnsafeRegion was true.
          */
         MOZ_ASSERT(gcNumber == gc->gcNumber(), "GC ran inside an AutoAssertOnGC scope.");
     }
 }
 
-/* static */ void
-JS::AutoAssertOnGC::VerifyIsSafeToGC(JSRuntime* rt)
-{
-    if (rt->gc.isInsideUnsafeRegion())
-        MOZ_CRASH("[AutoAssertOnGC] possible GC in GC-unsafe region");
-}
-
+#ifdef DEBUG
 JS::AutoAssertNoAlloc::AutoAssertNoAlloc(JSContext* cx)
   : gc(nullptr)
 {
     disallowAlloc(cx);
 }
 
 void JS::AutoAssertNoAlloc::disallowAlloc(JSRuntime* rt)
 {
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -1493,17 +1493,17 @@ ArrayBufferViewObject::notifyBufferDetac
             return;
         as<TypedArrayObject>().notifyBufferDetached(cx, newData);
     } else {
         as<OutlineTypedObject>().notifyBufferDetached(newData);
     }
 }
 
 uint8_t*
-ArrayBufferViewObject::dataPointerUnshared(const JS::AutoAssertOnGC& nogc)
+ArrayBufferViewObject::dataPointerUnshared(const JS::AutoRequireNoGC& nogc)
 {
     if (is<DataViewObject>())
         return static_cast<uint8_t*>(as<DataViewObject>().dataPointer());
     if (is<TypedArrayObject>()) {
         MOZ_ASSERT(!as<TypedArrayObject>().isSharedMemory());
         return static_cast<uint8_t*>(as<TypedArrayObject>().viewDataUnshared());
     }
     return as<TypedObject>().typedMem(nogc);
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -435,17 +435,17 @@ class ArrayBufferViewObject : public JSO
     void notifyBufferDetached(JSContext* cx, void* newData);
 
 #ifdef DEBUG
     bool isSharedMemory();
 #endif
 
     // By construction we only need unshared variants here.  See
     // comments in ArrayBufferObject.cpp.
-    uint8_t* dataPointerUnshared(const JS::AutoAssertOnGC&);
+    uint8_t* dataPointerUnshared(const JS::AutoRequireNoGC&);
     void setDataPointerUnshared(uint8_t* data);
 
     static void trace(JSTracer* trc, JSObject* obj);
 };
 
 bool
 ToClampedIndex(JSContext* cx, HandleValue v, uint32_t length, uint32_t* out);
 
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -358,17 +358,17 @@ ExecuteState::pushInterpreterFrame(JSCon
 # pragma optimize("g", off)
 #endif
 bool
 js::RunScript(JSContext* cx, RunState& state)
 {
     JS_CHECK_RECURSION(cx, return false);
 
     // Since any script can conceivably GC, make sure it's safe to do so.
-    JS::AutoAssertOnGC::VerifyIsSafeToGC(cx->runtime());
+    cx->runtime()->gc.verifyIsSafeToGC();
 
     if (!Debugger::checkNoExecute(cx, state.script()))
         return false;
 
 #if defined(MOZ_HAVE_RDTSC)
     js::AutoStopwatch stopwatch(cx);
 #endif // defined(MOZ_HAVE_RDTSC)