Bug 1007164 - Throw on touching sentinel values in DebugScopeProxy by default but allow Debugger.Environment.prototype.getVariable access. (r=jimb)

js/src/jit-test/tests/debug/Frame-eval-12.js

@@ -4,16 +4,10 @@ var g = newGlobal();
 var dbg = new Debugger(g);
 
 // capture arguments object and test function
-var hits = 0;
 dbg.onDebuggerStatement = function (frame) {
-    try {
+  var args = frame.older.environment.parent.getVariable('arguments');
-        frame.older.environment.parent.getVariable('arguments')
+  assertEq(args.missingArguments, true);
-    } catch (e) {
-        assertEq(''+e, "Error: Debugger scope is not live");
-        hits++;
-    }
 };
 g.eval("function h() { debugger; }");
 g.eval("function f() { var x = 0; return function() { x++; h() } }");
 g.eval("f('ponies')()");
-assertEq(hits, 1);

js/src/js.msg

@@ -356,7 +356,7 @@ MSG_DEF(JSMSG_INTRINSIC_NOT_DEFINED,  302, 1, JSEXN_REFERENCEERR, "no intrinsic
 MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA, 303, 2, JSEXN_ERR,      "{0} is being assigned a {1}, but already has one")
 MSG_DEF(JSMSG_PAR_ARRAY_BAD_ARG,      304, 0, JSEXN_RANGEERR, "invalid parallel method argument")
 MSG_DEF(JSMSG_REGEXP_RUNTIME_ERROR,   305, 0, JSEXN_INTERNALERR, "an error occurred while executing regular expression")
-MSG_DEF(JSMSG_UNUSED306,              306, 0, JSEXN_NONE, "")
+MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT,    306, 0, JSEXN_ERR, "variable has been optimized out")
 MSG_DEF(JSMSG_UNUSED307,              307, 0, JSEXN_NONE, "")
 MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_CONFLICT, 308, 0, JSEXN_ERR, "no conflict resolution function provided")
 MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_BOUNDS, 309, 0, JSEXN_ERR, "index in scatter vector out of bounds")

js/src/vm/CommonPropertyNames.h

@@ -147,6 +147,7 @@
     macro(of, of, "of") \
     macro(offset, offset, "offset") \
     macro(optimizedOut, optimizedOut, "optimizedOut") \
+    macro(missingArguments, missingArguments, "missingArguments") \
     macro(outOfMemory, outOfMemory, "out of memory") \
     macro(parseFloat, parseFloat, "parseFloat") \
     macro(parseInt, parseInt, "parseInt") \

js/src/vm/Debugger.cpp

@@ -755,15 +755,23 @@ Debugger::wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp)
             vp.setObject(*dobj);
         }
     } else if (vp.isMagic()) {
-        // Other magic values should not have escaped.
-        MOZ_ASSERT(vp.whyMagic() == JS_OPTIMIZED_OUT);
-
         RootedObject optObj(cx, NewBuiltinClassInstance(cx, &JSObject::class_));
         if (!optObj)
             return false;
 
+        // We handle two sentinel values: missing arguments (overloading
+        // JS_OPTIMIZED_ARGUMENTS) and optimized out slots (JS_OPTIMIZED_OUT).
+        // Other magic values should not have escaped.
+        PropertyName *name;
+        if (vp.whyMagic() == JS_OPTIMIZED_ARGUMENTS) {
+            name = cx->names().missingArguments;
+        } else {
+            MOZ_ASSERT(vp.whyMagic() == JS_OPTIMIZED_OUT);
+            name = cx->names().optimizedOut;
+        }
+
         RootedValue trueVal(cx, BooleanValue(true));
-        if (!JSObject::defineProperty(cx, optObj, cx->names().optimizedOut, trueVal))
+        if (!JSObject::defineProperty(cx, optObj, name, trueVal))
             return false;
 
         vp.setObject(*optObj);
@@ -5986,8 +5994,18 @@ DebuggerEnv_getVariable(JSContext *cx, unsigned argc, Value *vp)
 
         /* This can trigger getters. */
         ErrorCopier ec(ac, dbg->toJSObject());
-        if (!JSObject::getGeneric(cx, env, env, id, &v))
+
-            return false;
+        // For DebugScopeObjects, we get sentinel values for optimized out
+        // slots and arguments instead of throwing (the default behavior).
+        //
+        // See wrapDebuggeeValue for how the sentinel values are wrapped.
+        if (env->is<DebugScopeObject>()) {
+            if (!env->as<DebugScopeObject>().getMaybeSentinelValue(cx, id, &v))
+                return false;
+        } else {
+            if (!JSObject::getGeneric(cx, env, env, id, &v))
+                return false;
+        }
     }
 
     if (!dbg->wrapDebuggeeValue(cx, &v))

