Bug 642772: Don't recreate a class during enumeration, if it has been deleted (r=bhackett)

In SM, classes are lazily resolved. If we detect that a class about to be used
has not yet been resolved, then we resolve it. However, the way that we decided
that they were resolved was broken. If the global object had a String property,
then it had been resolved. So what happened when we deleted the String
property? Well, it got resolved again.

Instead of using the String property of the global object, we now use the
contructor slot on the global object. This works fine for String, but some
classes don't have a constructor, like Math and JSON. For those classes, we set
the constructor slot to True. In either case, we can now tell that a class is
resolved if the constructor slot in not Undefined.

js/src/jit-test/tests/basic/bug642772-1.js

@@ -0,0 +1,19 @@
+var n1 = Number.prototype.toFixed;
+var s1 = String.prototype.split;
+delete Number;
+delete String;
+
+var n2 = (5).toFixed;
+var s2 = ("foo").split;
+
+// Check enumeration doesn't resurrect deleted standard classes
+for (x in this) {}
+
+// Ensure the prototypes are shared.
+var n3 = (5).toFixed;
+var s3 = ("foo").split;
+
+assertEq(s1, s2);
+assertEq(s1, s3);
+assertEq(n1, n2);
+assertEq(n1, n3);

js/src/jit-test/tests/basic/bug642772-2.js

@@ -0,0 +1,104 @@
+function failWrapper(callback) {
+    try {
+        callback(); // this should fail
+        throw "test-error"; // and if it didn't we have a problem`
+    } catch (e) {
+        if (e == "test-error")
+            throw ("Testing error when running " + callback.toString());
+    }
+}
+
+
+print ("Deleting standard classes");
+delete Function;
+delete Object;
+delete Array;
+delete Boolean;
+delete JSON;
+delete Date;
+delete Math;
+delete Number;
+delete String;
+delete Regexp;
+delete XML;
+delete Reflect;
+delete Proxy;
+delete Error;
+delete Iterator;
+delete Generator;
+delete StopIteration;
+delete Float32Array;
+delete Float64Array;
+delete Int16Array;
+delete Int32Array;
+delete Int32Array;
+delete Uint16Array;
+delete Uint32Array;
+delete Uint8Array;
+delete Uint8ClampedArray;
+delete Weakmap;
+
+
+print ("Accessing standard classes shouldn't recreate them");
+failWrapper(function () { Function; });
+failWrapper(function () { Object; });
+failWrapper(function () { Array; });
+failWrapper(function () { Boolean; });
+failWrapper(function () { JSON; });
+failWrapper(function () { Date; });
+failWrapper(function () { Math; });
+failWrapper(function () { Number; });
+failWrapper(function () { String; });
+failWrapper(function () { Regexp; });
+failWrapper(function () { XML; });
+failWrapper(function () { Reflect; });
+failWrapper(function () { Proxy; });
+failWrapper(function () { Error; });
+failWrapper(function () { Iterator; });
+failWrapper(function () { Generator; });
+failWrapper(function () { StopIteration; });
+failWrapper(function () { Float32Array; });
+failWrapper(function () { Float64Array; });
+failWrapper(function () { Int16Array; });
+failWrapper(function () { Int32Array; });
+failWrapper(function () { Int32Array; });
+failWrapper(function () { Uint16Array; });
+failWrapper(function () { Uint32Array; });
+failWrapper(function () { Uint8Array; });
+failWrapper(function () { Uint8ClampedArray; });
+failWrapper(function () { Weakmap; });
+
+
+print ("Enumerate over the global object");
+for (c in this) {}
+
+print ("That shouldn't have recreated the standard classes either");
+failWrapper(function () { Function; });
+failWrapper(function () { Object; });
+failWrapper(function () { Array; });
+failWrapper(function () { Boolean; });
+failWrapper(function () { JSON; });
+failWrapper(function () { Date; });
+failWrapper(function () { Math; });
+failWrapper(function () { Number; });
+failWrapper(function () { String; });
+failWrapper(function () { Regexp; });
+failWrapper(function () { XML; });
+failWrapper(function () { Reflect; });
+failWrapper(function () { Proxy; });
+failWrapper(function () { Error; });
+failWrapper(function () { Iterator; });
+failWrapper(function () { Generator; });
+failWrapper(function () { StopIteration; });
+failWrapper(function () { Float32Array; });
+failWrapper(function () { Float64Array; });
+failWrapper(function () { Int16Array; });
+failWrapper(function () { Int32Array; });
+failWrapper(function () { Int32Array; });
+failWrapper(function () { Uint16Array; });
+failWrapper(function () { Uint32Array; });
+failWrapper(function () { Uint8Array; });
+failWrapper(function () { Uint8ClampedArray; });
+failWrapper(function () { Weakmap; });
+
+print ("success");

