From 8ba65d7192539012766a2679278a092dee6f03d8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 30 May 2012 23:27:51 -0700 Subject: [PATCH] Bug 758509 (part 1) - Create JSScript before starting bytecode generation. r=luke. --HG-- extra : rebase_source : d2b311715b74f4a1b74aea9673f38b2ca5ae9fa5 --- js/src/frontend/BytecodeCompiler.cpp | 18 ++- js/src/frontend/BytecodeEmitter.cpp | 25 +++- js/src/frontend/BytecodeEmitter.h | 6 +- .../tests/gc/jsscript-mark-children.js | 24 +++ js/src/jsobj.cpp | 2 +- js/src/jsscript.cpp | 138 +++++++++++------- js/src/jsscript.h | 43 +++--- js/src/vm/GlobalObject.cpp | 10 +- 8 files changed, 166 insertions(+), 100 deletions(-) create mode 100644 js/src/jit-test/tests/gc/jsscript-mark-children.js diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp index 654f1bedfd3..53166a3b5be 100644 --- a/js/src/frontend/BytecodeCompiler.cpp +++ b/js/src/frontend/BytecodeCompiler.cpp @@ -112,7 +112,12 @@ frontend::CompileScript(JSContext *cx, JSObject *scopeChain, StackFrame *callerF if (!tc.init()) return NULL; - BytecodeEmitter bce(&parser, &sc, lineno, noScriptRval, needScriptGlobal); + Rooted script(cx); + script = JSScript::Create(cx); + if (!script) + return NULL; + + BytecodeEmitter bce(&parser, &sc, script, lineno, noScriptRval, needScriptGlobal); if (!bce.init()) return NULL; @@ -240,9 +245,7 @@ frontend::CompileScript(JSContext *cx, JSObject *scopeChain, StackFrame *callerF JS_ASSERT(bce.version() == version); - Rooted script(cx); - script = JSScript::NewScriptFromEmitter(cx, &bce); - if (!script) + if (!script->fullyInitFromEmitter(cx, &bce)) return NULL; JS_ASSERT(script->savedCallerFun == savedCallerFun); @@ -276,7 +279,12 @@ frontend::CompileFunctionBody(JSContext *cx, JSFunction *fun, if (!funtc.init()) return false; - BytecodeEmitter funbce(&parser, &funsc, lineno, + Rooted script(cx); + script = JSScript::Create(cx); + if (!script) + return false; + + BytecodeEmitter funbce(&parser, &funsc, script, lineno, /* noScriptRval = */ false, /* needsScriptGlobal = */ false); if (!funbce.init()) return false; diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 9be1aea0de7..2557a87370b 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -65,10 +65,11 @@ NewTryNote(JSContext *cx, BytecodeEmitter *bce, JSTryNoteKind kind, unsigned sta static JSBool SetSrcNoteOffset(JSContext *cx, BytecodeEmitter *bce, unsigned index, unsigned which, ptrdiff_t offset); -BytecodeEmitter::BytecodeEmitter(Parser *parser, SharedContext *sc, unsigned lineno, - bool noScriptRval, bool needScriptGlobal) +BytecodeEmitter::BytecodeEmitter(Parser *parser, SharedContext *sc, Handle script, + unsigned lineno, bool noScriptRval, bool needScriptGlobal) : sc(sc), parent(NULL), + script(sc->context, script), parser(parser), atomIndices(sc->context), stackDepth(0), maxStackDepth(0), @@ -2656,9 +2657,16 @@ frontend::EmitFunctionScript(JSContext *cx, BytecodeEmitter *bce, ParseNode *bod bce->switchToMain(); } - return EmitTree(cx, bce, body) && - Emit1(cx, bce, JSOP_STOP) >= 0 && - JSScript::NewScriptFromEmitter(cx, bce); + if (!EmitTree(cx, bce, body)) + return false; + + if (Emit1(cx, bce, JSOP_STOP) < 0) + return false; + + if (!bce->script->fullyInitFromEmitter(cx, bce)) + return false; + + return true; } static bool @@ -4854,7 +4862,12 @@ EmitFunc(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn) sc.setFunMightAliasLocals(); // inherit funMightAliasLocals from parent sc.bindings.transfer(cx, &funbox->bindings); - BytecodeEmitter bce2(bce->parser, &sc, pn->pn_pos.begin.lineno, + Rooted script(cx); + script = JSScript::Create(cx); + if (!script) + return false; + + BytecodeEmitter bce2(bce->parser, &sc, script, pn->pn_pos.begin.lineno, /* noScriptRval = */ false, /* needsScriptGlobal = */ false); if (!bce2.init()) return false; diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 226d2e9e969..d0ff16e493e 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -77,6 +77,8 @@ struct BytecodeEmitter BytecodeEmitter *parent; /* enclosing function or global context */ + Rooted script; /* the JSScript we're ultimately producing */ + struct { jsbytecode *base; /* base of JS bytecode vector */ jsbytecode *limit; /* one byte beyond end of bytecode */ @@ -91,7 +93,7 @@ struct BytecodeEmitter Parser *parser; /* the parser */ OwnedAtomIndexMapPtr atomIndices; /* literals indexed for mapping */ - unsigned firstLine; /* first line, for JSScript::NewScriptFromEmitter */ + unsigned firstLine; /* first line, for JSScript::initFromEmitter */ int stackDepth; /* current stack depth in script frame */ unsigned maxStackDepth; /* maximum stack depth so far */ @@ -130,7 +132,7 @@ struct BytecodeEmitter bool inForInit:1; /* emitting init expr of for; exclude 'in' */ - BytecodeEmitter(Parser *parser, SharedContext *sc, unsigned lineno, + BytecodeEmitter(Parser *parser, SharedContext *sc, Handle script, unsigned lineno, bool noScriptRval, bool needScriptGlobal); bool init(); diff --git a/js/src/jit-test/tests/gc/jsscript-mark-children.js b/js/src/jit-test/tests/gc/jsscript-mark-children.js new file mode 100644 index 00000000000..449212a7edc --- /dev/null +++ b/js/src/jit-test/tests/gc/jsscript-mark-children.js @@ -0,0 +1,24 @@ +// Bug 758509 changed things so that a JSScript is partially initialized when +// it is created, which is prior to bytecode generation; full initialization +// only occurs after bytecode generation. This means that +// JSScript::markChildren() must deal with partially-initialized JSScripts. +// This test forces that to happen, because each let block allocates a +// StaticBlockObject. All that should happen is that we don't crash. + +let t = 0; +gczeal(2,1); +eval("\ +let x = 3, y = 4;\ +let (x = x+0, y = 12) { t += (x + y); } \ +let (x = x+1, y = 12) { t += (x + y); } \ +let (x = x+2, y = 12) { t += (x + y); } \ +let (x = x+3, y = 12) { t += (x + y); } \ +let (x = x+4, y = 12) { t += (x + y); } \ +let (x = x+5, y = 12) { t += (x + y); } \ +let (x = x+6, y = 12) { t += (x + y); } \ +let (x = x+7, y = 12) { t += (x + y); } \ +let (x = x+8, y = 12) { t += (x + y); } \ +let (x = x+9, y = 12) { t += (x + y); } \ +t += (x + y);\ +assertEq(t, 202);\ +"); diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 5378e0b084e..5fa80dce3af 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -870,7 +870,7 @@ class EvalScriptGuard } void setNewScript(JSScript *script) { - /* NewScriptFromEmitter has already called js_CallNewScriptHook. */ + /* JSScript::initFromEmitter has already called js_CallNewScriptHook. */ JS_ASSERT(!script_ && script); script_ = script; script_->isActiveEval = true; diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 3f83eee6964..18c0d43686d 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -582,10 +582,10 @@ js::XDRScript(XDRState *xdr, JSScript **scriptp, JSScript *parentScript) /* Note: version is packed into the 32b space with another 16b value. */ JSVersion version_ = JSVersion(version & JS_BITMASK(16)); JS_ASSERT((version_ & VersionFlags::FULL_MASK) == unsigned(version_)); - script = JSScript::NewScript(cx, length, nsrcnotes, natoms, nobjects, - nregexps, ntrynotes, nconsts, nClosedArgs, - nClosedVars, nTypeSets, version_); - if (!script) + script = JSScript::Create(cx); + if (!script || !script->partiallyInit(cx, length, nsrcnotes, natoms, nobjects, + nregexps, ntrynotes, nconsts, nClosedArgs, + nClosedVars, nTypeSets, version_)) return JS_FALSE; script->bindings.transfer(cx, &bindings); @@ -1092,6 +1092,17 @@ ScriptDataSize(uint32_t length, uint32_t nsrcnotes, uint32_t natoms, return size; } +JSScript * +JSScript::Create(JSContext *cx) +{ + JSScript *script = js_NewGCScript(cx); + if (!script) + return NULL; + + PodZero(script); + return script; +} + static inline uint8_t * AllocScriptData(JSContext *cx, size_t size) { @@ -1103,28 +1114,25 @@ AllocScriptData(JSContext *cx, size_t size) return data; } -JSScript * -JSScript::NewScript(JSContext *cx, uint32_t length, uint32_t nsrcnotes, uint32_t natoms, - uint32_t nobjects, uint32_t nregexps, uint32_t ntrynotes, uint32_t nconsts, - uint16_t nClosedArgs, uint16_t nClosedVars, uint32_t nTypeSets, JSVersion version) +bool +JSScript::partiallyInit(JSContext *cx, uint32_t length, uint32_t nsrcnotes, uint32_t natoms, + uint32_t nobjects, uint32_t nregexps, uint32_t ntrynotes, uint32_t nconsts, + uint16_t nClosedArgs, uint16_t nClosedVars, uint32_t nTypeSets, + JSVersion version) { + JSScript *script = this; + size_t size = ScriptDataSize(length, nsrcnotes, natoms, nobjects, nregexps, ntrynotes, nconsts, nClosedArgs, nClosedVars); + script->data = AllocScriptData(cx, size); + if (!script->data) + return false; - uint8_t *data = AllocScriptData(cx, size); - if (!data) - return NULL; - - JSScript *script = js_NewGCScript(cx); - if (!script) { - Foreground::free_(data); - return NULL; - } - - PodZero(script); - script->data = data; script->length = length; + script->version = version; + JS_ASSERT(script->getVersion() == version); + new (&script->bindings) Bindings(cx); uint8_t *cursor = data; @@ -1206,44 +1214,52 @@ JSScript::NewScript(JSContext *cx, uint32_t length, uint32_t nsrcnotes, uint32_t script->code = (jsbytecode *)cursor; JS_ASSERT(cursor + length * sizeof(jsbytecode) + nsrcnotes * sizeof(jssrcnote) == data + size); -#ifdef DEBUG - script->id_ = 0; -#endif - - JS_ASSERT(script->getVersion() == version); - return script; + return true; } -JSScript * -JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) +bool +JSScript::fullyInitTrivial(JSContext *cx, JSVersion version) { - uint32_t mainLength, prologLength, nfixed; - Rooted script(cx); - const char *filename; - JSFunction *fun; + JSScript *script = this; + + if (!script->partiallyInit(cx, /* length = */ 1, /* nsrcnotes = */ 1, 0, 0, 0, 0, 0, 0, 0, 0, + version)) + return false; + + script->noScriptRval = true; + + script->code[0] = JSOP_STOP; + script->notes()[0] = SRC_NULL; + + return true; +} + +bool +JSScript::fullyInitFromEmitter(JSContext *cx, BytecodeEmitter *bce) +{ + JSScript *script = this; /* The counts of indexed things must be checked during code generation. */ JS_ASSERT(bce->atomIndices->count() <= INDEX_LIMIT); JS_ASSERT(bce->objectList.length <= INDEX_LIMIT); JS_ASSERT(bce->regexpList.length <= INDEX_LIMIT); - mainLength = bce->offset(); - prologLength = bce->prologOffset(); + uint32_t mainLength = bce->offset(); + uint32_t prologLength = bce->prologOffset(); if (!bce->sc->bindings.ensureShape(cx)) - return NULL; + return false; uint32_t nsrcnotes = uint32_t(bce->countFinalSourceNotes()); uint16_t nClosedArgs = uint16_t(bce->closedArgs.length()); JS_ASSERT(nClosedArgs == bce->closedArgs.length()); uint16_t nClosedVars = uint16_t(bce->closedVars.length()); JS_ASSERT(nClosedVars == bce->closedVars.length()); - script = NewScript(cx, prologLength + mainLength, nsrcnotes, - bce->atomIndices->count(), bce->objectList.length, - bce->regexpList.length, bce->ntrynotes, bce->constList.length(), - nClosedArgs, nClosedVars, bce->typesetCount, bce->version()); - if (!script) - return NULL; + if (!script->partiallyInit(cx, prologLength + mainLength, nsrcnotes, bce->atomIndices->count(), + bce->objectList.length, bce->regexpList.length, bce->ntrynotes, + bce->constList.length(), nClosedArgs, nClosedVars, + bce->typesetCount, bce->version())) + return false; bce->sc->bindings.makeImmutable(); @@ -1251,22 +1267,22 @@ JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) script->mainOffset = prologLength; PodCopy(script->code, bce->prologBase(), prologLength); PodCopy(script->main(), bce->base(), mainLength); - nfixed = bce->sc->inFunction() ? bce->sc->bindings.numVars() : 0; + uint32_t nfixed = bce->sc->inFunction() ? bce->sc->bindings.numVars() : 0; JS_ASSERT(nfixed < SLOTNO_LIMIT); script->nfixed = uint16_t(nfixed); InitAtomMap(cx, bce->atomIndices.getMap(), script->atoms); - filename = bce->parser->tokenStream.getFilename(); + const char *filename = bce->parser->tokenStream.getFilename(); if (filename) { script->filename = SaveScriptFilename(cx, filename); if (!script->filename) - return NULL; + return false; } script->lineno = bce->firstLine; if (script->nfixed + bce->maxStackDepth >= JS_BIT(16)) { ReportCompileErrorNumber(cx, bce->tokenStream(), NULL, JSREPORT_ERROR, JSMSG_NEED_DIET, "script"); - return NULL; + return false; } script->nslots = script->nfixed + bce->maxStackDepth; @@ -1276,7 +1292,7 @@ JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) // never trigger. Oh well. if (bce->sc->staticLevel > UINT_MAX) { JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_TOO_DEEP, js_function_str); - return NULL; + return false; } script->staticLevel = uint16_t(bce->sc->staticLevel); @@ -1296,12 +1312,12 @@ JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) if (sourceMap) { if (!script->setSourceMap(cx, sourceMap)) { cx->free_(sourceMap); - return NULL; + return false; } } if (!FinishTakingSrcNotes(cx, bce, script->notes())) - return NULL; + return false; if (bce->ntrynotes != 0) FinishTakingTryNotes(bce, script->trynotes()); if (bce->objectList.length != 0) @@ -1343,7 +1359,7 @@ JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) script->bindings.transfer(cx, &bce->sc->bindings); - fun = NULL; + JSFunction *fun = NULL; if (bce->sc->inFunction()) { JS_ASSERT(!bce->noScriptRval); JS_ASSERT(!bce->needScriptGlobal); @@ -1369,7 +1385,7 @@ JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) bce->parent->checkSingletonContext(); if (!script->typeSetFunction(cx, fun, singleton)) - return NULL; + return false; fun->setScript(script); script->globalObject = fun->getParent() ? &fun->getParent()->global() : NULL; @@ -1404,7 +1420,7 @@ JSScript::NewScriptFromEmitter(JSContext *cx, BytecodeEmitter *bce) if (cx->hasRunOption(JSOPTION_PCCOUNT)) (void) script->initScriptCounts(cx); - return script; + return true; } size_t @@ -1463,6 +1479,11 @@ js::CallDestroyScriptHook(FreeOp *fop, JSScript *script) void JSScript::finalize(FreeOp *fop) { + // NOTE: this JSScript may be partially initialized at this point. E.g. we + // may have created it and partially initialized it with + // JSScript::Create(), but not yet finished initializing it with + // fullyInitFromEmitter() or fullyInitTrivial(). + CallDestroyScriptHook(fop, this); JS_ASSERT_IF(principals, originPrincipals); @@ -1482,8 +1503,10 @@ JSScript::finalize(FreeOp *fop) destroySourceMap(fop); destroyDebugScript(fop); - JS_POISON(data, 0xdb, computedSizeOfData()); - fop->free_(data); + if (data) { + JS_POISON(data, 0xdb, computedSizeOfData()); + fop->free_(data); + } } namespace js { @@ -1771,14 +1794,12 @@ js::CloneScript(JSContext *cx, HandleScript src) /* Now that all fallible allocation is complete, create the GC thing. */ - JSScript *dst = js_NewGCScript(cx); + JSScript *dst = JSScript::Create(cx); if (!dst) { Foreground::free_(data); return NULL; } - PodZero(dst); - new (&dst->bindings) Bindings(cx); dst->bindings.transfer(cx, &bindings); @@ -2089,6 +2110,11 @@ JSScript::clearTraps(FreeOp *fop) void JSScript::markChildren(JSTracer *trc) { + // NOTE: this JSScript may be partially initialized at this point. E.g. we + // may have created it and partially initialized it with + // JSScript::Create(), but not yet finished initializing it with + // fullyInitFromEmitter() or fullyInitTrivial(). + JS_ASSERT_IF(trc->runtime->gcStrictCompartmentChecking, compartment()->isCollecting()); for (uint32_t i = 0; i < natoms; ++i) { diff --git a/js/src/jsscript.h b/js/src/jsscript.h index 89fa11f9989..d649ced4b41 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -417,7 +417,7 @@ struct JSScript : public js::gc::Cell public: jsbytecode *code; /* bytecodes and their immediate operands */ uint8_t *data; /* pointer to variable-length data array (see - comment above NewScript() for details) */ + comment above Create() for details) */ const char *filename; /* source filename or null */ js::HeapPtrAtom *atoms; /* maps immediate index to literal struct */ @@ -513,8 +513,7 @@ struct JSScript : public js::gc::Cell private: // The bits in this field indicate the presence/non-presence of several - // optional arrays in |data|. See the comments above NewScript() for - // details. + // optional arrays in |data|. See the comments above Create() for details. ArrayBitsT hasArrayBits; // 1-bit fields. @@ -565,24 +564,19 @@ struct JSScript : public js::gc::Cell // End of fields. Start methods. // - /* - * Two successively less primitive ways to make a new JSScript. The first - * does *not* call a non-null cx->runtime->newScriptHook -- only the second, - * NewScriptFromEmitter, calls this optional debugger hook. - * - * The NewScript function can't know whether the script it creates belongs - * to a function, or is top-level or eval code, but the debugger wants access - * to the newly made script's function, if any -- so callers of NewScript - * are responsible for notifying the debugger after successfully creating any - * kind (function or other) of new JSScript. - */ public: - static JSScript *NewScript(JSContext *cx, uint32_t length, uint32_t nsrcnotes, uint32_t natoms, - uint32_t nobjects, uint32_t nregexps, - uint32_t ntrynotes, uint32_t nconsts, - uint16_t nClosedArgs, uint16_t nClosedVars, uint32_t nTypeSets, - JSVersion version); - static JSScript *NewScriptFromEmitter(JSContext *cx, js::BytecodeEmitter *bce); + static JSScript *Create(JSContext *cx); + + // Three ways ways to initialize a JSScript. Callers of partiallyInit() + // and fullyInitTrivial() are responsible for notifying the debugger after + // successfully creating any kind (function or other) of new JSScript. + // However, callers of fullyInitFromEmitter() do not need to do this. + bool partiallyInit(JSContext *cx, uint32_t length, uint32_t nsrcnotes, uint32_t natoms, + uint32_t nobjects, uint32_t nregexps, uint32_t ntrynotes, uint32_t nconsts, + uint16_t nClosedArgs, uint16_t nClosedVars, uint32_t nTypeSets, + JSVersion version); + bool fullyInitTrivial(JSContext *cx, JSVersion version); // inits a JSOP_STOP-only script + bool fullyInitFromEmitter(JSContext *cx, js::BytecodeEmitter *bce); void setVersion(JSVersion v) { version = v; } @@ -945,10 +939,11 @@ JS_STATIC_ASSERT(sizeof(JSScript::ArrayBitsT) * 8 >= JSScript::LIMIT); JS_STATIC_ASSERT(sizeof(JSScript) % js::gc::Cell::CellSize == 0); /* - * New-script-hook calling is factored from NewScriptFromEmitter so that it - * and callers of XDRScript can share this code. In the case of callers - * of XDRScript, the hook should be invoked only after successful decode - * of any owning function (the fun parameter) or script object (null fun). + * New-script-hook calling is factored from JSScript::fullyInitFromEmitter() so + * that it and callers of XDRScript() can share this code. In the case of + * callers of XDRScript(), the hook should be invoked only after successful + * decode of any owning function (the fun parameter) or script object (null + * fun). */ extern JS_FRIEND_API(void) js_CallNewScriptHook(JSContext *cx, JSScript *script, JSFunction *fun); diff --git a/js/src/vm/GlobalObject.cpp b/js/src/vm/GlobalObject.cpp index 184e6e442b6..e13941d7cce 100644 --- a/js/src/vm/GlobalObject.cpp +++ b/js/src/vm/GlobalObject.cpp @@ -111,13 +111,11 @@ GlobalObject::initFunctionAndObjectClasses(JSContext *cx) JS_ASSERT(proto == functionProto); functionProto->flags |= JSFUN_PROTOTYPE; - JSScript *script = - JSScript::NewScript(cx, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, JSVERSION_DEFAULT); - if (!script) + Rooted script(cx); + script = JSScript::Create(cx); + if (!script || !script->fullyInitTrivial(cx, JSVERSION_DEFAULT)) return NULL; - script->noScriptRval = true; - script->code[0] = JSOP_STOP; - script->code[1] = SRC_NULL; + functionProto->initScript(script); functionProto->getType(cx)->interpretedFunction = functionProto; script->setFunction(functionProto);