Bug 1221378: Use a dedicated flag on JS::Zone to disable allocation metadata collection, instead of abusing AutoEnterAnalysis. r=fitzgen
authorJim Blandy <jimb@mozilla.com>
Mon, 22 Feb 2016 12:21:56 -0800
changeset 333210 0d691fc145b8b4097f5adc7a16ef47b2e0a1a307
parent 333209 0bede6f91fa308e128cf01bd0c55fb8832621856
child 333211 1a994a5e9932f4d11ac2fbb98ad995bac4d74e4a
push id11295
push userbmo:rail@mozilla.com
push dateMon, 22 Feb 2016 23:51:29 +0000
reviewersfitzgen
bugs1221378
milestone47.0a1
Bug 1221378: Use a dedicated flag on JS::Zone to disable allocation metadata collection, instead of abusing AutoEnterAnalysis. r=fitzgen
js/src/devtools/automation/cgc-jittest-timeouts.txt
js/src/gc/Zone.cpp
js/src/gc/Zone.h
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsfriendapi.cpp
js/src/jsobjinlines.h
js/src/vm/TypeInference-inl.h
--- a/js/src/devtools/automation/cgc-jittest-timeouts.txt
+++ b/js/src/devtools/automation/cgc-jittest-timeouts.txt
@@ -1,8 +1,9 @@
+SIMD/nursery-overflow.js
 asm.js/testParallelCompile.js
 auto-regress/bug653395.js
 auto-regress/bug654392.js
 auto-regress/bug675251.js
 baseline/bug847446.js
 baseline/bug852175.js
 basic/bug632964-regexp.js
 basic/bug656261.js
@@ -14,14 +15,14 @@ basic/testManyVars.js
 basic/testTypedArrayInit.js
 gc/bug-1014972.js
 gc/bug-1246593.js
 gc/bug-906236.js
 gc/bug-906241.js
 ion/bug787921.js
 parallel/alloc-many-objs.js
 parallel/alloc-too-many-objs.js
+saved-stacks/bug-1006876-too-much-recursion.js
 self-test/assertDeepEq.js
 v8-v5/check-earley-boyer.js
 v8-v5/check-raytrace.js
 v8-v5/check-regexp.js
 v8-v5/check-splay.js
-SIMD/nursery-overflow.js
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -20,16 +20,17 @@
 using namespace js;
 using namespace js::gc;
 
 Zone * const Zone::NotOnList = reinterpret_cast<Zone*>(1);
 
 JS::Zone::Zone(JSRuntime* rt)
   : JS::shadow::Zone(rt, &rt->gc.marker),
     debuggers(nullptr),
+    suppressObjectMetadataCallback(false),
     arenas(rt),
     types(this),
     compartments(),
     gcGrayRoots(),
     gcMallocBytes(0),
     gcMallocGCTriggered(false),
     usage(&rt->gc.usage),
     gcDelayBytes(0),
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -274,16 +274,27 @@ struct Zone : public JS::shadow::Zone,
     // Side map for storing a unique ids for cells, independent of address.
     js::gc::UniqueIdMap uniqueIds_;
 
   public:
     bool hasDebuggers() const { return debuggers && debuggers->length(); }
     DebuggerVector* getDebuggers() const { return debuggers; }
     DebuggerVector* getOrCreateDebuggers(JSContext* cx);
 
