From e193d1faaaa2a85f27f422c0e9035f14679d5210 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Wed, 25 Jun 2014 15:35:36 -0700 Subject: [PATCH] Bug 1022773 - Return value rooting for JS engine, r=terrence --HG-- extra : rebase_source : 3d0934277a0edb7d3a22c12823b50840de9f6161 --- js/src/builtin/Eval.cpp | 4 +- js/src/jsapi.cpp | 117 +++++++++++++++++++++++---------------- js/src/jscompartment.cpp | 2 +- js/src/jscompartment.h | 2 +- js/src/jsfun.cpp | 2 +- js/src/jsscript.cpp | 18 +++--- js/src/jsscript.h | 2 +- js/src/vm/Debugger.cpp | 2 +- js/src/vm/ForkJoin.cpp | 2 +- js/src/vm/Stack.cpp | 7 ++- 10 files changed, 91 insertions(+), 67 deletions(-) diff --git a/js/src/builtin/Eval.cpp b/js/src/builtin/Eval.cpp index ebfccbfd7fa..76bc6889ce9 100644 --- a/js/src/builtin/Eval.cpp +++ b/js/src/builtin/Eval.cpp @@ -311,7 +311,7 @@ EvalKernel(JSContext *cx, const CallArgs &args, EvalType evalType, AbstractFrame esg.lookupInEvalCache(flatStr, callerScript, pc); if (!esg.foundScript()) { - JSScript *maybeScript; + RootedScript maybeScript(cx); unsigned lineno; const char *filename; JSPrincipals *originPrincipals; @@ -387,7 +387,7 @@ js::DirectEvalStringFromIon(JSContext *cx, esg.lookupInEvalCache(flatStr, callerScript, pc); if (!esg.foundScript()) { - JSScript *maybeScript; + RootedScript maybeScript(cx); const char *filename; unsigned lineno; JSPrincipals *originPrincipals; diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index c447f80e2e2..8f540d2847f 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1109,54 +1109,58 @@ JS_TransplantObject(JSContext *cx, HandleObject origobj, HandleObject target) JS_ASSERT(!origobj->is()); JS_ASSERT(!target->is()); - 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(); + { + // Scope to make ~AutoMaybeTouchDeadZones do its GC before the return value is on the stack. + AutoMaybeTouchDeadZones agc(cx); + AutoDisableProxyCheck adpc(cx->runtime()); - // 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); + JSCompartment *destination = target->compartment(); - if (!JSObject::swap(cx, newIdentity, target)) - MOZ_CRASH(); - } else { - // Otherwise, we use |target| for the new identity object. - newIdentity = target; - } + 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(); - // 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(); + // 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); - // 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)) + 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(); - 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 @@ -4639,11 +4643,16 @@ JS::FinishOffThreadScript(JSContext *maybecx, JSRuntime *rt, void *token) #ifdef JS_THREADSAFE JS_ASSERT(CurrentThreadCanAccessRuntime(rt)); - Maybe 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 @@ -4717,9 +4726,9 @@ JS::CompileFunction(JSContext *cx, HandleObject obj, const ReadOnlyCompileOption 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) @@ -5135,13 +5144,12 @@ JS::Construct(JSContext *cx, HandleValue fval, const JS::HandleValueArray& args, return InvokeConstructor(cx, fval, args.length(), args.begin(), rval.address()); } -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 @@ -5174,6 +5182,17 @@ JS_New(JSContext *cx, HandleObject ctor, const JS::HandleValueArray& inputArgs) 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) { diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index a65ad71d89a..a4b8ea3f9e9 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -924,7 +924,7 @@ JSCompartment::removeDebuggeeUnderGC(FreeOp *fop, } 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(); diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 04d13e0b573..552be34ffce 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -425,7 +425,7 @@ struct JSCompartment 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: diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 18284fb8e0b..572fcc1a05d 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1541,7 +1541,7 @@ FunctionConstructor(JSContext *cx, unsigned argc, Value *vp, GeneratorKind gener bool isStarGenerator = generatorKind == StarGenerator; JS_ASSERT(generatorKind != LegacyGenerator); - JSScript *maybeScript = nullptr; + RootedScript maybeScript(cx); const char *filename; unsigned lineno; JSPrincipals *originPrincipals; diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 05303919f93..324b37323f9 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -2884,29 +2884,29 @@ js_GetScriptLineExtent(JSScript *script) } 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; @@ -2921,10 +2921,10 @@ js::DescribeScriptedCallerForCompilation(JSContext *cx, JSScript **maybeScript, // 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; } } diff --git a/js/src/jsscript.h b/js/src/jsscript.h index f725b7690d0..2bb0f6ec2e3 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -2023,7 +2023,7 @@ enum LineOption { }; 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); diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 8ad6359cb53..5ed23fce808 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -2197,7 +2197,7 @@ 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; } diff --git a/js/src/vm/ForkJoin.cpp b/js/src/vm/ForkJoin.cpp index a6a539f3562..dc6f2d9024d 100644 --- a/js/src/vm/ForkJoin.cpp +++ b/js/src/vm/ForkJoin.cpp @@ -2201,7 +2201,7 @@ class ParallelSpewer if (cx) { jsbytecode *pc; - JSScript *script = cx->currentScript(&pc); + RootedScript script(cx, cx->currentScript(&pc)); if (script && pc) { NonBuiltinScriptFrameIter iter(cx); if (iter.done()) { diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp index a67aea8c343..106b3a07498 100644 --- a/js/src/vm/Stack.cpp +++ b/js/src/vm/Stack.cpp @@ -16,6 +16,7 @@ #include "jit/BaselineFrame.h" #include "jit/JitCompartment.h" #endif +#include "js/GCAPI.h" #include "vm/Opcodes.h" #include "jit/JitFrameIterator-inl.h" @@ -31,7 +32,7 @@ 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 @@ -666,6 +667,8 @@ FrameIter::FrameIter(ThreadSafeContext *cx, SavedOption savedOption) , ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr) #endif { + // settleOnActivation can only GC if principals are given. + JS::AutoSuppressGCAnalysis nogc; settleOnActivation(); } @@ -676,6 +679,8 @@ FrameIter::FrameIter(ThreadSafeContext *cx, ContextOption contextOption, , ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr) #endif { + // settleOnActivation can only GC if principals are given. + JS::AutoSuppressGCAnalysis nogc; settleOnActivation(); }