Bug 1249193 - Fix Debugger.Frame.this to work correctly if we're still in the script's prologue. r=shu
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 29 Feb 2016 09:10:28 +0100
changeset 324304 cb28a396b8846c4f4d863974c3cacaac0a8cddbb
parent 324303 41a76dfcce45e8216516b390752dfe3365b0c777
child 324305 f09fc69822a53c51075d0025c65732b3342525a8
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1249193
milestone47.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 1249193 - Fix Debugger.Frame.this to work correctly if we're still in the script's prologue. r=shu
js/src/jit-test/tests/debug/Frame-this-06.js
js/src/jit-test/tests/debug/Frame-this-09.js
js/src/jit-test/tests/debug/Frame-this-10.js
js/src/vm/ScopeObject.cpp
--- a/js/src/jit-test/tests/debug/Frame-this-06.js
+++ b/js/src/jit-test/tests/debug/Frame-this-06.js
@@ -14,9 +14,9 @@ dbg.onEnterFrame = function (frame) {
 g.eval("var foo = function() { 'use strict'; }; foo.call(33);");
 assertEq(evalThis.return, 33);
 assertEq(frameThis, 33);
 
 // Non-strict, this has to be boxed.
 g.eval("var bar = function() { }; bar.call(22);");
 assertEq(typeof evalThis.return, "object");
 assertEq(evalThis.return.unsafeDereference().valueOf(), 22);
-assertEq(frameThis.optimizedOut, true); // Frame.this currently doesn't box missing primitive this.
+assertEq(frameThis.unsafeDereference().valueOf(), 22);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Frame-this-09.js
@@ -0,0 +1,45 @@
+// Ensure |Frame.this| returns the right value even if we're still in the
+// script's prologue, before JSOP_FUNCTIONTHIS.
+
+var g = newGlobal();
+var dbg = new Debugger(g);
+var hits = 0;
+dbg.onEnterFrame = function (frame) {
+    if (frame.type === 'eval')
+        return;
+    hits++;
+
+    var frameThis = frame.this;
+    if (frameThis !== null && typeof frameThis === "object")
+        g.gotThis = frameThis.unsafeDereference();
+    else
+        g.gotThis = frameThis;
+
+    assertEq(frame.this, frameThis, "getter should not return a different object");
+};
+
+// Strict mode, function uses |this|.
+g.eval("function strictfun() { 'use strict'; return this; }");
+g.eval("strictfun.call(Math); assertEq(gotThis, Math);");
+g.eval("strictfun.call(true); assertEq(gotThis, true);");
+g.eval("strictfun.call();     assertEq(gotThis, undefined);");
+
+// Strict mode, function doesn't use |this|.
+g.eval("function strictfunNoThis() { 'use strict'; }");
+g.eval("strictfunNoThis.call(Math); assertEq(gotThis, Math);");
+g.eval("strictfunNoThis.call(true); assertEq(gotThis, true);");
+g.eval("strictfunNoThis.call(null); assertEq(gotThis, null);");
+
+// Non-strict mode (primitive |this| is boxed), function uses |this|.
+g.eval("function nonstrictfun() { return this; }");
+g.eval("nonstrictfun.call(Math); assertEq(gotThis, Math);");
+g.eval("nonstrictfun.call(null); assertEq(gotThis, this);");
+g.eval("nonstrictfun.call();     assertEq(gotThis, this);");
+
+// Non-strict mode (primitive |this| is boxed), function doesn't use |this|.
+g.eval("function nonstrictfunNoThis() {}");
+g.eval("nonstrictfunNoThis.call(Math); assertEq(gotThis, Math);");
+g.eval("nonstrictfunNoThis.call(null); assertEq(gotThis, this);");
+g.eval("nonstrictfunNoThis.call();     assertEq(gotThis, this);");
+
+assertEq(hits, 12);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Frame-this-10.js
@@ -0,0 +1,42 @@
+// Check the Frame.this getter always returns the same object for a given frame.
+// Primitive this-values should not be boxed multiple times.
+
+var g = newGlobal();
+var dbg = new Debugger(g);
+var framesEntered = 0;
+var framesPopped = 0;
+var numSteps = 0;
+dbg.onEnterFrame = function (frame) {
+    if (frame.type === 'eval')
+        return;
+    framesEntered++;
+
+    var frameThis = frame.this;
+    assertEq(frame.this, frameThis);
+
+    frame.onPop = function() {
+        framesPopped++;
+        assertEq(frame.this, frameThis);
+    };
+
+    frame.onStep = function() {
+        numSteps++;
+        assertEq(frame.this, frameThis);
+    }
+
+    g.gotThis = frameThis.unsafeDereference();
+};
+
+g.eval("function nonstrictfun() { return this; }");
+g.eval("nonstrictfun.call(Math); assertEq(gotThis, Math);");
+g.eval("nonstrictfun.call(true); assertEq(gotThis.valueOf(), true);");
+g.eval("nonstrictfun.call(); assertEq(gotThis, this);");
+
+g.eval("function nonstrictfunNoThis() { return 1; }");
+g.eval("nonstrictfunNoThis.call(Math); assertEq(gotThis, Math);");
+g.eval("nonstrictfunNoThis.call(true); assertEq(gotThis.valueOf(), true);");
+g.eval("nonstrictfunNoThis.call(); assertEq(gotThis, this);");
+
+assertEq(framesEntered, 6);
+assertEq(framesPopped, 6);
+assertEq(numSteps > 15, true);
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -3166,26 +3166,53 @@ js::GetThisValueForDebuggerMaybeOptimize
             return true;
         }
 
         if (si.type() != ScopeIter::Call || si.fun().hasLexicalThis())
             continue;
 
         RootedScript script(cx, si.fun().nonLazyScript());
 
