From 12879a07ec4cc19dec0d59d48bfae59574130109 Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Tue, 3 Jan 2012 11:50:07 -0800 Subject: [PATCH] Bug 714109 - Add missing barriers to Generator; r=billm The generator object stores aside values from the stack of the generator function when the generator is not running. These values need to properly root objects in the nursery. --- js/src/jsiter.cpp | 24 +++++++-------- js/src/jsiter.h | 2 +- js/src/vm/Stack.cpp | 72 ++++++++++++++++++++++++++++++++++++--------- js/src/vm/Stack.h | 13 +++++++- 4 files changed, 83 insertions(+), 28 deletions(-) diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index f3a57f37d6f..a4dc52dabf3 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -1371,16 +1371,11 @@ MarkGenerator(JSTracer *trc, JSGenerator *gen) */ JS_ASSERT(size_t(gen->regs.sp - fp->slots()) <= fp->numSlots()); - /* - * Currently, generators are not mjitted. Still, (overflow) args can be - * pushed by the mjit and need to be conservatively marked. Technically, the - * formal args and generator slots are safe for exact marking, but since the - * plan is to eventually mjit generators, it makes sense to future-proof - * this code and save someone an hour later. - */ - MarkStackRangeConservatively(trc, gen->floatingStack, fp->formalArgsEnd()); + MarkValueRange(trc, (HeapValue *)fp->formalArgsEnd() - gen->floatingStack, + gen->floatingStack, "Generator Floating Args"); js_TraceStackFrame(trc, fp); - MarkStackRangeConservatively(trc, fp->slots(), gen->regs.sp); + MarkValueRange(trc, gen->regs.sp - fp->slots(), + (HeapValue *)fp->slots(), "Generator Floating Stack"); } static void @@ -1469,14 +1464,18 @@ js_NewGenerator(JSContext *cx) (-1 + /* one Value included in JSGenerator */ vplen + VALUES_PER_STACK_FRAME + - stackfp->numSlots()) * sizeof(Value); + stackfp->numSlots()) * sizeof(HeapValue); + + JS_ASSERT(nbytes % sizeof(Value) == 0); + JS_STATIC_ASSERT(sizeof(StackFrame) % sizeof(HeapValue) == 0); JSGenerator *gen = (JSGenerator *) cx->malloc_(nbytes); if (!gen) return NULL; + SetValueRangeToUndefined((Value *)gen, nbytes / sizeof(Value)); /* Cut up floatingStack space. */ - Value *genvp = gen->floatingStack; + HeapValue *genvp = gen->floatingStack; StackFrame *genfp = reinterpret_cast(genvp + vplen); /* Initialize JSGenerator. */ @@ -1487,7 +1486,8 @@ js_NewGenerator(JSContext *cx) /* Copy from the stack to the generator's floating frame. */ gen->regs.rebaseFromTo(stackRegs, *genfp); - genfp->stealFrameAndSlots(genvp, stackfp, stackvp, stackRegs.sp); + genfp->stealFrameAndSlots( + genfp, genvp, stackfp, stackvp, stackRegs.sp); genfp->initFloatingGenerator(); obj->setPrivate(gen); diff --git a/js/src/jsiter.h b/js/src/jsiter.h index d8a8bff04cf..85e5c0518c2 100644 --- a/js/src/jsiter.h +++ b/js/src/jsiter.h @@ -237,7 +237,7 @@ struct JSGenerator { js::FrameRegs regs; JSObject *enumerators; js::StackFrame *floating; - js::Value floatingStack[1]; + js::HeapValue floatingStack[1]; js::StackFrame *floatingFrame() { return floating; diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp index 1954eafa76b..379254a41c6 100644 --- a/js/src/vm/Stack.cpp +++ b/js/src/vm/Stack.cpp @@ -38,6 +38,7 @@ * * ***** END LICENSE BLOCK ***** */ +#include "jscntxt.h" #include "jsgcmark.h" #include "methodjit/MethodJIT.h" #include "Stack.h" @@ -125,21 +126,31 @@ StackFrame::initDummyFrame(JSContext *cx, JSObject &chain) setScopeChainNoCallObj(chain); } +template void -StackFrame::stealFrameAndSlots(Value *vp, StackFrame *otherfp, - Value *othervp, Value *othersp) +StackFrame::stealFrameAndSlots(StackFrame *fp, T *vp, StackFrame *otherfp, U *othervp, + Value *othersp) { - JS_ASSERT(vp == (Value *)this - ((Value *)otherfp - othervp)); - JS_ASSERT(othervp == otherfp->actualArgs() - 2); + JS_ASSERT((U *)vp == (U *)this - ((U *)otherfp - othervp)); + JS_ASSERT((Value *)othervp == otherfp->actualArgs() - 2); JS_ASSERT(othersp >= otherfp->slots()); JS_ASSERT(othersp <= otherfp->base() + otherfp->numSlots()); + JS_ASSERT((T *)fp - vp == (U *)otherfp - othervp); - PodCopy(vp, othervp, othersp - othervp); - JS_ASSERT(vp == this->actualArgs() - 2); + /* Copy args, StackFrame, and slots. */ + U *srcend = (U *)otherfp->formalArgsEnd(); + T *dst = vp; + for (U *src = othervp; src < srcend; src++, dst++) + *dst = *src; - /* Catch bad-touching of non-canonical args (e.g., generator_trace). */ - if (otherfp->hasOverflowArgs()) - Debug_SetValueRangeToCrashOnTouch(othervp, othervp + 2 + otherfp->numFormalArgs()); + *fp = *otherfp; + if (doPostBarrier) + fp->writeBarrierPost(); + + srcend = (U *)othersp; + dst = (T *)fp->slots(); + for (U *src = (U *)otherfp->slots(); src < srcend; src++, dst++) + *dst = *src; /* * Repoint Call, Arguments, Block and With objects to the new live frame. @@ -166,6 +177,37 @@ StackFrame::stealFrameAndSlots(Value *vp, StackFrame *otherfp, } } +/* Note: explicit instantiation for js_NewGenerator located in jsiter.cpp. */ +template void StackFrame::stealFrameAndSlots( + StackFrame *, Value *, + StackFrame *, HeapValue *, Value *); +template void StackFrame::stealFrameAndSlots( + StackFrame *, HeapValue *, + StackFrame *, Value *, Value *); + +void +StackFrame::writeBarrierPost() +{ + /* This needs to follow the same rules as in js_TraceStackFrame. */ + if (scopeChain_) + JSObject::writeBarrierPost(scopeChain_, (void *)&scopeChain_); + if (isDummyFrame()) + return; + if (hasArgsObj()) + JSObject::writeBarrierPost(argsObj_, (void *)&argsObj_); + if (isScriptFrame()) { + if (isFunctionFrame()) { + JSFunction::writeBarrierPost((JSObject *)exec.fun, (void *)&exec.fun); + if (isEvalFrame()) + JSScript::writeBarrierPost(u.evalScript, (void *)&u.evalScript); + } else { + JSScript::writeBarrierPost(exec.script, (void *)&exec.script); + } + } + if (hasReturnValue()) + HeapValue::writeBarrierPost(rval_, &rval_); +} + #ifdef DEBUG JSObject *const StackFrame::sInvalidScopeChain = (JSObject *)0xbeef; #endif @@ -755,8 +797,8 @@ bool ContextStack::pushGeneratorFrame(JSContext *cx, JSGenerator *gen, GeneratorFrameGuard *gfg) { StackFrame *genfp = gen->floatingFrame(); - Value *genvp = gen->floatingStack; - uintN vplen = (Value *)genfp - genvp; + HeapValue *genvp = gen->floatingStack; + uintN vplen = (HeapValue *)genfp - genvp; uintN nvars = vplen + VALUES_PER_STACK_FRAME + genfp->numSlots(); Value *firstUnused = ensureOnTop(cx, REPORT_ERROR, nvars, CAN_EXTEND, &gfg->pushedSeg_); @@ -782,7 +824,8 @@ ContextStack::pushGeneratorFrame(JSContext *cx, JSGenerator *gen, GeneratorFrame JSObject::writeBarrierPre(genobj); /* Copy from the generator's floating frame to the stack. */ - stackfp->stealFrameAndSlots(stackvp, genfp, genvp, gen->regs.sp); + stackfp->stealFrameAndSlots( + stackfp, stackvp, genfp, genvp, gen->regs.sp); stackfp->resetGeneratorPrev(cx); stackfp->unsetFloatingGenerator(); gfg->regs_.rebaseFromTo(gen->regs, *stackfp); @@ -798,7 +841,7 @@ ContextStack::popGeneratorFrame(const GeneratorFrameGuard &gfg) { JSGenerator *gen = gfg.gen_; StackFrame *genfp = gen->floatingFrame(); - Value *genvp = gen->floatingStack; + HeapValue *genvp = gen->floatingStack; const FrameRegs &stackRegs = gfg.regs_; StackFrame *stackfp = stackRegs.fp(); @@ -806,7 +849,8 @@ ContextStack::popGeneratorFrame(const GeneratorFrameGuard &gfg) /* Copy from the stack to the generator's floating frame. */ gen->regs.rebaseFromTo(stackRegs, *genfp); - genfp->stealFrameAndSlots(genvp, stackfp, stackvp, stackRegs.sp); + genfp->stealFrameAndSlots( + genfp, genvp, stackfp, stackvp, stackRegs.sp); genfp->setFloatingGenerator(); /* ~FrameGuard/popFrame will finish the popping. */ diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index 0687a873262..b3eb30db3dd 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -430,7 +430,14 @@ class StackFrame const Value &thisv, JSObject &scopeChain, ExecuteType type); /* Used when activating generators. */ - void stealFrameAndSlots(Value *vp, StackFrame *otherfp, Value *othervp, Value *othersp); + enum TriggerPostBarriers { + DoPostBarrier = true, + NoPostBarrier = false + }; + template + void stealFrameAndSlots(StackFrame *fp, T *vp, StackFrame *otherfp, U *othervp, + Value *othersp); + void writeBarrierPost(); /* Perhaps one fine day we will remove dummy frames. */ void initDummyFrame(JSContext *cx, JSObject &chain); @@ -988,6 +995,10 @@ class StackFrame /* Return value */ + bool hasReturnValue() const { + return !!(flags_ & HAS_RVAL); + } + const Value &returnValue() { if (!(flags_ & HAS_RVAL)) rval_.setUndefined();