Bug 738479 - Debugger.Script objects whose referents are non-compile-and-go are crashy. Also fixes bug 746973. r=jimb.

--HG--
extra : rebase_source : 406065559a1e26485503b6d1e4d7eeea9070321f

js/src/jit-test/tests/debug/breakpoint-09.js

@@ -0,0 +1,13 @@
+// Setting a breakpoint in an eval script that is not on the stack. Bug 746973.
+// We don't assert that the breakpoint actually hits because that depends on
+// the eval cache, an implementation detail.
+
+var g = newGlobal('new-compartment');
+var dbg = Debugger(g);
+g.eval("function f() { return eval('2+2'); }");
+var s;
+dbg.onNewScript = function (script) { s = script; };
+g.f();
+for (var offset of s.getLineOffsets(s.startLine))
+    s.setBreakpoint(offset, {hit: function () {}});
+assertEq(g.f(), 4);

js/src/jit-test/tests/debug/breakpoint-noncng.js

@@ -0,0 +1,20 @@
+// Breakpoints work in non-compile-and-go code. Bug 738479.
+
+var g = newGlobal('new-compartment');
+g.s = '';
+var dbg = new Debugger;
+var gw = dbg.addDebuggee(g);
+g.evaluateNonCompileAndGo(
+    "function f() {\n" +  // fscript.startLine
+    "    s += 'a';\n" +   // fscript.startLine + 1
+    "    s += 'b';\n" +   // fscript.startLine + 2
+    "}\n");
+
+var fscript = gw.makeDebuggeeValue(g.f).script;
+var handler = {hit: function (frame) { g.s += '1'; }};
+for (var pc of fscript.getLineOffsets(fscript.startLine + 2))
+    fscript.setBreakpoint(pc, handler);
+
+g.f();
+
+assertEq(g.s, "a1b");

js/src/shell/js.cpp

@@ -822,7 +822,7 @@ EvaluateWithLocation(JSContext *cx, unsigned argc, jsval *vp)
 }
 
 static JSBool
-Evaluate(JSContext *cx, unsigned argc, jsval *vp)
+EvaluateCommon(JSContext *cx, unsigned argc, jsval *vp, bool compileAndGo)
 {
     if (argc != 1 || !JSVAL_IS_STRING(JS_ARGV(cx, vp)[0])) {
         JS_ReportErrorNumber(cx, my_GetErrorMessage, NULL,
@@ -848,7 +848,30 @@ Evaluate(JSContext *cx, unsigned argc, jsval *vp)
         return false;
     }
 
-    return JS_EvaluateUCScript(cx, thisobj, codeChars, codeLength, "@evaluate", 0, vp);
+    if (compileAndGo) {
+        // JS_EvaluateUCScript always enables the compile-and-go option.
+        return JS_EvaluateUCScript(cx, thisobj, codeChars, codeLength, "@evaluate", 1, vp);
+    } else {
+        uint32_t saved = JS_GetOptions(cx);
+
+        JS_SetOptions(cx, saved & ~JSOPTION_COMPILE_N_GO);
+        JSScript *script = JS_CompileUCScript(cx, thisobj, codeChars, codeLength, "@evaluate", 1);
+        JS_SetOptions(cx, saved);
+
+        return script && JS_ExecuteScript(cx, thisobj, script, vp);
+    }
+}
+
+static JSBool
+Evaluate(JSContext *cx, unsigned argc, jsval *vp)
+{
+    return EvaluateCommon(cx, argc, vp, true);
+}
+
+static JSBool
+EvaluateNonCompileAndGo(JSContext *cx, unsigned argc, jsval *vp)
+{
+    return EvaluateCommon(cx, argc, vp, false);
 }
 
 static JSString *
@@ -3485,6 +3508,10 @@ static JSFunctionSpecWithHelp shell_functions[] = {
 "evaluate(code)",
 "  Evaluate code as though it were the contents of a file."),
 
+    JS_FN_HELP("evaluateNonCompileAndGo", EvaluateNonCompileAndGo, 1, 0,
+"evaluateNonCompileAndGo(code)",
+"  Evaluate code like evaluate() but with compile-and-go turned off."),
+
     JS_FN_HELP("evalWithLocation", EvaluateWithLocation, 3, 0,
 "evalWithLocation(code, filename, lineno)",
 "  Eval code as if loaded from the given filename and line number."),

js/src/vm/Debugger.cpp

@@ -261,28 +261,6 @@ BreakpointSite::BreakpointSite(JSScript *script, jsbytecode *pc)
     JS_INIT_CLIST(&breakpoints);
 }
 
