js_InitExceptionClasses was skipping part js_InitClass, leading to "Assertion failure: proto->canProvideEmptyShape". Bug 624968, r=Waldo.
authorJason Orendorff <jorendorff@mozilla.com>
Mon, 24 Jan 2011 17:32:44 -0600
changeset 61705 f727e6cd2f06fe07d2e1d5d2b7979a051223c053
parent 61704 caca24ca36cd721eb9e4ed371fa40258d3e7a278
child 61706 02d1466338929adbfb4365c47402f8a617ad0bf0
push idunknown
push userunknown
push dateunknown
reviewersWaldo
bugs624968
milestone2.0b11pre
js_InitExceptionClasses was skipping part js_InitClass, leading to "Assertion failure: proto->canProvideEmptyShape". Bug 624968, r=Waldo.
js/src/jsexn.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/tests/js1_8_5/regress/jstests.list
js/src/tests/js1_8_5/regress/regress-624968.js
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -1002,100 +1002,67 @@ GetExceptionProtoKey(intN exn)
     JS_ASSERT(JSEXN_ERR <= exn);
     JS_ASSERT(exn < JSEXN_LIMIT);
     return (JSProtoKey) (JSProto_Error + exn);
 }
 
 JSObject *
 js_InitExceptionClasses(JSContext *cx, JSObject *obj)
 {
-    jsval roots[3];
-    JSObject *obj_proto, *error_proto;
-
     /*
      * If lazy class initialization occurs for any Error subclass, then all
      * classes are initialized, starting with Error.  To avoid reentry and
      * redundant initialization, we must not pass a null proto parameter to
      * NewNonFunction below, when called for the Error superclass.  We need to
      * ensure that Object.prototype is the proto of Error.prototype.
      *
      * See the equivalent code to ensure that parent_proto is non-null when
      * js_InitClass calls NewObject, in jsobj.cpp.
      */
+    JSObject *obj_proto;
     if (!js_GetClassPrototype(cx, obj, JSProto_Object, &obj_proto))
         return NULL;
 
-    PodArrayZero(roots);
-    AutoArrayRooter tvr(cx, JS_ARRAY_LENGTH(roots), Valueify(roots));
-
-#ifdef __GNUC__
-    error_proto = NULL;   /* quell GCC overwarning */
-#endif
-
-    jsval empty = STRING_TO_JSVAL(cx->runtime->emptyString);
-
-    /* Initialize the prototypes first. */
+    /* Define all error constructors. */
+    Value empty = StringValue(cx->runtime->emptyString);
+    jsid nameId = ATOM_TO_JSID(cx->runtime->atomState.nameAtom);
+    jsid messageId = ATOM_TO_JSID(cx->runtime->atomState.messageAtom);
+    jsid fileNameId = ATOM_TO_JSID(cx->runtime->atomState.fileNameAtom);
+    jsid lineNumberId = ATOM_TO_JSID(cx->runtime->atomState.lineNumberAtom);
+    JSObject *error_proto = NULL;
     for (intN i = JSEXN_ERR; i != JSEXN_LIMIT; i++) {
-        /* Make the prototype for the current constructor name. */
+        JSProtoKey protoKey = GetExceptionProtoKey(i);
+        JSAtom *atom = cx->runtime->atomState.classAtoms[protoKey];
         JSObject *proto =
-            NewNonFunction<WithProto::Class>(cx, &js_ErrorClass, (i != JSEXN_ERR) ? error_proto : obj_proto, obj);
+            DefineConstructorAndPrototype(cx, obj, protoKey, atom,
+                                          (i == JSEXN_ERR) ? obj_proto : error_proto,
+                                          &js_ErrorClass, Exception, 1,
+                                          NULL, (i == JSEXN_ERR) ? exception_methods : NULL,
+                                          NULL, NULL);
         if (!proto)
             return NULL;
-        if (i == JSEXN_ERR) {
-            error_proto = proto;
-            roots[0] = OBJECT_TO_JSVAL(proto);
-        } else {
-            // We cannot share the root for error_proto and other prototypes
-            // as error_proto must be rooted until the function returns.
-            roots[1] = OBJECT_TO_JSVAL(proto);
-        }
+        JS_ASSERT(proto->privateData == NULL);
 
-        /* So exn_finalize knows whether to destroy private data. */
-        proto->setPrivate(NULL);
-
-        /* Make a constructor function for the current name. */
-        JSProtoKey protoKey = GetExceptionProtoKey(i);
-        
-        jsid id = ATOM_TO_JSID(cx->runtime->atomState.classAtoms[protoKey]);
-        JSFunction *fun = js_DefineFunction(cx, obj, id, Exception, 1, JSFUN_CONSTRUCTOR);
-        if (!fun)
-            return NULL;
-        roots[2] = OBJECT_TO_JSVAL(FUN_OBJECT(fun));
+        if (i == JSEXN_ERR)
+            error_proto = proto;
 
-        /* Make this constructor make objects of class Exception. */
-        FUN_CLASP(fun) = &js_ErrorClass;
-
-        /* Make the prototype and constructor links. */
-        if (!js_SetClassPrototype(cx, FUN_OBJECT(fun), proto,
-                                  JSPROP_READONLY | JSPROP_PERMANENT)) {
-            return NULL;
-        }
-
-        /* Add the name property to the prototype. */
-        if (!JS_DefineProperty(cx, proto, js_name_str, STRING_TO_JSVAL(JSID_TO_STRING(id)),
-                               NULL, NULL, JSPROP_ENUMERATE)) {
-            return NULL;
-        }
-
-        /* Finally, stash the constructor for later uses. */
-        if (!js_SetClassObject(cx, obj, protoKey, FUN_OBJECT(fun), proto))
-            return NULL;
-
-        /* Set default values. */
-        if (!JS_DefineProperty(cx, proto, js_message_str, empty, NULL, NULL, JSPROP_ENUMERATE) ||
-            !JS_DefineProperty(cx, proto, js_fileName_str, empty, NULL, NULL, JSPROP_ENUMERATE) ||
-            !JS_DefineProperty(cx, proto, js_lineNumber_str, JSVAL_ZERO, NULL, NULL,
-                               JSPROP_ENUMERATE)) {
+        /* Add properties to the prototype. */
+        JSAutoResolveFlags rf(cx, JSRESOLVE_QUALIFIED | JSRESOLVE_DECLARING);
+        if (!js_DefineNativeProperty(cx, proto, nameId, StringValue(atom),
+                                     PropertyStub, PropertyStub, JSPROP_ENUMERATE, 0, 0, NULL) ||
+            !js_DefineNativeProperty(cx, proto, messageId, empty,
+                                     PropertyStub, PropertyStub, JSPROP_ENUMERATE, 0, 0, NULL) ||
+            !js_DefineNativeProperty(cx, proto, fileNameId, empty,
+                                     PropertyStub, PropertyStub, JSPROP_ENUMERATE, 0, 0, NULL) ||
+            !js_DefineNativeProperty(cx, proto, lineNumberId, Valueify(JSVAL_ZERO),
+                                     PropertyStub, PropertyStub, JSPROP_ENUMERATE, 0, 0, NULL)) {
             return NULL;
         }
     }
 
-    if (!JS_DefineFunctions(cx, error_proto, exception_methods))
-        return NULL;
-
     return error_proto;
 }
 
 const JSErrorFormatString*
 js_GetLocalizedErrorMessage(JSContext* cx, void *userRef, const char *locale,
                             const uintN errorNumber)
 {
     const JSErrorFormatString *errorString = NULL;
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3780,52 +3780,25 @@ DefineStandardSlot(JSContext *cx, JSObje
             return true;
         }
     }
 
     named = obj->defineProperty(cx, id, v, PropertyStub, PropertyStub, attrs);
     return named;
 }
 
