Bug 1118559 - Make checking if a slot is aliased less confusing. (r=jandem)
authorShu-yu Guo <shu@rfrn.org>
Fri, 09 Jan 2015 19:54:48 -0800
changeset 223108 1831405086dc4c3a8b10b632da3f62ba43b85beb
parent 223107 18bc11b32f6f637b3358a63d54e91a4aaa5c428a
child 223109 cadb53efd449dfb7f4f8af292b7421da2746835e
push id28082
push usercbook@mozilla.com
push dateMon, 12 Jan 2015 10:44:52 +0000
treeherdermozilla-central@643589c3ef94 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1118559
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1118559 - Make checking if a slot is aliased less confusing. (r=jandem)
js/src/frontend/BytecodeEmitter.cpp
js/src/jit/CompileInfo.h
js/src/jsscript.cpp
js/src/jsscript.h
js/src/vm/ScopeObject.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -1499,18 +1499,18 @@ BytecodeEmitter::isAliasedName(ParseNode
          * shape for the call object with with more than one name for a given
          * slot (which violates internal engine invariants). All this means that
          * the '|| sc->allLocalsAliased()' disjunct is incorrect since it will
          * mark both parameters in function(x,x) as aliased.
          */
         return script->formalIsAliased(pn->pn_cookie.slot());
       case Definition::VAR:
       case Definition::GLOBALCONST:
-        MOZ_ASSERT_IF(sc->allLocalsAliased(), script->varIsAliased(pn->pn_cookie.slot()));
-        return script->varIsAliased(pn->pn_cookie.slot());
+        MOZ_ASSERT_IF(sc->allLocalsAliased(), script->cookieIsAliased(pn->pn_cookie));
+        return script->cookieIsAliased(pn->pn_cookie);
       case Definition::PLACEHOLDER:
       case Definition::NAMED_LAMBDA:
       case Definition::MISSING:
         MOZ_CRASH("unexpected dn->kind");
     }
     return false;
 }
 