js/src/vm/Debugger.h

@@ -472,6 +472,10 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
      *
      * If *vp is a magic JS_OPTIMIZED_OUT value, this produces a plain object
      * of the form { optimizedOut: true }.
+     *
+     * If *vp is a magic JS_OPTIMIZED_ARGUMENTS value signifying missing
+     * arguments, this produces a plain object of the form { missingArguments:
+     * true }.
      */
     bool wrapDebuggeeValue(JSContext *cx, MutableHandleValue vp);
 

js/src/vm/ScopeObject.cpp

@@ -29,6 +29,7 @@ using namespace js::types;
 using mozilla::PodZero;
 
 typedef Rooted<ArgumentsObject *> RootedArgumentsObject;
+typedef MutableHandle<ArgumentsObject *> MutableHandleArgumentsObject;
 
 /*****************************************************************************/
 
@@ -1118,6 +1119,12 @@ class DebugScopeProxy : public BaseProxyHandler
 {
     enum Action { SET, GET };
 
+    enum AccessResult {
+        ACCESS_UNALIASED,
+        ACCESS_GENERIC,
+        ACCESS_LOST
+    };
+
     /*
      * This function handles access to unaliased locals/formals. Since they are
      * unaliased, the values of these variables are not stored in the slots of
@@ -1137,13 +1144,17 @@ class DebugScopeProxy : public BaseProxyHandler
      *     - and there was not a DebugScopeObject yet associated with the
      *       scope, then the unaliased values are lost and not recoverable.
      *
-     * handleUnaliasedAccess returns 'true' if the access was unaliased and
+     * Callers should check accessResult for non-failure results:
-     * completed by handleUnaliasedAccess.
+     *  - ACCESS_UNALIASED if the access was unaliased and completed
+     *  - ACCESS_GENERIC   if the access was aliased or the property not found
+     *  - ACCESS_LOST      if the value has been lost to the debugger
      */
-    bool handleUnaliasedAccess(JSContext *cx, Handle<DebugScopeObject*> debugScope, Handle<ScopeObject*> scope,
+    bool handleUnaliasedAccess(JSContext *cx, Handle<DebugScopeObject*> debugScope,
-                               jsid id, Action action, MutableHandleValue vp)
+                               Handle<ScopeObject*> scope, jsid id, Action action,
+                               MutableHandleValue vp, AccessResult *accessResult)
     {
         JS_ASSERT(&debugScope->scope() == scope);
+        *accessResult = ACCESS_GENERIC;
         ScopeIterVal *maybeLiveScope = DebugScopes::hasLiveScope(*scope);
 
         /* Handle unaliased formals, vars, and consts at function scope. */
@@ -1158,12 +1169,12 @@ class DebugScopeProxy : public BaseProxyHandler
             while (bi && NameToId(bi->name()) != id)
                 bi++;
             if (!bi)
-                return false;
+                return true;
 
             if (bi->kind() == Binding::VARIABLE || bi->kind() == Binding::CONSTANT) {
                 uint32_t i = bi.frameIndex();
                 if (script->varIsAliased(i))
-                    return false;
+                    return true;
 
                 if (maybeLiveScope) {
                     AbstractFramePtr frame = maybeLiveScope->frame();
@@ -1178,14 +1189,16 @@ class DebugScopeProxy : public BaseProxyHandler
                         snapshot->setDenseElement(bindings.numArgs() + i, vp);
                 } else {
                     /* The unaliased value has been lost to the debugger. */
-                    if (action == GET)
+                    if (action == GET) {
-                        vp.set(MagicValue(JS_OPTIMIZED_OUT));
+                        *accessResult = ACCESS_LOST;
+                        return true;
+                    }
                 }
             } else {
                 JS_ASSERT(bi->kind() == Binding::ARGUMENT);
                 unsigned i = bi.frameIndex();
                 if (script->formalIsAliased(i))
-                    return false;
+                    return true;
 
                 if (maybeLiveScope) {
                     AbstractFramePtr frame = maybeLiveScope->frame();
@@ -1207,14 +1220,17 @@ class DebugScopeProxy : public BaseProxyHandler
                         snapshot->setDenseElement(i, vp);
                 } else {
                     /* The unaliased value has been lost to the debugger. */
-                    if (action == GET)
+                    if (action == GET) {
-                        vp.set(MagicValue(JS_OPTIMIZED_OUT));
+                        *accessResult = ACCESS_LOST;
+                        return true;
+                    }
                 }
 
                 if (action == SET)
                     TypeScript::SetArgument(cx, script, i, vp);
             }
 
+            *accessResult = ACCESS_UNALIASED;
             return true;
         }
 
@@ -1223,11 +1239,11 @@ class DebugScopeProxy : public BaseProxyHandler
             Rooted<ClonedBlockObject *> block(cx, &scope->as<ClonedBlockObject>());
             Shape *shape = block->lastProperty()->search(cx, id);
             if (!shape)
-                return false;
+                return true;
 
             unsigned i = block->staticBlock().shapeToIndex(*shape);
             if (block->staticBlock().isAliased(i))
-                return false;
+                return true;
 
             if (maybeLiveScope) {
                 AbstractFramePtr frame = maybeLiveScope->frame();
@@ -1244,13 +1260,14 @@ class DebugScopeProxy : public BaseProxyHandler
                     block->setVar(i, vp, DONT_CHECK_ALIASING);
             }
 
+            *accessResult = ACCESS_UNALIASED;
             return true;
         }
 
         /* The rest of the internal scopes do not have unaliased vars. */
         JS_ASSERT(scope->is<DeclEnvObject>() || scope->is<DynamicWithObject>() ||
                   scope->as<CallObject>().isForEval());
