Bug 889543 - Use AutoCycleDetector instead of CloneMemory; r=till
authorTerrence Cole <terrence@mozilla.com>
Tue, 11 Mar 2014 09:00:16 -0700
changeset 176528 d29c145b465c21c40f96faf5d6cbe19e0874dcad
parent 176527 0228344cc188bdfd1ca0eb75b921904baa58c3fe
child 176529 456f206bd24f6442998889dad143dea74d205bb6
push id26524
push userryanvm@gmail.com
push dateTue, 01 Apr 2014 20:44:18 +0000
treeherdermozilla-central@0ff6afce0133 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs889543
milestone31.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 889543 - Use AutoCycleDetector instead of CloneMemory; r=till
js/src/vm/SelfHosting.cpp
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -965,115 +965,111 @@ JSRuntime::markSelfHostingGlobal(JSTrace
 }
 
 bool
 JSRuntime::isSelfHostingCompartment(JSCompartment *comp)
 {
     return selfHostingGlobal_->compartment() == comp;
 }
 
-// CloneMemory maps objects to each other which may be in different
-// runtimes. This class should only be used within an AutoSuppressGC,
-// so that issues of managing which objects should be traced can be ignored.
-typedef HashMap<JSObject *, JSObject *> CloneMemory;
+static bool
+CloneValue(JSContext *cx, HandleValue selfHostedValue, MutableHandleValue vp);
 
 static bool
-CloneValue(JSContext *cx, const Value &selfHostedValue, MutableHandleValue vp, CloneMemory &clonedObjects);
-
-static bool
-GetUnclonedValue(JSContext *cx, JSObject *selfHostedObject, jsid id, Value *vp)
+GetUnclonedValue(JSContext *cx, HandleObject selfHostedObject, HandleId id, MutableHandleValue vp)
 {
-    *vp = UndefinedValue();
+    vp.setUndefined();
 
     if (JSID_IS_INT(id)) {
         size_t index = JSID_TO_INT(id);
         if (index < selfHostedObject->getDenseInitializedLength() &&
             !selfHostedObject->getDenseElement(index).isMagic(JS_ELEMENTS_HOLE))
         {
-            *vp = selfHostedObject->getDenseElement(JSID_TO_INT(id));
+            vp.set(selfHostedObject->getDenseElement(JSID_TO_INT(id)));
             return true;
         }
     }
 
     // Since all atoms used by self hosting are marked as permanent, any
     // attempt to look up a non-permanent atom will fail. We should only
     // see such atoms when code is looking for properties on the self
     // hosted global which aren't present.
     if (JSID_IS_STRING(id) && !JSID_TO_STRING(id)->isPermanentAtom()) {
         JS_ASSERT(selfHostedObject->is<GlobalObject>());
-        gc::AutoSuppressGC suppress(cx);
         RootedValue value(cx, IdToValue(id));
         return js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_NO_SUCH_SELF_HOSTED_PROP,
                                         JSDVG_IGNORE_STACK, value, NullPtr(), nullptr, nullptr);
     }
 
-    Shape *shape = selfHostedObject->nativeLookupPure(id);
+    RootedShape shape(cx, selfHostedObject->nativeLookupPure(id));
     if (!shape) {
-        gc::AutoSuppressGC suppress(cx);
         RootedValue value(cx, IdToValue(id));
         return js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_NO_SUCH_SELF_HOSTED_PROP,
                                         JSDVG_IGNORE_STACK, value, NullPtr(), nullptr, nullptr);
     }
 
     JS_ASSERT(shape->hasSlot() && shape->hasDefaultGetter());
-    *vp = selfHostedObject->getSlot(shape->slot());
+    vp.set(selfHostedObject->getSlot(shape->slot()));
     return true;
 }
 
 static bool
