Bug 1118559 - Make checking if a slot is aliased less confusing. r=jandem, a=sledru
authorShu-yu Guo <shu@rfrn.org>
Fri, 09 Jan 2015 19:54:48 -0800
changeset 242811 89b21469facd1756e61f597bbfb2b0a491de50c4
parent 242810 4aac2ed783b82f247b4bdf42048034c5d3427845
child 242812 6d673033b92b84467f4a29f0adc789904cc4bd86
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, sledru
bugs1118559
milestone36.0a2
Bug 1118559 - Make checking if a slot is aliased less confusing. r=jandem, a=sledru
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;
 }
 
@@ -3074,25 +3074,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)
@@ -3550,38 +3545,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());
@@ -3648,34 +3642,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
@@ -1372,17 +1372,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