[wasm] [interpreter] Handle stack unwinding

If an exception is thrown and the wasm interpreter entry frame is
unwound, also the internal frames in the interpreter need to be unwound.
We did not do so before, leaving a corrupted internal state of the wasm
interpreter. Thus reusing it would fail.
This CL fixes this and adds a test which reenters a previously unwound
wasm interpreter. It checks that this works and the correct stack is
returned.
This test also requires support for calling an imported function which
throws, so this change is also included here.

R=ahaas@chromium.org, titzer@chromium.org
BUG=v8:5822

Change-Id: I12fb843f7a371a4e618b4ac63ed3299667a03a82
Reviewed-on: https://chromium-review.googlesource.com/453938
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43937}

src/isolate.cc

@@ -1341,13 +1341,14 @@ Object* Isolate::UnwindAndFindHandler() {
         }
         break;
 
-      case StackFrame::WASM_INTERPRETER_ENTRY:
-        // TODO(clemensh): Handle unwinding interpreted wasm frames (stored in
-        // the WasmInterpreter C++ object).
+      case StackFrame::WASM_INTERPRETER_ENTRY: {
         if (trap_handler::IsThreadInWasm()) {
           trap_handler::ClearThreadInWasm();
         }
-        break;
+        WasmInterpreterEntryFrame* interpreter_frame =
+            WasmInterpreterEntryFrame::cast(frame);
+        interpreter_frame->wasm_instance()->debug_info()->Unwind(frame->fp());
+      } break;
 
       default:
         // All other types can not handle exception.

src/wasm/wasm-debug.cc

@@ -154,9 +154,7 @@ class InterpreterHandle {
     WasmInterpreter::Thread* thread = interpreter_.GetThread(0);
     // We do not support reentering an already running interpreter at the moment
     // (like INTERPRETER -> JS -> WASM -> INTERPRETER).
-    DCHECK(thread->state() == WasmInterpreter::STOPPED ||
-           thread->state() == WasmInterpreter::FINISHED ||
-           thread->state() == WasmInterpreter::TRAPPED);
+    DCHECK_EQ(0, thread->GetFrameCount());
     thread->Reset();
     thread->InitFrame(&module()->functions[func_index], wasm_args.start());
     bool finished = false;
@@ -180,8 +178,11 @@ class InterpreterHandle {
           // And hard exit the execution.
           return false;
         } break;
-        // STOPPED and RUNNING should never occur here.
         case WasmInterpreter::State::STOPPED:
+          // Then an exception happened, and the stack was unwound.
+          DCHECK_EQ(0, thread->GetFrameCount());
+          return false;
+        // RUNNING should never occur here.
         case WasmInterpreter::State::RUNNING:
         default:
           UNREACHABLE();
@@ -329,6 +330,17 @@ class InterpreterHandle {
         new wasm::InterpretedFrame(thread->GetMutableFrame(idx)));
   }
 
