Bug 1330489 - Ignore imports for wasm debug frames and discard frames in activation on throw. r?luke draft
authorYury Delendik <ydelendik@mozilla.com>
Thu, 12 Jan 2017 13:55:18 -0600
changeset 460797 4ca3c0208ff527e4f303133da2b884f2c08c9cc9
parent 460618 91f5293e9a89056565493ed5073c3842b0ee9fdc
child 542145 0cc6a4fbe168c9932feb1bdfa8ac372717f8f154
push id41497
push userydelendik@mozilla.com
push dateFri, 13 Jan 2017 21:22:10 +0000
reviewersluke
bugs1330489
milestone53.0a1
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,