Bug 1330489 - Ignore imports for wasm debug frames and discard frames in activation on throw. r=luke
authorYury Delendik <ydelendik@mozilla.com>
Thu, 12 Jan 2017 13:55:18 -0600
changeset 329497 5ac5cb6fc6ef755378ae70cdbffe76f0b44a30df
parent 329496 94c2a93e9024f705bdd8be6bbb51228fb57a48c0
child 329521 75a039db121f8c8778805cfdc4e9fb6621fdec75
push id36069
push userryanvm@gmail.com
push dateSun, 15 Jan 2017 20:04:42 +0000
treeherderautoland@5ac5cb6fc6ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1330489
milestone53.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 1330489 - Ignore imports for wasm debug frames and discard frames in activation on throw. r=luke MozReview-Commit-ID: HmZkqnUvZXm
js/src/jit-test/tests/debug/bug1330489-sps.js
js/src/jit-test/tests/debug/bug1330489.js
js/src/jit-test/tests/wasm/profiling.js
js/src/vm/Stack.cpp
js/src/vm/Stack.h
js/src/wasm/WasmFrameIterator.cpp
js/src/wasm/WasmFrameIterator.h
js/src/wasm/WasmStubs.cpp
js/src/wasm/WasmTypes.cpp
js/src/wasm/WasmTypes.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/bug1330489-sps.js
@@ -0,0 +1,44 @@
+// |jit-test| test-also-wasm-baseline; error: TestComplete
+
+load(libdir + "asserts.js");
+
+if (!wasmIsSupported())
+    throw "TestComplete";
+
+// Single-step profiling currently only works in the ARM simulator
+if (!getBuildConfiguration()["arm-simulator"])
+    throw "TestComplete";
+
+enableSPSProfiling();
+enableSingleStepProfiling();
+
+var g = newGlobal();
+g.parent = this;
+g.eval("Debugger(parent).onExceptionUnwind = function () {};");
+
+let module = new WebAssembly.Module(wasmTextToBinary(`
+    (module
+        (import $imp "a" "b" (result i32))
+        (memory 1 1)
+        (table 2 2 anyfunc)
+        (elem (i32.const 0) $imp $def)
+        (func $def (result i32) (i32.load (i32.const 0)))
+        (type $v2i (func (result i32)))
+        (func $call (param i32) (result i32) (call_indirect $v2i (get_local 0)))
+        (export "call" $call)
+    )
+`));
+
+let instance = new WebAssembly.Instance(module, {
+    a: { b: function () { throw "test"; } }
+});
+
+try {
+    instance.exports.call(0);
+    assertEq(false, true);
+} catch (e) {
+    assertEq(e, "test");
+}
+
+disableSPSProfiling();
+throw "TestComplete";
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/bug1330489.js
@@ -0,0 +1,36 @@
+// |jit-test| test-also-wasm-baseline; error: TestComplete
+
+load(libdir + "asserts.js");
+
+if (!wasmIsSupported())
+    throw "TestComplete";
+
+var g = newGlobal();
+g.parent = this;
+g.eval("Debugger(parent).onExceptionUnwind = function () {};");
+
+let module = new WebAssembly.Module(wasmTextToBinary(`
+    (module
+        (import $imp "a" "b" (result i32))
+        (memory 1 1)
+        (table 2 2 anyfunc)
+        (elem (i32.const 0) $imp $def)
+        (func $def (result i32) (i32.load (i32.const 0)))
+        (type $v2i (func (result i32)))
+        (func $call (param i32) (result i32) (call_indirect $v2i (get_local 0)))
+        (export "call" $call)
+    )
+`));
+
+let instance = new WebAssembly.Instance(module, {
+    a: { b: function () { throw "test"; } }
+});
+
+try {
+    instance.exports.call(0);
+    assertEq(false, true);
+} catch (e) {
+    assertEq(e, "test");
+}
+
+throw "TestComplete";
--- a/js/src/jit-test/tests/wasm/profiling.js
+++ b/js/src/jit-test/tests/wasm/profiling.js
@@ -122,32 +122,32 @@ function testError(code, error, expect)
 }
 
 testError(
 `(module
     (func $foo (unreachable))
     (func (export "") (call $foo))
 )`,
 WebAssembly.RuntimeError,
-["", ">", "1,>", "0,1,>", "trap handling,0,1,>", "inline stub,0,1,>", "trap handling,0,1,>", "inline stub,0,1,>", ""]);
+["", ">", "1,>", "0,1,>", "trap handling,0,1,>", "inline stub,0,1,>", "trap handling,0,1,>", ""]);
 
 testError(
 `(module
     (type $good (func))
     (type $bad (func (param i32)))
     (func $foo (call_indirect $bad (i32.const 1) (i32.const 0)))
     (func $bar (type $good))
     (table anyfunc (elem $bar))
     (export "" $foo)
 )`,
 WebAssembly.RuntimeError,
 // Technically we have this one *one-instruction* interval where
 // the caller is lost (the stack with "1,>"). It's annoying to fix and shouldn't
 // mess up profiles in practice so we ignore it.
-["", ">", "0,>", "1,0,>", "1,>", "trap handling,0,>", "inline stub,0,>", "trap handling,0,>", "inline stub,0,>", ""]);
+["", ">", "0,>", "1,0,>", "1,>", "trap handling,0,>", "inline stub,0,>", "trap handling,0,>", ""]);
 
 (function() {
     var e = wasmEvalText(`
     (module
         (func $foo (result i32) (i32.const 42))
         (export "foo" $foo)
         (func $bar (result i32) (i32.const 13))
         (table 10 anyfunc)
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -540,17 +540,17 @@ FrameIter::settleOnActivation()
             }
 
             nextJitFrame();
             data_.state_ = JIT;
             return;
         }
 
         if (activation->isWasm()) {
-            data_.wasmFrames_ = wasm::FrameIterator(*data_.activations_->asWasm());
+            data_.wasmFrames_ = wasm::FrameIterator(data_.activations_->asWasm());
 
             if (data_.wasmFrames_.done()) {
                 ++data_.activations_;
                 continue;
             }
 
             data_.pc_ = nullptr;
             data_.state_ = WASM;
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1712,16 +1712,19 @@ class WasmActivation : public Activation
     // Written by JIT code:
     static unsigned offsetOfEntrySP() { return offsetof(WasmActivation, entrySP_); }
     static unsigned offsetOfFP() { return offsetof(WasmActivation, fp_); }
     static unsigned offsetOfExitReason() { return offsetof(WasmActivation, exitReason_); }
 
     // Read/written from SIGSEGV handler:
     void setResumePC(void* pc) { resumePC_ = pc; }
     void* resumePC() const { return resumePC_; }
+
+    // Used by wasm::FrameIterator during stack unwinding.
+    void unwindFP(uint8_t* fp) { fp_ = fp; }
 };
 
 // A FrameIter walks over the runtime's stack of JS script activations,
 // abstracting over whether the JS scripts were running in the interpreter or
 // different modes of compiled code.
 //
 // FrameIter is parameterized by what it includes in the stack iteration:
 //  - When provided, the optional JSPrincipal argument will cause FrameIter to
--- a/js/src/wasm/WasmFrameIterator.cpp
+++ b/js/src/wasm/WasmFrameIterator.cpp
@@ -54,36 +54,38 @@ TlsDataFromFP(void *fp)
 
 FrameIterator::FrameIterator()
   : activation_(nullptr),
     code_(nullptr),
     callsite_(nullptr),
     codeRange_(nullptr),
     fp_(nullptr),
     pc_(nullptr),
+    unwind_(Unwind::False),
     missingFrameMessage_(false)
 {
     MOZ_ASSERT(done());
 }
 
-FrameIterator::FrameIterator(const WasmActivation& activation)
-  : activation_(&activation),
+FrameIterator::FrameIterator(WasmActivation* activation, Unwind unwind)
+  : activation_(activation),
     code_(nullptr),
     callsite_(nullptr),
     codeRange_(nullptr),
-    fp_(activation.fp()),
+    fp_(activation->fp()),
     pc_(nullptr),
+    unwind_(unwind),
     missingFrameMessage_(false)
 {
     if (fp_) {
         settle();
         return;
     }
 
-    void* pc = activation.resumePC();
+    void* pc = activation_->resumePC();
     if (!pc) {
         MOZ_ASSERT(done());
         return;
     }
     pc_ = (uint8_t*)pc;
 
     code_ = activation_->compartment()->wasm.lookupCode(pc);
     MOZ_ASSERT(code_);
@@ -151,16 +153,19 @@ FrameIterator::settle()
       case CodeRange::ImportJitExit:
       case CodeRange::ImportInterpExit:
       case CodeRange::TrapExit:
       case CodeRange::DebugTrap:
       case CodeRange::Inline:
       case CodeRange::FarJumpIsland:
         MOZ_CRASH("Should not encounter an exit during iteration");
     }
+
+    if (unwind_ == Unwind::True)
+        activation_->unwindFP(fp_);
 }
 
 const char*
 FrameIterator::filename() const
 {
     MOZ_ASSERT(!done());
     return code_->metadata().filename.get();
 }
@@ -224,17 +229,19 @@ FrameIterator::instance() const
     return TlsDataFromFP(fp_ + callsite_->stackDepth())->instance;
 }
 
 bool
 FrameIterator::debugEnabled() const
 {
     MOZ_ASSERT(!done() && code_);
     MOZ_ASSERT_IF(!missingFrameMessage_, codeRange_->kind() == CodeRange::Function);
-    return code_->metadata().debugEnabled;
+    // Only non-imported functions can have debug frames.
+    return code_->metadata().debugEnabled &&
+           codeRange_->funcIndex() >= code_->metadata().funcImports.length();
 }
 
 DebugFrame*
 FrameIterator::debugFrame() const
 {
     MOZ_ASSERT(!done() && debugEnabled());
     // The fp() points to wasm::Frame.
     void* buf = static_cast<uint8_t*>(fp_ + callsite_->stackDepth()) - DebugFrame::offsetOfFrame();
--- a/js/src/wasm/WasmFrameIterator.h
+++ b/js/src/wasm/WasmFrameIterator.h
@@ -46,29 +46,34 @@ struct TrapOffset;
 //
 // The one exception is that this iterator may be called from the interrupt
 // callback which may be called asynchronously from asm.js code; in this case,
 // the backtrace may not be correct. That being said, we try our best printing
 // an informative message to the user and at least the name of the innermost
 // function stack frame.
 class FrameIterator
 {
-    const WasmActivation* activation_;
+  public:
+    enum class Unwind { True, False };
+
+  private:
+    WasmActivation* activation_;
     const Code* code_;
     const CallSite* callsite_;
     const CodeRange* codeRange_;
     uint8_t* fp_;
     uint8_t* pc_;
+    Unwind unwind_;
     bool missingFrameMessage_;
 
     void settle();
 
   public:
     explicit FrameIterator();
-    explicit FrameIterator(const WasmActivation& activation);
+    explicit FrameIterator(WasmActivation* activation, Unwind unwind = Unwind::False);
     void operator++();
     bool done() const;
     const char* filename() const;
     const char16_t* displayURL() const;
     bool mutedErrors() const;
     JSAtom* functionDisplayAtom() const;
     unsigned lineOrBytecode() const;
     const CodeRange* codeRange() const { return codeRange_; }
--- a/js/src/wasm/WasmStubs.cpp
+++ b/js/src/wasm/WasmStubs.cpp
@@ -1126,27 +1126,34 @@ wasm::GenerateThrowStub(MacroAssembler& 
 {
     masm.haltingAlign(CodeAlignment);
 
     masm.bind(throwLabel);
 
     Offsets offsets;
     offsets.begin = masm.currentOffset();
 
+    // The following HandleThrow call sets fp of this WasmActivation to null.
     masm.andToStackPtr(Imm32(~(ABIStackAlignment - 1)));
     if (ShadowStackSpace)
         masm.subFromStackPtr(Imm32(ShadowStackSpace));
-    masm.call(SymbolicAddress::HandleDebugThrow);
+    masm.call(SymbolicAddress::HandleThrow);
 
-    // We are about to pop all frames in this WasmActivation. Set fp to null to
-    // maintain the invariant that fp is either null or pointing to a valid
-    // frame.
     Register scratch = ABINonArgReturnReg0;
     masm.loadWasmActivationFromSymbolicAddress(scratch);
-    masm.storePtr(ImmWord(0), Address(scratch, WasmActivation::offsetOfFP()));
+
+#ifdef DEBUG
+    // We are about to pop all frames in this WasmActivation. Checking if fp is
+    // set to null to maintain the invariant that fp is either null or pointing
+    // to a valid frame.
+    Label ok;
+    masm.branchPtr(Assembler::Equal, Address(scratch, WasmActivation::offsetOfFP()), ImmWord(0), &ok);
+    masm.breakpoint();
+    masm.bind(&ok);
+#endif
 
     masm.setFramePushed(FramePushedForEntrySP);
     masm.loadStackPtr(Address(scratch, WasmActivation::offsetOfEntrySP()));
     masm.Pop(scratch);
     masm.PopRegsInMask(NonVolatileRegs);
     MOZ_ASSERT(masm.framePushed() == 0);
 
     masm.mov(ImmWord(0), ReturnReg);
--- a/js/src/wasm/WasmTypes.cpp
+++ b/js/src/wasm/WasmTypes.cpp
@@ -98,19 +98,20 @@ WasmHandleExecutionInterrupt()
 
     return success;
 }
 
 static bool
 WasmHandleDebugTrap()
 {
     WasmActivation* activation = JSRuntime::innermostWasmActivation();
+    MOZ_ASSERT(activation);
     JSContext* cx = activation->cx();
 
-    FrameIterator iter(*activation);
+    FrameIterator iter(activation);
     MOZ_ASSERT(iter.debugEnabled());
     const CallSite* site = iter.debugTrapCallsite();
     MOZ_ASSERT(site);
     if (site->kind() == CallSite::EnterFrame) {
         if (!iter.instance()->enterFrameTrapsEnabled())
             return true;
         DebugFrame* frame = iter.debugFrame();
         frame->setIsDebuggee();
@@ -133,22 +134,23 @@ WasmHandleDebugTrap()
         return ok;
     }
     // TODO baseline debug traps
     MOZ_CRASH();
     return true;
 }
 
 static void
-WasmHandleDebugThrow()
+WasmHandleThrow()
 {
     WasmActivation* activation = JSRuntime::innermostWasmActivation();
+    MOZ_ASSERT(activation);
     JSContext* cx = activation->cx();
 
-    for (FrameIterator iter(*activation); !iter.done(); ++iter) {
+    for (FrameIterator iter(activation, FrameIterator::Unwind::True); !iter.done(); ++iter) {
         if (!iter.debugEnabled())
             continue;
 
         DebugFrame* frame = iter.debugFrame();
 
         JSTrapStatus status = Debugger::onExceptionUnwind(cx, frame);
         if (status == JSTRAP_RETURN) {
             // Unexpected trap return -- raising error since throw recovery
@@ -344,18 +346,18 @@ wasm::AddressOf(SymbolicAddress imm, Exc
       case SymbolicAddress::InterruptUint32:
         return cx->runtimeAddressOfInterruptUint32();
       case SymbolicAddress::ReportOverRecursed:
         return FuncCast(WasmReportOverRecursed, Args_General0);
       case SymbolicAddress::HandleExecutionInterrupt:
         return FuncCast(WasmHandleExecutionInterrupt, Args_General0);
       case SymbolicAddress::HandleDebugTrap:
         return FuncCast(WasmHandleDebugTrap, Args_General0);
-      case SymbolicAddress::HandleDebugThrow:
-        return FuncCast(WasmHandleDebugThrow, Args_General0);
+      case SymbolicAddress::HandleThrow:
+        return FuncCast(WasmHandleThrow, Args_General0);
       case SymbolicAddress::ReportTrap:
         return FuncCast(WasmReportTrap, Args_General1);
       case SymbolicAddress::ReportOutOfBounds:
         return FuncCast(WasmReportOutOfBounds, Args_General0);
       case SymbolicAddress::ReportUnalignedAccess:
         return FuncCast(WasmReportUnalignedAccess, Args_General0);
       case SymbolicAddress::CallImport_Void:
         return FuncCast(Instance::callImport_void, Args_General4);
--- a/js/src/wasm/WasmTypes.h
+++ b/js/src/wasm/WasmTypes.h
@@ -1012,17 +1012,17 @@ enum class SymbolicAddress
     LogD,
     PowD,
     ATan2D,
     Context,
     InterruptUint32,
     ReportOverRecursed,
     HandleExecutionInterrupt,
     HandleDebugTrap,
-    HandleDebugThrow,
+    HandleThrow,
     ReportTrap,
     ReportOutOfBounds,
     ReportUnalignedAccess,
     CallImport_Void,
     CallImport_I32,
     CallImport_I64,
     CallImport_F64,
     CoerceInPlace_ToInt32,