Bug 950118 - Don't allow the object metadata hook to reenter JS, r=jimb.
authorBrian Hackett <bhackett1024@gmail.com>
Sat, 14 Dec 2013 16:29:43 -0800
changeset 160532 bec71542c0553d95e1f54dbc2f1f33c6ea200f3e
parent 160531 24ba20d0a8e5f312bdb8868d29887c251c80cd8a
child 160533 c22a4358920a9eeab2ededf65090b2344b802d02
push id25836
push userphilringnalda@gmail.com
push dateMon, 16 Dec 2013 01:58:54 +0000
treeherdermozilla-central@f8fea5ea69a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs950118
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 950118 - Don't allow the object metadata hook to reenter JS, r=jimb.
js/src/builtin/TestingFunctions.cpp
js/src/jit-test/tests/basic/metadata-hook.js
js/src/jsobjinlines.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -20,16 +20,17 @@
 #include "jit/AsmJSLink.h"
 #include "js/StructuredClone.h"
 #include "vm/ForkJoin.h"
 #include "vm/GlobalObject.h"
 #include "vm/Interpreter.h"
 #include "vm/ProxyObject.h"
 
 #include "jscntxtinlines.h"
+#include "jsobjinlines.h"
 
 using namespace js;
 using namespace JS;
 
 using mozilla::ArrayLength;
 
 // If fuzzingSafe is set, remove functionality that could cause problems with
 // fuzzers. Set this via the environment variable MOZ_FUZZING_SAFE.