+  void Unwind(Address frame_pointer) {
+    // TODO(clemensh): Use frame_pointer.
+    USE(frame_pointer);
+
+    using ExceptionResult = WasmInterpreter::Thread::ExceptionHandlingResult;
+    ExceptionResult result =
+        interpreter()->GetThread(0)->HandleException(isolate_);
+    // TODO(wasm): Handle exceptions caught in wasm land.
+    CHECK_EQ(ExceptionResult::UNWOUND, result);
+  }
+
   uint64_t NumInterpretedCalls() {
     DCHECK_EQ(1, interpreter()->GetThreadCount());
     return interpreter()->GetThread(0)->NumInterpretedCalls();
@@ -498,6 +510,10 @@ std::unique_ptr<wasm::InterpretedFrame> WasmDebugInfo::GetInterpretedFrame(
   return GetInterpreterHandle(this)->GetInterpretedFrame(frame_pointer, idx);
 }
 
+void WasmDebugInfo::Unwind(Address frame_pointer) {
+  return GetInterpreterHandle(this)->Unwind(frame_pointer);
+}
+
 uint64_t WasmDebugInfo::NumInterpretedCalls() {
   auto handle = GetInterpreterHandleOrNull(this);
   return handle ? handle->NumInterpretedCalls() : 0;

src/wasm/wasm-interpreter.cc

@@ -1006,11 +1006,13 @@ class ThreadImpl {
   WasmInterpreter::State Run() {
     DCHECK(state_ == WasmInterpreter::STOPPED ||
            state_ == WasmInterpreter::PAUSED);
+    // Execute in chunks of {kRunSteps} steps as long as we did not trap or
+    // unwind.
     do {
       TRACE("  => Run()\n");
       state_ = WasmInterpreter::RUNNING;
       Execute(frames_.back().code, frames_.back().pc, kRunSteps);
-    } while (state_ == WasmInterpreter::STOPPED);
+    } while (state_ == WasmInterpreter::STOPPED && !frames_.empty());
     return state_;
   }
 
@@ -1071,6 +1073,22 @@ class ThreadImpl {
 
   void ClearBreakFlags() { break_flags_ = WasmInterpreter::BreakFlag::None; }
 
+  // Handle a thrown exception. Returns whether the exception was handled inside
+  // wasm. Unwinds the interpreted stack accordingly.
+  WasmInterpreter::Thread::ExceptionHandlingResult HandleException(
+      Isolate* isolate) {
+    DCHECK(isolate->has_pending_exception());
+    // TODO(wasm): Add wasm exception handling.
+    USE(isolate->pending_exception());
+    TRACE("----- UNWIND -----\n");
+    // TODO(clemensh): Only clear the portion of the stack belonging to the
+    // current activation of the interpreter.
+    stack_.clear();
+    frames_.clear();
+    state_ = WasmInterpreter::STOPPED;
+    return WasmInterpreter::Thread::UNWOUND;
+  }
+
  private:
   // Entries on the stack of functions being evaluated.
   struct Frame {
@@ -1466,7 +1484,7 @@ class ThreadImpl {
           InterpreterCode* target = codemap()->GetCode(operand.index);
           if (target->function->imported) {
             CommitPc(pc);
-            CallImportedFunction(operand.index);
+            if (!CallImportedFunction(operand.index)) return;
             PAUSE_IF_BREAK_FLAG(AfterCall);
             len = 1 + operand.length;
             break;  // bump pc
@@ -1814,7 +1832,11 @@ class ThreadImpl {
 #endif  // DEBUG
   }
 
-  void CallImportedFunction(uint32_t function_index) {
+  // Call imported function. Return true if the function returned normally or a
+  // thrown exception was handled inside this wasm activation. Returns false if
+  // the stack was fully unwound. A pending exception will be set in the latter
+  // case.
+  bool CallImportedFunction(uint32_t function_index) {
     Handle<HeapObject> target = codemap()->GetImportedFunction(function_index);
     if (target.is_null()) {
       // The function does not have a js-compatible signature.
@@ -1847,9 +1869,10 @@ class ThreadImpl {
     MaybeHandle<Object> maybe_retval = Execution::Call(
         isolate, target, isolate->global_proxy(), num_args, args.data());
     if (maybe_retval.is_null()) {
-      // TODO(clemensh): Exception handling here.
-      UNIMPLEMENTED();
+      auto result = HandleException(isolate);
+      return result == WasmInterpreter::Thread::HANDLED;
     }
+
     Handle<Object> retval = maybe_retval.ToHandleChecked();
     // TODO(clemensh): Call ToNumber on retval.
     // Pop arguments of the stack.
@@ -1859,6 +1882,7 @@ class ThreadImpl {
       DCHECK_EQ(1, called_fun->sig->return_count());
       stack_.push_back(NumberToWasmVal(retval, called_fun->sig->GetReturn()));
     }
+    return true;
   }
 };
 
@@ -1895,6 +1919,10 @@ WasmInterpreter::State WasmInterpreter::Thread::Step() {
 }
 void WasmInterpreter::Thread::Pause() { return ToImpl(this)->Pause(); }
 void WasmInterpreter::Thread::Reset() { return ToImpl(this)->Reset(); }
+WasmInterpreter::Thread::ExceptionHandlingResult
+WasmInterpreter::Thread::HandleException(Isolate* isolate) {
+  return ToImpl(this)->HandleException(isolate);
+}
 pc_t WasmInterpreter::Thread::GetBreakpointPc() {
   return ToImpl(this)->GetBreakpointPc();
 }

src/wasm/wasm-interpreter.h

@@ -113,11 +113,11 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
   //                       +---------------Run()-----------+
   //                       V                               |
   // STOPPED ---Run()-->  RUNNING  ------Pause()-----+-> PAUSED  <------+
-  //                       | | |                    /      |            |
-  //                       | | +---- Breakpoint ---+       +-- Step() --+
-  //                       | |
-  //                       | +------------ Trap --------------> TRAPPED
-  //                       +------------- Finish -------------> FINISHED
+  //  ^                   | | | |                   /      |            |
+  //  +- HandleException -+ | | +--- Breakpoint ---+       +-- Step() --+
+  //                        | |
+  //                        | +---------- Trap --------------> TRAPPED
+  //                        +----------- Finish -------------> FINISHED
   enum State { STOPPED, RUNNING, PAUSED, FINISHED, TRAPPED };
 
   // Tells a thread to pause after certain instructions.
@@ -134,6 +134,8 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     Thread() = delete;
 
    public:
+    enum ExceptionHandlingResult { HANDLED, UNWOUND };
+
     // Execution control.
     State state();
     void InitFrame(const WasmFunction* function, WasmVal* args);
@@ -141,6 +143,9 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     State Step();
     void Pause();
     void Reset();
+    // Handle the pending exception in the passed isolate. Unwind the stack
+    // accordingly. Return whether the exception was handled inside wasm.
+    ExceptionHandlingResult HandleException(Isolate* isolate);
 
     // Stack inspection and modification.
     pc_t GetBreakpointPc();

src/wasm/wasm-objects.h

@@ -487,6 +487,10 @@ class WasmDebugInfo : public FixedArray {
   std::unique_ptr<wasm::InterpretedFrame> GetInterpretedFrame(
       Address frame_pointer, int idx);
 
+  // Unwind the interpreted stack belonging to the passed interpreter entry
+  // frame.
+  void Unwind(Address frame_pointer);
+
   // Returns the number of calls / function frames executed in the interpreter.
   uint64_t NumInterpretedCalls();
 

test/mjsunit/wasm/interpreter.js

@@ -108,3 +108,34 @@ function checkStack(stack, expected_lines) {
     ]);
   }
 })();
+
+(function testThrowFromImport() {
+  function func() {
+    throw new Error('thrown from imported function');
+  }
+  var builder = new WasmModuleBuilder();
+  builder.addImport("mod", "func", kSig_v_v);
+  builder.addFunction('main', kSig_v_v)
+      .addBody([kExprCallFunction, 0])
+      .exportFunc();
+  var instance = builder.instantiate({mod: {func: func}});
+  // Test that this does not mess up internal state by executing it three times.
+  for (var i = 0; i < 3; ++i) {
+    var interpreted_before = % WasmNumInterpretedCalls(instance);
+    var stack;
+    try {
+      instance.exports.main();
+      assertUnreachable();
+    } catch (e) {
+      stack = e.stack;
+    }
+    assertEquals(interpreted_before + 1, % WasmNumInterpretedCalls(instance));
+    checkStack(stripPath(stack), [
+      'Error: thrown from imported function',                   // -
+      /^    at func \(interpreter.js:\d+:11\)$/,                // -
+      '    at main (<WASM>[1]+1)',                              // -
+      /^    at testThrowFromImport \(interpreter.js:\d+:24\)/,  // -
+      /^    at interpreter.js:\d+:3$/
+    ]);
+  }
+})();