js/src/jit-test/tests/basic/bug642772-3.js

@@ -0,0 +1,5 @@
+// Catch memory leaks when enumerating over the global object.
+
+for (let z = 1; z <= 16000; ++z) {
+  for each (y in this);
+}

js/src/jsapi.cpp

@@ -1774,8 +1774,7 @@ JS_ResolveStandardClass(JSContext *cx, JSObject *obj, jsid id, JSBool *resolved)
         if (stdnm->clasp->flags & JSCLASS_IS_ANONYMOUS)
             return JS_TRUE;
 
-        JSProtoKey key = JSCLASS_CACHED_PROTO_KEY(stdnm->clasp);
-        if (obj->getReservedSlot(key).isObject())
+        if (IsStandardClassResolved(obj, stdnm->clasp))
             return JS_TRUE;
 
         if (!stdnm->init(cx, obj))
@@ -1796,7 +1795,10 @@ JS_EnumerateStandardClasses(JSContext *cx, JSObject *obj)
     assertSameCompartment(cx, obj);
     rt = cx->runtime;
 
-    /* Check whether we need to bind 'undefined' and define it if so. */
+    /*
+     * Check whether we need to bind 'undefined' and define it if so.
+     * Since ES5 15.1.1.3 undefined can't be deleted.
+     */
     atom = rt->atomState.typeAtoms[JSTYPE_VOID];
     if (!obj->nativeContains(ATOM_TO_JSID(atom)) &&
         !obj->defineProperty(cx, ATOM_TO_JSID(atom), UndefinedValue(),
@@ -1805,12 +1807,12 @@ JS_EnumerateStandardClasses(JSContext *cx, JSObject *obj)
         return JS_FALSE;
     }
 
-    /* Initialize any classes that have not been resolved yet. */
+    /* Initialize any classes that have not been initialized yet. */
     for (i = 0; standard_class_atoms[i].init; i++) {
-        atom = OFFSET_TO_ATOM(rt, standard_class_atoms[i].atomOffset);
-        if (!obj->nativeContains(ATOM_TO_JSID(atom)) &&
-            !standard_class_atoms[i].init(cx, obj)) {
-            return JS_FALSE;
+        if (!js::IsStandardClassResolved(obj, standard_class_atoms[i].clasp) &&
+            !standard_class_atoms[i].init(cx, obj))
+        {
+                return JS_FALSE;
         }
     }
 

js/src/jsmath.cpp

@@ -881,5 +881,8 @@ js_InitMathClass(JSContext *cx, JSObject *obj)
         return NULL;
     if (!JS_DefineConstDoubles(cx, Math, math_constants))
         return NULL;
+
+    MarkStandardClassInitializedNoProto(obj, &js_MathClass);
+
     return Math;
 }

js/src/jsobj.cpp

@@ -3976,6 +3976,37 @@ bad:
     return NULL;
 }
 
