Bug 1001090 - Part 2b: Fix unwinding all scopes to not use pc. (r=jimb)

js/src/jit/IonFrames.cpp

@@ -555,7 +555,7 @@ HandleExceptionBaseline(JSContext *cx, const JitFrameIterator &frame, ResumeFrom
 
         // Unwind scope chain (pop block objects).
         if (cx->isExceptionPending()) {
-            *unwoundScopeToPc = script->main() + tn->start;
+            *unwoundScopeToPc = UnwindScopeToTryPc(script, tn);
             UnwindScope(cx, si, *unwoundScopeToPc);
         }
 

js/src/jit/VMFunctions.cpp

@@ -809,8 +809,8 @@ DebugEpilogue(JSContext *cx, BaselineFrame *frame, jsbytecode *pc, bool ok)
 {
     // Unwind scope chain to stack depth 0.
     ScopeIter si(frame, pc, cx);
+    UnwindAllScopes(cx, si);
     jsbytecode *unwindPc = frame->script()->main();
-    UnwindScope(cx, si, unwindPc);
     frame->setUnwoundScopeOverridePc(unwindPc);
 
     // If ScriptDebugEpilogue returns |true| we have to return the frame's

js/src/vm/Interpreter.cpp

@@ -866,6 +866,25 @@ js::EnterWithOperation(JSContext *cx, AbstractFramePtr frame, HandleValue val,
     return true;
 }
 
+static void
+PopScope(JSContext *cx, ScopeIter &si)
+{
+    switch (si.type()) {
+      case ScopeIter::Block:
+        if (cx->compartment()->debugMode())
+            DebugScopes::onPopBlock(cx, si);
+        if (si.staticBlock().needsClone())
+            si.frame().popBlock(cx);
+        break;
+      case ScopeIter::With:
+        si.frame().popWith(cx);
+        break;
+      case ScopeIter::Call:
+      case ScopeIter::StrictEvalScope:
+        break;
+    }
+}
+
 // Unwind scope chain and iterator to match the static scope corresponding to
 // the given bytecode position.
 void
@@ -876,28 +895,42 @@ js::UnwindScope(JSContext *cx, ScopeIter &si, jsbytecode *pc)
 
     Rooted<NestedScopeObject *> staticScope(cx, si.frame().script()->getStaticScope(pc));
 
-    for (; si.staticScope() != staticScope; ++si) {
+    for (; si.staticScope() != staticScope; ++si)
-        switch (si.type()) {
+        PopScope(cx, si);
-          case ScopeIter::Block:
+}
-            if (cx->compartment()->debugMode())
+
-                DebugScopes::onPopBlock(cx, si);
+// Unwind all scopes. This is needed because block scopes may cover the
-            if (si.staticBlock().needsClone())
+// first bytecode at a script's main(). e.g.,
-                si.frame().popBlock(cx);
+//
-            break;
+//     function f() { { let i = 0; } }
-          case ScopeIter::With:
+//
-            si.frame().popWith(cx);
+// will have no pc location distinguishing the first block scope from the
-            break;
+// outermost function scope.
-          case ScopeIter::Call:
+void
-          case ScopeIter::StrictEvalScope:
+js::UnwindAllScopes(JSContext *cx, ScopeIter &si)
-            break;
+{
-        }
+    for (; !si.done(); ++si)
-    }
+        PopScope(cx, si);
+}
+
+// Compute the pc needed to unwind the scope to the beginning of a try
+// block. We cannot unwind to *after* the JSOP_TRY, because that might be the
+// first opcode of an inner scope, with the same problem as above. e.g.,
+//
+// try { { let x; } }
+//
+// will have no pc location distinguishing the try block scope from the inner
+// let block scope.
+jsbytecode *
+js::UnwindScopeToTryPc(JSScript *script, JSTryNote *tn)
+{
+    return script->main() + tn->start - js_CodeSpec[JSOP_TRY].length;
 }
 
 static void
 ForcedReturn(JSContext *cx, ScopeIter &si, InterpreterRegs &regs)
 {
-    UnwindScope(cx, si, regs.fp()->script()->main());
+    UnwindAllScopes(cx, si);
     regs.setToEndOfScript();
 }
 
@@ -1022,24 +1055,8 @@ HandleError(JSContext *cx, InterpreterRegs &regs)
         for (TryNoteIter tni(cx, regs); !tni.done(); ++tni) {
             JSTryNote *tn = *tni;
 
-            // Unwind the scope to the beginning of the JSOP_TRY. We cannot
+            // Unwind the scope to the beginning of the JSOP_TRY.
-            // unwind to *after* the JSOP_TRY, because that might be the first
+            UnwindScope(cx, si, UnwindScopeToTryPc(regs.fp()->script(), tn));
-            // opcode of an inner scope. Consider the following:
-            //
-            // try {
-            //   { let x; }
-            // }
-            //
-            // This would generate
-            //
-            // 0000: try
-            // 0001: undefined
-            // 0002: initlet 0
-            //
-            // If we unwound to 0001, we would be unwinding to the inner
-            // scope, and not the scope of the try { }.
-            UnwindScope(cx, si, (regs.fp()->script()->main() + tn->start -
-                                 js_CodeSpec[JSOP_TRY].length));
 
             /*
              * Set pc to the first bytecode after the the try note to point

js/src/vm/Interpreter.h

@@ -325,6 +325,15 @@ HasInstance(JSContext *cx, HandleObject obj, HandleValue v, bool *bp);
 extern void
 UnwindScope(JSContext *cx, ScopeIter &si, jsbytecode *pc);
 
+// Unwind all scopes.
+extern void
+UnwindAllScopes(JSContext *cx, ScopeIter &si);
+
+// Compute the pc needed to unwind the scope to the beginning of the block
+// pointed to by the try note.
+extern jsbytecode *
+UnwindScopeToTryPc(JSScript *script, JSTryNote *tn);
+
 /*
  * Unwind for an uncatchable exception. This means not running finalizers, etc;
  * just preserving the basic engine stack invariants.