+    /*
+     * When true, skip calling the metadata callback. We use this:
+     * - to avoid invoking the callback recursively;
+     * - to avoid observing lazy prototype setup (which confuses callbacks that
+     *   want to use the types being set up!);
+     * - to avoid attaching allocation stacks to allocation stack nodes, which
+     *   is silly
+     * And so on.
+     */
+    bool suppressObjectMetadataCallback;
+
     js::gc::ArenaLists arenas;
 
     js::TypeZone types;
 
     /* Live weakmaps in this zone. */
     mozilla::LinkedList<js::WeakMapBase> gcWeakMapList;
 
     // The set of compartments in this zone.
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -1188,20 +1188,33 @@ AutoSetNewObjectMetadata::AutoSetNewObje
 AutoSetNewObjectMetadata::~AutoSetNewObjectMetadata()
 {
     // If we don't have a cx, we didn't change the metadata state, so no need to
     // reset it here.
     if (!cx_)
         return;
 
     if (!cx_->isExceptionPending() && cx_->compartment()->hasObjectPendingMetadata()) {
-        RootedObject obj(cx_, cx_->compartment()->objectMetadataState.as<PendingMetadata>());
+        // This destructor often runs upon exit from a function that is
+        // returning an unrooted pointer to a Cell. The allocation metadata
+        // callback often allocates; if it causes a GC, then the Cell pointer
+        // being returned won't be traced or relocated.
+        //
+        // The only extant callbacks are those internal to SpiderMonkey that
+        // capture the JS stack. In fact, we're considering removing general
+        // callbacks altogther in bug 1236748. Since it's not running arbitrary
+        // code, it's adequate to simply suppress GC while we run the callback.
+        AutoSuppressGC autoSuppressGC(cx_);
+
+        JSObject* obj = cx_->compartment()->objectMetadataState.as<PendingMetadata>();
+
         // Make sure to restore the previous state before setting the object's
         // metadata. SetNewObjectMetadata asserts that the state is not
         // PendingMetadata in order to ensure that metadata callbacks are called
         // in order.
         cx_->compartment()->objectMetadataState = prevState_;
+
         obj = SetNewObjectMetadata(cx_, obj);
     } else {
         cx_->compartment()->objectMetadataState = prevState_;
     }
 }
 
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -935,11 +935,32 @@ class MOZ_RAII AutoWrapperRooter : priva
 
     friend void JS::AutoGCRooter::trace(JSTracer* trc);
 
   private:
     WrapperValue value;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+class MOZ_RAII AutoSuppressObjectMetadataCallback {
+    JS::Zone* zone;
+    bool saved;
+
+  public:
+    explicit AutoSuppressObjectMetadataCallback(ExclusiveContext* cx)
+      : AutoSuppressObjectMetadataCallback(cx->compartment()->zone())
+    { }
+
+    explicit AutoSuppressObjectMetadataCallback(JS::Zone* zone)
+      : zone(zone),
+        saved(zone->suppressObjectMetadataCallback)
+    {
+        zone->suppressObjectMetadataCallback = true;
+    }
+
+    ~AutoSuppressObjectMetadataCallback() {
+        zone->suppressObjectMetadataCallback = saved;
+    }
+};
+
 } /* namespace js */
 
 #endif /* jscompartment_h */
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -133,18 +133,17 @@ JS_NewObjectWithUniqueType(JSContext* cx
     if (!JS_SplicePrototype(cx, obj, proto))
         return nullptr;
     return obj;
 }
 
 JS_FRIEND_API(JSObject*)
 JS_NewObjectWithoutMetadata(JSContext* cx, const JSClass* clasp, JS::Handle<JSObject*> proto)
 {
-    // Use an AutoEnterAnalysis to suppress invocation of the metadata callback.
-    AutoEnterAnalysis enter(cx);
+    AutoSuppressObjectMetadataCallback suppressMetadata(cx);
     return JS_NewObjectWithGivenProto(cx, clasp, proto);
 }
 
 JS_FRIEND_API(JSPrincipals*)
 JS_GetCompartmentPrincipals(JSCompartment* compartment)
 {
     return compartment->principals();
 }
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -289,21 +289,20 @@ static MOZ_ALWAYS_INLINE MOZ_WARN_UNUSED
 SetNewObjectMetadata(ExclusiveContext* cxArg, JSObject* obj)
 {
     MOZ_ASSERT(!cxArg->compartment()->hasObjectPendingMetadata());
 
     // The metadata callback is invoked for each object created on the main
     // thread, except when analysis/compilation is active, to avoid recursion.
     if (JSContext* cx = cxArg->maybeJSContext()) {
         if (MOZ_UNLIKELY((size_t)cx->compartment()->hasObjectMetadataCallback()) &&
-            !cx->zone()->types.activeAnalysis)
+            !cx->zone()->suppressObjectMetadataCallback)
         {
-            // Use AutoEnterAnalysis to prohibit both any GC activity under the
-            // callback, and any reentering of JS via Invoke() etc.
-            AutoEnterAnalysis enter(cx);
+            // Don't collect metadata on objects that represent metadata.
+            AutoSuppressObjectMetadataCallback suppressMetadata(cx);
 
             RootedObject rooted(cx, obj);
             cx->compartment()->setNewObjectMetadata(cx, rooted);
             return rooted;
         }
     }
 
     return obj;
@@ -360,16 +359,21 @@ JSObject::create(js::ExclusiveContext* c
 
     // JSFunction's fixed slots expect POD-style initialization.
     if (group->clasp()->isJSFunction()) {
         MOZ_ASSERT(kind == js::gc::AllocKind::FUNCTION ||
                    kind == js::gc::AllocKind::FUNCTION_EXTENDED);
         size_t size =
             kind == js::gc::AllocKind::FUNCTION ? sizeof(JSFunction) : sizeof(js::FunctionExtended);
         memset(obj->as<JSFunction>().fixedSlots(), 0, size - sizeof(js::NativeObject));
+        if (kind == js::gc::AllocKind::FUNCTION_EXTENDED) {
+            // SetNewObjectMetadata may gc, which will be unhappy if flags &
+            // EXTENDED doesn't match the arena's AllocKind.
+            obj->as<JSFunction>().setFlags(JSFunction::EXTENDED);
+        }
     }
 
     if (group->clasp()->shouldDelayMetadataCallback())
         cx->compartment()->setObjectPendingMetadata(cx, obj);
     else
         obj = SetNewObjectMetadata(cx, obj);
 
     js::gc::TraceCreateObject(obj);
--- a/js/src/vm/TypeInference-inl.h
+++ b/js/src/vm/TypeInference-inl.h
@@ -278,27 +278,30 @@ struct AutoEnterAnalysis
     gc::AutoSuppressGC suppressGC;
 
     // Allow clearing inference info on OOM during incremental sweeping.
     AutoClearTypeInferenceStateOnOOM oom;
 
     // Pending recompilations to perform before execution of JIT code can resume.
     RecompileInfoVector pendingRecompiles;
 
+    // Prevent us from calling the objectMetadataCallback.
+    js::AutoSuppressObjectMetadataCallback suppressMetadata;
+
     FreeOp* freeOp;
     Zone* zone;
 
     explicit AutoEnterAnalysis(ExclusiveContext* cx)
-      : suppressGC(cx), oom(cx->zone())
+      : suppressGC(cx), oom(cx->zone()), suppressMetadata(cx)
     {
         init(cx->defaultFreeOp(), cx->zone());
     }
 
     AutoEnterAnalysis(FreeOp* fop, Zone* zone)
-      : suppressGC(zone->runtimeFromMainThread()), oom(zone)
+      : suppressGC(zone->runtimeFromMainThread()), oom(zone), suppressMetadata(zone)
     {
         init(fop, zone);
     }
 
     ~AutoEnterAnalysis()
     {
         if (this != zone->types.activeAnalysis)
             return;