-        return false;
+        return true;
     }
 
     static bool isArguments(JSContext *cx, jsid id)
@@ -1276,31 +1293,33 @@ class DebugScopeProxy : public BaseProxyHandler
     }
 
     /*
-     * This function creates an arguments object when the debugger requests
+     * This function checks if an arguments object needs to be created when
-     * 'arguments' for a function scope where the arguments object has been
+     * the debugger requests 'arguments' for a function scope where the
-     * optimized away (either because the binding is missing altogether or
+     * arguments object has been optimized away (either because the binding is
-     * because !ScriptAnalysis::needsArgsObj).
+     * missing altogether or because !ScriptAnalysis::needsArgsObj).
      */
-    static bool checkForMissingArguments(JSContext *cx, jsid id, ScopeObject &scope,
+    static bool isMissingArguments(JSContext *cx, jsid id, ScopeObject &scope)
-                                         ArgumentsObject **maybeArgsObj)
     {
-        *maybeArgsObj = nullptr;
+        return isArguments(cx, id) && isFunctionScope(scope) &&
+               !scope.as<CallObject>().callee().nonLazyScript()->needsArgsObj();
+    }
 
-        if (!isArguments(cx, id) || !isFunctionScope(scope))
+    /*
-            return true;
+     * Create a missing arguments object. If the function returns true but
-
+     * argsObj is null, it means the scope is dead.
-        if (scope.as<CallObject>().callee().nonLazyScript()->needsArgsObj())
+     */
-            return true;
+    static bool createMissingArguments(JSContext *cx, jsid id, ScopeObject &scope,
+                                       MutableHandleArgumentsObject argsObj)
+    {
+        MOZ_ASSERT(isMissingArguments(cx, id, scope));
+        argsObj.set(nullptr);
 
         ScopeIterVal *maybeScope = DebugScopes::hasLiveScope(scope);
-        if (!maybeScope) {
+        if (!maybeScope)
-            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE,
+            return true;
-                                 "Debugger scope");
-            return false;
-        }
 
-        *maybeArgsObj = ArgumentsObject::createUnexpected(cx, maybeScope->frame());
+        argsObj.set(ArgumentsObject::createUnexpected(cx, maybeScope->frame()));
-        return true;
+        return !!argsObj;
     }
 
   public:
@@ -1336,51 +1355,118 @@ class DebugScopeProxy : public BaseProxyHandler
         Rooted<DebugScopeObject*> debugScope(cx, &proxy->as<DebugScopeObject>());
         Rooted<ScopeObject*> scope(cx, &debugScope->scope());
 
