From 5eb8c809dbe05fc77817eec7d75b737eda043477 Mon Sep 17 00:00:00 2001 From: Terrence Cole Date: Wed, 26 Sep 2012 15:39:33 -0700 Subject: [PATCH] Bug 793823 - Store an InternalHandle in BindingIter; r=billm Currently, BindingIter stores a Bindings*: this is sometimes a pointer into a JSScript, which causes the analysis to poison it. This patch straightforwardly replaces this Bindings* with an InternalBindingsHandle. --HG-- extra : rebase_source : bc780da603be1ae24fbbdf2d304d3fdb53de130e --- js/src/frontend/BytecodeEmitter.cpp | 9 +++++---- js/src/jsanalyze.cpp | 3 ++- js/src/jsdbgapi.cpp | 2 +- js/src/jsfun.cpp | 2 +- js/src/jsopcode.cpp | 6 ++++-- js/src/jsscript.cpp | 24 +++++++++++++----------- js/src/jsscript.h | 18 +++++++++++------- js/src/vm/Debugger.cpp | 3 ++- js/src/vm/ScopeObject.cpp | 8 +++++--- 9 files changed, 44 insertions(+), 31 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 6c572a2d7c24..4382224b06ef 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -886,14 +886,14 @@ ClonedBlockDepth(BytecodeEmitter *bce) } static uint16_t -AliasedNameToSlot(JSScript *script, PropertyName *name) +AliasedNameToSlot(HandleScript script, PropertyName *name) { /* * Beware: BindingIter may contain more than one Binding for a given name * (in the case of |function f(x,x) {}|) but only one will be aliased. */ unsigned slot = CallObject::RESERVED_SLOTS; - for (BindingIter bi(script->bindings); ; bi++) { + for (BindingIter bi(script); ; bi++) { if (bi->aliased()) { if (bi->name() == name) return slot; @@ -2621,7 +2621,8 @@ frontend::EmitFunctionScript(JSContext *cx, BytecodeEmitter *bce, ParseNode *bod bce->switchToProlog(); if (Emit1(cx, bce, JSOP_ARGUMENTS) < 0) return false; - unsigned varIndex = bce->script->bindings.argumentsVarIndex(cx); + InternalBindingsHandle bindings(bce->script, &bce->script->bindings); + unsigned varIndex = Bindings::argumentsVarIndex(cx, bindings); if (bce->script->varIsAliased(varIndex)) { ScopeCoordinate sc; sc.hops = 0; @@ -4925,7 +4926,7 @@ EmitFunc(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn) return false; } else { #ifdef DEBUG - BindingIter bi(bce->script->bindings); + BindingIter bi(bce->script); while (bi->name() != fun->atom()) bi++; JS_ASSERT(bi->kind() == VARIABLE || bi->kind() == CONSTANT || bi->kind() == ARGUMENT); diff --git a/js/src/jsanalyze.cpp b/js/src/jsanalyze.cpp index 2c015c0a5844..b7d18ae88a77 100644 --- a/js/src/jsanalyze.cpp +++ b/js/src/jsanalyze.cpp @@ -141,7 +141,8 @@ ScriptAnalysis::analyzeBytecode(JSContext *cx) bool allVarsAliased = script_->compartment()->debugMode(); bool allArgsAliased = allVarsAliased || script_->argumentsHasVarBinding(); - for (BindingIter bi(script_->bindings); bi; bi++) { + RootedScript script(cx, script_); + for (BindingIter bi(script); bi; bi++) { if (bi->kind() == ARGUMENT) escapedSlots[ArgSlot(bi.frameIndex())] = allArgsAliased || bi->aliased(); else diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 41a524d76bb8..485323586081 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -411,7 +411,7 @@ extern JS_PUBLIC_API(uintptr_t *) JS_GetFunctionLocalNameArray(JSContext *cx, JSFunction *fun, void **markp) { BindingVector bindings(cx); - if (!FillBindingVector(fun->script()->bindings, &bindings)) + if (!FillBindingVector(fun->script(), &bindings)) return NULL; /* Munge data into the API this method implements. Avert your eyes! */ diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 682afa2ff5d8..308d01886e98 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -679,7 +679,7 @@ js::FunctionToString(JSContext *cx, HandleFunction fun, bool bodyOnly, bool lamb // Fish out the argument names. BindingVector *localNames = cx->new_(cx); js::ScopedDeletePtr freeNames(localNames); - if (!FillBindingVector(script->bindings, localNames)) + if (!FillBindingVector(script, localNames)) return NULL; for (unsigned i = 0; i < fun->nargs; i++) { if ((i && !out.append(", ")) || diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index 1b262d7d26e1..98bb1cf8d682 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -1078,7 +1078,8 @@ SetPrinterLocalNames(JSContext *cx, JSScript *script, JSPrinter *jp) BindingVector *localNames = NULL; if (script->bindings.count() > 0) { localNames = cx->new_(cx); - if (!localNames || !FillBindingVector(script->bindings, localNames)) + RootedScript script_(cx, script); + if (!localNames || !FillBindingVector(script_, localNames)) return false; } jp->localNames = localNames; @@ -6047,7 +6048,8 @@ ExpressionDecompiler::init() localNames = cx->new_(cx); if (!localNames) return false; - if (!FillBindingVector(script->bindings, localNames)) + RootedScript script_(cx, script); + if (!FillBindingVector(script_, localNames)) return false; return true; diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 581c6895ef23..d4bef4e4a033 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -50,18 +50,18 @@ using namespace js; using namespace js::gc; using namespace js::frontend; -unsigned -Bindings::argumentsVarIndex(JSContext *cx) const +/* static */ unsigned +Bindings::argumentsVarIndex(JSContext *cx, InternalBindingsHandle bindings) { HandlePropertyName arguments = cx->names().arguments; - BindingIter bi(*this); + BindingIter bi(bindings); while (bi->name() != arguments) bi++; return bi.frameIndex(); } bool -Bindings::initWithTemporaryStorage(JSContext *cx, InternalHandle self, +Bindings::initWithTemporaryStorage(JSContext *cx, InternalBindingsHandle self, unsigned numArgs, unsigned numVars, Binding *bindingArray) { @@ -106,7 +106,7 @@ Bindings::initWithTemporaryStorage(JSContext *cx, InternalHandle self return false; #endif - BindingIter bi(*self); + BindingIter bi(self); unsigned slot = CallObject::RESERVED_SLOTS; for (unsigned i = 0, n = self->count(); i < n; i++, bi++) { if (!bi->aliased()) @@ -151,7 +151,7 @@ Bindings::switchToScriptStorage(Binding *newBindingArray) } bool -Bindings::clone(JSContext *cx, InternalHandle self, +Bindings::clone(JSContext *cx, InternalBindingsHandle self, uint8_t *dstScriptData, HandleScript srcScript) { @@ -186,13 +186,13 @@ XDRScriptBindings(XDRState *xdr, LifoAllocScope &las, unsigned numArgs, un JSContext *cx = xdr->cx(); if (mode == XDR_ENCODE) { - for (BindingIter bi(script->bindings); bi; bi++) { + for (BindingIter bi(script); bi; bi++) { JSAtom *atom = bi->name(); if (!XDRAtom(xdr, &atom)) return false; } - for (BindingIter bi(script->bindings); bi; bi++) { + for (BindingIter bi(script); bi; bi++) { uint8_t u8 = (uint8_t(bi->kind()) << 1) | uint8_t(bi->aliased()); if (!xdr->codeUint8(&u8)) return false; @@ -225,7 +225,7 @@ XDRScriptBindings(XDRState *xdr, LifoAllocScope &las, unsigned numArgs, un bindingArray[i] = Binding(name, kind, aliased); } - InternalHandle bindings(script, &script->bindings); + InternalBindingsHandle bindings(script, &script->bindings); if (!Bindings::initWithTemporaryStorage(cx, bindings, numArgs, numVars, bindingArray)) return false; } @@ -261,8 +261,9 @@ Bindings::trace(JSTracer *trc) } bool -js::FillBindingVector(Bindings &bindings, BindingVector *vec) +js::FillBindingVector(HandleScript fromScript, BindingVector *vec) { + InternalBindingsHandle bindings(fromScript, &fromScript->bindings); for (BindingIter bi(bindings); bi; bi++) { if (!vec->append(*bi)) return false; @@ -2552,7 +2553,8 @@ JSScript::argumentsOptimizationFailed(JSContext *cx, JSScript *script_) script->needsArgsObj_ = true; - const unsigned var = script->bindings.argumentsVarIndex(cx); + InternalBindingsHandle bindings(script, &script->bindings); + const unsigned var = Bindings::argumentsVarIndex(cx, bindings); /* * By design, the apply-arguments optimization is only made when there diff --git a/js/src/jsscript.h b/js/src/jsscript.h index eae25c6462b2..4c89418227df 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -126,6 +126,9 @@ class Binding JS_STATIC_ASSERT(sizeof(Binding) == sizeof(uintptr_t)); +class Bindings; +typedef InternalHandle InternalBindingsHandle; + /* * Formal parameters and local variables are stored in a shape tree * path encapsulated within this class. This class represents bindings for @@ -168,7 +171,7 @@ class Bindings * storage is release, switchToScriptStorage must be called, providing a * pointer into the Binding array stored in script->data. */ - static bool initWithTemporaryStorage(JSContext *cx, InternalHandle self, + static bool initWithTemporaryStorage(JSContext *cx, InternalBindingsHandle self, unsigned numArgs, unsigned numVars, Binding *bindingArray); @@ -178,7 +181,8 @@ class Bindings * Clone srcScript's bindings (as part of js::CloneScript). dstScriptData * is the pointer to what will eventually be dstScript->data. */ - static bool clone(JSContext *cx, InternalHandle self, uint8_t *dstScriptData, HandleScript srcScript); + static bool clone(JSContext *cx, InternalBindingsHandle self, uint8_t *dstScriptData, + HandleScript srcScript); unsigned numArgs() const { return numArgs_; } unsigned numVars() const { return numVars_; } @@ -188,7 +192,7 @@ class Bindings Shape *callObjShape() const { return callObjShape_; } /* Convenience method to get the var index of 'arguments'. */ - unsigned argumentsVarIndex(JSContext *cx) const; + static unsigned argumentsVarIndex(JSContext *cx, InternalBindingsHandle); /* Return whether the binding at bindingIndex is aliased. */ bool bindingIsAliased(unsigned bindingIndex); @@ -900,14 +904,14 @@ namespace js { */ class BindingIter { - const Bindings *bindings_; + const InternalBindingsHandle bindings_; unsigned i_; friend class Bindings; - BindingIter(const Bindings &bindings, unsigned i) : bindings_(&bindings), i_(i) {} public: - explicit BindingIter(const Bindings &bindings) : bindings_(&bindings), i_(0) {} + explicit BindingIter(const InternalBindingsHandle &bindings) : bindings_(bindings), i_(0) {} + explicit BindingIter(const HandleScript &script) : bindings_(script, &script->bindings), i_(0) {} bool done() const { return i_ == bindings_->count(); } operator bool() const { return !done(); } @@ -931,7 +935,7 @@ class BindingIter typedef Vector BindingVector; extern bool -FillBindingVector(Bindings &bindings, BindingVector *vec); +FillBindingVector(HandleScript fromScript, BindingVector *vec); /* * Iterator over the aliased formal bindings in ascending index order. This can diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index d97e75ee1cb0..86902ff8c966 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -3741,7 +3741,8 @@ DebuggerObject_getParameterNames(JSContext *cx, unsigned argc, Value *vp) if (fun->nargs > 0) { BindingVector bindings(cx); - if (!FillBindingVector(fun->script()->bindings, &bindings)) + RootedScript script(cx, fun->script()); + if (!FillBindingVector(script, &bindings)) return false; for (size_t i = 0; i < fun->nargs; i++) { Value v; diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index a16bad5315d0..0621ae89ed3e 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/js/src/vm/ScopeObject.cpp @@ -1124,7 +1124,7 @@ class DebugScopeProxy : public BaseProxyHandler return false; Bindings &bindings = script->bindings; - BindingIter bi(script->bindings); + BindingIter bi(script); while (bi && NameToId(bi->name()) != id) bi++; if (!bi) @@ -1395,7 +1395,8 @@ class DebugScopeProxy : public BaseProxyHandler * they must be manually appended here. */ if (scope.isCall() && !scope.asCall().isForEval()) { - for (BindingIter bi(scope.asCall().callee().script()->bindings); bi; bi++) { + RootedScript script(cx, scope.asCall().callee().script()); + for (BindingIter bi(script); bi; bi++) { if (!bi->aliased() && !props.append(NameToId(bi->name()))) return false; } @@ -1433,7 +1434,8 @@ class DebugScopeProxy : public BaseProxyHandler * a manual search is necessary. */ if (!found && scope.isCall() && !scope.asCall().isForEval()) { - for (BindingIter bi(scope.asCall().callee().script()->bindings); bi; bi++) { + RootedScript script(cx, scope.asCall().callee().script()); + for (BindingIter bi(script); bi; bi++) { if (!bi->aliased() && NameToId(bi->name()) == id) { found = true; break;