Bug 1397049 - Fix debugger 'this' in functions with let. r=jorendorff a=gchang
authorTed Campbell <tcampbell@mozilla.com>
Thu, 16 Nov 2017 00:39:36 -0500
changeset 442505 47c97637717fc01e6abcf769bb00ce3cc7a16276
parent 442504 33e95d96edd254f38dcd29bae7f9974ebfeea703
child 442506 168cc6fc9f82604937187b106e830f80d4e7f736
push id8236
push userapavel@mozilla.com
push dateMon, 27 Nov 2017 10:06:01 +0000
treeherdermozilla-beta@47c97637717f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, gchang
bugs1397049
milestone58.0
Bug 1397049 - Fix debugger 'this' in functions with let. r=jorendorff a=gchang When the debugger evaluates code in the debuggee frame, the parser may fail to detect we are in a function context and will compute 'this' incorrectly as a result. This patch fixes the environment chain traversal around DebugEnvironmentProxy to more accurately determine the binding type of 'this' within a function. MozReview-Commit-ID: GzRDOJLK8fx
js/src/frontend/Parser.cpp
js/src/jit-test/tests/debug/bug1397049.js
js/src/vm/EnvironmentObject.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -336,32 +336,35 @@ EvalSharedContext::EvalSharedContext(JSC
   : SharedContext(cx, Kind::Eval, directives, extraWarnings),
     enclosingScope_(cx, enclosingScope),
     bindings(cx)
 {
     computeAllowSyntax(enclosingScope);
     computeInWith(enclosingScope);
     computeThisBinding(enclosingScope);
 
-    // Like all things Debugger, Debugger.Frame.eval needs special
-    // handling. Since the environment chain of such evals are non-syntactic
-    // (DebuggerEnvironmentProxy is not an EnvironmentObject), computing the
-    // this binding with respect to enclosingScope is incorrect if the
-    // Debugger.Frame is a function frame. Recompute the this binding if we
-    // are such an eval.
+    // If this eval is in response to Debugger.Frame.eval, we may have been
+    // passed an incomplete scope chain. In order to better determine the 'this'
+    // binding type, we traverse the environment chain, looking for a CallObject
+    // and recompute the binding type based on its body scope.
+    //
+    // NOTE: A non-debug eval in a non-syntactic environment will also trigger
+    // this code. In that case, we should still compute the same binding type.
     if (enclosingEnv && enclosingScope->hasOnChain(ScopeKind::NonSyntactic)) {
-        // For Debugger.Frame.eval with bindings, the environment chain may
-        // have more than the DebugEnvironmentProxy.
         JSObject* env = enclosingEnv;
         while (env) {
+            // Look at target of any DebugEnvironmentProxy, but be sure to use
+            // enclosingEnvironment() of the proxy itself.
+            JSObject* unwrapped = env;
             if (env->is<DebugEnvironmentProxy>())
-                env = &env->as<DebugEnvironmentProxy>().environment();
-
-            if (env->is<CallObject>()) {
-                computeThisBinding(env->as<CallObject>().callee().nonLazyScript()->bodyScope());
+                unwrapped = &env->as<DebugEnvironmentProxy>().environment();
+
+            if (unwrapped->is<CallObject>()) {
+                JSFunction* callee = &unwrapped->as<CallObject>().callee();
+                computeThisBinding(callee->nonLazyScript()->bodyScope());
                 break;
             }
 
             env = env->enclosingEnvironment();
         }
     }
 }
 
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/bug1397049.js
@@ -0,0 +1,40 @@
+// Run debugger in its own global
+let g = newGlobal();
+g.target = this;
+g.evaluate(`
+    let d = new Debugger;
+    let gw = d.addDebuggee(target);
+
+    d.onDebuggerStatement = function(frame)
+    {
+        frame = frame.older;
+
+        let res = frame.eval("this");
+        assertEq(res.return, frame.this);
+
+        res = frame.evalWithBindings("this", {x:42});
+        assertEq(res.return, frame.this);
+    }
+`);
+
+// Debugger statement affects parse so hide in another function
+function brk() { debugger; }
+
+function f1() {
+    var temp = "string";
+    brk();
+}
+
+function f2() {
+    let temp = "string";
+    brk();
+}
+
+function f3() {
+    const temp = "string";
+    brk();
+}
+
+f1.call({});
+f2.call({});
+f3.call({});
--- a/js/src/vm/EnvironmentObject.h
+++ b/js/src/vm/EnvironmentObject.h
@@ -903,16 +903,19 @@ class DebugEnvironmentProxy : public Pro
      * frame that has been popped.
      */
     static const unsigned SNAPSHOT_SLOT = 1;
 
   public:
     static DebugEnvironmentProxy* create(JSContext* cx, EnvironmentObject& env,
                                          HandleObject enclosing);
 
+    // NOTE: The environment may be a debug hollow with invalid
+    // enclosingEnvironment. Always use the enclosingEnvironment accessor on
+    // the DebugEnvironmentProxy in order to walk the environment chain.
     EnvironmentObject& environment() const;
     JSObject& enclosingEnvironment() const;
 
     /* May only be called for proxies to function call objects. */
     ArrayObject* maybeSnapshot() const;
     void initSnapshot(ArrayObject& snapshot);
 
     // Currently, the 'declarative' environments are function, module, and