@@ -3085,25 +3085,25 @@ frontend::EmitFunctionScript(ExclusiveCo
 
     FunctionBox *funbox = bce->sc->asFunctionBox();
     if (funbox->argumentsHasLocalBinding()) {
         MOZ_ASSERT(bce->offset() == 0);  /* See JSScript::argumentsBytecode. */
         bce->switchToProlog();
         if (Emit1(cx, bce, JSOP_ARGUMENTS) < 0)
             return false;
         InternalBindingsHandle bindings(bce->script, &bce->script->bindings);
-        uint32_t varIndex = Bindings::argumentsVarIndex(cx, bindings);
-        if (bce->script->varIsAliased(varIndex)) {
+        BindingIter bi = Bindings::argumentsBinding(cx, bindings);
+        if (bce->script->bindingIsAliased(bi)) {
             ScopeCoordinate sc;
             sc.setHops(0);
             JS_ALWAYS_TRUE(LookupAliasedNameSlot(bce, bce->script, cx->names().arguments, &sc));
             if (!EmitAliasedVarOp(cx, JSOP_SETALIASEDVAR, sc, DontCheckLexical, bce))
                 return false;
         } else {
-            if (!EmitUnaliasedVarOp(cx, JSOP_SETLOCAL, varIndex, DontCheckLexical, bce))
+            if (!EmitUnaliasedVarOp(cx, JSOP_SETLOCAL, bi.localIndex(), DontCheckLexical, bce))
                 return false;
         }
         if (Emit1(cx, bce, JSOP_POP) < 0)
             return false;
         bce->switchToMain();
     }
 
     /*
--- a/js/src/jit/CompileInfo.h
+++ b/js/src/jit/CompileInfo.h
@@ -374,19 +374,25 @@ class CompileInfo
             return false;
 
         uint32_t arg = index - firstArgSlot();
         if (arg < nargs())
             return script()->formalIsAliased(arg);
 
         uint32_t local = index - firstLocalSlot();
         if (local < nlocals()) {
-            // First, check if this local is body-level.
+            // First, check if this local is body-level. If we have a slot for
+            // it, it is by definition unaliased. Aliased body-level locals do
+            // not have fixed slots on the frame and live in the CallObject.
+            //
+            // Note that this is not true for lexical (block-scoped)
+            // bindings. Such bindings, even when aliased, may be considered
+            // part of the "fixed" part (< nlocals()) of the frame.
             if (local < nbodyfixed())
-                return script()->bodyLevelLocalIsAliased(local);
+                return false;
 
             // Otherwise, it might be part of a block scope.
             for (; staticScope; staticScope = staticScope->enclosingNestedScope()) {
                 if (!staticScope->is<StaticBlockObject>())
                     continue;
                 StaticBlockObject &blockObj = staticScope->as<StaticBlockObject>();
                 if (blockObj.localOffset() < local) {
                     if (local - blockObj.localOffset() < blockObj.numVariables())
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -58,29 +58,24 @@ using namespace js::gc;
 using namespace js::frontend;
 
 using mozilla::PodCopy;
 using mozilla::PodZero;
 using mozilla::RotateLeft;
 
 typedef Rooted<GlobalObject *> RootedGlobalObject;
 
-/* static */ uint32_t
-Bindings::argumentsVarIndex(ExclusiveContext *cx, InternalBindingsHandle bindings,
-                            uint32_t *unaliasedSlot)
+/* static */ BindingIter
+Bindings::argumentsBinding(ExclusiveContext *cx, InternalBindingsHandle bindings)
 {
     HandlePropertyName arguments = cx->names().arguments;
     BindingIter bi(bindings);
     while (bi->name() != arguments)
         bi++;
-
-    if (unaliasedSlot)
-        *unaliasedSlot = bi->aliased() ? UINT32_MAX : bi.frameIndex();
-
-    return bi.localIndex();
+    return bi;
 }
 
 bool
 Bindings::initWithTemporaryStorage(ExclusiveContext *cx, InternalBindingsHandle self,
                                    uint32_t numArgs, uint32_t numVars,
                                    uint32_t numBodyLevelLexicals, uint32_t numBlockScoped,
                                    uint32_t numUnaliasedVars, uint32_t numUnaliasedBodyLevelLexicals,
                                    Binding *bindingArray)
@@ -3548,38 +3543,37 @@ js::SetFrameArgumentsObject(JSContext *c
                             HandleScript script, JSObject *argsobj)
 {
     /*
      * Replace any optimized arguments in the frame with an explicit arguments
      * object. Note that 'arguments' may have already been overwritten.
      */
 
     InternalBindingsHandle bindings(script, &script->bindings);
-    uint32_t unaliasedSlot;
-    const uint32_t var = Bindings::argumentsVarIndex(cx, bindings, &unaliasedSlot);
-
-    if (script->varIsAliased(var)) {
+    BindingIter bi = Bindings::argumentsBinding(cx, bindings);
+
+    if (script->bindingIsAliased(bi)) {
         /*
          * Scan the script to find the slot in the call object that 'arguments'
          * is assigned to.
          */
         jsbytecode *pc = script->code();
         while (*pc != JSOP_ARGUMENTS)
             pc += GetBytecodeLength(pc);
         pc += JSOP_ARGUMENTS_LENGTH;
         MOZ_ASSERT(*pc == JSOP_SETALIASEDVAR);
 
         // Note that here and below, it is insufficient to only check for
         // JS_OPTIMIZED_ARGUMENTS, as Ion could have optimized out the
         // arguments slot.
         if (IsOptimizedPlaceholderMagicValue(frame.callObj().as<ScopeObject>().aliasedVar(ScopeCoordinate(pc))))
             frame.callObj().as<ScopeObject>().setAliasedVar(cx, ScopeCoordinate(pc), cx->names().arguments, ObjectValue(*argsobj));
     } else {
-        if (IsOptimizedPlaceholderMagicValue(frame.unaliasedLocal(unaliasedSlot)))
-            frame.unaliasedLocal(unaliasedSlot) = ObjectValue(*argsobj);
+        if (IsOptimizedPlaceholderMagicValue(frame.unaliasedLocal(bi.frameIndex())))
+            frame.unaliasedLocal(bi.frameIndex()) = ObjectValue(*argsobj);
     }
 }
 
 /* static */ bool
 JSScript::argumentsOptimizationFailed(JSContext *cx, HandleScript script)
 {
     MOZ_ASSERT(script->functionNonDelazifying());
     MOZ_ASSERT(script->analyzedArgsUsage());
@@ -3646,34 +3640,35 @@ JSScript::argumentsOptimizationFailed(JS
             SetFrameArgumentsObject(cx, frame, script, argsobj);
         }
     }
 
     return true;
 }
 
 bool
-JSScript::varIsAliased(uint32_t varSlot)
+JSScript::bindingIsAliased(const BindingIter &bi)
 {
-    return bodyLevelLocalIsAliased(varSlot);
-}
-
-bool
-JSScript::bodyLevelLocalIsAliased(uint32_t localSlot)
-{
-    return bindings.bindingIsAliased(bindings.numArgs() + localSlot);
+    return bindings.bindingIsAliased(bi.i_);
 }
 
 bool
 JSScript::formalIsAliased(unsigned argSlot)
 {
+    MOZ_ASSERT(argSlot < bindings.numArgs());
     return bindings.bindingIsAliased(argSlot);
 }
 
 bool
+JSScript::cookieIsAliased(const frontend::UpvarCookie &cookie)
+{
+    return bindings.bindingIsAliased(bindings.numArgs() + cookie.slot());
+}
+
+bool
 JSScript::formalLivesInArgumentsObject(unsigned argSlot)
 {
     return argsObjAliasesFormals() && !formalIsAliased(argSlot);
 }
 
 LazyScript::LazyScript(JSFunction *fun, void *table, uint64_t packedFields, uint32_t begin, uint32_t end, uint32_t lineno, uint32_t column)
   : script_(nullptr),
     function_(fun),
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -47,16 +47,17 @@ class LazyScript;
 class RegExpObject;
 struct SourceCompressionTask;
 class Shape;
 class WatchpointMap;
 class NestedScopeObject;
 
 namespace frontend {
     struct BytecodeEmitter;
+    class UpvarCookie;
 }
 
 }
 
 /*
  * Type of try note associated with each catch or finally block, and also with
  * for-in and other kinds of loops. Non-for-in loops do not need these notes
  * for exception unwinding, but storing their boundaries here is helpful for
@@ -280,32 +281,31 @@ class Bindings
     uint32_t numArgs() const { return numArgs_; }
     uint32_t numVars() const { return numVars_; }
     uint32_t numBodyLevelLexicals() const { return numBodyLevelLexicals_; }
     uint32_t numBlockScoped() const { return numBlockScoped_; }
     uint32_t numBodyLevelLocals() const { return numVars_ + numBodyLevelLexicals_; }
     uint32_t numUnaliasedBodyLevelLocals() const { return numUnaliasedVars_ + numUnaliasedBodyLevelLexicals_; }
     uint32_t numAliasedBodyLevelLocals() const { return numBodyLevelLocals() - numUnaliasedBodyLevelLocals(); }
     uint32_t numLocals() const { return numVars() + numBodyLevelLexicals() + numBlockScoped(); }
-    uint32_t numUnaliasedLocals() const { return numUnaliasedVars() + numUnaliasedBodyLevelLexicals() + numBlockScoped(); }
+    uint32_t numFixedLocals() const { return numUnaliasedVars() + numUnaliasedBodyLevelLexicals() + numBlockScoped(); }
     uint32_t lexicalBegin() const { return numArgs() + numVars(); }
     uint32_t aliasedBodyLevelLexicalBegin() const { return aliasedBodyLevelLexicalBegin_; }
 
     uint32_t numUnaliasedVars() const { return numUnaliasedVars_; }
     uint32_t numUnaliasedBodyLevelLexicals() const { return numUnaliasedBodyLevelLexicals_; }
 
     // Return the size of the bindingArray.
     uint32_t count() const { return numArgs() + numVars() + numBodyLevelLexicals(); }
 
     /* Return the initial shape of call objects created for this scope. */
     Shape *callObjShape() const { return callObjShape_; }
 
     /* Convenience method to get the var index of 'arguments'. */
-    static uint32_t argumentsVarIndex(ExclusiveContext *cx, InternalBindingsHandle,
-                                      uint32_t *unaliasedSlot = nullptr);
+    static BindingIter argumentsBinding(ExclusiveContext *cx, InternalBindingsHandle);
 
     /* Return whether the binding at bindingIndex is aliased. */
     bool bindingIsAliased(uint32_t bindingIndex);
 
     /* Return whether this scope has any aliased bindings. */
     bool hasAnyAliasedBindings() const {
         if (!callObjShape_)
             return false;
@@ -1085,17 +1085,17 @@ class JSScript : public js::gc::TenuredC
         return column_;
     }
 
     void setColumn(size_t column) { column_ = column; }
 
     // The fixed part of a stack frame is comprised of vars (in function code)
     // and block-scoped locals (in all kinds of code).
     size_t nfixed() const {
-        return function_ ? bindings.numUnaliasedLocals() : bindings.numBlockScoped();
+        return function_ ? bindings.numFixedLocals() : bindings.numBlockScoped();
     }
 
     // Number of fixed slots reserved for vars.  Only nonzero for function
     // code.
     size_t nfixedvars() const {
         return function_ ? bindings.numUnaliasedVars() : 0;
     }
 
@@ -1655,21 +1655,23 @@ class JSScript : public js::gc::TenuredC
             return false;
 
         jsbytecode *pc = code();
         if (noScriptRval() && JSOp(*pc) == JSOP_FALSE)
             ++pc;
         return JSOp(*pc) == JSOP_RETRVAL;
     }
 
-    bool varIsAliased(uint32_t varSlot);
-    bool bodyLevelLocalIsAliased(uint32_t localSlot);
+    bool bindingIsAliased(const js::BindingIter &bi);
     bool formalIsAliased(unsigned argSlot);
     bool formalLivesInArgumentsObject(unsigned argSlot);
 
+    // Frontend-only.
+    bool cookieIsAliased(const js::frontend::UpvarCookie &cookie);
+
   private:
     /* Change this->stepMode to |newValue|. */
     void setNewStepMode(js::FreeOp *fop, uint32_t newValue);
 
     bool ensureHasDebugScript(JSContext *cx);
     js::DebugScript *debugScript();
     js::DebugScript *releaseDebugScript();
     void destroyDebugScript(js::FreeOp *fop);
@@ -1729,16 +1731,17 @@ namespace js {
  *  - next, variables, from index 0 to numLocals
  */
 class BindingIter
 {
     const InternalBindingsHandle bindings_;
     uint32_t i_;
     uint32_t unaliasedLocal_;
 
+    friend class ::JSScript;
     friend class Bindings;
 
   public:
     explicit BindingIter(const InternalBindingsHandle &bindings)
       : bindings_(bindings), i_(0), unaliasedLocal_(0) {}
     explicit BindingIter(const HandleScript &script)
       : bindings_(script, &script->bindings), i_(0), unaliasedLocal_(0) {}
 
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -1348,17 +1348,17 @@ class DebugScopeProxy : public BaseProxy
             Bindings &bindings = script->bindings;
             BindingIter bi(script);
             while (bi && NameToId(bi->name()) != id)
                 bi++;
             if (!bi)
                 return true;
 
             if (bi->kind() == Binding::VARIABLE || bi->kind() == Binding::CONSTANT) {
-                if (script->bodyLevelLocalIsAliased(bi.localIndex()))
+                if (script->bindingIsAliased(bi))
                     return true;
 
                 uint32_t i = bi.frameIndex();
                 if (maybeLiveScope) {
                     AbstractFramePtr frame = maybeLiveScope->frame();
                     if (action == GET)
                         vp.set(frame.unaliasedLocal(i));
                     else