[wasm][debug] Remove debugging capabilities from WasmDebugInfo

This CL removes all debugging capabilities from the API provided by
WasmDebugInfo:
- setting and removing breakpoints
- stepping
- stack inspection

The WasmDebugInfo is still kept, since it's used from tests to
instantiate the interpreter.

R=thibaudm@chromium.org, bmeurer@chromium.org

Bug: v8:10389
Change-Id: I297bfc8df2104dae8b9f9f9b13078026c47698da
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2164791Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67418}

src/debug/debug-scope-iterator.cc

@@ -129,11 +129,9 @@ bool DebugScopeIterator::SetVariableValue(v8::Local<v8::String> name,
 }
 
 DebugWasmScopeIterator::DebugWasmScopeIterator(Isolate* isolate,
-                                               StandardFrame* frame,
-                                               int inlined_frame_index)
+                                               StandardFrame* frame)
     : isolate_(isolate),
       frame_(frame),
-      inlined_frame_index_(inlined_frame_index),
       type_(debug::ScopeIterator::ScopeTypeModule) {}
 
 bool DebugWasmScopeIterator::Done() {
@@ -172,13 +170,6 @@ v8::Local<v8::Object> DebugWasmScopeIterator::GetObject() {
       return Utils::ToLocal(wasm::GetModuleScopeObject(instance));
     }
     case debug::ScopeIterator::ScopeTypeLocal: {
-      if (frame_->is_wasm_interpreter_entry()) {
-        Handle<WasmDebugInfo> debug_info(
-            WasmInterpreterEntryFrame::cast(frame_)->debug_info(), isolate_);
-        return Utils::ToLocal(WasmDebugInfo::GetLocalScopeObject(
-            debug_info, frame_->fp(), inlined_frame_index_));
-      }
-      // Compiled code.
       DCHECK(frame_->is_wasm_compiled());
       wasm::DebugInfo* debug_info =
           WasmCompiledFrame::cast(frame_)->native_module()->GetDebugInfo();
@@ -186,13 +177,6 @@ v8::Local<v8::Object> DebugWasmScopeIterator::GetObject() {
           isolate_, frame_->pc(), frame_->fp(), frame_->callee_fp()));
     }
     case debug::ScopeIterator::ScopeTypeWasmExpressionStack: {
-      if (frame_->is_wasm_interpreter_entry()) {
-        Handle<WasmDebugInfo> debug_info(
-            WasmInterpreterEntryFrame::cast(frame_)->debug_info(), isolate_);
-        return Utils::ToLocal(WasmDebugInfo::GetStackScopeObject(
-            debug_info, frame_->fp(), inlined_frame_index_));
-      }
-      // Compiled code.
       DCHECK(frame_->is_wasm_compiled());
       wasm::DebugInfo* debug_info =
           WasmCompiledFrame::cast(frame_)->native_module()->GetDebugInfo();

src/debug/debug-scope-iterator.h

@@ -39,8 +39,7 @@ class DebugScopeIterator final : public debug::ScopeIterator {
 
 class DebugWasmScopeIterator final : public debug::ScopeIterator {
  public:
-  DebugWasmScopeIterator(Isolate* isolate, StandardFrame* frame,
-                         int inlined_frame_index);
+  DebugWasmScopeIterator(Isolate* isolate, StandardFrame* frame);
 
   bool Done() override;
   void Advance() override;
@@ -57,7 +56,6 @@ class DebugWasmScopeIterator final : public debug::ScopeIterator {
  private:
   Isolate* isolate_;
   StandardFrame* frame_;
-  int inlined_frame_index_;
   ScopeType type_;
 };
 }  // namespace internal

src/debug/debug-stack-trace-iterator.cc

@@ -160,8 +160,8 @@ DebugStackTraceIterator::GetScopeIterator() const {
   DCHECK(!Done());
   StandardFrame* frame = iterator_.frame();
   if (frame->is_wasm()) {
-    return std::make_unique<DebugWasmScopeIterator>(isolate_, iterator_.frame(),
-                                                    inlined_frame_index_);
+    return std::make_unique<DebugWasmScopeIterator>(isolate_,
+                                                    iterator_.frame());
   }
   return std::make_unique<DebugScopeIterator>(isolate_, frame_inspector_.get());
 }

src/debug/debug.cc

@@ -1026,14 +1026,6 @@ void Debug::PrepareStep(StepAction step_action) {
     thread_local_.last_frame_count_ = current_frame_count;
     // No longer perform the current async step.
     clear_suspended_generator();
-  } else if (frame->is_wasm_interpreter_entry()) {
-    // Handle stepping in wasm functions via the wasm interpreter.
-    WasmInterpreterEntryFrame* wasm_frame =
-        WasmInterpreterEntryFrame::cast(frame);
-    if (wasm_frame->NumberOfActiveFrames() > 0) {
-      wasm_frame->debug_info().PrepareStep(step_action);
-      return;
-    }
   } else if (frame->is_wasm_compiled()) {
     // Handle stepping in Liftoff code.
     WasmCompiledFrame* wasm_frame = WasmCompiledFrame::cast(frame);

src/wasm/module-instantiate.cc

@@ -533,13 +533,6 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
     if (thrower_->error()) return {};
   }
 
-  //--------------------------------------------------------------------------
-  // Debugging support.
-  //--------------------------------------------------------------------------
-  // Set all breakpoints that were set on the shared module.
-  WasmScript::SetBreakpointsOnNewInstance(
-      handle(module_object_->script(), isolate_), instance);
-
   //--------------------------------------------------------------------------
   // Create a wrapper for the start function.
   //--------------------------------------------------------------------------

src/wasm/wasm-debug.cc

@@ -91,8 +91,6 @@ class InterpreterHandle {
   Isolate* isolate_;
   const WasmModule* module_;
   WasmInterpreter interpreter_;
-  StepAction next_step_action_ = StepNone;
-  int last_step_stack_depth_ = 0;
   std::unordered_map<Address, uint32_t> activations_;
 
   uint32_t StartActivation(Address frame_pointer) {
@@ -146,18 +144,6 @@ class InterpreterHandle {
   WasmInterpreter* interpreter() { return &interpreter_; }
   const WasmModule* module() const { return module_; }
 
-  void PrepareStep(StepAction step_action) {
-    next_step_action_ = step_action;
-    last_step_stack_depth_ = CurrentStackDepth();
-  }
-
-  void ClearStepping() { next_step_action_ = StepNone; }
-
-  int CurrentStackDepth() {
-    DCHECK_EQ(1, interpreter()->GetThreadCount());
-    return interpreter()->GetThread(0)->GetFrameCount();
-  }
-
   // Returns true if exited regularly, false if a trap/exception occurred and
   // was not handled inside this activation. In the latter case, a pending
   // exception will have been set on the isolate.
@@ -179,11 +165,10 @@ class InterpreterHandle {
     bool finished = false;
     while (!finished) {
       // TODO(clemensb): Add occasional StackChecks.
-      WasmInterpreter::State state = ContinueExecution(thread);
+      WasmInterpreter::State state = thread->Run();
       switch (state) {
         case WasmInterpreter::State::PAUSED:
-          NotifyDebugEventListeners(thread);
-          break;
+          UNREACHABLE();
         case WasmInterpreter::State::FINISHED:
           // Perfect, just break the switch and exit the loop.
           finished = true;
@@ -231,109 +216,9 @@ class InterpreterHandle {
 
     FinishActivation(frame_pointer, activation_id);
 
-    // If we do stepping and it exits wasm interpreter then debugger need to
-    // prepare for it.
-    if (next_step_action_ != StepNone) {
-      // Enter the debugger.
-      DebugScope debug_scope(isolate_->debug());
-
-      isolate_->debug()->PrepareStep(StepOut);
-    }
-    ClearStepping();
-
     return true;
   }
 
-  WasmInterpreter::State ContinueExecution(WasmInterpreter::Thread* thread) {
-    switch (next_step_action_) {
-      case StepNone:
-        return thread->Run();
-      case StepIn:
-        return thread->Step();
-      case StepOut:
-        thread->AddBreakFlags(WasmInterpreter::BreakFlag::AfterReturn);
-        return thread->Run();
-      case StepNext: {
-        int stack_depth = thread->GetFrameCount();
-        if (stack_depth == last_step_stack_depth_) return thread->Step();
-        thread->AddBreakFlags(stack_depth > last_step_stack_depth_
-                                  ? WasmInterpreter::BreakFlag::AfterReturn
-                                  : WasmInterpreter::BreakFlag::AfterCall);
-        return thread->Run();
-      }
-      default:
-        UNREACHABLE();
-    }
-  }
-
-  Handle<WasmInstanceObject> GetInstanceObject() {
-    StackTraceFrameIterator it(isolate_);
-    WasmInterpreterEntryFrame* frame =
-        WasmInterpreterEntryFrame::cast(it.frame());
-    Handle<WasmInstanceObject> instance_obj(frame->wasm_instance(), isolate_);
-    // Check that this is indeed the instance which is connected to this
-    // interpreter.
-    DCHECK_EQ(this, Managed<InterpreterHandle>::cast(
-                        instance_obj->debug_info().interpreter_handle())
-                        .raw());
-    return instance_obj;
-  }
-
-  void NotifyDebugEventListeners(WasmInterpreter::Thread* thread) {
-    // Enter the debugger.
-    DebugScope debug_scope(isolate_->debug());
-
-    // Check whether we hit a breakpoint.
-    if (isolate_->debug()->break_points_active()) {
-      Handle<WasmModuleObject> module_object(
-          GetInstanceObject()->module_object(), isolate_);
-      Handle<Script> script(module_object->script(), isolate_);
-      int position = GetTopPosition(module_object);
-      Handle<FixedArray> breakpoints;
-      if (WasmScript::CheckBreakPoints(isolate_, script, position)
-              .ToHandle(&breakpoints)) {
-        // We hit one or several breakpoints. Clear stepping, notify the
-        // listeners and return.
-        ClearStepping();
-        isolate_->debug()->OnDebugBreak(breakpoints);
-        return;
-      }
-    }
-
-    // We did not hit a breakpoint, so maybe this pause is related to stepping.
-    bool hit_step = false;
-    switch (next_step_action_) {
-      case StepNone:
-        break;
-      case StepIn:
-        hit_step = true;
-        break;
-      case StepOut:
-        hit_step = thread->GetFrameCount() < last_step_stack_depth_;
-        break;
-      case StepNext: {
-        hit_step = thread->GetFrameCount() == last_step_stack_depth_;
-        break;
-      }
-      default:
-        UNREACHABLE();
-    }
-    if (!hit_step) return;
-    ClearStepping();
-    isolate_->debug()->OnDebugBreak(isolate_->factory()->empty_fixed_array());
-  }
-
-  int GetTopPosition(Handle<WasmModuleObject> module_object) {
-    DCHECK_EQ(1, interpreter()->GetThreadCount());
-    WasmInterpreter::Thread* thread = interpreter()->GetThread(0);
-    DCHECK_LT(0, thread->GetFrameCount());
-
-    auto frame = thread->GetFrame(thread->GetFrameCount() - 1);
-    return GetWasmFunctionOffset(module_object->module(),
-                                 frame->function()->func_index) +
-           frame->pc();
-  }
-
   std::vector<std::pair<uint32_t, int>> GetInterpretedStack(
       Address frame_pointer) {
     DCHECK_EQ(1, interpreter()->GetThreadCount());
@@ -381,65 +266,6 @@ class InterpreterHandle {
     return interpreter()->GetThread(0)->NumInterpretedCalls();
   }
 
-  Handle<JSObject> GetLocalScopeObject(InterpretedFrame* frame,
-                                       Handle<WasmDebugInfo> debug_info) {
-    Isolate* isolate = isolate_;
-
-    Handle<JSObject> local_scope_object =
-        isolate_->factory()->NewJSObjectWithNullProto();
-    // Fill parameters and locals.
-    int num_locals = frame->GetLocalCount();
-    DCHECK_LE(frame->GetParameterCount(), num_locals);
-    if (num_locals > 0) {
-      Handle<JSObject> locals_obj =
-          isolate_->factory()->NewJSObjectWithNullProto();
-      Handle<String> locals_name =
-          isolate_->factory()->InternalizeString(StaticCharVector("locals"));
-      JSObject::AddProperty(isolate, local_scope_object, locals_name,
-                            locals_obj, NONE);
-      NativeModule* native_module =
-          debug_info->wasm_instance().module_object().native_module();
-      for (int i = 0; i < num_locals; ++i) {
-        Handle<Name> name;
-        if (!GetLocalNameString(isolate, native_module,
-                                frame->function()->func_index, i)
-                 .ToHandle(&name)) {
-          name = PrintFToOneByteString<true>(isolate_, "var%d", i);
-        }
-        WasmValue value = frame->GetLocalValue(i);
-        Handle<Object> value_obj = WasmValueToValueObject(isolate_, value);
-        // {name} can be a string representation of an element index.
-        LookupIterator::Key lookup_key{isolate, name};
-        LookupIterator it(isolate, locals_obj, lookup_key, locals_obj,
-                          LookupIterator::OWN_SKIP_INTERCEPTOR);
-        if (it.IsFound()) continue;
-        Object::AddDataProperty(&it, value_obj, NONE,
-                                Just(ShouldThrow::kThrowOnError),
-                                StoreOrigin::kNamed)
-            .Check();
-      }
-    }
-    return local_scope_object;
-  }
-
-  Handle<JSObject> GetStackScopeObject(InterpretedFrame* frame,
-                                       Handle<WasmDebugInfo> debug_info) {
-    // Fill stack values.
-    int stack_count = frame->GetStackHeight();
-    // Use an object without prototype instead of an Array, for nicer displaying
-    // in DevTools. For Arrays, the length field and prototype is displayed,
-    // which does not make too much sense here.
-    Handle<JSObject> stack_scope_obj =
-        isolate_->factory()->NewJSObjectWithNullProto();
-    for (int i = 0; i < stack_count; ++i) {
-      WasmValue value = frame->GetStackValue(i);
-      Handle<Object> value_obj = WasmValueToValueObject(isolate_, value);
-      JSObject::AddDataElement(stack_scope_obj, static_cast<uint32_t>(i),
-                               value_obj, NONE);
-    }
-    return stack_scope_obj;
-  }
-
  private:
   DISALLOW_COPY_AND_ASSIGN(InterpreterHandle);
 };
@@ -1137,39 +963,6 @@ wasm::WasmInterpreter* WasmDebugInfo::SetupForTesting(
   return interp_handle->raw()->interpreter();
 }
 
-// static
-void WasmDebugInfo::PrepareStepIn(Handle<WasmDebugInfo> debug_info,
-                                  int func_index) {
-  Isolate* isolate = debug_info->GetIsolate();
-  auto* handle = GetOrCreateInterpreterHandle(isolate, debug_info);
-  RedirectToInterpreter(debug_info, Vector<int>(&func_index, 1));
-  const wasm::WasmFunction* func = &handle->module()->functions[func_index];
-  handle->interpreter()->PrepareStepIn(func);
-  // Debug break would be considered as a step-in inside wasm.
-  handle->PrepareStep(StepAction::StepIn);
-}
-
-// static
-void WasmDebugInfo::SetBreakpoint(Handle<WasmDebugInfo> debug_info,
-                                  int func_index, int offset) {
-  Isolate* isolate = debug_info->GetIsolate();
-  auto* handle = GetOrCreateInterpreterHandle(isolate, debug_info);
-  RedirectToInterpreter(debug_info, Vector<int>(&func_index, 1));
-  const wasm::WasmFunction* func = &handle->module()->functions[func_index];
-  handle->interpreter()->SetBreakpoint(func, offset, true);
-}
-
-// static
-void WasmDebugInfo::ClearBreakpoint(Handle<WasmDebugInfo> debug_info,
-                                    int func_index, int offset) {
-  Isolate* isolate = debug_info->GetIsolate();
-  auto* handle = GetOrCreateInterpreterHandle(isolate, debug_info);
-  // TODO(leese): If there are no more breakpoints left it would be good to
-  // undo redirecting to the interpreter.
-  const wasm::WasmFunction* func = &handle->module()->functions[func_index];
-  handle->interpreter()->SetBreakpoint(func, offset, false);
-}
-
 // static
 void WasmDebugInfo::RedirectToInterpreter(Handle<WasmDebugInfo> debug_info,
                                           Vector<int> func_indexes) {
@@ -1206,10 +999,6 @@ void WasmDebugInfo::RedirectToInterpreter(Handle<WasmDebugInfo> debug_info,
   }
 }
 
-void WasmDebugInfo::PrepareStep(StepAction step_action) {
-  GetInterpreterHandle(*this)->PrepareStep(step_action);
-}
-
 // static
 bool WasmDebugInfo::RunInterpreter(Isolate* isolate,
                                    Handle<WasmDebugInfo> debug_info,
@@ -1243,22 +1032,6 @@ uint64_t WasmDebugInfo::NumInterpretedCalls() {
   return handle ? handle->NumInterpretedCalls() : 0;
 }
 
-// static
-Handle<JSObject> WasmDebugInfo::GetLocalScopeObject(
-    Handle<WasmDebugInfo> debug_info, Address frame_pointer, int frame_index) {
-  auto* interp_handle = GetInterpreterHandle(*debug_info);
-  auto frame = interp_handle->GetInterpretedFrame(frame_pointer, frame_index);
-  return interp_handle->GetLocalScopeObject(frame.get(), debug_info);
-}
-
-// static
-Handle<JSObject> WasmDebugInfo::GetStackScopeObject(
-    Handle<WasmDebugInfo> debug_info, Address frame_pointer, int frame_index) {
-  auto* interp_handle = GetInterpreterHandle(*debug_info);
-  auto frame = interp_handle->GetInterpretedFrame(frame_pointer, frame_index);
-  return interp_handle->GetStackScopeObject(frame.get(), debug_info);
-}
-
 // static
 Handle<Code> WasmDebugInfo::GetCWasmEntry(Handle<WasmDebugInfo> debug_info,
                                           const wasm::FunctionSig* sig) {
@@ -1521,38 +1294,6 @@ void WasmScript::AddBreakpointToInfo(Handle<Script> script, int position,
   new_breakpoint_infos->set(insert_pos, *breakpoint_info);
 }
 
-void WasmScript::SetBreakpointsOnNewInstance(
-    Handle<Script> script, Handle<WasmInstanceObject> instance) {
-  if (!script->has_wasm_breakpoint_infos()) return;
-  Isolate* isolate = script->GetIsolate();
-  Handle<WasmDebugInfo> debug_info =
-      WasmInstanceObject::GetOrCreateDebugInfo(instance);
-
-  Handle<FixedArray> breakpoint_infos(script->wasm_breakpoint_infos(), isolate);
-  // If the array exists, it should not be empty.
-  DCHECK_LT(0, breakpoint_infos->length());
-
-  for (int i = 0, e = breakpoint_infos->length(); i < e; ++i) {
-    Handle<Object> obj(breakpoint_infos->get(i), isolate);
-    if (obj->IsUndefined(isolate)) {
-      for (; i < e; ++i) {
-        DCHECK(breakpoint_infos->get(i).IsUndefined(isolate));
-      }
-      break;
-    }
-    Handle<BreakPointInfo> breakpoint_info = Handle<BreakPointInfo>::cast(obj);
-    int position = breakpoint_info->source_position();
-
-    // Find the function for this breakpoint, and set the breakpoint.
-    const wasm::WasmModule* module = script->wasm_native_module()->module();
-    int func_index = GetContainingWasmFunction(module, position);
-    DCHECK_LE(0, func_index);
-    const wasm::WasmFunction& func = module->functions[func_index];
-    int offset_in_func = position - func.code.offset();
-    WasmDebugInfo::SetBreakpoint(debug_info, func_index, offset_in_func);
-  }
-}
-
 // static
 bool WasmScript::GetPossibleBreakpoints(
     wasm::NativeModule* native_module, const v8::debug::Location& start,

src/wasm/wasm-objects.h

@@ -843,28 +843,11 @@ class WasmDebugInfo : public Struct {
   V8_EXPORT_PRIVATE static wasm::WasmInterpreter* SetupForTesting(
       Handle<WasmInstanceObject>);
 
-  // Prepare WasmDebugInfo for stepping in the given function.
-  V8_EXPORT_PRIVATE static void PrepareStepIn(Handle<WasmDebugInfo>,
-                                              int func_index);
-
-  // Set a breakpoint in the given function at the given byte offset within that
-  // function. This will redirect all future calls to this function to the
-  // interpreter and will always pause at the given offset.
-  V8_EXPORT_PRIVATE static void SetBreakpoint(Handle<WasmDebugInfo>,
-                                              int func_index, int offset);
-
-  // Clear a previously set breakpoint in the given function at the given byte
-  // offset within that function.
-  V8_EXPORT_PRIVATE static void ClearBreakpoint(Handle<WasmDebugInfo>,
-                                                int func_index, int offset);
-
   // Make a set of functions always execute in the interpreter without setting
   // breakpoints.
   V8_EXPORT_PRIVATE static void RedirectToInterpreter(Handle<WasmDebugInfo>,
                                                       Vector<int> func_indexes);
 
-  void PrepareStep(StepAction);
-
   // Execute the specified function in the interpreter. Read arguments from the
   // {argument_values} vector and write to {return_values} on regular exit.
   // The frame_pointer will be used to identify the new activation of the
@@ -890,18 +873,6 @@ class WasmDebugInfo : public Struct {
   // Returns the number of calls / function frames executed in the interpreter.
   V8_EXPORT_PRIVATE uint64_t NumInterpretedCalls();
 
-  // Get local scope details for a specific interpreted frame. It contains
-  // information about parameters, locals, and stack values.
-  static Handle<JSObject> GetLocalScopeObject(Handle<WasmDebugInfo>,
-                                              Address frame_pointer,
-                                              int frame_index);
-
-  // Get stack scope details for a specific interpreted frame. It contains
-  // information about stack values.
-  static Handle<JSObject> GetStackScopeObject(Handle<WasmDebugInfo>,
-                                              Address frame_pointer,
-                                              int frame_index);
-
   V8_EXPORT_PRIVATE static Handle<Code> GetCWasmEntry(Handle<WasmDebugInfo>,
                                                       const wasm::FunctionSig*);
 
@@ -942,9 +913,6 @@ class WasmScript : public AllStatic {
   V8_EXPORT_PRIVATE static bool ClearBreakPointById(Handle<Script>,
                                                     int breakpoint_id);
 
-  static void SetBreakpointsOnNewInstance(Handle<Script>,
-                                          Handle<WasmInstanceObject>);
-
   // Get a list of all possible breakpoints within a given range of this module.
   V8_EXPORT_PRIVATE static bool GetPossibleBreakpoints(
       wasm::NativeModule* native_module, const debug::Location& start,