Bug 1136397 - Ensure OSR frame scripts have debug instrumentation. r=jandem, a=lmandel
authorShu-yu Guo <shu@rfrn.org>
Wed, 25 Feb 2015 16:15:11 -0800
changeset 250243 666a1aafecfd
parent 250242 610aae9b5e36
child 250244 8be609272977
push id4524
push userryanvm@gmail.com
push date2015-03-04 18:49 +0000
treeherdermozilla-beta@666a1aafecfd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem, lmandel
bugs1136397
milestone37.0
Bug 1136397 - Ensure OSR frame scripts have debug instrumentation. r=jandem, a=lmandel
js/src/jit-test/tests/debug/execution-observability-06.js
js/src/jit/BaselineJIT.cpp
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/execution-observability-06.js
@@ -0,0 +1,24 @@
+// Test that OSR respect debuggeeness.
+
+var g = newGlobal();
+var dbg = new Debugger(g);
+
+g.eval("" + function f(c) {
+  if (c == 0)
+    return;
+  if (c == 2)
+    debugger;
+  f(c-1);
+  acc = 0;
+  for (var i = 0; i < 100; i++)
+    acc += i;
+});
+
+var log = "";
+dbg.onDebuggerStatement = function (frame) {
+  frame.onPop = function f() { log += "p"; }
+};
+
+g.eval("f(2)");
+
+assertEq(log, "p");
--- a/js/src/jit/BaselineJIT.cpp
+++ b/js/src/jit/BaselineJIT.cpp
@@ -167,17 +167,17 @@ jit::EnterBaselineAtBranch(JSContext *cx
     BaselineScript *baseline = fp->script()->baselineScript();
 
     EnterJitData data(cx);
     data.jitcode = baseline->nativeCodeForPC(fp->script(), pc);
 
     // Skip debug breakpoint/trap handler, the interpreter already handled it
     // for the current op.
     if (fp->isDebuggee()) {
-        MOZ_ASSERT(baseline->hasDebugInstrumentation());
+        MOZ_RELEASE_ASSERT(baseline->hasDebugInstrumentation());
         data.jitcode += MacroAssembler::ToggledCallSize(data.jitcode);
     }
 
     data.osrFrame = fp;
     data.osrNumStackValues = fp->script()->nfixed() + cx->interpreterRegs().stackDepth();
 
     RootedValue thisv(cx);
 
@@ -315,16 +315,39 @@ jit::CanEnterBaselineAtBranch(JSContext 
        if (!obj)
            return Method_Skipped;
        fp->functionThis().setObject(*obj);
    }
 
    if (!CheckFrame(fp))
        return Method_CantCompile;
 
+   // This check is needed in the following corner case. Consider a function h,
+   //
+   //   function h(x) {
+   //      h(false);
+   //      if (!x)
+   //        return;
+   //      for (var i = 0; i < N; i++)
+   //         /* do stuff */
+   //   }
+   //
+   // Suppose h is not yet compiled in baseline and is executing in the
+   // interpreter. Let this interpreter frame be f_older. The debugger marks
+   // f_older as isDebuggee. At the point of the recursive call h(false), h is
+   // compiled in baseline without debug instrumentation, pushing a baseline
+   // frame f_newer. The debugger never flags f_newer as isDebuggee, and never
+   // recompiles h. When the recursive call returns and execution proceeds to
+   // the loop, the interpreter attempts to OSR into baseline. Since h is
+   // already compiled in baseline, execution jumps directly into baseline
+   // code. This is incorrect as h's baseline script does not have debug
+   // instrumentation.
+   if (fp->isDebuggee() && !Debugger::ensureExecutionObservabilityOfOsrFrame(cx, fp))
+       return Method_Error;
+
    RootedScript script(cx, fp->script());
    return CanEnterBaselineJIT(cx, script, fp);
 }
 
 MethodStatus
 jit::CanEnterBaselineMethod(JSContext *cx, RunState &state)
 {
     if (state.isInvoke()) {
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1994,16 +1994,29 @@ Debugger::ensureExecutionObservabilityOf
 {
     if (script->isDebuggee())
         return true;
     ExecutionObservableScript obs(cx, script);
     return updateExecutionObservability(cx, obs, Observing);
 }
 
 /* static */ bool
+Debugger::ensureExecutionObservabilityOfOsrFrame(JSContext *cx, InterpreterFrame *frame)
+{
+    MOZ_ASSERT(frame->isDebuggee());
+    if (frame->script()->hasBaselineScript() &&
+        frame->script()->baselineScript()->hasDebugInstrumentation())
+    {
+        return true;
+    }
+    ExecutionObservableFrame obs(frame);
+    return updateExecutionObservabilityOfFrames(cx, obs, Observing);
+}
+
+/* static */ bool
 Debugger::ensureExecutionObservabilityOfFrame(JSContext *cx, AbstractFramePtr frame)
 {
     MOZ_ASSERT_IF(frame.script()->isDebuggee(), frame.isDebuggee());
     if (frame.isDebuggee())
         return true;
     ExecutionObservableFrame obs(frame);
     return updateExecutionObservabilityOfFrames(cx, obs, Observing);
 }
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -429,16 +429,18 @@ class Debugger : private mozilla::Linked
     static bool updateExecutionObservabilityOfFrames(JSContext *cx, const ExecutionObservableSet &obs,
                                                      IsObserving observing);
     static bool updateExecutionObservabilityOfScripts(JSContext *cx, const ExecutionObservableSet &obs,
                                                       IsObserving observing);
     static bool updateExecutionObservability(JSContext *cx, ExecutionObservableSet &obs,
                                              IsObserving observing);
 
   public:
+    static bool ensureExecutionObservabilityOfOsrFrame(JSContext *cx, InterpreterFrame *frame);
+
     // Public for DebuggerScript_setBreakpoint.
     static bool ensureExecutionObservabilityOfScript(JSContext *cx, JSScript *script);
 
   private:
     static bool ensureExecutionObservabilityOfFrame(JSContext *cx, AbstractFramePtr frame);
     static bool ensureExecutionObservabilityOfCompartment(JSContext *cx, JSCompartment *comp);
 
     static bool hookObservesAllExecution(Hook which);