Bug 1427729: Have JitFrameIter properly unwind JitActivation when transitioning from JS to wasm; r=jandem

MozReview-Commit-ID: K5RQga66lKz

--HG--
extra : rebase_source : 36c6e58e8bf01a63138f1888f25c7c26cacce40a

js/src/jit-test/tests/wasm/regress/debug-exception-in-fast-import.js

@@ -0,0 +1,20 @@
+g = newGlobal();
+g.parent = this;
+g.eval("(" + function() {
+    Debugger(parent).onExceptionUnwind = function(frame) {}
+} + ")()")
+
+o = {};
+
+let { exports } = wasmEvalText(`
+  (module (import $imp "" "inc") (func) (func $start (call $imp)) (start $start) (export "" $start))
+`, {
+    "": {
+        inc: function() { o = o.p; }
+    }
+});
+
+// after instanciation, the start function has been executed and o is undefined.
+// This second call will throw in the imported function:
+
+assertErrorMessage(exports[""], TypeError, /undefined/);

js/src/jit/BaselineDebugModeOSR.h

@@ -93,7 +93,7 @@ class DebugModeOSRVolatileJitFrameIter : public JitFrameIter
 
   public:
     explicit DebugModeOSRVolatileJitFrameIter(JSContext* cx)
-      : JitFrameIter(cx->activation()->asJit())
+      : JitFrameIter(cx->activation()->asJit(), /* mustUnwindActivation */ true)
     {
         stack = &cx->liveVolatileJitFrameIter_.ref();
         prev = *stack;

js/src/jit/JitFrames.cpp

@@ -591,10 +591,7 @@ struct AutoResetLastProfilerFrameOnReturnFromException
 void
 HandleExceptionWasm(JSContext* cx, wasm::WasmFrameIter* iter, ResumeFromException* rfe)
 {
-    // Maintain the wasm invariant that we have wasm frames when unwinding.
-    JitActivation* act = cx->activation()->asJit();
-    act->setWasmExitFP((const wasm::Frame*) act->jsExitFP());
-
+    MOZ_ASSERT(cx->activation()->asJit()->hasWasmExitFP());
     rfe->kind = ResumeFromException::RESUME_WASM;
     rfe->framePointer = (uint8_t*) wasm::FailFP;
     rfe->stackPointer = (uint8_t*) wasm::HandleThrow(cx, *iter);
@@ -736,19 +733,8 @@ HandleException(ResumeFromException* rfe)
                 return;
         }
 
-        JitFrameLayout* current = frame.isScripted() ? frame.jsFrame() : nullptr;
-
         ++iter;
 
-        if (current) {
-            // Unwind the frame by updating packedExitFP. This is necessary so
-            // that (1) debugger exception unwind and leave frame hooks don't
-            // see this frame when they use ScriptFrameIter, and (2)
-            // ScriptFrameIter does not crash when accessing an IonScript
-            // that's destroyed by the ionScript->decref call.
-            EnsureBareExitFrame(cx->activation()->asJit(), current);
-        }
-
         if (overrecursed) {
             // We hit an overrecursion error during bailout. Report it now.
             ReportOverRecursed(cx);

js/src/vm/Stack.cpp

@@ -482,6 +482,7 @@ JitFrameIter::operator=(const JitFrameIter& another)
     MOZ_ASSERT(this != &another);
 
     act_ = another.act_;
+    mustUnwindActivation_ = another.mustUnwindActivation_;
 
     if (isSome())
         iter_.destroy();
@@ -498,9 +499,10 @@ JitFrameIter::operator=(const JitFrameIter& another)
     return *this;
 }
 
-JitFrameIter::JitFrameIter(jit::JitActivation* act)
+JitFrameIter::JitFrameIter(jit::JitActivation* act, bool mustUnwindActivation)
 {
     act_ = act;
+    mustUnwindActivation_ = mustUnwindActivation;
     MOZ_ASSERT(act->hasExitFP(), "packedExitFP is used to determine if JSJit or wasm");
     if (act->hasJSExitFP()) {
         iter_.construct<jit::JSJitFrameIter>(act);
@@ -557,6 +559,10 @@ JitFrameIter::settle()
         // popped.
 
         wasm::Frame* prevFP = (wasm::Frame*) jitFrame.prevFp();
+
+        if (mustUnwindActivation_)
+            act_->setWasmExitFP(prevFP);
+
         iter_.destroy();
         iter_.construct<wasm::WasmFrameIter>(act_, prevFP);
         MOZ_ASSERT(!asWasm().done());
@@ -568,12 +574,28 @@ void
 JitFrameIter::operator++()
 {
     MOZ_ASSERT(isSome());
-    if (isJSJit())
+    if (isJSJit()) {
+        const jit::JSJitFrameIter& jitFrame = asJSJit();
+
+        jit::JitFrameLayout* prevFrame = nullptr;
+        if (mustUnwindActivation_ && jitFrame.isScripted())
+            prevFrame = jitFrame.jsFrame();
+
         ++asJSJit();
-    else if (isWasm())
+
+        if (prevFrame) {
+            // Unwind the frame by updating packedExitFP. This is necessary
+            // so that (1) debugger exception unwind and leave frame hooks
+            // don't see this frame when they use ScriptFrameIter, and (2)
+            // ScriptFrameIter does not crash when accessing an IonScript
+            // that's destroyed by the ionScript->decref call.
+            EnsureBareExitFrame(act_, prevFrame);
+        }
+    } else if (isWasm()) {
         ++asWasm();
-    else
+    } else {
         MOZ_CRASH("unhandled case");
+    }
     settle();
 }
 

js/src/vm/Stack.h

@@ -1780,12 +1780,13 @@ class JitFrameIter
   protected:
     jit::JitActivation* act_;
     mozilla::MaybeOneOf<jit::JSJitFrameIter, wasm::WasmFrameIter> iter_;
+    bool mustUnwindActivation_;
 
     void settle();
 
   public:
-    JitFrameIter() : act_(nullptr), iter_() {}
-    explicit JitFrameIter(jit::JitActivation* activation);
+    JitFrameIter() : act_(nullptr), iter_(), mustUnwindActivation_(false) {}
+    explicit JitFrameIter(jit::JitActivation* activation, bool mustUnwindActivation = false);
 
     explicit JitFrameIter(const JitFrameIter& another);
     JitFrameIter& operator=(const JitFrameIter& another);