Bug 1022773 - Return value rooting for JS engine, r=terrence, a=abillings
authorSteve Fink <sfink@mozilla.com>
Tue, 01 Jul 2014 14:23:32 -0700
changeset 207556 07d419bd7fd0ef4244d27b8f9b5dbd56b076d2e5
parent 207555 17e4424469045396e42f3c8067fabda05a47b67a
child 207557 97ef8ef6033f0eb4947e2230fffeef5f36c0f5f4
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, abillings
bugs1022773
milestone32.0a2
Bug 1022773 - Return value rooting for JS engine, r=terrence, a=abillings
js/jsd/jsd_stak.cpp
js/src/builtin/Eval.cpp
js/src/jsapi.cpp
js/src/jscompartment.cpp
js/src/jscompartment.h
js/src/jsfun.cpp
js/src/jsscript.cpp
js/src/jsscript.h
js/src/vm/Debugger.cpp
js/src/vm/ForkJoin.cpp
js/src/vm/Stack.cpp
--- a/js/jsd/jsd_stak.cpp
+++ b/js/jsd/jsd_stak.cpp
@@ -465,32 +465,34 @@ jsd_EvaluateScriptInStackFrame(JSDContex
 
 JSString*
 jsd_ValToStringInStackFrame(JSDContext* jsdc, 
                             JSDThreadState* jsdthreadstate,
                             JSDStackFrameInfo* jsdframe,
                             jsval val)
 {
     bool valid;
-    JSString* retval;
     JSExceptionState* exceptionState;
+    JSContext *cx = jsdthreadstate->context;
 
     JSD_LOCK_THREADSTATES(jsdc);
     valid = jsd_IsValidFrameInThreadState(jsdc, jsdthreadstate, jsdframe);
     JSD_UNLOCK_THREADSTATES(jsdc);
 
     if( ! valid )
         return nullptr;
 
-    AutoPushJSContext cx(jsdthreadstate->context);
-
+    JS::RootedString retval(cx);
     JS::RootedValue v(cx, val);
-    exceptionState = JS_SaveExceptionState(cx);
-    retval = JS::ToString(cx, v);
-    JS_RestoreExceptionState(cx, exceptionState);
+    {
+        AutoPushJSContext cx(jsdthreadstate->context);
+        exceptionState = JS_SaveExceptionState(cx);
+        retval = JS::ToString(cx, v);
+        JS_RestoreExceptionState(cx, exceptionState);
+    }
 
     return retval;
 }
 
 bool
 jsd_IsValidThreadState(JSDContext*        jsdc, 
                        JSDThreadState*    jsdthreadstate)
 {
--- a/js/src/builtin/Eval.cpp
+++ b/js/src/builtin/Eval.cpp
@@ -266,17 +266,17 @@ EvalKernel(JSContext *cx, const CallArgs
         return ejr == EvalJSON_Success;
 
     EvalScriptGuard esg(cx);
 
     if (evalType == DIRECT_EVAL && caller.isNonEvalFunctionFrame())
         esg.lookupInEvalCache(flatStr, callerScript, pc);
 
     if (!esg.foundScript()) {
-        JSScript *maybeScript;
+        RootedScript maybeScript(cx);
         unsigned lineno;
         const char *filename;
         JSPrincipals *originPrincipals;
         uint32_t pcOffset;
         DescribeScriptedCallerForCompilation(cx, &maybeScript, &filename, &lineno, &pcOffset,
                                              &originPrincipals,
                                              evalType == DIRECT_EVAL
                                              ? CALLED_FROM_JSOP_EVAL
@@ -336,17 +336,17 @@ js::DirectEvalStringFromIon(JSContext *c
     if (ejr != EvalJSON_NotJSON)
         return ejr == EvalJSON_Success;
 
     EvalScriptGuard esg(cx);
 
     esg.lookupInEvalCache(flatStr, callerScript, pc);
 
     if (!esg.foundScript()) {
-        JSScript *maybeScript;
+        RootedScript maybeScript(cx);
         const char *filename;
         unsigned lineno;
         JSPrincipals *originPrincipals;
         uint32_t pcOffset;
         DescribeScriptedCallerForCompilation(cx, &maybeScript, &filename, &lineno, &pcOffset,
                                               &originPrincipals, CALLED_FROM_JSOP_EVAL);
 
         const char *introducerFilename = filename;
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1085,64 +1085,68 @@ JS_WrapId(JSContext *cx, JS::MutableHand
 JS_PUBLIC_API(JSObject *)
 JS_TransplantObject(JSContext *cx, HandleObject origobj, HandleObject target)
 {
     AssertHeapIsIdle(cx);
     JS_ASSERT(origobj != target);
     JS_ASSERT(!origobj->is<CrossCompartmentWrapperObject>());
     JS_ASSERT(!target->is<CrossCompartmentWrapperObject>());
 
-    AutoMaybeTouchDeadZones agc(cx);
-    AutoDisableProxyCheck adpc(cx->runtime());
-
-    JSCompartment *destination = target->compartment();
     RootedValue origv(cx, ObjectValue(*origobj));
     RootedObject newIdentity(cx);
 
-    if (origobj->compartment() == destination) {
-        // If the original object is in the same compartment as the
-        // destination, then we know that we won't find a wrapper in the
-        // destination's cross compartment map and that the same
-        // object will continue to work.
-        if (!JSObject::swap(cx, origobj, target))
-            MOZ_CRASH();
-        newIdentity = origobj;
-    } else if (WrapperMap::Ptr p = destination->lookupWrapper(origv)) {
-        // There might already be a wrapper for the original object in
-        // the new compartment. If there is, we use its identity and swap
-        // in the contents of |target|.
-        newIdentity = &p->value().get().toObject();
-
-        // When we remove origv from the wrapper map, its wrapper, newIdentity,
-        // must immediately cease to be a cross-compartment wrapper. Neuter it.
-        destination->removeWrapper(p);
-        NukeCrossCompartmentWrapper(cx, newIdentity);
-
-        if (!JSObject::swap(cx, newIdentity, target))
+    {
+        // Scope to make ~AutoMaybeTouchDeadZones do its GC before the return value is on the stack.
+        AutoMaybeTouchDeadZones agc(cx);
+        AutoDisableProxyCheck adpc(cx->runtime());
+
+        JSCompartment *destination = target->compartment();
+
+        if (origobj->compartment() == destination) {
+            // If the original object is in the same compartment as the
+            // destination, then we know that we won't find a wrapper in the
+            // destination's cross compartment map and that the same
+            // object will continue to work.
+            if (!JSObject::swap(cx, origobj, target))
+                MOZ_CRASH();
+            newIdentity = origobj;
+        } else if (WrapperMap::Ptr p = destination->lookupWrapper(origv)) {
+            // There might already be a wrapper for the original object in
+            // the new compartment. If there is, we use its identity and swap
+            // in the contents of |target|.
+            newIdentity = &p->value().get().toObject();
+
+            // When we remove origv from the wrapper map, its wrapper, newIdentity,
+            // must immediately cease to be a cross-compartment wrapper. Neuter it.
+            destination->removeWrapper(p);
+            NukeCrossCompartmentWrapper(cx, newIdentity);
+
+            if (!JSObject::swap(cx, newIdentity, target))
+                MOZ_CRASH();
+        } else {
+            // Otherwise, we use |target| for the new identity object.
+            newIdentity = target;
+        }
+
+        // Now, iterate through other scopes looking for references to the
+        // old object, and update the relevant cross-compartment wrappers.
+        if (!RemapAllWrappersForObject(cx, origobj, newIdentity))
             MOZ_CRASH();
-    } else {
-        // Otherwise, we use |target| for the new identity object.
-        newIdentity = target;
-    }
-
-    // Now, iterate through other scopes looking for references to the
-    // old object, and update the relevant cross-compartment wrappers.
-    if (!RemapAllWrappersForObject(cx, origobj, newIdentity))
-        MOZ_CRASH();
-
-    // Lastly, update the original object to point to the new one.
-    if (origobj->compartment() != destination) {
-        RootedObject newIdentityWrapper(cx, newIdentity);
-        AutoCompartment ac(cx, origobj);
-        if (!JS_WrapObject(cx, &newIdentityWrapper))
-            MOZ_CRASH();
-        JS_ASSERT(Wrapper::wrappedObject(newIdentityWrapper) == newIdentity);
-        if (!JSObject::swap(cx, origobj, newIdentityWrapper))
-            MOZ_CRASH();
-        origobj->compartment()->putWrapper(cx, CrossCompartmentKey(newIdentity), origv);
+
+        // Lastly, update the original object to point to the new one.
+        if (origobj->compartment() != destination) {
+            RootedObject newIdentityWrapper(cx, newIdentity);
+            AutoCompartment ac(cx, origobj);
+            if (!JS_WrapObject(cx, &newIdentityWrapper))
+                MOZ_CRASH();
+            JS_ASSERT(Wrapper::wrappedObject(newIdentityWrapper) == newIdentity);
+            if (!JSObject::swap(cx, origobj, newIdentityWrapper))
+                MOZ_CRASH();
+            origobj->compartment()->putWrapper(cx, CrossCompartmentKey(newIdentity), origv);
+        }
     }
 
     // The new identity object might be one of several things. Return it to avoid
     // ambiguity.
     return newIdentity;
 }
 
 /*
@@ -4712,21 +4716,26 @@ JS::CompileOffThread(JSContext *cx, cons
 }
 
 JS_PUBLIC_API(JSScript *)
 JS::FinishOffThreadScript(JSContext *maybecx, JSRuntime *rt, void *token)
 {
 #ifdef JS_THREADSAFE
     JS_ASSERT(CurrentThreadCanAccessRuntime(rt));
 
-    Maybe<AutoLastFrameCheck> lfc;
-    if (maybecx)
-        lfc.construct(maybecx);
-
-    return HelperThreadState().finishParseTask(maybecx, rt, token);
+    if (maybecx) {
+        RootedScript script(maybecx);
+        {
+            AutoLastFrameCheck lfc(maybecx);
+            script = HelperThreadState().finishParseTask(maybecx, rt, token);
+        }
+        return script;
+    } else {
+        return HelperThreadState().finishParseTask(maybecx, rt, token);
+    }
 #else
     MOZ_ASSUME_UNREACHABLE("Off thread compilation is not available.");
 #endif
 }
 
 JS_PUBLIC_API(JSScript *)
 JS_CompileScript(JSContext *cx, JS::HandleObject obj, const char *ascii,
                  size_t length, const JS::CompileOptions &options)
@@ -4790,19 +4799,19 @@ JS_PUBLIC_API(bool)
 JS::CompileFunction(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
                     const char *name, unsigned nargs, const char *const *argnames,
                     SourceBufferHolder &srcBuf, MutableHandleFunction fun)
 {
     JS_ASSERT(!cx->runtime()->isAtomsCompartment(cx->compartment()));
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
+    RootedAtom funAtom(cx);
     AutoLastFrameCheck lfc(cx);
 
-    RootedAtom funAtom(cx);
     if (name) {
         funAtom = Atomize(cx, name, strlen(name));
         if (!funAtom)
             return false;
     }
 
     AutoNameVector formals(cx);
     for (unsigned i = 0; i < nargs; i++) {
@@ -5201,23 +5210,22 @@ JS::Call(JSContext *cx, HandleValue this
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, thisv, fval, args);
     AutoLastFrameCheck lfc(cx);
 
     return Invoke(cx, thisv, fval, args.length(), args.begin(), rval);
 }
 
-JS_PUBLIC_API(JSObject *)
-JS_New(JSContext *cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
+static JSObject *
+JS_NewHelper(JSContext *cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, ctor, inputArgs);
-    AutoLastFrameCheck lfc(cx);
 
     // This is not a simple variation of JS_CallFunctionValue because JSOP_NEW
     // is not a simple variation of JSOP_CALL. We have to determine what class
     // of object to create, create it, and clamp the return value to an object,
     // among other details. InvokeConstructor does the hard work.
     InvokeArgs args(cx);
     if (!args.init(inputArgs.length()))
         return nullptr;
@@ -5240,16 +5248,27 @@ JS_New(JSContext *cx, HandleObject ctor,
                                  bytes.ptr());
         }
         return nullptr;
     }
 
     return &args.rval().toObject();
 }
 
+JS_PUBLIC_API(JSObject *)
+JS_New(JSContext *cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
+{
+    RootedObject obj(cx);
+    {
+        AutoLastFrameCheck lfc(cx);
+        obj = JS_NewHelper(cx, ctor, inputArgs);
+    }
+    return obj;
+}
+
 JS_PUBLIC_API(JSInterruptCallback)
 JS_SetInterruptCallback(JSRuntime *rt, JSInterruptCallback callback)
 {
     JSInterruptCallback old = rt->interruptCallback;
     rt->interruptCallback = callback;
     return old;
 }
 
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -920,17 +920,17 @@ JSCompartment::removeDebuggeeUnderGC(Fre
     if (debuggees.empty()) {
         debugModeBits &= ~DebugFromJS;
         if (wasEnabled && !debugMode())
             DebugScopes::onCompartmentLeaveDebugMode(this);
     }
 }
 
 void
-JSCompartment::clearBreakpointsIn(FreeOp *fop, js::Debugger *dbg, JSObject *handler)
+JSCompartment::clearBreakpointsIn(FreeOp *fop, js::Debugger *dbg, HandleObject handler)
 {
     for (gc::ZoneCellIter i(zone(), gc::FINALIZE_SCRIPT); !i.done(); i.next()) {
         JSScript *script = i.get<JSScript>();
         if (script->compartment() == this && script->hasAnyBreakpointsOrStepMode())
             script->clearBreakpointsIn(fop, dbg, handler);
     }
 }
 
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@@ -418,17 +418,17 @@ struct JSCompartment
     void removeDebuggeeUnderGC(js::FreeOp *fop, js::GlobalObject *global,
                                js::GlobalObjectSet::Enum *debuggeesEnum = nullptr);
     void removeDebuggeeUnderGC(js::FreeOp *fop, js::GlobalObject *global,
                                js::AutoDebugModeInvalidation &invalidate,
                                js::GlobalObjectSet::Enum *debuggeesEnum = nullptr);
     bool setDebugModeFromC(JSContext *cx, bool b,
                            js::AutoDebugModeInvalidation &invalidate);
 
-    void clearBreakpointsIn(js::FreeOp *fop, js::Debugger *dbg, JSObject *handler);
+    void clearBreakpointsIn(js::FreeOp *fop, js::Debugger *dbg, JS::HandleObject handler);
     void clearTraps(js::FreeOp *fop);
 
   private:
     void sweepBreakpoints(js::FreeOp *fop);
 
   public:
     js::WatchpointMap *watchpointMap;
 
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1528,17 +1528,17 @@ FunctionConstructor(JSContext *cx, unsig
     AutoKeepAtoms keepAtoms(cx->perThreadData);
     AutoNameVector formals(cx);
 
     bool hasRest = false;
 
     bool isStarGenerator = generatorKind == StarGenerator;
     JS_ASSERT(generatorKind != LegacyGenerator);
 
-    JSScript *maybeScript = nullptr;
+    RootedScript maybeScript(cx);
     const char *filename;
     unsigned lineno;
     JSPrincipals *originPrincipals;
     uint32_t pcOffset;
     DescribeScriptedCallerForCompilation(cx, &maybeScript, &filename, &lineno, &pcOffset,
                                          &originPrincipals);
 
     const char *introductionType = "Function";
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -2805,57 +2805,57 @@ js_GetScriptLineExtent(JSScript *script)
         if (maxLineNo < lineno)
             maxLineNo = lineno;
     }
 
     return 1 + maxLineNo - script->lineno();
 }
 
 void
-js::DescribeScriptedCallerForCompilation(JSContext *cx, JSScript **maybeScript,
+js::DescribeScriptedCallerForCompilation(JSContext *cx, MutableHandleScript maybeScript,
                                          const char **file, unsigned *linenop,
                                          uint32_t *pcOffset, JSPrincipals **origin,
                                          LineOption opt)
 {
     if (opt == CALLED_FROM_JSOP_EVAL) {
         jsbytecode *pc = nullptr;
-        *maybeScript = cx->currentScript(&pc);
+        maybeScript.set(cx->currentScript(&pc));
         JS_ASSERT(JSOp(*pc) == JSOP_EVAL || JSOp(*pc) == JSOP_SPREADEVAL);
         JS_ASSERT(*(pc + (JSOp(*pc) == JSOP_EVAL ? JSOP_EVAL_LENGTH
                                                  : JSOP_SPREADEVAL_LENGTH)) == JSOP_LINENO);
-        *file = (*maybeScript)->filename();
+        *file = maybeScript->filename();
         *linenop = GET_UINT16(pc + (JSOp(*pc) == JSOP_EVAL ? JSOP_EVAL_LENGTH
                                                            : JSOP_SPREADEVAL_LENGTH));
-        *pcOffset = pc - (*maybeScript)->code();
-        *origin = (*maybeScript)->originPrincipals();
+        *pcOffset = pc - maybeScript->code();
+        *origin = maybeScript->originPrincipals();
         return;
     }
 
     NonBuiltinFrameIter iter(cx);
 
     if (iter.done()) {
-        *maybeScript = nullptr;
+        maybeScript.set(nullptr);
         *file = nullptr;
         *linenop = 0;
         *pcOffset = 0;
         *origin = cx->compartment()->principals;
         return;
     }
 
     *file = iter.scriptFilename();
     *linenop = iter.computeLine();
     *origin = iter.originPrincipals();
 
     // These values are only used for introducer fields which are debugging
     // information and can be safely left null for asm.js frames.
     if (iter.hasScript()) {
-        *maybeScript = iter.script();
-        *pcOffset = iter.pc() - (*maybeScript)->code();
+        maybeScript.set(iter.script());
+        *pcOffset = iter.pc() - maybeScript->code();
     } else {
-        *maybeScript = nullptr;
+        maybeScript.set(nullptr);
         *pcOffset = 0;
     }
 }
 
 template <class T>
 static inline T *
 Rebase(JSScript *dst, JSScript *src, T *srcp)
 {
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -1977,17 +1977,17 @@ PCToLineNumber(unsigned startLine, jssrc
  */
 
 enum LineOption {
     CALLED_FROM_JSOP_EVAL,
     NOT_CALLED_FROM_JSOP_EVAL
 };
 
 extern void
-DescribeScriptedCallerForCompilation(JSContext *cx, JSScript **maybeScript,
+DescribeScriptedCallerForCompilation(JSContext *cx, MutableHandleScript maybeScript,
                                      const char **file, unsigned *linenop,
                                      uint32_t *pcOffset, JSPrincipals **origin,
                                      LineOption opt = NOT_CALLED_FROM_JSOP_EVAL);
 
 bool
 CloneFunctionScript(JSContext *cx, HandleFunction original, HandleFunction clone,
                     NewObjectKind newKind = GenericObject);
 
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2192,17 +2192,17 @@ Debugger::getNewestFrame(JSContext *cx, 
 }
 
 bool
 Debugger::clearAllBreakpoints(JSContext *cx, unsigned argc, Value *vp)
 {
     THIS_DEBUGGER(cx, argc, vp, "clearAllBreakpoints", args, dbg);
     for (GlobalObjectSet::Range r = dbg->debuggees.all(); !r.empty(); r.popFront())
         r.front()->compartment()->clearBreakpointsIn(cx->runtime()->defaultFreeOp(),
-                                                     dbg, nullptr);
+                                                     dbg, NullPtr());
     return true;
 }
 
 bool
 Debugger::construct(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
--- a/js/src/vm/ForkJoin.cpp
+++ b/js/src/vm/ForkJoin.cpp
@@ -1927,17 +1927,17 @@ class ParallelSpewer
     }
 
     void beginOp(JSContext *cx, const char *name) {
         if (!active[SpewOps])
             return;
 
         if (cx) {
             jsbytecode *pc;
-            JSScript *script = cx->currentScript(&pc);
+            RootedScript script(cx, cx->currentScript(&pc));
             if (script && pc) {
                 NonBuiltinScriptFrameIter iter(cx);
                 if (iter.done()) {
                     spew(SpewOps, "%sBEGIN %s%s (%s:%u)", bold(), name, reset(),
                          script->filename(), PCToLineNumber(script, pc));
                 } else {
                     spew(SpewOps, "%sBEGIN %s%s (%s:%u -> %s:%u)", bold(), name, reset(),
                          iter.script()->filename(), PCToLineNumber(iter.script(), iter.pc()),
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -11,32 +11,33 @@
 #include "jscntxt.h"
 
 #include "gc/Marking.h"
 #ifdef JS_ION
 #include "jit/AsmJSModule.h"
 #include "jit/BaselineFrame.h"
 #include "jit/JitCompartment.h"
 #endif
+#include "js/GCAPI.h"
 #include "vm/Opcodes.h"
 
 #include "jit/JitFrameIterator-inl.h"
 #include "vm/Interpreter-inl.h"
 #include "vm/Probes-inl.h"
 #include "vm/ScopeObject-inl.h"
 
 using namespace js;
 
 using mozilla::PodCopy;
 
 /*****************************************************************************/
 
 void
 InterpreterFrame::initExecuteFrame(JSContext *cx, JSScript *script, AbstractFramePtr evalInFramePrev,
-                             const Value &thisv, JSObject &scopeChain, ExecuteType type)
+                                   const Value &thisv, JSObject &scopeChain, ExecuteType type)
 {
     /*
      * See encoding of ExecuteType. When GLOBAL isn't set, we are executing a
      * script in the context of another frame and the frame type is determined
      * by the context.
      */
     flags_ = type | HAS_SCOPECHAIN;
 
@@ -663,26 +664,30 @@ FrameIter::Data::Data(const FrameIter::D
 }
 
 FrameIter::FrameIter(ThreadSafeContext *cx, SavedOption savedOption)
   : data_(cx, savedOption, CURRENT_CONTEXT, nullptr)
 #ifdef JS_ION
   , ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr)
 #endif
 {
+    // settleOnActivation can only GC if principals are given.
+    JS::AutoSuppressGCAnalysis nogc;
     settleOnActivation();
 }
 
 FrameIter::FrameIter(ThreadSafeContext *cx, ContextOption contextOption,
                      SavedOption savedOption)
   : data_(cx, savedOption, contextOption, nullptr)
 #ifdef JS_ION
   , ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr)
 #endif
 {
+    // settleOnActivation can only GC if principals are given.
+    JS::AutoSuppressGCAnalysis nogc;
     settleOnActivation();
 }
 
 FrameIter::FrameIter(JSContext *cx, ContextOption contextOption,
                      SavedOption savedOption, JSPrincipals *principals)
   : data_(cx, savedOption, contextOption, principals)
 #ifdef JS_ION
   , ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr)