Bug 746601 - unexpected 'arguments' read from the debugger should work like f.arguments (r=jimb)

js/src/jit-test/tests/debug/Environment-gc-01.js

@@ -9,11 +9,11 @@ assertEq(g.f(22), 44);
 dbg.onEnterFrame = undefined;
 
 assertEq(env.find("x"), env);
-assertEq(env.names().join(","), "x");
+assertEq(env.names().join(","), "arguments,x");
 
 gc();
 g.gc(g);
 gc(env);
 
 assertEq(env.find("x"), env);
-assertEq(env.names().join(","), "x");
+assertEq(env.names().join(","), "arguments,x");

js/src/jit-test/tests/debug/Frame-environment-04.js

@@ -5,7 +5,7 @@ g.eval("function f(x) { return 2 * x; }");
 var dbg = Debugger(g);
 var hits = 0;
 dbg.onEnterFrame = function (frame) {
-    assertEq(frame.environment.names().join(","), "x");
+    assertEq(frame.environment.names().join(","), "arguments,x");
     hits++;
 };
 assertEq(g.f(22), 44);

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

@@ -1,15 +1,23 @@
-// Test that 'arguments' access in a function that doesn't expect 'arguments'
-// doesn't crash.
-
-// TODO bug 659577: the debugger should be improved to throw an error in such
-// cases rather than silently returning whatever it finds on the scope chain.
+// The arguments can escape from a function via a debugging hook.
 
 var g = newGlobal('new-compartment');
 var dbg = new Debugger(g);
 
 // capture arguments object and test function
+var args, testfn;
 dbg.onDebuggerStatement = function (frame) {
-    args = frame.eval("arguments");
+    args = frame.eval("arguments").return;
+    testfn = frame.eval("test").return;
 };
 g.eval("function f() { debugger; }");
+g.eval("var test = " + function test(args) {
+        assertEq(args.length, 3);
+        assertEq(args[0], this);
+        assertEq(args[1], f);
+        assertEq(args[2].toString(), "[object Object]");
+        return 42;
+    } + ";");
 g.eval("f(this, f, {});");
+
+var cv = testfn.apply(null, [args]);
+assertEq(cv.return, 42);

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

@@ -0,0 +1,15 @@
+// The arguments can escape from a function via a debugging hook.
+
+var g = newGlobal('new-compartment');
+var dbg = new Debugger(g);
+
+// capture arguments object and test function
+var hits = 0;
+dbg.onDebuggerStatement = function (frame) {
+    assertEq(frame.older.eval('arguments[0]').return, 'ponies');
+    hits++;
+};
+g.eval("function g() { debugger; }");
+g.eval("function f() { g(); }");
+g.eval("f('ponies')");
+assertEq(hits, 1);

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

@@ -0,0 +1,19 @@
+// The arguments can escape from a function via a debugging hook.
+
+var g = newGlobal('new-compartment');
+var dbg = new Debugger(g);
+
+// capture arguments object and test function
+var hits = 0;
+dbg.onDebuggerStatement = function (frame) {
+    try {
+        frame.older.environment.parent.getVariable('arguments')
+    } 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/jit-test/tests/debug/Frame-evalWithBindings-03.js

@@ -1,5 +1,4 @@
 // arguments works in evalWithBindings (it does not interpose a function scope)
-// when the function expects 'arguments'
 var g = newGlobal('new-compartment');
 var dbg = new Debugger;
 var global = dbg.addDebuggee(g);
@@ -12,6 +11,6 @@ dbg.onDebuggerStatement = function (frame) {
         assertEq(frame.evalWithBindings("arguments[i]", {i: i}).return, frame.arguments[i]);
     hits++;
 };
-g.eval("function f() { eval('arguments'); debugger; }");
+g.eval("function f() { debugger; }");
 g.eval("f(undefined, -0, NaN, '\uffff', Array.prototype, Math, f);");
 assertEq(hits, 1);

js/src/jit-test/tests/debug/Frame-onPop-21.js

@@ -16,6 +16,7 @@ function handlePop(c) {
     // Arguments must be live.
     assertEq(this.eval('a').return, 'frieze');
     assertEq(this.eval('b = "architrave"').return, 'architrave');
+    assertEq(this.eval('arguments[1]').return, 'architrave');
     assertEq(this.eval('b').return, 'architrave');
 
     // function-scope variables must be live.

js/src/vm/Debugger.cpp

@@ -3012,7 +3012,7 @@ CheckThisFrame(JSContext *cx, const CallArgs &args, const char *fnname, bool che
         }
         if (checkLive) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_NOT_LIVE,
-                                 "Debugger.Frame", fnname, "stack frame");
+                                 "Debugger.Frame");
             return NULL;
         }
     }

js/src/vm/ScopeObject.cpp

@@ -308,10 +308,7 @@ CallObject::getVarOp(JSContext *cx, JSObject *obj, jsid id, Value *vp)
     else
         *vp = callobj.var(i);
 
-    /* This can only happen via the debugger. Bug 659577 will remove it. */
-    if (vp->isMagic(JS_OPTIMIZED_ARGUMENTS))
-        *vp = UndefinedValue();
-
+    JS_ASSERT(!vp->isMagic(JS_OPTIMIZED_ARGUMENTS));
     return true;
 }
 
@@ -1273,8 +1270,60 @@ namespace js {
  *    proxy can either hide these optimizations or make the situation more
  *    clear to the debugger. An example is 'arguments'.
  */
-struct DebugScopeProxy : BaseProxyHandler
+class DebugScopeProxy : public BaseProxyHandler
 {
+    static bool isArguments(JSContext *cx, jsid id)
+    {
+        return id == NameToId(cx->runtime->atomState.argumentsAtom);
+    }
+
+    static bool isFunctionScope(ScopeObject &scope)
+    {
+        return scope.isCall() && !scope.asCall().isForEval();
+    }
+
+    /*
+     * In theory, every function scope contains an 'arguments' bindings.
+     * However, the engine only adds a binding if 'arguments' is used in the
+     * function body. Thus, from the debugger's perspective, 'arguments' may be
+     * missing from the list of bindings.
+     */
+    static bool isMissingArgumentsBinding(ScopeObject &scope)
+    {
+        return isFunctionScope(scope) &&
+               !scope.asCall().getCalleeFunction()->script()->argumentsHasLocalBinding();
+    }
+
+    /*
+     * This function creates an arguments object when the debugger requests
+     * 'arguments' for a function scope where the arguments object has been
+     * optimized away (either because the binding is missing altogether or
+     * because !ScriptAnalysis::needsArgsObj).
+     */
+    static bool checkForMissingArguments(JSContext *cx, jsid id, ScopeObject &scope,
+                                         ArgumentsObject **maybeArgsObj)
+    {
+        *maybeArgsObj = NULL;
+
+        if (!isArguments(cx, id) || !isFunctionScope(scope))
+            return true;
+
+        JSScript *script = scope.asCall().getCalleeFunction()->script();
+        if (script->needsArgsObj())
+            return true;
+
+        StackFrame *fp = scope.maybeStackFrame();
+        if (!fp) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_NOT_LIVE,
+                                 "Debugger scope");
+            return false;
+        }
+
+        *maybeArgsObj = ArgumentsObject::createUnexpected(cx, fp);
+        return true;
+    }
+
+  public:
     static int family;
     static DebugScopeProxy singleton;
 
@@ -1289,15 +1338,38 @@ struct DebugScopeProxy : BaseProxyHandler
     bool getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, bool set,
                                   PropertyDescriptor *desc) MOZ_OVERRIDE
     {
-        AutoAllowUnaliasedVarAccess a(cx);
         ScopeObject &scope = proxy->asDebugScope().scope();
+
+        ArgumentsObject *maybeArgsObj;
+        if (!checkForMissingArguments(cx, id, scope, &maybeArgsObj))
+            return false;
+
+        if (maybeArgsObj) {
+            PodZero(desc);
+            desc->obj = proxy;
+            desc->attrs = JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT;
+            desc->value = ObjectValue(*maybeArgsObj);
+            return true;
+        }
+
+        AutoAllowUnaliasedVarAccess a(cx);
         return JS_GetPropertyDescriptorById(cx, &scope, id, JSRESOLVE_QUALIFIED, desc);
     }
 
     bool get(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, Value *vp) MOZ_OVERRIDE
     {
-        AutoAllowUnaliasedVarAccess a(cx);
         ScopeObject &scope = proxy->asDebugScope().scope();
+
+        ArgumentsObject *maybeArgsObj;
+        if (!checkForMissingArguments(cx, id, scope, &maybeArgsObj))
+            return false;
+
+        if (maybeArgsObj) {
+            *vp = ObjectValue(*maybeArgsObj);
+            return true;
+        }
+
+        AutoAllowUnaliasedVarAccess a(cx);
         return scope.getGeneric(cx, &scope, id, vp);
     }
 
@@ -1317,6 +1389,13 @@ struct DebugScopeProxy : BaseProxyHandler
     bool getOwnPropertyNames(JSContext *cx, JSObject *proxy, AutoIdVector &props) MOZ_OVERRIDE
     {
         ScopeObject &scope = proxy->asDebugScope().scope();
+
+        if (isMissingArgumentsBinding(scope) &&
+            !props.append(NameToId(cx->runtime->atomState.argumentsAtom)))
+        {
+            return false;
+        }
+
         return GetPropertyNames(cx, &scope, JSITER_OWNONLY, &props);
     }
 
@@ -1330,8 +1409,32 @@ struct DebugScopeProxy : BaseProxyHandler
     bool enumerate(JSContext *cx, JSObject *proxy, AutoIdVector &props) MOZ_OVERRIDE
     {
         ScopeObject &scope = proxy->asDebugScope().scope();
+
+        if (isMissingArgumentsBinding(scope) &&
+            !props.append(NameToId(cx->runtime->atomState.argumentsAtom)))
+        {
+            return false;
+        }
+
         return GetPropertyNames(cx, &scope, 0, &props);
     }
+
+    bool has(JSContext *cx, JSObject *proxy, jsid id, bool *bp) MOZ_OVERRIDE
+    {
+        ScopeObject &scope = proxy->asDebugScope().scope();
+
+        if (isArguments(cx, id) && isFunctionScope(scope)) {
+            *bp = true;
+            return true;
+        }
+
+        JSBool found;
+        if (!JS_HasPropertyById(cx, &scope, id, &found))
+            return false;
+
+        *bp = found;
+        return true;
+    }
 };
 
 }  /* namespace js */