Bug 1251529: Replace allocation metadata callback with a builder class. r=fitzgen
authorJim Blandy <jimb@mozilla.com>
Wed, 12 Aug 2015 15:17:16 -0700
changeset 347722 fc76f66bf11f5800a7692ccf71add7ef8b01640d
parent 347721 ad5ff46b72e7168e38d1c6c4cbe3b370ee0537b5
child 347723 12f6c52e4b4df6527c98593c31840a8c68e2bf5f
push id14653
push userolivier@olivieryiptong.com
push dateTue, 05 Apr 2016 19:21:01 +0000
reviewersfitzgen
bugs1251529
milestone48.0a1
Bug 1251529: Replace allocation metadata callback with a builder class. r=fitzgen
js/src/builtin/TestingFunctions.cpp
js/src/builtin/TypedObject.cpp
js/src/jit-test/tests/gc/bug-1252154.js
js/src/jsapi-tests/testSavedStacks.cpp
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/vm/Debugger.cpp
js/src/vm/HelperThreads.cpp
js/src/vm/SavedStacks.cpp
js/src/vm/SavedStacks.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1139,17 +1139,17 @@ CallFunctionWithAsyncStack(JSContext* cx
                                          JS::AutoSetAsyncStackForNewCalls::AsyncCallKind::EXPLICIT);
     return Call(cx, UndefinedHandleValue, function,
                 JS::HandleValueArray::empty(), args.rval());
 }
 
 static bool
 EnableTrackAllocations(JSContext* cx, unsigned argc, Value* vp)
 {
-    SetAllocationMetadataBuilder(cx, SavedStacksMetadataBuilder);
+    SetAllocationMetadataBuilder(cx, &SavedStacks::metadataBuilder);
     return true;
 }
 
 static bool
 DisableTrackAllocations(JSContext* cx, unsigned argc, Value* vp)
 {
     SetAllocationMetadataBuilder(cx, nullptr);
     return true;
@@ -1684,69 +1684,78 @@ DisplayName(JSContext* cx, unsigned argc
     }
 
     JSFunction* fun = &args[0].toObject().as<JSFunction>();
     JSString* str = fun->displayAtom();
     args.rval().setString(str ? str : cx->runtime()->emptyString);
     return true;
 }
 
-static JSObject*
-ShellAllocationMetadataBuilder(JSContext* cx, HandleObject)
+class ShellAllocationMetadataBuilder : public AllocationMetadataBuilder {
+  public:
+    virtual JSObject* build(JSContext *cx, HandleObject) const override;
+
+    static const ShellAllocationMetadataBuilder metadataBuilder;
+};
+
+JSObject*
+ShellAllocationMetadataBuilder::build(JSContext* cx, HandleObject) const
 {
     AutoEnterOOMUnsafeRegion oomUnsafe;
 
     RootedObject obj(cx, NewBuiltinClassInstance<PlainObject>(cx));
     if (!obj)
-        oomUnsafe.crash("ShellAllocationMetadataBuilder");
+        oomUnsafe.crash("ShellAllocationMetadataBuilder::build");
 
     RootedObject stack(cx, NewDenseEmptyArray(cx));
     if (!stack)
-        oomUnsafe.crash("ShellAllocationMetadataBuilder");
+        oomUnsafe.crash("ShellAllocationMetadataBuilder::build");
 
     static int createdIndex = 0;
     createdIndex++;
 
     if (!JS_DefineProperty(cx, obj, "index", createdIndex, 0,
                            JS_STUBGETTER, JS_STUBSETTER))
     {
-        oomUnsafe.crash("ShellAllocationMetadataBuilder");
+        oomUnsafe.crash("ShellAllocationMetadataBuilder::build");
     }
 
     if (!JS_DefineProperty(cx, obj, "stack", stack, 0,
                            JS_STUBGETTER, JS_STUBSETTER))
     {
-        oomUnsafe.crash("ShellAllocationMetadataBuilder");
+        oomUnsafe.crash("ShellAllocationMetadataBuilder::build");
     }
 
     int stackIndex = 0;
     RootedId id(cx);
     RootedValue callee(cx);
     for (NonBuiltinScriptFrameIter iter(cx); !iter.done(); ++iter) {
         if (iter.isFunctionFrame() && iter.compartment() == cx->compartment()) {
             id = INT_TO_JSID(stackIndex);
             RootedObject callee(cx, iter.callee(cx));
             if (!JS_DefinePropertyById(cx, stack, id, callee, 0,
                                        JS_STUBGETTER, JS_STUBSETTER))
             {
-                oomUnsafe.crash("ShellAllocationMetadataBuilder");
+                oomUnsafe.crash("ShellAllocationMetadataBuilder::build");
             }
             stackIndex++;
         }
     }
 
     return obj;
 }
 