-/*
- * Precondition: script is live, meaning either it is a non-held script that is
- * on the stack or a held script that hasn't been GC'd.
- */
-static GlobalObject *
-ScriptGlobal(JSContext *cx, JSScript *script, GlobalObject *scriptGlobal)
-{
-    if (scriptGlobal)
-        return scriptGlobal;
-
-    /*
-     * The referent is a non-held script. There is no direct reference from
-     * script to the scope, so find it on the stack.
-     */
-    for (AllFramesIter i(cx->stack.space()); ; ++i) {
-        JS_ASSERT(!i.done());
-        if (i.fp()->maybeScript() == script)
-            return &i.fp()->global();
-    }
-    JS_NOT_REACHED("ScriptGlobal: live non-held script not on stack");
-}
-
 void
 BreakpointSite::recompile(FreeOp *fop)
 {
@@ -1138,6 +1116,18 @@ Debugger::onTrap(JSContext *cx, Value *vp)
         if (!site || !site->hasBreakpoint(bp))
             continue;
 
+
+        /*
+         * There are two reasons we have to check whether dbg is enabled and
+         * debugging scriptGlobal.
+         *
+         * One is just that one breakpoint handler can disable other Debuggers
+         * or remove debuggees.
+         *
+         * The other has to do with non-compile-and-go scripts, which have no
+         * specific global--until they are executed. Only now do we know which
+         * global the script is running against.
+         */
         Debugger *dbg = bp->debugger;
         if (dbg->enabled && dbg->debuggees.lookup(scriptGlobal)) {
             AutoCompartment ac(cx, dbg->object);
@@ -2832,6 +2822,26 @@ DebuggerScript_getLineOffsets(JSContext *cx, unsigned argc, Value *vp)
     return true;
 }
 
+bool
+Debugger::observesScript(JSScript *script) const
+{
+    if (!enabled)
+        return false;
+
+    /* Does the script have a global stored in it? */
+    if (GlobalObject *global = script->getGlobalObjectOrNull())
+        return observesGlobal(global);
+
+    /* Is the script in a compartment this Debugger is debugging? */
+    JSCompartment *comp = script->compartment();
+    for (GlobalObjectSet::Range r = debuggees.all(); !r.empty(); r.popFront()) {
+        if (r.front()->compartment() == comp)
+            return true;
+    }
+
+    return false;
+}
+
 static JSBool
 DebuggerScript_setBreakpoint(JSContext *cx, unsigned argc, Value *vp)
 {
@@ -2839,8 +2849,7 @@ DebuggerScript_setBreakpoint(JSContext *cx, unsigned argc, Value *vp)
     THIS_DEBUGSCRIPT_SCRIPT(cx, argc, vp, "setBreakpoint", args, obj, script);
     Debugger *dbg = Debugger::fromChildJSObject(obj);
 
-    RootedVar<GlobalObject*> scriptGlobal(cx, script->getGlobalObjectOrNull());
+    if (!dbg->observesScript(script)) {
-    if (!dbg->observesGlobal(ScriptGlobal(cx, script, scriptGlobal))) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_NOT_DEBUGGING);
         return false;
     }
@@ -2854,6 +2863,7 @@ DebuggerScript_setBreakpoint(JSContext *cx, unsigned argc, Value *vp)
         return false;
 
     jsbytecode *pc = script->code + offset;
+    RootedVar<GlobalObject *> scriptGlobal(cx, script->getGlobalObjectOrNull());
     BreakpointSite *site = script->getOrCreateBreakpointSite(cx, pc, scriptGlobal);
     if (!site)
         return false;

js/src/vm/Debugger.h

@@ -283,6 +283,7 @@ class Debugger {
     inline bool observesNewScript() const;
     inline bool observesGlobal(GlobalObject *global) const;
     inline bool observesFrame(StackFrame *fp) const;
+    bool observesScript(JSScript *script) const;
 
     /*
      * If env is NULL, call vp->setNull() and return true. Otherwise, find or