-        RootedArgumentsObject maybeArgsObj(cx);
+        if (isMissingArguments(cx, id, *scope)) {
-        if (!checkForMissingArguments(cx, id, *scope, maybeArgsObj.address()))
+            RootedArgumentsObject argsObj(cx);
-            return false;
+            if (!createMissingArguments(cx, id, *scope, &argsObj))
+                return false;
+
+            if (!argsObj) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE,
+                                     "Debugger scope");
+                return false;
+            }
 
-        if (maybeArgsObj) {
             desc.object().set(debugScope);
             desc.setAttributes(JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT);
-            desc.value().setObject(*maybeArgsObj);
+            desc.value().setObject(*argsObj);
             desc.setGetter(nullptr);
             desc.setSetter(nullptr);
             return true;
         }
 
         RootedValue v(cx);
-        if (handleUnaliasedAccess(cx, debugScope, scope, id, GET, &v)) {
+        AccessResult access;
+        if (!handleUnaliasedAccess(cx, debugScope, scope, id, GET, &v, &access))
+            return false;
+
+        switch (access) {
+          case ACCESS_UNALIASED:
             desc.object().set(debugScope);
             desc.setAttributes(JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT);
             desc.value().set(v);
             desc.setGetter(nullptr);
             desc.setSetter(nullptr);
             return true;
+          case ACCESS_GENERIC:
+            return JS_GetOwnPropertyDescriptorById(cx, scope, id, desc);
+          case ACCESS_LOST:
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_OPTIMIZED_OUT);
+            return false;
+          default:
+            MOZ_ASSUME_UNREACHABLE("bad AccessResult");
         }
-
-        return JS_GetOwnPropertyDescriptorById(cx, scope, id, desc);
     }
 
-    bool get(JSContext *cx, HandleObject proxy, HandleObject receiver,  HandleId id,
+    bool get(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id,
              MutableHandleValue vp) MOZ_OVERRIDE
     {
         Rooted<DebugScopeObject*> debugScope(cx, &proxy->as<DebugScopeObject>());
         Rooted<ScopeObject*> scope(cx, &proxy->as<DebugScopeObject>().scope());
 
-        RootedArgumentsObject maybeArgsObj(cx);
+        if (isMissingArguments(cx, id, *scope)) {
-        if (!checkForMissingArguments(cx, id, *scope, maybeArgsObj.address()))
+            RootedArgumentsObject argsObj(cx);
-            return false;
+            if (!createMissingArguments(cx, id, *scope, &argsObj))
+                return false;
 
-        if (maybeArgsObj) {
+            if (!argsObj) {
-            vp.set(ObjectValue(*maybeArgsObj));
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_LIVE,
+                                     "Debugger scope");
+                return false;
+            }
+
+            vp.setObject(*argsObj);
             return true;
         }
 
-        if (handleUnaliasedAccess(cx, debugScope, scope, id, GET, vp))
+        AccessResult access;
-            return true;
+        if (!handleUnaliasedAccess(cx, debugScope, scope, id, GET, vp, &access))
+            return false;
 
-        return JSObject::getGeneric(cx, scope, scope, id, vp);
+        switch (access) {
+          case ACCESS_UNALIASED:
+            return true;
+          case ACCESS_GENERIC:
+            return JSObject::getGeneric(cx, scope, scope, id, vp);
+          case ACCESS_LOST:
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_DEBUG_OPTIMIZED_OUT);
+            return false;
+          default:
+            MOZ_ASSUME_UNREACHABLE("bad AccessResult");
+        }
+    }
+
+    /*
+     * Like 'get', but returns sentinel values instead of throwing on
+     * exceptional cases.
+     */
+    bool getMaybeSentinelValue(JSContext *cx, Handle<DebugScopeObject *> debugScope, HandleId id,
+                               MutableHandleValue vp)
+    {
+        Rooted<ScopeObject*> scope(cx, &debugScope->scope());
+
+        if (isMissingArguments(cx, id, *scope)) {
+            RootedArgumentsObject argsObj(cx);
+            if (!createMissingArguments(cx, id, *scope, &argsObj))
+                return false;
+            vp.set(argsObj ? ObjectValue(*argsObj) : MagicValue(JS_OPTIMIZED_ARGUMENTS));
+            return true;
+        }
+
+        AccessResult access;
+        if (!handleUnaliasedAccess(cx, debugScope, scope, id, GET, vp, &access))
+            return false;
+
+        switch (access) {
+          case ACCESS_UNALIASED:
+            return true;
+          case ACCESS_GENERIC:
+            return JSObject::getGeneric(cx, scope, scope, id, vp);
+          case ACCESS_LOST:
+            vp.setMagic(JS_OPTIMIZED_OUT);
+            return true;
+          default:
+            MOZ_ASSUME_UNREACHABLE("bad AccessResult");
+        }
     }
 
     bool set(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id, bool strict,
@@ -1388,9 +1474,19 @@ class DebugScopeProxy : public BaseProxyHandler
     {
         Rooted<DebugScopeObject*> debugScope(cx, &proxy->as<DebugScopeObject>());
         Rooted<ScopeObject*> scope(cx, &proxy->as<DebugScopeObject>().scope());
-        if (handleUnaliasedAccess(cx, debugScope, scope, id, SET, vp))
+
+        AccessResult access;
+        if (!handleUnaliasedAccess(cx, debugScope, scope, id, SET, vp, &access))
+            return false;
+
+        switch (access) {
+          case ACCESS_UNALIASED:
             return true;
-        return JSObject::setGeneric(cx, scope, scope, id, vp, strict);
+          case ACCESS_GENERIC:
+            return JSObject::setGeneric(cx, scope, scope, id, vp, strict);
+          default:
+            MOZ_ASSUME_UNREACHABLE("bad AccessResult");
+        }
     }
 
     bool defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
@@ -1551,6 +1647,13 @@ DebugScopeObject::isForDeclarative() const
     return s.is<CallObject>() || s.is<BlockObject>() || s.is<DeclEnvObject>();
 }
 