+namespace js {
+
 JSObject *
-js_InitClass(JSContext *cx, JSObject *obj, JSObject *parent_proto,
-             Class *clasp, Native constructor, uintN nargs,
-             JSPropertySpec *ps, JSFunctionSpec *fs,
-             JSPropertySpec *static_ps, JSFunctionSpec *static_fs)
-{
-    JSAtom *atom;
-    JSProtoKey key;
-    JSFunction *fun;
-    bool named = false;
-
-    atom = js_Atomize(cx, clasp->name, strlen(clasp->name), 0);
-    if (!atom)
-        return NULL;
-
-    /*
-     * When initializing a standard class, if no parent_proto (grand-proto of
-     * instances of the class, parent-proto of the class's prototype object)
-     * is given, we must use Object.prototype if it is available.  Otherwise,
-     * we could look up the wrong binding for a class name in obj.  Example:
-     *
-     *   String = Array;
-     *   print("hi there".join);
-     *
-     * should print undefined, not Array.prototype.join.  This is required by
-     * ECMA-262, alas.  It might have been better to make String readonly and
-     * permanent in the global object, instead -- but that's too big a change
-     * to swallow at this point.
-     */
-    key = JSCLASS_CACHED_PROTO_KEY(clasp);
-    if (key != JSProto_Null &&
-        !parent_proto &&
-        !js_GetClassPrototype(cx, obj, JSProto_Object, &parent_proto)) {
-        return NULL;
-    }
-
+DefineConstructorAndPrototype(JSContext *cx, JSObject *obj, JSProtoKey key, JSAtom *atom,
+                              JSObject *protoProto, Class *clasp,
+                              Native constructor, uintN nargs,
+                              JSPropertySpec *ps, JSFunctionSpec *fs,
+                              JSPropertySpec *static_ps, JSFunctionSpec *static_fs)
+{
     /*
      * Create a prototype object for this class.
      *
      * FIXME: lazy standard (built-in) class initialization and even older
      * eager boostrapping code rely on all of these properties:
      *
      * 1. NewObject attempting to compute a default prototype object when
      *    passed null for proto; and
@@ -3837,26 +3810,27 @@ js_InitClass(JSContext *cx, JSObject *ob
      * 3. NewObject allocating a JSFunction-sized GC-thing when clasp is
      *    &js_FunctionClass, not a JSObject-sized (smaller) GC-thing.
      *
      * The JS_NewObjectForGivenProto and JS_NewObject APIs also allow clasp to
      * be &js_FunctionClass (we could break compatibility easily). But fixing
      * (3) is not enough without addressing the bootstrapping dependency on (1)
      * and (2).
      */
-    JSObject *proto = NewObject<WithProto::Class>(cx, clasp, parent_proto, obj);
+    JSObject *proto = NewObject<WithProto::Class>(cx, clasp, protoProto, obj);
     if (!proto)
         return NULL;
 
     proto->syncSpecialEquality();
-    
+
     /* After this point, control must exit via label bad or out. */
     AutoObjectRooter tvr(cx, proto);
 
     JSObject *ctor;
+    bool named = 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) {
@@ -3864,17 +3838,17 @@ js_InitClass(JSContext *cx, JSObject *ob
                            ? JSPROP_READONLY | JSPROP_PERMANENT
                            : 0;
             if (!DefineStandardSlot(cx, obj, key, atom, ObjectValue(*proto), attrs, named))
                 goto bad;
         }
 
         ctor = proto;
     } else {
-        fun = js_NewFunction(cx, NULL, constructor, nargs, JSFUN_CONSTRUCTOR, obj, atom);
+        JSFunction *fun = js_NewFunction(cx, NULL, constructor, nargs, JSFUN_CONSTRUCTOR, obj, atom);
         if (!fun)
             goto bad;
 
         AutoValueRooter tvr2(cx, ObjectValue(*fun));
         if (!DefineStandardSlot(cx, obj, key, atom, tvr2.value(), 0, named))
             goto bad;
 
         /*
@@ -3962,16 +3936,51 @@ js_InitClass(JSContext *cx, JSObject *ob
 bad:
     if (named) {
         Value rval;
         obj->deleteProperty(cx, ATOM_TO_JSID(atom), &rval, false);
     }
     return NULL;
 }
 
+}
+
+JSObject *
+js_InitClass(JSContext *cx, JSObject *obj, JSObject *protoProto,
+             Class *clasp, Native constructor, uintN nargs,
+             JSPropertySpec *ps, JSFunctionSpec *fs,
+             JSPropertySpec *static_ps, JSFunctionSpec *static_fs)
+{
+    JSAtom *atom = js_Atomize(cx, clasp->name, strlen(clasp->name), 0);
+    if (!atom)
+        return NULL;
+
+    /*
+     * All instances of the class will inherit properties from the prototype
+     * object we are about to create (in DefineConstructorAndPrototype), which
+     * in turn will inherit from protoProto.
+     *
+     * When initializing a standard class (other than Object), if protoProto is
+     * null, default to the Object prototype object. The engine's internal uses
+     * of js_InitClass depend on this nicety. Note that in
+     * js_InitFunctionAndObjectClasses, we specially hack the resolving table
+     * and then depend on js_GetClassPrototype here leaving protoProto NULL and
+     * returning true.
+     */
+    JSProtoKey key = JSCLASS_CACHED_PROTO_KEY(clasp);
+    if (key != JSProto_Null &&
+        !protoProto &&
+        !js_GetClassPrototype(cx, obj, JSProto_Object, &protoProto)) {
+        return NULL;
+    }
+
+    return DefineConstructorAndPrototype(cx, obj, key, atom, protoProto, clasp, constructor, nargs,
+                                         ps, fs, static_ps, static_fs);
+}
+
 bool
 JSObject::allocSlots(JSContext *cx, size_t newcap)
 {
     uint32 oldcap = numSlots();
 
     JS_ASSERT(newcap >= oldcap && !hasSlotsArray());
 
     if (newcap > NSLOTS_LIMIT) {
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1487,16 +1487,25 @@ js_PropertyIsEnumerable(JSContext *cx, J
 #ifdef OLD_GETTER_SETTER_METHODS
 JS_FRIEND_API(JSBool) js_obj_defineGetter(JSContext *cx, uintN argc, js::Value *vp);
 JS_FRIEND_API(JSBool) js_obj_defineSetter(JSContext *cx, uintN argc, js::Value *vp);
 #endif
 
 extern JSObject *
 js_InitObjectClass(JSContext *cx, JSObject *obj);
 
+namespace js {
+JSObject *
+DefineConstructorAndPrototype(JSContext *cx, JSObject *obj, JSProtoKey key, JSAtom *atom,
+                              JSObject *protoProto, Class *clasp,
+                              Native constructor, uintN nargs,
+                              JSPropertySpec *ps, JSFunctionSpec *fs,
+                              JSPropertySpec *static_ps, JSFunctionSpec *static_fs);
+}
+
 extern JSObject *
 js_InitClass(JSContext *cx, JSObject *obj, JSObject *parent_proto,
              js::Class *clasp, js::Native constructor, uintN nargs,
              JSPropertySpec *ps, JSFunctionSpec *fs,
              JSPropertySpec *static_ps, JSFunctionSpec *static_fs);
 
 /*
  * Select Object.prototype method names shared between jsapi.cpp and jsobj.cpp.
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -1099,17 +1099,17 @@ namespace detail
 {
 template <bool withProto, bool isFunction>
 static JS_ALWAYS_INLINE JSObject *
 NewObject(JSContext *cx, js::Class *clasp, JSObject *proto, JSObject *parent,
           gc::FinalizeKind kind)
 {
     /* Bootstrap the ur-object, and make it the default prototype object. */
     if (withProto == WithProto::Class && !proto) {
-        if (!FindProto (cx, clasp, parent, &proto))
+        if (!FindProto(cx, clasp, parent, &proto))
           return NULL;
     }
 
     /*
      * Allocate an object from the GC heap and initialize all its fields before
      * doing any operation that can potentially trigger GC. Functions have a
      * larger non-standard allocation size.
      *
@@ -1182,17 +1182,17 @@ template <WithProto::e withProto>
 static JS_ALWAYS_INLINE JSObject *
 NewObject(JSContext *cx, js::Class *clasp, JSObject *proto, JSObject *parent)
 {
     gc::FinalizeKind kind = gc::GetGCObjectKind(JSCLASS_RESERVED_SLOTS(clasp));
     return NewObject<withProto>(cx, clasp, proto, parent, kind);
 }
 
 /*
- * As for js_GetGCObjectKind, where numSlots is a guess at the final size of
+ * As for gc::GetGCObjectKind, where numSlots is a guess at the final size of
  * the object, zero if the final size is unknown.
  */
 static inline gc::FinalizeKind
 GuessObjectGCKind(size_t numSlots, bool isArray)
 {
     if (numSlots)
         return gc::GetGCObjectKind(numSlots);
     return isArray ? gc::FINALIZE_OBJECT8 : gc::FINALIZE_OBJECT4;
--- a/js/src/tests/js1_8_5/regress/jstests.list
+++ b/js/src/tests/js1_8_5/regress/jstests.list
@@ -83,8 +83,9 @@ script regress-619003-1.js
 script regress-619003-2.js
 script regress-620376-1.js
 script regress-620376-2.js
 script regress-621814.js
 script regress-620750.js
 script regress-624199.js
 script regress-624547.js
 script regress-626436.js
+script regress-624968.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/js1_8_5/regress/regress-624968.js
@@ -0,0 +1,9 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+// Contributor: Bob Clary <bclary@bclary.com>
+
+try {
+    new {prototype: TypeError.prototype};
+} catch (e) {}
+
+reportCompare(0, 0, 'ok');