From ea20ea661fc1987ec2fadad783455d14a813daf0 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Tue, 9 Oct 2012 12:02:30 -0700 Subject: [PATCH] Bug 795173: Make Debugger.Object.prototype.evalInGlobalWithBindings pass the right 'this' value. r=jorendorff This also adds a test for Debugger.Frame.prototype.evalWithBindings, to make sure it puts new variables on the right object; I couldn't find any existing tests for the non-strict case (Frame-evalWithBindings-11.js handles the strict mode case). --- .../tests/debug/Frame-evalWithBindings-11.js | 18 ++++++++ .../tests/debug/Object-evalInGlobal-06.js | 8 ++++ js/src/jsdbgapi.cpp | 6 ++- js/src/vm/Debugger.cpp | 45 ++++++++++++------- js/src/vm/Debugger.h | 5 ++- 5 files changed, 63 insertions(+), 19 deletions(-) create mode 100644 js/src/jit-test/tests/debug/Frame-evalWithBindings-11.js create mode 100644 js/src/jit-test/tests/debug/Object-evalInGlobal-06.js diff --git a/js/src/jit-test/tests/debug/Frame-evalWithBindings-11.js b/js/src/jit-test/tests/debug/Frame-evalWithBindings-11.js new file mode 100644 index 00000000000..b1df83a19c8 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-evalWithBindings-11.js @@ -0,0 +1,18 @@ +// var statements in non-strict evalWithBindings code behave like non-strict direct eval. +var g = newGlobal(); +var dbg = new Debugger(g); +var log; +dbg.onDebuggerStatement = function (frame) { + log += 'd'; + assertEq(frame.evalWithBindings("var i = v; 42;", { v: 'inner' }).return, 42); +}; + +g.i = 'outer'; +log = ''; +assertEq(g.eval('debugger; i;'), 'inner'); +assertEq(log, 'd'); + +g.j = 'outer'; +log = ''; +assertEq(g.eval('debugger; j;'), 'outer'); +assertEq(log, 'd'); diff --git a/js/src/jit-test/tests/debug/Object-evalInGlobal-06.js b/js/src/jit-test/tests/debug/Object-evalInGlobal-06.js new file mode 100644 index 00000000000..329cdf3e16c --- /dev/null +++ b/js/src/jit-test/tests/debug/Object-evalInGlobal-06.js @@ -0,0 +1,8 @@ +// Debugger.Object.prototype.evalInGlobal sets 'this' to the global. + +var dbg = new Debugger; +var g1 = newGlobal(); +var g1w = dbg.addDebuggee(g1); + +assertEq(g1w.evalInGlobal('this').return, g1w); +assertEq(g1w.evalInGlobalWithBindings('this', { x:42 }).return, g1w); diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 4b1496880f4..3ebd9af5baa 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -747,8 +747,12 @@ JS_EvaluateUCInStackFrame(JSContext *cx, JSStackFrame *fpArg, StackFrame *fp = Valueify(fpArg); + if (!ComputeThis(cx, fp)) + return false; + RootedValue thisv(cx, fp->thisValue()); + js::AutoCompartment ac(cx, env); - return EvaluateInEnv(cx, env, fp, chars, length, filename, lineno, rval); + return EvaluateInEnv(cx, env, thisv, fp, chars, length, filename, lineno, rval); } JS_PUBLIC_API(JSBool) diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 4e202c93383..c3c43f0107d 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -3390,25 +3390,27 @@ DebuggerFrame_setOnPop(JSContext *cx, unsigned argc, Value *vp) return true; } +/* + * Evaluate |chars[0..length-1]| in the environment |env|, treating that + * source as appearing starting at |lineno| in |filename|. Store the return + * value in |*rval|. Use |thisv| as the 'this' value. + * + * If |fp| is non-NULL, evaluate as for a direct eval in that frame; |env| + * must be either |fp|'s DebugScopeObject, or some extension of that + * environment; either way, |fp|'s scope is where newly declared variables + * go. In this case, |fp| must have a computed 'this' value, equal to |thisv|. + */ JSBool -js::EvaluateInEnv(JSContext *cx, Handle env, StackFrame *fp, const jschar *chars, - unsigned length, const char *filename, unsigned lineno, Value *rval) +js::EvaluateInEnv(JSContext *cx, Handle env, HandleValue thisv, StackFrame *fp, + const jschar *chars, unsigned length, const char *filename, unsigned lineno, + Value *rval) { assertSameCompartment(cx, env, fp); + JS_ASSERT_IF(fp, thisv.get() == fp->thisValue()); JS_ASSERT(!IsPoisonedPtr(chars)); SkipRoot skip(cx, &chars); - RootedValue thisv(cx); - if (fp) { - /* Execute assumes an already-computed 'this" value. */ - if (!ComputeThis(cx, fp)) - return false; - thisv = fp->thisValue(); - } else { - thisv = ObjectValue(*env); - } - /* * NB: This function breaks the assumption that the compiler can see all * calls and properly compute a static level. In practice, any non-zero @@ -3480,9 +3482,20 @@ DebuggerGenericEval(JSContext *cx, const char *fullMethodName, else ac.construct(cx, scope); - Rooted env(cx, fp ? GetDebugScopeForFrame(cx, fp) : scope); - if (!env) - return false; + RootedValue thisv(cx); + Rooted env(cx); + if (fp) { + /* ExecuteInEnv requires 'fp' to have a computed 'this" value. */ + if (!ComputeThis(cx, fp)) + return false; + thisv = fp->thisValue(); + env = GetDebugScopeForFrame(cx, fp); + if (!env) + return false; + } else { + thisv = ObjectValue(*scope); + env = scope; + } /* If evalWithBindings, create the inner environment. */ if (bindings) { @@ -3505,7 +3518,7 @@ DebuggerGenericEval(JSContext *cx, const char *fullMethodName, /* Run the code and produce the completion value. */ Value rval; JS::Anchor anchor(stable); - bool ok = EvaluateInEnv(cx, env, fp, stable->chars(), stable->length(), + bool ok = EvaluateInEnv(cx, env, thisv, fp, stable->chars(), stable->length(), "debugger eval code", 1, &rval); return dbg->receiveCompletionValue(ac, ok, rval, vp); } diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index ba7c0bdf1c1..d21031de73f 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -523,8 +523,9 @@ Debugger::onNewScript(JSContext *cx, JSScript *script, GlobalObject *compileAndG } extern JSBool -EvaluateInEnv(JSContext *cx, Handle env, StackFrame *fp, const jschar *chars, - unsigned length, const char *filename, unsigned lineno, Value *rval); +EvaluateInEnv(JSContext *cx, Handle env, HandleValue thisv, StackFrame *fp, + const jschar *chars, unsigned length, const char *filename, unsigned lineno, + Value *rval); }