+const ShellAllocationMetadataBuilder ShellAllocationMetadataBuilder::metadataBuilder;
+
 static bool
 EnableShellAllocationMetadataBuilder(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    SetAllocationMetadataBuilder(cx, ShellAllocationMetadataBuilder);
+    SetAllocationMetadataBuilder(cx, &ShellAllocationMetadataBuilder::metadataBuilder);
 
     args.rval().setUndefined();
     return true;
 }
 
 static bool
 GetAllocationMetadata(JSContext* cx, unsigned argc, Value* vp)
 {
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -2279,19 +2279,19 @@ const ObjectOps TypedObject::objectOps_ 
         JS_NULL_CLASS_SPEC,                              \
         JS_NULL_CLASS_EXT,                               \
         &TypedObject::objectOps_                         \
     }
 
 DEFINE_TYPEDOBJ_CLASS(OutlineTransparentTypedObject, OutlineTypedObject::obj_trace, 0);
 DEFINE_TYPEDOBJ_CLASS(OutlineOpaqueTypedObject,      OutlineTypedObject::obj_trace, 0);
 DEFINE_TYPEDOBJ_CLASS(InlineTransparentTypedObject,  InlineTypedObject::obj_trace,
-                      JSCLASS_DELAY_METADATA_CALLBACK);
+                      JSCLASS_DELAY_METADATA_BUILDER);
 DEFINE_TYPEDOBJ_CLASS(InlineOpaqueTypedObject,       InlineTypedObject::obj_trace,
-                      JSCLASS_DELAY_METADATA_CALLBACK);
+                      JSCLASS_DELAY_METADATA_BUILDER);
 
 static int32_t
 LengthForType(TypeDescr& descr)
 {
     switch (descr.kind()) {
       case type::Scalar:
       case type::Reference:
       case type::Struct:
--- a/js/src/jit-test/tests/gc/bug-1252154.js
+++ b/js/src/jit-test/tests/gc/bug-1252154.js
@@ -1,11 +1,11 @@
 // Bug 1252154: Inline typed array objects need delayed metadata collection.
 // Shouldn't crash.
 
 if (!this.hasOwnProperty("TypedObject"))
   quit();
 
 gczeal(7,1);
-enableShellObjectMetadataCallback();
+enableShellAllocationMetadataBuilder();
 var T = TypedObject;
 var AT = new T.ArrayType(T.Any,10);
 var v = new AT();
--- a/js/src/jsapi-tests/testSavedStacks.cpp
+++ b/js/src/jsapi-tests/testSavedStacks.cpp
@@ -11,17 +11,17 @@
 #include "builtin/TestingFunctions.h"
 #include "jsapi-tests/tests.h"
 #include "vm/ArrayObject.h"
 #include "vm/SavedStacks.h"
 
 BEGIN_TEST(testSavedStacks_withNoStack)
 {
     JSCompartment* compartment = js::GetContextCompartment(cx);
-    compartment->setAllocationMetadataBuilder(js::SavedStacksMetadataBuilder);
+    compartment->setAllocationMetadataBuilder(&js::SavedStacks::metadataBuilder);
     JS::RootedObject obj(cx, js::NewDenseEmptyArray(cx));
     compartment->setAllocationMetadataBuilder(nullptr);
     return true;
 }
 END_TEST(testSavedStacks_withNoStack)
 
 BEGIN_TEST(testSavedStacks_ApiDefaultValues)
 {
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -868,38 +868,38 @@ JSCompartment::clearTables()
         baseShapes.clear();
     if (initialShapes.initialized())
         initialShapes.clear();
     if (savedStacks_.initialized())
         savedStacks_.clear();
 }
 
 void
-JSCompartment::setAllocationMetadataBuilder(js::AllocationMetadataBuilder callback)
+JSCompartment::setAllocationMetadataBuilder(const js::AllocationMetadataBuilder *builder)
 {
     // Clear any jitcode in the runtime, which behaves differently depending on
     // whether there is a creation callback.
     ReleaseAllJITCode(runtime_->defaultFreeOp());
 
-    allocationMetadataBuilder = callback;
+    allocationMetadataBuilder = builder;
 }
 
 void
 JSCompartment::clearObjectMetadata()
 {
     js_delete(objectMetadataTable);
     objectMetadataTable = nullptr;
 }
 
 void
 JSCompartment::setNewObjectMetadata(JSContext* cx, HandleObject obj)
 {
     assertSameCompartment(cx, this, obj);
 
-    if (JSObject* metadata = allocationMetadataBuilder(cx, obj)) {
+    if (JSObject* metadata = allocationMetadataBuilder->build(cx, obj)) {
         AutoEnterOOMUnsafeRegion oomUnsafe;
         assertSameCompartment(cx, metadata);
         if (!objectMetadataTable) {
             objectMetadataTable = cx->new_<ObjectWeakMap>(cx);
             if (!objectMetadataTable || !objectMetadataTable->init())
                 oomUnsafe.crash("setNewObjectMetadata");
         }
         if (!objectMetadataTable->add(cx, obj, metadata))
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -362,17 +362,17 @@ struct JSCompartment
     inline js::GlobalObject* unsafeUnbarrieredMaybeGlobal() const;
 
     inline void initGlobal(js::GlobalObject& global);
 
   public:
     void*                        data;
 
   private:
-    js::AllocationMetadataBuilder     allocationMetadataBuilder;
+    const js::AllocationMetadataBuilder *allocationMetadataBuilder;
 
     js::SavedStacks              savedStacks_;
 
     js::WrapperMap               crossCompartmentWrappers;
 
   public:
     /* Last time at which an animation was played for a global in this compartment. */
     int64_t                      lastAnimationTime;
@@ -601,18 +601,20 @@ struct JSCompartment
     void clearTables();
 
     static void fixupCrossCompartmentWrappersAfterMovingGC(JSTracer* trc);
     void fixupInitialShapeTable();
     void fixupAfterMovingGC();
     void fixupGlobal();
 
     bool hasAllocationMetadataBuilder() const { return allocationMetadataBuilder; }
-    js::AllocationMetadataBuilder getAllocationMetadataBuilder() const { return allocationMetadataBuilder; }
-    void setAllocationMetadataBuilder(js::AllocationMetadataBuilder builder);
+    const js::AllocationMetadataBuilder* getAllocationMetadataBuilder() const {
+        return allocationMetadataBuilder;
+    }
+    void setAllocationMetadataBuilder(const js::AllocationMetadataBuilder* builder);
     void forgetAllocationMetadataBuilder() {
         allocationMetadataBuilder = nullptr;
     }
     void setNewObjectMetadata(JSContext* cx, JS::HandleObject obj);
     void clearObjectMetadata();
     const void* addressOfMetadataBuilder() const {
         return &allocationMetadataBuilder;
     }
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1229,17 +1229,17 @@ js::AutoCTypesActivityCallback::AutoCTyp
 {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
 
     if (callback)
         callback(cx, beginType);
 }
 
 JS_FRIEND_API(void)
-js::SetAllocationMetadataBuilder(JSContext* cx, AllocationMetadataBuilder callback)
+js::SetAllocationMetadataBuilder(JSContext* cx, const AllocationMetadataBuilder *callback)
 {
     cx->compartment()->setAllocationMetadataBuilder(callback);
 }
 
 JS_FRIEND_API(JSObject*)
 js::GetAllocationMetadata(JSObject* obj)
 {
     ObjectWeakMap* map = obj->compartment()->objectMetadataTable;
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2660,26 +2660,34 @@ class MOZ_RAII JS_FRIEND_API(AutoCTypesA
     void DoEndCallback() {
         if (callback) {
             callback(cx, endType);
             callback = nullptr;
         }
     }
 };
 
-typedef JSObject*
-(* AllocationMetadataBuilder)(JSContext* cx, JS::HandleObject obj);
+// Abstract base class for objects that build allocation metadata for JavaScript
+// values.
+struct AllocationMetadataBuilder {
+    // Return a metadata object for the newly constructed object |obj|, or
+    // nullptr if there's no metadata to attach.
+    //
+    // Implementations should treat all errors as fatal; there is no way to
+    // report errors from this callback.
+    virtual JSObject* build(JSContext *cx, JS::HandleObject obj) const { return nullptr; }
+};
 
 /**
  * Specify a callback to invoke when creating each JS object in the current
  * compartment, which may return a metadata object to associate with the
  * object.
  */
 JS_FRIEND_API(void)
-SetAllocationMetadataBuilder(JSContext* cx, AllocationMetadataBuilder callback);
+SetAllocationMetadataBuilder(JSContext* cx, const AllocationMetadataBuilder *callback);
 
 /** Get the metadata associated with an object. */
 JS_FRIEND_API(JSObject*)
 GetAllocationMetadata(JSObject* obj);
 
 JS_FRIEND_API(bool)
 GetElementsWithAdder(JSContext* cx, JS::HandleObject obj, JS::HandleObject receiver,
                      uint32_t begin, uint32_t end, js::ElementAdder* adder);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2426,17 +2426,17 @@ Debugger::updateObservesAsmJSOnDebuggees
 
 
 /*** Allocations Tracking *************************************************************************/
 
 /* static */ bool
 Debugger::cannotTrackAllocations(const GlobalObject& global)
 {
     auto existingCallback = global.compartment()->getAllocationMetadataBuilder();
-    return existingCallback && existingCallback != SavedStacksMetadataBuilder;
+    return existingCallback && existingCallback != &SavedStacks::metadataBuilder;
 }
 
 /* static */ bool
 Debugger::isObservedByDebuggerTrackingAllocations(const GlobalObject& debuggee)
 {
     if (auto* v = debuggee.getDebuggers()) {
         for (auto p = v->begin(); p != v->end(); p++) {
             if ((*p)->trackingAllocationSites && (*p)->enabled) {
@@ -2456,17 +2456,17 @@ Debugger::addAllocationsTracking(JSConte
     MOZ_ASSERT(isObservedByDebuggerTrackingAllocations(*debuggee));
 
     if (Debugger::cannotTrackAllocations(*debuggee)) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                              JSMSG_OBJECT_METADATA_CALLBACK_ALREADY_SET);
         return false;
     }
 
-    debuggee->compartment()->setAllocationMetadataBuilder(SavedStacksMetadataBuilder);
+    debuggee->compartment()->setAllocationMetadataBuilder(&SavedStacks::metadataBuilder);
     debuggee->compartment()->chooseAllocationSamplingProbability();
     return true;
 }
 
 /* static */ void
 Debugger::removeAllocationsTracking(GlobalObject& global)
 {
     // If there are still Debuggers that are observing allocations, we cannot
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -457,17 +457,17 @@ bool
 js::StartOffThreadParseScript(JSContext* cx, const ReadOnlyCompileOptions& options,
                               const char16_t* chars, size_t length,
                               JS::OffThreadCompileCallback callback, void* callbackData)
 {
     // Suppress GC so that calls below do not trigger a new incremental GC
     // which could require barriers on the atoms compartment.
     gc::AutoSuppressGC nogc(cx);
     gc::AutoAssertNoNurseryAlloc noNurseryAlloc(cx->runtime());
-    AutoSuppressObjectMetadataCallback suppressMetadata(cx);
+    AutoSuppressAllocationMetadataBuilder suppressMetadata(cx);
 
     JSObject* global = CreateGlobalForOffThreadParse(cx, ParseTaskKind::Script, nogc);
     if (!global)
         return false;
 
     ScopedJSDeletePtr<ExclusiveContext> helpercx(
         cx->new_<ExclusiveContext>(cx->runtime(), (PerThreadData*) nullptr,
                                    ExclusiveContext::Context_Exclusive));
@@ -494,17 +494,17 @@ bool
 js::StartOffThreadParseModule(JSContext* cx, const ReadOnlyCompileOptions& options,
                               const char16_t* chars, size_t length,
                               JS::OffThreadCompileCallback callback, void* callbackData)
 {
     // Suppress GC so that calls below do not trigger a new incremental GC
     // which could require barriers on the atoms compartment.
     gc::AutoSuppressGC nogc(cx);
     gc::AutoAssertNoNurseryAlloc noNurseryAlloc(cx->runtime());
-    AutoSuppressObjectMetadataCallback suppressMetadata(cx);
+    AutoSuppressAllocationMetadataBuilder suppressMetadata(cx);
 
     JSObject* global = CreateGlobalForOffThreadParse(cx, ParseTaskKind::Module, nogc);
     if (!global)
         return false;
 
     ScopedJSDeletePtr<ExclusiveContext> helpercx(
         cx->new_<ExclusiveContext>(cx->runtime(), (PerThreadData*) nullptr,
                                    ExclusiveContext::Context_Exclusive));
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1470,17 +1470,17 @@ SavedStacks::chooseSamplingProbability(J
         bernoulli.setRandomState(seed[0], seed[1]);
         bernoulliSeeded = true;
     }
 
     bernoulli.setProbability(probability);
 }
 
 JSObject*
-SavedStacksMetadataBuilder(JSContext* cx, HandleObject target)
+SavedStacks::MetadataBuilder::build(JSContext* cx, HandleObject target) const
 {
     RootedObject obj(cx, target);
 
     SavedStacks& stacks = cx->compartment()->savedStacks();
     if (!stacks.bernoulli.trial())
         return nullptr;
 
     AutoEnterOOMUnsafeRegion oomUnsafe;
@@ -1490,16 +1490,18 @@ SavedStacksMetadataBuilder(JSContext* cx
 
     if (!Debugger::onLogAllocationSite(cx, obj, frame, JS_GetCurrentEmbedderTime()))
         oomUnsafe.crash("SavedStacksMetadataBuilder");
 
     MOZ_ASSERT_IF(frame, !frame->is<WrapperObject>());
     return frame;
 }
 
+const SavedStacks::MetadataBuilder SavedStacks::metadataBuilder;
+
 #ifdef JS_CRASH_DIAGNOSTICS
 void
 CompartmentChecker::check(SavedStacks* stacks)
 {
     if (&compartment->savedStacks() != stacks) {
         printf("*** Compartment SavedStacks mismatch: %p vs. %p\n",
                (void*) &compartment->savedStacks(), stacks);
         MOZ_CRASH();
--- a/js/src/vm/SavedStacks.h
+++ b/js/src/vm/SavedStacks.h
@@ -144,17 +144,16 @@ namespace js {
 // In the case of z, the `SavedFrame` accessors are called with the `SavedFrame`
 // object in the `this` value, and the content compartment as the cx's current
 // compartment. Similar to the case of y, only the B and C frames are exposed
 // because the cx's current compartment's principals do not subsume A's captured
 // principals.
 
 class SavedStacks {
     friend class SavedFrame;
-    friend JSObject* SavedStacksMetadataBuilder(JSContext* cx, HandleObject target);
     friend bool JS::ubi::ConstructSavedFrameStackSlow(JSContext* cx,
                                                       JS::ubi::StackFrame& ubiFrame,
                                                       MutableHandleObject outSavedFrameStack);
 
   public:
     SavedStacks()
       : frames(),
         bernoulliSeeded(false),
@@ -175,16 +174,24 @@ class SavedStacks {
 
     // Set the sampling random number generator's state to |state0| and
     // |state1|. One or the other must be non-zero. See the comments for
     // mozilla::non_crypto::XorShift128PlusRNG::setState for details.
     void     setRNGState(uint64_t state0, uint64_t state1) { bernoulli.setRandomState(state0, state1); }
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
+    // An alloction metadata builder that marks cells with the JavaScript stack
+    // at which they were allocated.
+    class MetadataBuilder : public AllocationMetadataBuilder {
+        virtual JSObject* build(JSContext *cx, HandleObject obj) const override;
+    };
+
+    static const MetadataBuilder metadataBuilder;
+
   private:
     SavedFrame::Set frames;
     bool bernoulliSeeded;
     mozilla::FastBernoulliTrial bernoulli;
     bool creatingSavedFrame;
 
     // Similar to mozilla::ReentrancyGuard, but instead of asserting against
     // reentrancy, just change the behavior of SavedStacks::saveCurrentStack to
@@ -294,18 +301,16 @@ class SavedStacks {
     // removes the dead script; the second will clear out the source atom since
     // it is no longer held by the table.
     using PCLocationMap = GCHashMap<PCKey, LocationValue, PCLocationHasher, SystemAllocPolicy>;
     PCLocationMap pcLocationMap;
 
     bool getLocation(JSContext* cx, const FrameIter& iter, MutableHandle<LocationValue> locationp);
 };
 
-JSObject* SavedStacksMetadataBuilder(JSContext* cx, HandleObject target);
-
 template <>
 class RootedBase<SavedStacks::LocationValue>
   : public SavedStacks::MutableLocationValueOperations<JS::Rooted<SavedStacks::LocationValue>>
 {};
 
 template <>
 class MutableHandleBase<SavedStacks::LocationValue>
   : public SavedStacks::MutableLocationValueOperations<JS::MutableHandle<SavedStacks::LocationValue>>