js_UnbrandAndClearSlots leaks, use JS_ClearScope and throw if compile-N-go scripts are run after JS_ClearScope (630072, r=brendan, a=blocker).
authorAndreas Gal <gal@mozilla.com>
Sun, 13 Feb 2011 20:55:33 -0800
changeset 62526 3a9a361402d2e052c143404602cf6bfcdafa63f2
parent 62525 7464e26236cb786b18d5f42573e2bdb8b47e188b
child 62527 da5f01b8838b58b3e2b6eb41e6f7d68d8aecea4b
push id1
push userroot
push dateTue, 10 Dec 2013 15:46:25 +0000
reviewersbrendan, blocker
bugs630072
milestone2.0b12pre
js_UnbrandAndClearSlots leaks, use JS_ClearScope and throw if compile-N-go scripts are run after JS_ClearScope (630072, r=brendan, a=blocker).
dom/base/nsJSEnvironment.cpp
js/src/js.msg
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsinterp.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsscope.h
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -3173,27 +3173,17 @@ nsJSContext::ClearScope(void *aGlobalObj
     // nothing else does.
     jsval window;
     if (!JS_GetProperty(mContext, obj, "window", &window)) {
       window = JSVAL_VOID;
 
       JS_ClearPendingException(mContext);
     }
 
-    // Hack fix for bug 611653. Originally, this always called JS_ClearScope,
-    // which was required to avoid leaks. But for native objects, the JS
-    // engine has an optimization that requires that permanent properties of
-    // the global object are never deleted. So instead, we call a new special
-    // API that clears the values of the global, thus avoiding leaks without
-    // deleting any properties.
-    if (obj->isNative()) {
-      js_UnbrandAndClearSlots(mContext, obj);
-    } else {
-      JS_ClearScope(mContext, obj);
-    }
+    JS_ClearScope(mContext, obj);
 
     if (xpc::WrapperFactory::IsXrayWrapper(obj)) {
       JS_ClearScope(mContext, &obj->getProxyExtra().toObject());
     }
 
     if (window != JSVAL_VOID) {
       if (!JS_DefineProperty(mContext, obj, "window", window,
                              JS_PropertyStub, JS_StrictPropertyStub,
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -343,9 +343,9 @@ MSG_DEF(JSMSG_SC_BAD_SERIALIZED_DATA, 26
 MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE,    261, 0, JSEXN_TYPEERR, "unsupported type for structured data")
 MSG_DEF(JSMSG_SC_RECURSION,           262, 0, JSEXN_INTERNALERR, "recursive object")
 MSG_DEF(JSMSG_CANT_WRAP_XML_OBJECT,   263, 0, JSEXN_TYPEERR, "can't wrap XML objects")
 MSG_DEF(JSMSG_BAD_CLONE_VERSION,      264, 0, JSEXN_ERR, "unsupported structured clone version")
 MSG_DEF(JSMSG_CANT_CLONE_OBJECT,      265, 0, JSEXN_TYPEERR, "can't clone object")
 MSG_DEF(JSMSG_NON_NATIVE_SCOPE,       266, 0, JSEXN_TYPEERR, "non-native scope object")
 MSG_DEF(JSMSG_STRICT_FUNCTION_STATEMENT, 267, 0, JSEXN_SYNTAXERR, "in strict mode code, functions may be declared only at top level or immediately within another function")
 MSG_DEF(JSMSG_INVALID_FOR_IN_INIT,    268, 0, JSEXN_SYNTAXERR, "for-in loop let declaration may not have an initializer")
-
+MSG_DEF(JSMSG_CLEARED_SCOPE,          269, 0, JSEXN_TYPEERR, "attempt to run compile-and-go script on a cleared scope")
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3057,18 +3057,18 @@ JS_NewGlobalObject(JSContext *cx, JSClas
     if (!obj)
         return NULL;
 
     obj->syncSpecialEquality();
 
     /* Construct a regexp statics object for this global object. */
     JSObject *res = regexp_statics_construct(cx, obj);
     if (!res ||
-        !js_SetReservedSlot(cx, obj, JSRESERVED_GLOBAL_REGEXP_STATICS,
-                            ObjectValue(*res))) {
+        !js_SetReservedSlot(cx, obj, JSRESERVED_GLOBAL_REGEXP_STATICS, ObjectValue(*res)) ||
+        !js_SetReservedSlot(cx, obj, JSRESERVED_GLOBAL_FLAGS, Int32Value(0))) {
         return NULL;
     }
 
     return obj;
 }
 
 JS_PUBLIC_API(JSObject *)
 JS_NewCompartmentAndGlobalObject(JSContext *cx, JSClass *clasp, JSPrincipals *principals)
@@ -3932,21 +3932,35 @@ JS_ClearScope(JSContext *cx, JSObject *o
     if (clearOp)
         clearOp(cx, obj);
 
     if (obj->isNative())
         js_ClearNative(cx, obj);
 
     /* Clear cached class objects on the global object. */
     if (obj->isGlobal()) {
+        /* This can return false but that doesn't mean it failed. */
+        obj->unbrand(cx);
+
         for (int key = JSProto_Null; key < JSProto_LIMIT * 3; key++)
             JS_SetReservedSlot(cx, obj, key, JSVAL_VOID);
 
+        /* Clear regexp statics. */
+        RegExpStatics::extractFrom(obj)->clear();
+
         /* Clear the CSP eval-is-allowed cache. */
         JS_SetReservedSlot(cx, obj, JSRESERVED_GLOBAL_EVAL_ALLOWED, JSVAL_VOID);
+
+        /*
+         * Mark global as cleared. If we try to execute any compile-and-go
+         * scripts from here on, we will throw.
+         */
+        int32 flags = obj->getReservedSlot(JSRESERVED_GLOBAL_FLAGS).toInt32();
+        flags |= JSGLOBAL_FLAGS_CLEARED;
+        JS_SetReservedSlot(cx, obj, JSRESERVED_GLOBAL_FLAGS, Jsvalify(Int32Value(flags)));
     }
 
     js_InitRandom(cx);
 }
 
 JS_PUBLIC_API(JSIdArray *)
 JS_Enumerate(JSContext *cx, JSObject *obj)
 {
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -1961,22 +1961,26 @@ struct JSClass {
 #define JSCLASS_MARK_IS_TRACE           (1<<(JSCLASS_HIGH_FLAGS_SHIFT+3))
 #define JSCLASS_INTERNAL_FLAG2          (1<<(JSCLASS_HIGH_FLAGS_SHIFT+4))
 
 /* Indicate whether the proto or ctor should be frozen. */
 #define JSCLASS_FREEZE_PROTO            (1<<(JSCLASS_HIGH_FLAGS_SHIFT+5))
 #define JSCLASS_FREEZE_CTOR             (1<<(JSCLASS_HIGH_FLAGS_SHIFT+6))
 
 /* Additional global reserved slots, beyond those for standard prototypes. */
-#define JSRESERVED_GLOBAL_SLOTS_COUNT     5
+#define JSRESERVED_GLOBAL_SLOTS_COUNT     6
 #define JSRESERVED_GLOBAL_THIS            (JSProto_LIMIT * 3)
 #define JSRESERVED_GLOBAL_THROWTYPEERROR  (JSRESERVED_GLOBAL_THIS + 1)
 #define JSRESERVED_GLOBAL_REGEXP_STATICS  (JSRESERVED_GLOBAL_THROWTYPEERROR + 1)
 #define JSRESERVED_GLOBAL_FUNCTION_NS     (JSRESERVED_GLOBAL_REGEXP_STATICS + 1)
 #define JSRESERVED_GLOBAL_EVAL_ALLOWED    (JSRESERVED_GLOBAL_FUNCTION_NS + 1)
+#define JSRESERVED_GLOBAL_FLAGS           (JSRESERVED_GLOBAL_EVAL_ALLOWED + 1)
+
+/* Global flags. */
+#define JSGLOBAL_FLAGS_CLEARED          0x1
 
 /*
  * ECMA-262 requires that most constructors used internally create objects
  * with "the original Foo.prototype value" as their [[Prototype]] (__proto__)
  * member initial value.  The "original ... value" verbiage is there because
  * in ECMA-262, global properties naming class objects are read/write and
  * deleteable, for the most part.
  *
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -614,16 +614,25 @@ NoSuchMethod(JSContext *cx, uintN argc, 
 
 namespace js {
 
 JS_REQUIRES_STACK bool
 RunScript(JSContext *cx, JSScript *script, JSStackFrame *fp)
 {
     JS_ASSERT(script);
 
+    /* FIXME: Once bug 470510 is fixed, make this an assert. */
+    if (script->compileAndGo) {
+        int32 flags = fp->scopeChain().getGlobal()->getReservedSlot(JSRESERVED_GLOBAL_FLAGS).toInt32();
+        if (flags & JSGLOBAL_FLAGS_CLEARED) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CLEARED_SCOPE);
+            return false;
+        }
+    }
+
 #ifdef JS_METHODJIT_SPEW
     JMCheckLogging();
 #endif
 
     AutoInterpPreparer prepareInterp(cx, script);
 
     JS_ASSERT(fp == cx->fp());
     JS_ASSERT(fp->script() == script);
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -4445,50 +4445,16 @@ JSObject::freeSlot(JSContext *cx, uint32
             last = slot;
             return true;
         }
     }
     vref.setUndefined();
     return false;
 }
 
-JS_FRIEND_API(void)
-js_UnbrandAndClearSlots(JSContext *cx, JSObject *obj)
-{
-    JS_ASSERT(obj->isNative());
-    JS_ASSERT(obj->isGlobal());
-
-    /* This can return false but that doesn't mean it failed. */
-    obj->unbrand(cx);
-
-    /*
-     * Clear the prototype cache. We must not clear the other global
-     * reserved slots, as other code will crash if they are arbitrarily
-     * reset (e.g., regexp statics).
-     */
-    for (int key = JSProto_Null; key < JSRESERVED_GLOBAL_THIS; key++)
-        JS_SetReservedSlot(cx, obj, key, JSVAL_VOID);
-
-    /*
-     * Clear the non-reserved slots.
-     */
-    ClearValueRange(obj->slots + JSCLASS_RESERVED_SLOTS(obj->clasp),
-                    obj->capacity - JSCLASS_RESERVED_SLOTS(obj->clasp),
-                    obj->clasp == &js_ArrayClass);
-
-    /*
-     * We just overwrote all slots to undefined, so the freelist has
-     * been trashed. We need to clear the head pointer or else we will
-     * crash later. This leaks slots but the object is all but dead
-     * anyway.
-     */
-    if (obj->hasPropertyTable())
-        obj->lastProperty()->getTable()->freelist = SHAPE_INVALID_SLOT;
-}
-
 /* JSBOXEDWORD_INT_MAX as a string */
 #define JSBOXEDWORD_INT_MAX_STRING "1073741823"
 
 /*
  * Convert string indexes that convert to int jsvals as ints to save memory.
  * Care must be taken to use this macro every time a property name is used, or
  * else double-sets, incorrect property cache misses, or other mistakes could
  * occur.
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1763,25 +1763,16 @@ js_SetPropertyHelper(JSContext *cx, JSOb
 /*
  * Change attributes for the given native property. The caller must ensure
  * that obj is locked and this function always unlocks obj on return.
  */
 extern JSBool
 js_SetNativeAttributes(JSContext *cx, JSObject *obj, js::Shape *shape,
                        uintN attrs);
 
-/*
- * Hack fix for bug 611653: Do not use for any other purpose.
- *
- * Unbrand and set all slot values to undefined (except reserved slots that
- * are not used for cached prototypes).
- */
-JS_FRIEND_API(void)
-js_UnbrandAndClearSlots(JSContext *cx, JSObject *obj);
-
 namespace js {
 
 /*
  * If obj has a data property methodid which is a function object for the given
  * native, return that function object. Otherwise, return NULL.
  */
 extern JSObject *
 HasNativeMethod(JSObject *obj, jsid methodid, Native native);
--- a/js/src/jsscope.h
+++ b/js/src/jsscope.h
@@ -292,17 +292,16 @@ CastAsPropertyOp(js::Class *clasp)
 
 struct Shape : public JSObjectMap
 {
     friend struct ::JSObject;
     friend struct ::JSFunction;
     friend class js::PropertyTree;
     friend class js::Bindings;
     friend bool IsShapeAboutToBeFinalized(JSContext *cx, const js::Shape *shape);
-    friend JS_FRIEND_API(void) ::js_UnbrandAndClearSlots(JSContext *cx, JSObject *obj);
 
     /* 
      * numLinearSearches starts at zero and is incremented initially on each
      * search() call.  Once numLinearSearches reaches MAX_LINEAR_SEARCHES
      * (which is a small integer), the table is created on the next search()
      * call, and the table pointer will be easily distinguishable from a small
      * integer.  The table can also be created when hashifying for dictionary
      * mode.