@@ -983,53 +984,67 @@ js::testingFunc_inParallelSection(JSCont
 {
     // If we were actually *in* a parallel section, then this function
     // would be inlined to TRUE in ion-generated code.
     JS_ASSERT(!InParallelSection());
     JS_SET_RVAL(cx, vp, JSVAL_FALSE);
     return true;
 }
 
-static const char *ObjectMetadataPropertyName = "__objectMetadataFunction__";
-
 static bool
 ShellObjectMetadataCallback(JSContext *cx, JSObject **pmetadata)
 {
-    RootedValue fun(cx);
-    if (!JS_GetProperty(cx, cx->global(), ObjectMetadataPropertyName, &fun))
+    RootedObject obj(cx, NewBuiltinClassInstance(cx, &JSObject::class_));
+    if (!obj)
+        return false;
+
+    RootedObject stack(cx, NewDenseEmptyArray(cx));
+    if (!stack)
         return false;
 
-    RootedValue rval(cx);
-    if (!Invoke(cx, UndefinedValue(), fun, 0, nullptr, &rval))
+    static int createdIndex = 0;
+    createdIndex++;
+
+    if (!JS_DefineProperty(cx, obj, "index", Int32Value(createdIndex),
+                           JS_PropertyStub, JS_StrictPropertyStub, 0))
+    {
         return false;
+    }
 
-    if (rval.isObject())
-        *pmetadata = &rval.toObject();
+    if (!JS_DefineProperty(cx, obj, "stack", ObjectValue(*stack),
+                           JS_PropertyStub, JS_StrictPropertyStub, 0))
+    {
+        return false;
+    }
 
+    int stackIndex = 0;
+    for (NonBuiltinScriptFrameIter iter(cx); !iter.done(); ++iter) {
+        if (iter.isFunctionFrame()) {
+            if (!JS_DefinePropertyById(cx, stack, INT_TO_JSID(stackIndex), ObjectValue(*iter.callee()),
+                                       JS_PropertyStub, JS_StrictPropertyStub, 0))
+            {
+                return false;
+            }
+            stackIndex++;
+        }
+    }
+
+    *pmetadata = obj;
     return true;
 }
 
 static bool
 SetObjectMetadataCallback(JSContext *cx, unsigned argc, jsval *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    args.rval().setUndefined();
+    bool enabled = argc ? ToBoolean(args[0]) : false;
+    SetObjectMetadataCallback(cx, enabled ? ShellObjectMetadataCallback : nullptr);
 
-    if (argc == 0 || !args[0].isObject() || !args[0].toObject().is<JSFunction>()) {
-        if (!JS_DeleteProperty(cx, cx->global(), ObjectMetadataPropertyName))
-            return false;
-        js::SetObjectMetadataCallback(cx, nullptr);
-        return true;
-    }
-
-    if (!JS_DefineProperty(cx, cx->global(), ObjectMetadataPropertyName, args[0], nullptr, nullptr, 0))
-        return false;
-
-    js::SetObjectMetadataCallback(cx, ShellObjectMetadataCallback);
+    args.rval().setUndefined();
     return true;
 }
 
 static bool
 SetObjectMetadata(JSContext *cx, unsigned argc, jsval *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     if (argc != 2 || !args[0].isObject() || !args[1].isObject()) {
--- a/js/src/jit-test/tests/basic/metadata-hook.js
+++ b/js/src/jit-test/tests/basic/metadata-hook.js
@@ -1,48 +1,34 @@
 
 x = [1,2,3];
 setObjectMetadata(x, {y:0});
 assertEq(getObjectMetadata(x).y, 0);
 
-incallback = false;
-count = 0;
-
-function callback(obj) {
-    if (incallback)
-	return null;
-    incallback = true;
-    var res =
-      {
-        count: ++count,
-        location: Error().stack,
-        message: Error().message // no .message => Error.prototype.message => ""
-      };
-    incallback = false;
-    return res;
-}
-callback({});
-
-setObjectMetadataCallback(callback);
+setObjectMetadataCallback(true);
 
 function Foo() {
   this.x = 0;
   this.y = 1;
 }
 
-function f() {
-  w = new Foo();
-  x = [1,2,3];
-  y = [2,3,5];
-  z = {a:0,b:1};
+function hello() {
+  function there() {
+    w = new Foo();
+    x = [1,2,3];
+    y = [2,3,5];
+    z = {a:0,b:1};
+  }
+  callee = there;
+  callee();
 }
-f();
+hello();
 
-var wc = getObjectMetadata(w).count;
-var xc = getObjectMetadata(x).count;
-var yc = getObjectMetadata(y).count;
-var zc = getObjectMetadata(z).count;
+var wc = getObjectMetadata(w).index;
+var xc = getObjectMetadata(x).index;
+var yc = getObjectMetadata(y).index;
+var zc = getObjectMetadata(z).index;
 
 assertEq(xc > wc, true);
 assertEq(yc > xc, true);
 assertEq(zc > yc, true);
-assertEq(/\.js/.test(getObjectMetadata(x).location), true);
-assertEq(getObjectMetadata(x).message, "");
+assertEq(getObjectMetadata(x).stack[0], callee);
+assertEq(getObjectMetadata(x).stack[1], hello);
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -1042,25 +1042,28 @@ DefineConstructorAndPrototype(JSContext 
                               const JSPropertySpec *static_ps, const JSFunctionSpec *static_fs,
                               JSObject **ctorp = nullptr,
                               gc::AllocKind ctorKind = JSFunction::FinalizeKind);
 
 static JS_ALWAYS_INLINE bool
 NewObjectMetadata(ExclusiveContext *cxArg, JSObject **pmetadata)
 {
     // The metadata callback is invoked before each created object, except when
-    // analysis/compilation/parsing is active as the callback may reenter JS.
+    // analysis/compilation is active, to avoid recursion.
     JS_ASSERT(!*pmetadata);
     if (JSContext *cx = cxArg->maybeJSContext()) {
         if (JS_UNLIKELY((size_t)cx->compartment()->hasObjectMetadataCallback()) &&
-            !cx->compartment()->activeAnalysis &&
-            !cx->runtime()->mainThread.activeCompilations)
+            !cx->compartment()->activeAnalysis)
         {
             JS::DisableGenerationalGC(cx->runtime());
-            gc::AutoSuppressGC suppress(cx);
+
+            // Use AutoEnterAnalysis to prohibit both any GC activity under the
+            // callback, and any reentering of JS via Invoke() etc.
+            types::AutoEnterAnalysis enter(cx);
+
             bool status = cx->compartment()->callObjectMetadataCallback(cx, pmetadata);
             JS::EnableGenerationalGC(cx->runtime());
             return status;
         }
     }
     return true;
 }