[wasm] [interpreter] Fix deallocation of InterpretedFrameImpl

We were passing a pointer to an object allocated as
{InterpretedFrameImpl} in an {std::unique_ptr<InterpretedFrame>}.
The default deleter then called {delete ptr;} on a ptr of type
{InterpretedFrame*}, even though that object was allocated as
{InterpretedFrameImpl}. This error might caught by validators.
Fix this by passing a custom deleter on the unique_ptr.

R=ahaas@chromium.org, ulan@chromium.org

Bug: v8:7231
Change-Id: Ia18114236384813c4878319209ae4535fda56c41
Reviewed-on: https://chromium-review.googlesource.com/834510Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50226}

src/debug/debug-frames.h

@@ -9,15 +9,11 @@
 #include "src/frames.h"
 #include "src/isolate.h"
 #include "src/objects.h"
+#include "src/wasm/wasm-interpreter.h"
 
 namespace v8 {
 namespace internal {
 
-// Forward declaration:
-namespace wasm {
-class InterpretedFrame;
-}
-
 class FrameInspector {
  public:
   FrameInspector(StandardFrame* frame, int inlined_frame_index,
@@ -61,7 +57,7 @@ class FrameInspector {
 
   StandardFrame* frame_;
   std::unique_ptr<DeoptimizedFrameInfo> deoptimized_frame_;
-  std::unique_ptr<wasm::InterpretedFrame> wasm_interpreted_frame_;
+  wasm::WasmInterpreter::FramePtr wasm_interpreted_frame_;
   Isolate* isolate_;
   Handle<Script> script_;
   Handle<Object> receiver_;

src/wasm/wasm-debug.cc

@@ -373,8 +373,8 @@ class InterpreterHandle {
     return stack;
   }
 
-  std::unique_ptr<wasm::InterpretedFrame> GetInterpretedFrame(
-      Address frame_pointer, int idx) {
+  WasmInterpreter::FramePtr GetInterpretedFrame(Address frame_pointer,
+                                                int idx) {
     DCHECK_EQ(1, interpreter()->GetThreadCount());
     WasmInterpreter::Thread* thread = interpreter()->GetThread(0);
 
@@ -790,7 +790,7 @@ std::vector<std::pair<uint32_t, int>> WasmDebugInfo::GetInterpretedStack(
   return GetInterpreterHandle(this)->GetInterpretedStack(frame_pointer);
 }
 
-std::unique_ptr<wasm::InterpretedFrame> WasmDebugInfo::GetInterpretedFrame(
+wasm::WasmInterpreter::FramePtr WasmDebugInfo::GetInterpretedFrame(
     Address frame_pointer, int idx) {
   return GetInterpreterHandle(this)->GetInterpretedFrame(frame_pointer, idx);
 }

src/wasm/wasm-interpreter.cc

@@ -2770,11 +2770,10 @@ pc_t WasmInterpreter::Thread::GetBreakpointPc() {
 int WasmInterpreter::Thread::GetFrameCount() {
   return ToImpl(this)->GetFrameCount();
 }
-std::unique_ptr<InterpretedFrame> WasmInterpreter::Thread::GetFrame(int index) {
+WasmInterpreter::FramePtr WasmInterpreter::Thread::GetFrame(int index) {
   DCHECK_LE(0, index);
   DCHECK_GT(GetFrameCount(), index);
-  return std::unique_ptr<InterpretedFrame>(
-      ToFrame(new InterpretedFrameImpl(ToImpl(this), index)));
+  return FramePtr(ToFrame(new InterpretedFrameImpl(ToImpl(this), index)));
 }
 WasmValue WasmInterpreter::Thread::GetReturnValue(int index) {
   return ToImpl(this)->GetReturnValue(index);
@@ -2935,6 +2934,9 @@ WasmValue InterpretedFrame::GetLocalValue(int index) const {
 WasmValue InterpretedFrame::GetStackValue(int index) const {
   return ToImpl(this)->GetStackValue(index);
 }
+void InterpretedFrame::Deleter::operator()(InterpretedFrame* ptr) {
+  delete ToImpl(ptr);
+}
 
 //============================================================================
 // Public API of the heap objects scope.

src/wasm/wasm-interpreter.h

@@ -71,6 +71,12 @@ class InterpretedFrame {
   WasmValue GetLocalValue(int index) const;
   WasmValue GetStackValue(int index) const;
 
+  // Deleter struct to delete the underlying InterpretedFrameImpl without
+  // violating language specifications.
+  struct Deleter {
+    void operator()(InterpretedFrame* ptr);
+  };
+
  private:
   friend class WasmInterpreter;
   // Don't instante InterpretedFrames; they will be allocated as
@@ -113,6 +119,8 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     AfterCall = 1 << 1
   };
 
+  using FramePtr = std::unique_ptr<InterpretedFrame, InterpretedFrame::Deleter>;
+
   // Representation of a thread in the interpreter.
   class V8_EXPORT_PRIVATE Thread {
     // Don't instante Threads; they will be allocated as ThreadImpl in the
@@ -139,7 +147,7 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     // TODO(clemensh): Make this uint32_t.
     int GetFrameCount();
     // The InterpretedFrame is only valid as long as the Thread is paused.
-    std::unique_ptr<InterpretedFrame> GetFrame(int index);
+    FramePtr GetFrame(int index);
     WasmValue GetReturnValue(int index = 0);
     TrapReason GetTrapReason();
 

src/wasm/wasm-objects.h

@@ -12,6 +12,7 @@
 #include "src/objects.h"
 #include "src/objects/script.h"
 #include "src/wasm/decoder.h"
+#include "src/wasm/wasm-interpreter.h"
 #include "src/wasm/wasm-limits.h"
 
 #include "src/heap/heap.h"
@@ -25,7 +26,6 @@ namespace wasm {
 class InterpretedFrame;
 class NativeModule;
 class WasmCode;
-class WasmInterpreter;
 struct WasmModule;
 class SignatureMap;
 typedef Address GlobalHandleAddress;
@@ -631,8 +631,8 @@ class WasmDebugInfo : public FixedArray {
   std::vector<std::pair<uint32_t, int>> GetInterpretedStack(
       Address frame_pointer);
 
-  std::unique_ptr<wasm::InterpretedFrame> GetInterpretedFrame(
-      Address frame_pointer, int frame_index);
+  wasm::WasmInterpreter::FramePtr GetInterpretedFrame(Address frame_pointer,
+                                                      int frame_index);
 
   // Unwind the interpreted stack belonging to the passed interpreter entry
   // frame.