+bool
+DebugScopeObject::getMaybeSentinelValue(JSContext *cx, HandleId id, MutableHandleValue vp)
+{
+    Rooted<DebugScopeObject *> self(cx, this);
+    return DebugScopeProxy::singleton.getMaybeSentinelValue(cx, self, id, vp);
+}
+
 bool
 js_IsDebugScopeSlow(ProxyObject *proxy)
 {

js/src/vm/ScopeObject.h

@@ -801,6 +801,10 @@ class DebugScopeObject : public ProxyObject
 
     /* Currently, the 'declarative' scopes are Call and Block. */
     bool isForDeclarative() const;
+
+    // Get a property by 'id', but returns sentinel values instead of throwing
+    // on exceptional cases.
+    bool getMaybeSentinelValue(JSContext *cx, HandleId id, MutableHandleValue vp);
 };
 
 /* Maintains per-compartment debug scope bookkeeping information. */

toolkit/devtools/server/actors/script.js

@@ -4547,6 +4547,8 @@ EnvironmentActor.prototype = {
       let arg = {};
 
       let value = this.obj.getVariable(name);
+      // The slot is optimized out.
+      // FIXME: Need actual UI, bug 941287.
       if (value && value.optimizedOut) {
         continue;
       }
@@ -4583,18 +4585,11 @@ EnvironmentActor.prototype = {
         continue;
       }
 
-      let value;
+      let value = this.obj.getVariable(name);
-      try {
+      // The slot is optimized out or arguments on a dead scope.
-        value = this.obj.getVariable(name);
+      // FIXME: Need actual UI, bug 941287.
-        if (value && value.optimizedOut) {
+      if (value && (value.optimizedOut || value.missingArguments)) {
-          continue;
+        continue;
-        }
-      } catch (e) {
-        // Avoid "Debugger scope is not live" errors for |arguments|, introduced
-        // in bug 746601.
-        if (name != "arguments") {
-          throw e;
-        }
       }
 
       // TODO: this part should be removed in favor of the commented-out part

toolkit/devtools/webconsole/utils.js

@@ -1087,11 +1087,10 @@ let DebuggerEnvironmentSupport = {
   getProperty: function(aObj, aName)
   {
     // TODO: we should use getVariableDescriptor() here - bug 725815.
-    let result = undefined;
+    let result = aObj.getVariable(aName);
-    try {
+    // FIXME: Need actual UI, bug 941287.
-      result = aObj.getVariable(aName);
+    if (result.optimizedOut || result.missingArguments) {
-    } catch (ex) {
+      return null;
-      // getVariable() throws for invalid identifiers.
     }
     return result === undefined ? null : { value: result };
   },