-CloneProperties(JSContext *cx, JSObject *selfHostedObject,
-                HandleObject clone, CloneMemory &clonedObjects)
+CloneProperties(JSContext *cx, HandleObject selfHostedObject, HandleObject clone)
 {
-    Vector<jsid> ids(cx);
+    AutoIdVector ids(cx);
 
     for (size_t i = 0; i < selfHostedObject->getDenseInitializedLength(); i++) {
         if (!selfHostedObject->getDenseElement(i).isMagic(JS_ELEMENTS_HOLE)) {
             if (!ids.append(INT_TO_JSID(i)))
                 return false;
         }
     }
 
     for (Shape::Range<NoGC> range(selfHostedObject->lastProperty()); !range.empty(); range.popFront()) {
         Shape &shape = range.front();
         if (shape.enumerable() && !ids.append(shape.propid()))
             return false;
     }
 
     RootedId id(cx);
     RootedValue val(cx);
+    RootedValue selfHostedValue(cx);
     for (uint32_t i = 0; i < ids.length(); i++) {
         id = ids[i];
-        Value selfHostedValue;
         if (!GetUnclonedValue(cx, selfHostedObject, id, &selfHostedValue))
             return false;
-        if (!CloneValue(cx, selfHostedValue, &val, clonedObjects) ||
+        if (!CloneValue(cx, selfHostedValue, &val) ||
             !JS_DefinePropertyById(cx, clone, id, val.get(), nullptr, nullptr, 0))
         {
             return false;
         }
     }
 
     return true;
 }
 
 static JSObject *
-CloneObject(JSContext *cx, JSObject *selfHostedObject, CloneMemory &clonedObjects)
+CloneObject(JSContext *cx, HandleObject selfHostedObject)
 {
-    DependentAddPtr<CloneMemory> p(cx, clonedObjects, selfHostedObject);
-    if (p)
-        return p->value();
+    AutoCycleDetector detect(cx, selfHostedObject);
+    if (!detect.init())
+        return nullptr;
+    if (detect.foundCycle()) {
+        JS_ReportError(cx, "SelfHosted cloning cannot handle cyclic object graphs.");
+        return nullptr;
+    }
+
     RootedObject clone(cx);
     if (selfHostedObject->is<JSFunction>()) {
-        JSFunction *selfHostedFunction = &selfHostedObject->as<JSFunction>();
+        RootedFunction selfHostedFunction(cx, &selfHostedObject->as<JSFunction>());
         bool hasName = selfHostedFunction->atom() != nullptr;
         js::gc::AllocKind kind = hasName
                                  ? JSFunction::ExtendedFinalizeKind
                                  : selfHostedFunction->getAllocKind();
-        clone = CloneFunctionObject(cx, HandleFunction::fromMarkedLocation(&selfHostedFunction),
-                                    cx->global(), kind, TenuredObject);
+        clone = CloneFunctionObject(cx, selfHostedFunction, cx->global(), kind, TenuredObject);
         // To be able to re-lazify the cloned function, its name in the
         // self-hosting compartment has to be stored on the clone.
         if (clone && hasName)
             clone->as<JSFunction>().setExtendedSlot(0, StringValue(selfHostedFunction->atom()));
     } else if (selfHostedObject->is<RegExpObject>()) {
         RegExpObject &reobj = selfHostedObject->as<RegExpObject>();
         RootedAtom source(cx, reobj.getSource());
         JS_ASSERT(source->isPermanentAtom());
@@ -1099,60 +1095,56 @@ CloneObject(JSContext *cx, JSObject *sel
     } else {
         JS_ASSERT(selfHostedObject->isNative());
         clone = NewObjectWithGivenProto(cx, selfHostedObject->getClass(), nullptr, cx->global(),
                                         selfHostedObject->tenuredGetAllocKind(),
                                         SingletonObject);
     }
     if (!clone)
         return nullptr;
-    if (!p.add(cx, clonedObjects, selfHostedObject, clone))
+    if (!CloneProperties(cx, selfHostedObject, clone))
         return nullptr;
-    if (!CloneProperties(cx, selfHostedObject, clone, clonedObjects)) {
-        clonedObjects.remove(selfHostedObject);
-        return nullptr;
-    }
     return clone;
 }
 
 static bool
-CloneValue(JSContext *cx, const Value &selfHostedValue, MutableHandleValue vp, CloneMemory &clonedObjects)
+CloneValue(JSContext *cx, HandleValue selfHostedValue, MutableHandleValue vp)
 {
     if (selfHostedValue.isObject()) {
-        JSObject *selfHostedObject = &selfHostedValue.toObject();
-        RootedObject clone(cx, CloneObject(cx, selfHostedObject, clonedObjects));
+        RootedObject selfHostedObject(cx, &selfHostedValue.toObject());
+        JSObject *clone = CloneObject(cx, selfHostedObject);
         if (!clone)
             return false;
         vp.setObject(*clone);
     } else if (selfHostedValue.isBoolean() || selfHostedValue.isNumber() || selfHostedValue.isNullOrUndefined()) {
         // Nothing to do here: these are represented inline in the value.
         vp.set(selfHostedValue);
     } else if (selfHostedValue.isString()) {
         if (!selfHostedValue.toString()->isFlat())
             MOZ_CRASH();
         JSFlatString *selfHostedString = &selfHostedValue.toString()->asFlat();
-        RootedString clone(cx, js_NewStringCopyN<CanGC>(cx,
-                                                        selfHostedString->chars(),
-                                                        selfHostedString->length()));
+        JSString *clone = js_NewStringCopyN<CanGC>(cx,
+                                                   selfHostedString->chars(),
+                                                   selfHostedString->length());
         if (!clone)
             return false;
         vp.setString(clone);
     } else {
-        MOZ_ASSUME_UNREACHABLE("Self-hosting CloneValue can't clone given value.");
+        MOZ_CRASH("Self-hosting CloneValue can't clone given value.");
     }
     return true;
 }
 
 bool
-JSRuntime::cloneSelfHostedFunctionScript(JSContext *cx, Handle<PropertyName*> name,
-                                         Handle<JSFunction*> targetFun)
+JSRuntime::cloneSelfHostedFunctionScript(JSContext *cx, HandlePropertyName name,
+                                         HandleFunction targetFun)
 {
     RootedId id(cx, NameToId(name));
-    Value funVal;
-    if (!GetUnclonedValue(cx, selfHostingGlobal_, id, &funVal))
+    RootedValue funVal(cx);
+    if (!GetUnclonedValue(cx, HandleObject::fromMarkedLocation(&selfHostingGlobal_), id, &funVal))
         return false;
 
     RootedFunction sourceFun(cx, &funVal.toObject().as<JSFunction>());
     // JSFunction::generatorKind can't handle lazy self-hosted functions, so we make sure there
     // aren't any.
     JS_ASSERT(!sourceFun->isGenerator());
     RootedScript sourceScript(cx, sourceFun->getOrCreateScript(cx));
     if (!sourceScript)
@@ -1168,37 +1160,34 @@ JSRuntime::cloneSelfHostedFunctionScript
     targetFun->setFlags((targetFun->flags() & ~JSFunction::INTERPRETED_LAZY) |
                         sourceFun->flags() | JSFunction::EXTENDED);
     targetFun->setScript(cscript);
     JS_ASSERT(targetFun->isExtended());
     return true;
 }
 
 bool
-JSRuntime::cloneSelfHostedValue(JSContext *cx, Handle<PropertyName*> name, MutableHandleValue vp)
+JSRuntime::cloneSelfHostedValue(JSContext *cx, HandlePropertyName name, MutableHandleValue vp)
 {
     RootedId id(cx, NameToId(name));
-    Value selfHostedValue;
-    if (!GetUnclonedValue(cx, selfHostingGlobal_, id, &selfHostedValue))
+    RootedValue selfHostedValue(cx);
+    if (!GetUnclonedValue(cx, HandleObject::fromMarkedLocation(&selfHostingGlobal_), id, &selfHostedValue))
         return false;
 
     /*
      * We don't clone if we're operating in the self-hosting global, as that
      * means we're currently executing the self-hosting script while
      * initializing the runtime (see JSRuntime::initSelfHosting).
      */
-    if (cx->global() != selfHostingGlobal_) {
-        gc::AutoSuppressGC suppress(cx);
-        CloneMemory clonedObjects(cx);
-        if (!clonedObjects.init() || !CloneValue(cx, selfHostedValue, vp, clonedObjects))
-            return false;
-    } else {
+    if (cx->global() == selfHostingGlobal_) {
         vp.set(selfHostedValue);
+        return true;
     }
-    return true;
+
+    return CloneValue(cx, selfHostedValue, vp);
 }
 
 JSFunction *
 js::SelfHostedFunction(JSContext *cx, HandlePropertyName propName)
 {
     RootedValue func(cx);
     if (!GlobalObject::getIntrinsicValue(cx, cx->global(), propName, &func))
         return nullptr;