[INFER] Cache standard class objects earlier to avoid reentrant class construction, bug 646393.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 30 Mar 2011 07:08:58 -0700
changeset 75889 81ee9f8d4c343c7bbc3e3268cee652723f20c4e0
parent 75888 93bc88428f167e29e1060e972d199af4543c35cd
child 75890 df80ae4a87769a8c7f294585e27986128e6d96fb
push id67
push userclegnitto@mozilla.com
push dateFri, 04 Nov 2011 22:39:41 +0000
treeherdermozilla-release@04778346a3b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs646393
milestone2.0b13pre
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
[INFER] Cache standard class objects earlier to avoid reentrant class construction, bug 646393.
js/src/jit-test/tests/basic/bug646393.js
js/src/jsobj.cpp
js/src/jsobj.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug646393.js
@@ -0,0 +1,4 @@
+try {
+  x.y;
+} catch(ex) {}
+x = Number(1);
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3860,16 +3860,19 @@ js_InitObjectClass(JSContext *cx, JSObje
 }
 
 static bool
 DefineStandardSlot(JSContext *cx, JSObject *obj, JSProtoKey key, JSAtom *atom,
                    const Value &v, uint32 attrs, bool &named)
 {
     jsid id = ATOM_TO_JSID(atom);
 
+    if (!cx->addTypePropertyId(obj->getType(), id, v))
+        return false;
+
     if (key != JSProto_Null) {
         /*
          * Initializing an actual standard class on a global object. If the
          * property is not yet present, force it into a new one bound to a
          * reserved slot. Otherwise, go through the normal property path.
          */
         JS_ASSERT(obj->isGlobal());
         JS_ASSERT(obj->isNative());
@@ -3891,16 +3894,38 @@ DefineStandardSlot(JSContext *cx, JSObje
     }
 
     named = obj->defineProperty(cx, id, v, PropertyStub, StrictPropertyStub, attrs);
     return named;
 }
 
 namespace js {
 
+static bool
+SetClassObject(JSContext *cx, JSObject *obj, JSProtoKey key, JSObject *cobj, JSObject *proto)
+{
+    JS_ASSERT(!obj->getParent());
+    if (!obj->isGlobal())
+        return true;
+
+    return js_SetReservedSlot(cx, obj, key, ObjectOrNullValue(cobj)) &&
+           js_SetReservedSlot(cx, obj, JSProto_LIMIT + key, ObjectOrNullValue(proto));
+}
+
+static void
+ClearClassObject(JSContext *cx, JSObject *obj, JSProtoKey key)
+{
+    JS_ASSERT(!obj->getParent());
+    if (!obj->isGlobal())
+        return;
+
+    obj->setSlot(key, UndefinedValue());
+    obj->setSlot(JSProto_LIMIT + key, UndefinedValue());
+}
+
 JSObject *
 DefineConstructorAndPrototype(JSContext *cx, JSObject *obj, JSProtoKey key, JSAtom *atom,
                               JSObject *protoProto, Class *clasp,
                               Native constructor, uintN nargs,
                               JSTypeHandler ctorHandler,
                               JSPropertySpec *ps, JSFunctionSpec *fs,
                               JSPropertySpec *static_ps, JSFunctionSpec *static_fs)
 {
@@ -3950,43 +3975,53 @@ DefineConstructorAndPrototype(JSContext 
 
     proto->syncSpecialEquality();
 
     /* After this point, control must exit via label bad or out. */
     AutoObjectRooter tvr(cx, proto);
 
     JSObject *ctor;
     bool named = false;
+    bool cached = false;
     if (!constructor) {
         /*
          * Lacking a constructor, name the prototype (e.g., Math) unless this
          * class (a) is anonymous, i.e. for internal use only; (b) the class
          * of obj (the global object) is has a reserved slot indexed by key;
          * and (c) key is not the null key.
          */
         if (!(clasp->flags & JSCLASS_IS_ANONYMOUS) || !obj->isGlobal() || key == JSProto_Null) {
             uint32 attrs = (clasp->flags & JSCLASS_IS_ANONYMOUS)
                            ? JSPROP_READONLY | JSPROP_PERMANENT
                            : 0;
-            if (!cx->addTypePropertyId(obj->getType(), ATOM_TO_JSID(atom), ObjectValue(*proto)))
-                goto bad;
             if (!DefineStandardSlot(cx, obj, key, atom, ObjectValue(*proto), attrs, named))
                 goto bad;
         }
 
         ctor = proto;
     } else {
         if (!ctorHandler)
             ctorHandler = JS_TypeHandlerDynamic;
 
         JSFunction *fun = js_NewFunction(cx, NULL, constructor, nargs, JSFUN_CONSTRUCTOR, obj, atom,
                                          ctorHandler, clasp->name);
-        if (!fun || !cx->addTypePropertyId(obj->getType(), ATOM_TO_JSID(atom), ObjectValue(*fun)))
+        if (!fun)
             goto bad;
 
+        /*
+         * Set the class object early for standard class constructors. Type
+         * inference may need to access these, and js_GetClassPrototype will
+         * fail if it tries to do a reentrant reconstruction of the class.
+         */
+        if (key != JSProto_Null && !(clasp->flags & JSCLASS_CONSTRUCT_PROTOTYPE) &&
+            !SetClassObject(cx, obj, key, fun, proto)) {
+            goto bad;
+        }
+        cached = true;
+
         AutoValueRooter tvr2(cx, ObjectValue(*fun));
         if (!DefineStandardSlot(cx, obj, key, atom, tvr2.value(), 0, named))
             goto bad;
 
         /*
          * Remember the class this function is a constructor for so that
          * we know to create an object of this class when we call the
          * constructor.
@@ -4066,26 +4101,28 @@ DefineConstructorAndPrototype(JSContext 
         JS_ASSERT_IF(ctor == proto, !(clasp->flags & JSCLASS_FREEZE_CTOR));
         if (proto && (clasp->flags & JSCLASS_FREEZE_PROTO) && !proto->freeze(cx))
             goto bad;
         if (ctor && (clasp->flags & JSCLASS_FREEZE_CTOR) && !ctor->freeze(cx))
             goto bad;
     }
 
     /* If this is a standard class, cache its prototype. */
-    if (key != JSProto_Null && !js_SetClassObject(cx, obj, key, ctor, proto))
+    if (!cached && key != JSProto_Null && !SetClassObject(cx, obj, key, ctor, proto))
         goto bad;
 
     return proto;
 
 bad:
     if (named) {
         Value rval;
         obj->deleteProperty(cx, ATOM_TO_JSID(atom), &rval, false);
     }
+    if (cached)
+        ClearClassObject(cx, obj, key);
     return NULL;
 }
 
 }
 
 JSObject *
 js_InitClass(JSContext *cx, JSObject *obj, JSObject *protoProto,
              Class *clasp, Native constructor, uintN nargs,
@@ -4335,27 +4372,16 @@ js_GetClassObject(JSContext *cx, JSObjec
             cobj = &v.toObject();
     }
 
     *objp = cobj;
     return true;
 }
 
 JSBool
-js_SetClassObject(JSContext *cx, JSObject *obj, JSProtoKey key, JSObject *cobj, JSObject *proto)
-{
-    JS_ASSERT(!obj->getParent());
-    if (!obj->isGlobal())
-        return JS_TRUE;
-
-    return js_SetReservedSlot(cx, obj, key, ObjectOrNullValue(cobj)) &&
-           js_SetReservedSlot(cx, obj, JSProto_LIMIT + key, ObjectOrNullValue(proto));
-}
-
-JSBool
 js_FindClassObject(JSContext *cx, JSObject *start, JSProtoKey protoKey,
                    Value *vp, Class *clasp)
 {
     JSStackFrame *fp;
     JSObject *obj, *cobj, *pobj;
     jsid id;
     JSProperty *prop;
     const Shape *shape;
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1559,20 +1559,16 @@ js_PopulateObject(JSContext *cx, JSObjec
 
 /*
  * Fast access to immutable standard objects (constructors and prototypes).
  */
 extern JSBool
 js_GetClassObject(JSContext *cx, JSObject *obj, JSProtoKey key,
                   JSObject **objp);
 
-extern JSBool
-js_SetClassObject(JSContext *cx, JSObject *obj, JSProtoKey key,
-                  JSObject *cobj, JSObject *prototype);
-
 /*
  * If protoKey is not JSProto_Null, then clasp is ignored. If protoKey is
  * JSProto_Null, clasp must non-null.
  */
 extern JSBool
 js_FindClassObject(JSContext *cx, JSObject *start, JSProtoKey key,
                    js::Value *vp, js::Class *clasp = NULL);