+        if (si.withinInitialFrame() &&
+            (pc < script->main() || !script->functionHasThisBinding()))
+        {
+            // Either we're in the script prologue and we may still have to
+            // initialize the this-binding (JSOP_FUNCTIONTHIS), or the script
+            // does not have a this-binding (because it doesn't use |this|).
+
+            // If our this-argument is an object, or we're in strict mode,
+            // the this-binding is always the same as our this-argument.
+            if (frame.thisArgument().isObject() || script->strict()) {
+                res.set(frame.thisArgument());
+                return true;
+            }
+
+            // Figure out if we already executed JSOP_FUNCTIONTHIS.
+            bool executedInitThisOp = false;
+            if (script->functionHasThisBinding()) {
+                jsbytecode* initThisPc = script->code();
+                while (*initThisPc != JSOP_FUNCTIONTHIS && initThisPc < script->main())
+                    initThisPc = GetNextPc(initThisPc);
+                executedInitThisOp = (pc > initThisPc);
+            }
+
+            if (!executedInitThisOp) {
+                // We didn't initialize the this-binding yet. Determine the
+                // correct |this| value for this frame (box primitives if not
+                // in strict mode), and assign it to the this-argument slot so
+                // JSOP_FUNCTIONTHIS will use it and not box a second time.
+                if (!GetFunctionThis(cx, frame, res))
+                    return false;
+                frame.thisArgument() = res;
+                return true;
+            }
+        }
+
         if (!script->functionHasThisBinding()) {
-            MOZ_ASSERT(!script->isDerivedClassConstructor(),
-                       "Derived class constructors always have a this-binding");
-
-            // If we're still inside `frame`, we can use the this-value passed
-            // to it, if it does not require boxing.
-            if (si.withinInitialFrame() && (frame.thisArgument().isObject() || script->strict()))
-                res.set(frame.thisArgument());
-            else
-                res.setMagic(JS_OPTIMIZED_OUT);
+            res.setMagic(JS_OPTIMIZED_OUT);
             return true;
         }
 
         BindingIter bi = Bindings::thisBinding(cx, script);
 
         if (script->bindingIsAliased(bi)) {
             RootedObject callObj(cx, &si.scope().as<CallObject>());
             return GetProperty(cx, callObj, callObj, cx->names().dotThis, res);