+/*
+ * Lazy standard classes need a way to indicate if they have been initialized.
+ * Otherwise, when we delete them, we might accidentally recreate them via a
+ * lazy initialization. We use the presence of a ctor or proto in the
+ * globalObject's slot to indicate that they've been constructed, but this only
+ * works for classes which have a proto and ctor. Classes which don't have one
+ * can call MarkStandardClassInitializedNoProto(), and we can always check
+ * whether a class is initialized by calling IsStandardClassResolved().
+ */
+bool
+IsStandardClassResolved(JSObject *obj, js::Class *clasp)
+{
+    JSProtoKey key = JSCLASS_CACHED_PROTO_KEY(clasp);
+
+    /* If the constructor is undefined, then it hasn't been initialized. */
+    return (obj->getReservedSlot(key) != UndefinedValue());
+}
+
+void
+MarkStandardClassInitializedNoProto(JSObject* obj, js::Class *clasp)
+{
+    JSProtoKey key = JSCLASS_CACHED_PROTO_KEY(clasp);
+
+    /*
+     * We use True so that it's obvious what we're doing (instead of, say,
+     * Null, which might be miscontrued as an error in setting Undefined).
+     */
+    if (obj->getReservedSlot(key) == UndefinedValue())
+        obj->setSlot(key, BooleanValue(true));
+}
+
 }
 
 JSObject *

js/src/jsobj.h

@@ -1548,6 +1548,13 @@ DefineConstructorAndPrototype(JSContext *cx, JSObject *obj, JSProtoKey key, JSAt
                               Native constructor, uintN nargs,
                               JSPropertySpec *ps, JSFunctionSpec *fs,
                               JSPropertySpec *static_ps, JSFunctionSpec *static_fs);
+
+bool
+IsStandardClassResolved(JSObject *obj, js::Class *clasp);
+
+void
+MarkStandardClassInitializedNoProto(JSObject *obj, js::Class *clasp);
+
 }
 
 extern JSObject *

js/src/json.cpp

@@ -1413,5 +1413,7 @@ js_InitJSONClass(JSContext *cx, JSObject *obj)
     if (!JS_DefineFunctions(cx, JSON, json_static_methods))
         return NULL;
 
+    MarkStandardClassInitializedNoProto(obj, &js_JSONClass);
+
     return JSON;
 }

js/src/jsproxy.cpp

@@ -1459,5 +1459,8 @@ js_InitProxyClass(JSContext *cx, JSObject *obj)
     }
     if (!JS_DefineFunctions(cx, module, static_methods))
         return NULL;
+
+    MarkStandardClassInitializedNoProto(obj, &js_ProxyClass);
+
     return module;
 }

js/src/jsreflect.cpp

@@ -3314,5 +3314,7 @@ js_InitReflectClass(JSContext *cx, JSObject *obj)
     if (!JS_DefineFunctions(cx, Reflect, static_methods))
         return NULL;
 
+    MarkStandardClassInitializedNoProto(obj, &js_ReflectClass);
+
     return Reflect;
 }

js/src/shell/js.cpp

@@ -3219,7 +3219,8 @@ ResolveClass(JSContext *cx, JSObject *obj, jsid id, JSBool *resolved)
 
     if (!*resolved) {
         if (JSID_IS_ATOM(id, CLASS_ATOM(cx, Reflect))) {
-            if (!js_InitReflectClass(cx, obj))
+            if (!IsStandardClassResolved(obj, &js_ReflectClass) &&
+                !js_InitReflectClass(cx, obj))
                 return JS_FALSE;
             *resolved = JS_TRUE;
         }

js/src/tests/js1_8_5/regress/jstests.list

@@ -90,7 +90,7 @@ script regress-624199.js
 script regress-624547.js
 script regress-624968.js
 script regress-626436.js
-fails-if(xulRuntime.shell) script regress-633741.js
+script regress-633741.js
 script regress-634210-1.js
 script regress-634210-2.js
 script regress-634210-3.js