Bug 1427729: Have JitFrameIter properly unwind JitActivation when transitioning from JS to wasm; r=jandem
authorBenjamin Bouvier <benj@benj.me>
Fri, 05 Jan 2018 18:41:25 +0100
changeset 450149 23185445f29d2af03a3992ff8f8a861b30e2d34d
parent 450148 2e7604aa6ad8c3020c73a635630084311da4e8ae
child 450150 d2a67cf4639a9bbff277421aa5823643d0d5dc05
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1427729
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1427729: Have JitFrameIter properly unwind JitActivation when transitioning from JS to wasm; r=jandem MozReview-Commit-ID: K5RQga66lKz
js/src/jit-test/tests/wasm/regress/debug-exception-in-fast-import.js
js/src/jit/BaselineDebugModeOSR.h
js/src/jit/JitFrames.cpp
js/src/vm/Stack.cpp
js/src/vm/Stack.h
new file mode 100644
--- /dev/null
+++ b/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/);
--- a/js/src/jit/BaselineDebugModeOSR.h
+++ b/js/src/jit/BaselineDebugModeOSR.h
@@ -88,17 +88,17 @@ class DebugModeOSRVolatileStub
 
 class DebugModeOSRVolatileJitFrameIter : public JitFrameIter
 {
     DebugModeOSRVolatileJitFrameIter** stack;
     DebugModeOSRVolatileJitFrameIter* prev;
 
   public:
     explicit DebugModeOSRVolatileJitFrameIter(JSContext* cx)
-      : JitFrameIter(cx->activation()->asJit())
+      : JitFrameIter(cx->activation()->asJit(), /* mustUnwindActivation */ true)
     {
         stack = &cx->liveVolatileJitFrameIter_.ref();
         prev = *stack;
         *stack = this;
     }
 
     ~DebugModeOSRVolatileJitFrameIter() {
         MOZ_ASSERT(*stack == this);
--- a/js/src/jit/JitFrames.cpp
+++ b/js/src/jit/JitFrames.cpp
@@ -586,20 +586,17 @@ struct AutoResetLastProfilerFrameOnRetur
         MOZ_CRASH("Invalid ResumeFromException type!");
         return nullptr;
     }
 };
 
 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);
     MOZ_ASSERT(iter->done());
 }
 
 void
 HandleException(ResumeFromException* rfe)
@@ -731,29 +728,18 @@ HandleException(ResumeFromException* rfe
             JSScript* script = frame.script();
             probes::ExitScript(cx, script, script->functionNonDelazifying(),
                                /* popProfilerFrame = */ false);
 
             if (rfe->kind == ResumeFromException::RESUME_FORCED_RETURN)
                 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);
         }
     }
 
     // Wasm sets its own value of SP in HandleExceptionWasm.
     if (iter.isJSJit())
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -477,35 +477,37 @@ JitFrameIter::JitFrameIter(const JitFram
 }
 
 JitFrameIter&
 JitFrameIter::operator=(const JitFrameIter& another)
 {
     MOZ_ASSERT(this != &another);
 
     act_ = another.act_;
+    mustUnwindActivation_ = another.mustUnwindActivation_;
 
     if (isSome())
         iter_.destroy();
     if (!another.isSome())
         return *this;
 
     if (another.isJSJit()) {
         iter_.construct<jit::JSJitFrameIter>(another.asJSJit());
     } else {
         MOZ_ASSERT(another.isWasm());
         iter_.construct<wasm::WasmFrameIter>(another.asWasm());
     }
 
     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);
     } else {
         MOZ_ASSERT(act->hasWasmExitFP());
         iter_.construct<wasm::WasmFrameIter>(act);
     }
     settle();
@@ -552,33 +554,53 @@ JitFrameIter::settle()
         // [WASM JIT EXIT FRAME ]
         // [JIT WASM ENTRY FRAME] <-- we're here.
         //
         // So prevFP points to the wasm jit exit FP, maintaing the invariant in
         // WasmFrameIter that the first frame is an exit frame and can be
         // 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());
         return;
     }
 }
 
 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();
 }
 
 OnlyJSJitFrameIter::OnlyJSJitFrameIter(jit::JitActivation* act)
   : JitFrameIter(act)
 {
     settle();
 }
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1775,22 +1775,23 @@ class InterpreterFrameIterator
 // TODO(bug 1360211) In particular, this can handle the transition from wasm to
 // ion and from ion to wasm, since these will be interleaved in the same
 // JitActivation.
 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);
 
     bool isSome() const { return !iter_.empty(); }
     void reset() { MOZ_ASSERT(isSome()); iter_.destroy(); }
 
     bool isJSJit() const { return isSome() && iter_.constructed<jit::JSJitFrameIter>(); }