[wasm] [interpreter] Refactor and extend InterpretedFrame

Similar to WasmInterpreter::Thread, we now also use the pimpl idiom for
InterpretedFrame, hiding the implementation completely in the .cc file.
This allows us to store just two things per InterpretedFrameImpl: The
corresponding thread, and the frame index.
The external interface changes to always return a std::unique_ptr,
because the object layout is not known via the public interface, hence
objects cannot be stack allocated. They also cannot be copied or passed
by value.

The frame inspection interface will be tested after another fix in
https://chromium-review.googlesource.com/474749.

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

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

src/wasm/wasm-debug.cc

@@ -303,10 +303,9 @@ class InterpreterHandle {
     WasmInterpreter::Thread* thread = interpreter()->GetThread(0);
     DCHECK_LT(0, thread->GetFrameCount());
 
-    wasm::InterpretedFrame frame =
-        thread->GetFrame(thread->GetFrameCount() - 1);
-    return compiled_module->GetFunctionOffset(frame.function()->func_index) +
-           frame.pc();
+    auto frame = thread->GetFrame(thread->GetFrameCount() - 1);
+    return compiled_module->GetFunctionOffset(frame->function()->func_index) +
+           frame->pc();
   }
 
   std::vector<std::pair<uint32_t, int>> GetInterpretedStack(
@@ -320,8 +319,8 @@ class InterpreterHandle {
     std::vector<std::pair<uint32_t, int>> stack;
     stack.reserve(frame_range.second - frame_range.first);
     for (uint32_t fp = frame_range.first; fp < frame_range.second; ++fp) {
-      wasm::InterpretedFrame frame = thread->GetFrame(fp);
-      stack.emplace_back(frame.function()->func_index, frame.pc());
+      auto frame = thread->GetFrame(fp);
+      stack.emplace_back(frame->function()->func_index, frame->pc());
     }
     return stack;
   }
@@ -336,8 +335,7 @@ class InterpreterHandle {
     DCHECK_LE(0, idx);
     DCHECK_GT(frame_range.second - frame_range.first, idx);
 
-    return std::unique_ptr<wasm::InterpretedFrame>(new wasm::InterpretedFrame(
-        thread->GetMutableFrame(frame_range.first + idx)));
+    return thread->GetFrame(frame_range.first + idx);
   }
 
   void Unwind(Address frame_pointer) {

src/wasm/wasm-interpreter.cc

@@ -1176,19 +1176,6 @@ class ThreadImpl {
     return static_cast<int>(frames_.size());
   }
 
-  template <typename FrameCons>
-  InterpretedFrame GetMutableFrame(int index, FrameCons frame_cons) {
-    DCHECK_LE(0, index);
-    DCHECK_GT(frames_.size(), index);
-    Frame* frame = &frames_[index];
-    DCHECK_GE(kMaxInt, frame->pc);
-    DCHECK_GE(kMaxInt, frame->sp);
-    DCHECK_GE(kMaxInt, frame->llimit());
-    return frame_cons(frame->code->function, static_cast<int>(frame->pc),
-                      static_cast<int>(frame->sp),
-                      static_cast<int>(frame->llimit()));
-  }
-
   WasmVal GetReturnValue(uint32_t index) {
     if (state_ == WasmInterpreter::TRAPPED) return WasmVal(0xdeadbeef);
     DCHECK_EQ(WasmInterpreter::FINISHED, state_);
@@ -1199,6 +1186,11 @@ class ThreadImpl {
     return stack_[act.sp + index];
   }
 
+  WasmVal GetStackValue(uint32_t index) {
+    DCHECK_GT(stack_.size(), index);
+    return stack_[index];
+  }
+
   TrapReason GetTrapReason() { return trap_reason_; }
 
   pc_t GetBreakpointPc() { return break_pc_; }
@@ -1282,6 +1274,8 @@ class ThreadImpl {
     unsigned arity;
   };
 
+  friend class InterpretedFrameImpl;
+
   CodeMap* codemap_;
   WasmInstance* instance_;
   ZoneVector<WasmVal> stack_;
@@ -2234,6 +2228,66 @@ class ThreadImpl {
   }
 };
 
+class InterpretedFrameImpl {
+ public:
+  InterpretedFrameImpl(ThreadImpl* thread, int index)
+      : thread_(thread), index_(index) {
+    DCHECK_LE(0, index);
+  }
+
+  const WasmFunction* function() const { return frame()->code->function; }
+
+  int pc() const {
+    DCHECK_LE(0, frame()->pc);
+    DCHECK_GE(kMaxInt, frame()->pc);
+    return static_cast<int>(frame()->pc);
+  }
+
+  int GetParameterCount() const {
+    DCHECK_GE(kMaxInt, function()->sig->parameter_count());
+    return static_cast<int>(function()->sig->parameter_count());
+  }
+
+  int GetLocalCount() const {
+    size_t num_locals = function()->sig->parameter_count() +
+                        frame()->code->locals.type_list.size();
+    DCHECK_GE(kMaxInt, num_locals);
+    return static_cast<int>(num_locals);
+  }
+
+  int GetStackHeight() const {
+    bool is_top_frame =
+        static_cast<size_t>(index_) + 1 == thread_->frames_.size();
+    size_t stack_limit =
+        is_top_frame ? thread_->stack_.size() : thread_->frames_[index_ + 1].sp;
+    DCHECK_LE(GetLocalCount(), stack_limit);
+    return static_cast<int>(stack_limit) - GetLocalCount();
+  }
+
+  WasmVal GetLocalValue(int index) const {
+    DCHECK_LE(0, index);
+    DCHECK_GT(GetLocalCount(), index);
+    return thread_->GetStackValue(static_cast<int>(frame()->sp) + index);
+  }
+
+  WasmVal GetStackValue(int index) const {
+    DCHECK_LE(0, index);
+    // Index must be within the number of stack values of this frame.
+    DCHECK_GT(GetStackHeight(), index);
+    return thread_->GetStackValue(static_cast<int>(frame()->sp) +
+                                  GetLocalCount() + index);
+  }
+
+ private:
+  ThreadImpl* thread_;
+  int index_;
+
+  ThreadImpl::Frame* frame() const {
+    DCHECK_GT(thread_->frames_.size(), index_);
+    return &thread_->frames_[index_];
+  }
+};
+
 // Converters between WasmInterpreter::Thread and WasmInterpreter::ThreadImpl.
 // Thread* is the public interface, without knowledge of the object layout.
 // This cast is potentially risky, but as long as we always cast it back before
@@ -2245,6 +2299,14 @@ ThreadImpl* ToImpl(WasmInterpreter::Thread* thread) {
   return reinterpret_cast<ThreadImpl*>(thread);
 }
 
+// Same conversion for InterpretedFrame and InterpretedFrameImpl.
+InterpretedFrame* ToFrame(InterpretedFrameImpl* impl) {
+  return reinterpret_cast<InterpretedFrame*>(impl);
+}
+const InterpretedFrameImpl* ToImpl(const InterpretedFrame* frame) {
+  return reinterpret_cast<const InterpretedFrameImpl*>(frame);
+}
+
 }  // namespace
 
 //============================================================================
@@ -2275,16 +2337,11 @@ pc_t WasmInterpreter::Thread::GetBreakpointPc() {
 int WasmInterpreter::Thread::GetFrameCount() {
   return ToImpl(this)->GetFrameCount();
 }
-const InterpretedFrame WasmInterpreter::Thread::GetFrame(int index) {
-  return GetMutableFrame(index);
-}
-InterpretedFrame WasmInterpreter::Thread::GetMutableFrame(int index) {
-  // We have access to the constructor of InterpretedFrame, but ThreadImpl has
-  // not. So pass it as a lambda (should all get inlined).
-  auto frame_cons = [](const WasmFunction* function, int pc, int fp, int sp) {
-    return InterpretedFrame(function, pc, fp, sp);
-  };
-  return ToImpl(this)->GetMutableFrame(index, frame_cons);
+std::unique_ptr<InterpretedFrame> WasmInterpreter::Thread::GetFrame(int index) {
+  DCHECK_LE(0, index);
+  DCHECK_GT(GetFrameCount(), index);
+  return std::unique_ptr<InterpretedFrame>(
+      ToFrame(new InterpretedFrameImpl(ToImpl(this), index)));
 }
 WasmVal WasmInterpreter::Thread::GetReturnValue(int index) {
   return ToImpl(this)->GetReturnValue(index);
@@ -2435,31 +2492,25 @@ ControlTransferMap WasmInterpreter::ComputeControlTransfersForTesting(
 //============================================================================
 // Implementation of the frame inspection interface.
 //============================================================================
+const WasmFunction* InterpretedFrame::function() const {
+  return ToImpl(this)->function();
+}
+int InterpretedFrame::pc() const { return ToImpl(this)->pc(); }
 int InterpretedFrame::GetParameterCount() const {
-  USE(fp_);
-  USE(sp_);
-  // TODO(clemensh): Return the correct number of parameters.
-  return 0;
+  return ToImpl(this)->GetParameterCount();
 }
-
-WasmVal InterpretedFrame::GetLocalVal(int index) const {
-  CHECK_GE(index, 0);
-  UNIMPLEMENTED();
-  WasmVal none;
-  none.type = kWasmStmt;
-  return none;
+int InterpretedFrame::GetLocalCount() const {
+  return ToImpl(this)->GetLocalCount();
 }
-
-WasmVal InterpretedFrame::GetExprVal(int pc) const {
-  UNIMPLEMENTED();
-  WasmVal none;
-  none.type = kWasmStmt;
-  return none;
+int InterpretedFrame::GetStackHeight() const {
+  return ToImpl(this)->GetStackHeight();
+}
+WasmVal InterpretedFrame::GetLocalValue(int index) const {
+  return ToImpl(this)->GetLocalValue(index);
+}
+WasmVal InterpretedFrame::GetStackValue(int index) const {
+  return ToImpl(this)->GetStackValue(index);
 }
-
-void InterpretedFrame::SetLocalVal(int index, WasmVal val) { UNIMPLEMENTED(); }
-
-void InterpretedFrame::SetExprVal(int pc, WasmVal val) { UNIMPLEMENTED(); }
 
 }  // namespace wasm
 }  // namespace internal

src/wasm/wasm-interpreter.h

@@ -82,30 +82,37 @@ FOREACH_UNION_MEMBER(DECLARE_CAST)
 #undef DECLARE_CAST
 
 // Representation of frames within the interpreter.
+//
+// Layout of a frame:
+// -----------------
+// stack slot #N  ‾\.
+// ...             |  stack entries: GetStackHeight(); GetStackValue()
+// stack slot #0  _/·
+// local #L       ‾\.
+// ...             |  locals: GetLocalCount(); GetLocalValue()
+// local #P+1      |
+// param #P        |   ‾\.
+// ...             |    | parameters: GetParameterCount(); GetLocalValue()
+// param #0       _/·  _/·
+// -----------------
+//
 class InterpretedFrame {
  public:
-  const WasmFunction* function() const { return function_; }
-  int pc() const { return pc_; }
+  const WasmFunction* function() const;
+  int pc() const;
 
-  //==========================================================================
-  // Stack frame inspection.
-  //==========================================================================
   int GetParameterCount() const;
-  WasmVal GetLocalVal(int index) const;
-  WasmVal GetExprVal(int pc) const;
-  void SetLocalVal(int index, WasmVal val);
-  void SetExprVal(int pc, WasmVal val);
+  int GetLocalCount() const;
+  int GetStackHeight() const;
+  WasmVal GetLocalValue(int index) const;
+  WasmVal GetStackValue(int index) const;
 
  private:
   friend class WasmInterpreter;
-
-  InterpretedFrame(const WasmFunction* function, int pc, int fp, int sp)
-      : function_(function), pc_(pc), fp_(fp), sp_(sp) {}
-
-  const WasmFunction* function_;
-  int pc_;
-  int fp_;
-  int sp_;
+  // Don't instante InterpretedFrames; they will be allocated as
+  // InterpretedFrameImpl in the interpreter implementation.
+  InterpretedFrame() = delete;
+  DISALLOW_COPY_AND_ASSIGN(InterpretedFrame);
 };
 
 // An interpreter capable of executing WASM.
@@ -154,8 +161,8 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     pc_t GetBreakpointPc();
     // TODO(clemensh): Make this uint32_t.
     int GetFrameCount();
-    const InterpretedFrame GetFrame(int index);
-    InterpretedFrame GetMutableFrame(int index);
+    // The InterpretedFrame is only valid as long as the Thread is paused.
+    std::unique_ptr<InterpretedFrame> GetFrame(int index);
     WasmVal GetReturnValue(int index = 0);
     TrapReason GetTrapReason();
 

src/wasm/wasm-objects.h

@@ -579,7 +579,7 @@ class WasmDebugInfo : public FixedArray {
       Address frame_pointer);
 
   std::unique_ptr<wasm::InterpretedFrame> GetInterpretedFrame(
-      Address frame_pointer, int idx);
+      Address frame_pointer, int frame_index);
 
   // Unwind the interpreted stack belonging to the passed interpreter entry
   // frame.