[wasm] Add support for deleting breakpoints

This is the first piece of the wasm debugging prototype. This change
adds support for removing breakpoints in WasmModuleObject. This change
does not introduce any ways of exposing this feature.

Code mostly pulled from Paolo Severini's prototype.

Bug: chromium:1010467
Change-Id: Ia2821c59e89aa7f234398bf41e145b907085b382
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1826902Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Eric Leese <leese@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64162}

src/wasm/wasm-debug.cc

@@ -527,6 +527,7 @@ wasm::WasmInterpreter* WasmDebugInfo::SetupForTesting(
   return interp_handle->raw()->interpreter();
 }
 
+// static
 void WasmDebugInfo::SetBreakpoint(Handle<WasmDebugInfo> debug_info,
                                   int func_index, int offset) {
   Isolate* isolate = debug_info->GetIsolate();
@@ -536,6 +537,18 @@ void WasmDebugInfo::SetBreakpoint(Handle<WasmDebugInfo> debug_info,
   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) {
   Isolate* isolate = debug_info->GetIsolate();

src/wasm/wasm-objects.cc

@@ -252,6 +252,7 @@ Handle<WasmModuleObject> WasmModuleObject::New(
   return module_object;
 }
 
+// static
 bool WasmModuleObject::SetBreakPoint(Handle<WasmModuleObject> module_object,
                                      int* position,
                                      Handle<BreakPoint> break_point) {
@@ -270,7 +271,7 @@ bool WasmModuleObject::SetBreakPoint(Handle<WasmModuleObject> module_object,
                              offset_in_func));
 
   // Insert new break point into break_positions of module object.
-  WasmModuleObject::AddBreakpoint(module_object, *position, break_point);
+  WasmModuleObject::AddBreakpointToInfo(module_object, *position, break_point);
 
   // Iterate over all instances of this module and tell them to set this new
   // breakpoint. We do this using the weak list of all instances.
@@ -291,6 +292,43 @@ bool WasmModuleObject::SetBreakPoint(Handle<WasmModuleObject> module_object,
   return true;
 }
 
+// static
+bool WasmModuleObject::ClearBreakPoint(Handle<WasmModuleObject> module_object,
+                                       int position,
+                                       Handle<BreakPoint> break_point) {
+  Isolate* isolate = module_object->GetIsolate();
+
+  // Find the function for this breakpoint.
+  const WasmModule* module = module_object->module();
+  int func_index = GetContainingWasmFunction(module, position);
+  if (func_index < 0) return false;
+  const WasmFunction& func = module->functions[func_index];
+  int offset_in_func = position - func.code.offset();
+
+  if (!WasmModuleObject::RemoveBreakpointFromInfo(module_object, position,
+                                                  break_point)) {
+    return false;
+  }
+
+  // Iterate over all instances of this module and tell them to remove this
+  // breakpoint. We do this using the weak list of all instances.
+  Handle<WeakArrayList> weak_instance_list(module_object->weak_instance_list(),
+                                           isolate);
+  for (int i = 0; i < weak_instance_list->length(); ++i) {
+    MaybeObject maybe_instance = weak_instance_list->Get(i);
+    if (maybe_instance->IsWeak()) {
+      Handle<WasmInstanceObject> instance(
+          WasmInstanceObject::cast(maybe_instance->GetHeapObjectAssumeWeak()),
+          isolate);
+      Handle<WasmDebugInfo> debug_info =
+          WasmInstanceObject::GetOrCreateDebugInfo(instance);
+      WasmDebugInfo::ClearBreakpoint(debug_info, func_index, offset_in_func);
+    }
+  }
+
+  return true;
+}
+
 namespace {
 
 int GetBreakpointPos(Isolate* isolate, Object break_point_info_or_undef) {
@@ -323,9 +361,10 @@ int FindBreakpointInfoInsertPos(Isolate* isolate,
 
 }  // namespace
 
-void WasmModuleObject::AddBreakpoint(Handle<WasmModuleObject> module_object,
-                                     int position,
-                                     Handle<BreakPoint> break_point) {
+// static
+void WasmModuleObject::AddBreakpointToInfo(
+    Handle<WasmModuleObject> module_object, int position,
+    Handle<BreakPoint> break_point) {
   Isolate* isolate = module_object->GetIsolate();
   Handle<FixedArray> breakpoint_infos;
   if (module_object->has_breakpoint_infos()) {
@@ -379,6 +418,39 @@ void WasmModuleObject::AddBreakpoint(Handle<WasmModuleObject> module_object,
   new_breakpoint_infos->set(insert_pos, *breakpoint_info);
 }
 
+// static
+bool WasmModuleObject::RemoveBreakpointFromInfo(
+    Handle<WasmModuleObject> module_object, int position,
+    Handle<BreakPoint> break_point) {
+  if (!module_object->has_breakpoint_infos()) return false;
+
+  Isolate* isolate = module_object->GetIsolate();
+  Handle<FixedArray> breakpoint_infos(module_object->breakpoint_infos(),
+                                      isolate);
+
+  int pos = FindBreakpointInfoInsertPos(isolate, breakpoint_infos, position);
+
+  // Does a BreakPointInfo object already exist for this position?
+  if (pos == breakpoint_infos->length()) return false;
+
+  Handle<BreakPointInfo> info(BreakPointInfo::cast(breakpoint_infos->get(pos)),
+                              isolate);
+  BreakPointInfo::ClearBreakPoint(isolate, info, break_point);
+
+  // Check if there are no more breakpoints at this location.
+  if (info->GetBreakPointCount(isolate) == 0) {
+    // Update array by moving breakpoints up one position.
+    for (int i = pos; i < breakpoint_infos->length() - 1; i++) {
+      Object entry = breakpoint_infos->get(i + 1);
+      breakpoint_infos->set(i, entry);
+      if (entry.IsUndefined(isolate)) break;
+    }
+    // Make sure last array element is empty as a result.
+    breakpoint_infos->set_undefined(breakpoint_infos->length() - 1);
+  }
+  return true;
+}
+
 void WasmModuleObject::SetBreakpointsOnNewInstance(
     Handle<WasmModuleObject> module_object,
     Handle<WasmInstanceObject> instance) {

src/wasm/wasm-objects.h

@@ -165,12 +165,15 @@ class WasmModuleObject : public JSObject {
                                               int* position,
                                               Handle<BreakPoint> break_point);
 
+  // Remove a previously set breakpoint at the given byte position inside the
+  // given module. If this breakpoint is not found this function returns false.
+  V8_EXPORT_PRIVATE static bool ClearBreakPoint(Handle<WasmModuleObject>,
+                                                int position,
+                                                Handle<BreakPoint> break_point);
+
   // Check whether this module was generated from asm.js source.
   inline bool is_asm_js();
 
-  static void AddBreakpoint(Handle<WasmModuleObject>, int position,
-                            Handle<BreakPoint> break_point);
-
   static void SetBreakpointsOnNewInstance(Handle<WasmModuleObject>,
                                           Handle<WasmInstanceObject>);
 
@@ -224,6 +227,14 @@ class WasmModuleObject : public JSObject {
                                                   int position);
 
   OBJECT_CONSTRUCTORS(WasmModuleObject, JSObject);
+
+ private:
+  // Helper functions that update the breakpoint info list.
+  static void AddBreakpointToInfo(Handle<WasmModuleObject>, int position,
+                                  Handle<BreakPoint> break_point);
+
+  static bool RemoveBreakpointFromInfo(Handle<WasmModuleObject>, int position,
+                                       Handle<BreakPoint> break_point);
 };
 
 // Representation of a WebAssembly.Table JavaScript-level object.
@@ -784,7 +795,7 @@ class WasmExportedFunctionData : public Struct {
   DECL_PRINTER(WasmExportedFunctionData)
   DECL_VERIFIER(WasmExportedFunctionData)
 
-// Layout description.
+  // Layout description.
   DEFINE_FIELD_OFFSET_CONSTANTS(
       HeapObject::kHeaderSize,
       TORQUE_GENERATED_WASM_EXPORTED_FUNCTION_DATA_FIELDS)
@@ -832,7 +843,7 @@ class WasmDebugInfo : public Struct {
   DECL_PRINTER(WasmDebugInfo)
   DECL_VERIFIER(WasmDebugInfo)
 
-// Layout description.
+  // Layout description.
   DEFINE_FIELD_OFFSET_CONSTANTS(HeapObject::kHeaderSize,
                                 TORQUE_GENERATED_WASM_DEBUG_INFO_FIELDS)
 
@@ -851,6 +862,11 @@ class WasmDebugInfo : public Struct {
   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>,

test/cctest/wasm/test-wasm-breakpoints.cc

@@ -63,8 +63,12 @@ class BreakHandler : public debug::DebugDelegate {
   struct BreakPoint {
     int position;
     Action action;
+    std::function<void(void)> pre_action;
     BreakPoint(int position, Action action)
-        : position(position), action(action) {}
+        : position(position), action(action), pre_action([]() {}) {}
+    BreakPoint(int position, Action action,
+               std::function<void(void)> pre_action)
+        : position(position), action(action), pre_action(pre_action) {}
   };
 
   explicit BreakHandler(Isolate* isolate,
@@ -96,6 +100,7 @@ class BreakHandler : public debug::DebugDelegate {
     auto summ = FrameSummary::GetTop(frame_it.frame()).AsWasmInterpreted();
     CHECK_EQ(expected_breaks_[count_].position, summ.byte_offset());
 
+    expected_breaks_[count_].pre_action();
     Action next_action = expected_breaks_[count_].action;
     switch (next_action) {
       case Continue:
@@ -112,8 +117,9 @@ class BreakHandler : public debug::DebugDelegate {
   }
 };
 
-void SetBreakpoint(WasmRunnerBase* runner, int function_index, int byte_offset,
-                   int expected_set_byte_offset = -1) {
+Handle<BreakPoint> SetBreakpoint(WasmRunnerBase* runner, int function_index,
+                                 int byte_offset,
+                                 int expected_set_byte_offset = -1) {
   int func_offset =
       runner->builder().GetFunctionAt(function_index)->code.offset();
   int code_offset = func_offset + byte_offset;
@@ -134,6 +140,25 @@ void SetBreakpoint(WasmRunnerBase* runner, int function_index, int byte_offset,
   Handle<WasmDebugInfo> debug_info =
       WasmInstanceObject::GetOrCreateDebugInfo(instance);
   WasmDebugInfo::SetBreakpoint(debug_info, function_index, set_byte_offset);
+
+  return break_point;
+}
+
+void ClearBreakpoint(WasmRunnerBase* runner, int function_index,
+                     int byte_offset, Handle<BreakPoint> break_point) {
+  int func_offset =
+      runner->builder().GetFunctionAt(function_index)->code.offset();
+  int code_offset = func_offset + byte_offset;
+  Handle<WasmInstanceObject> instance = runner->builder().instance_object();
+  Handle<WasmModuleObject> module_object(instance->module_object(),
+                                         runner->main_isolate());
+  CHECK(WasmModuleObject::ClearBreakPoint(module_object, code_offset,
+                                          break_point));
+  // Also clear breakpoint on the debug info of the instance directly, since the
+  // instance chain is not setup properly in tests.
+  Handle<WasmDebugInfo> debug_info =
+      WasmInstanceObject::GetOrCreateDebugInfo(instance);
+  WasmDebugInfo::ClearBreakpoint(debug_info, function_index, byte_offset);
 }
 
 // Wrapper with operator<<.
@@ -400,6 +425,104 @@ WASM_COMPILED_EXEC_TEST(WasmGetLocalsAndStack) {
   CHECK(!Execution::Call(isolate, main_fun_wrapper, global, 1, args).is_null());
 }
 
+WASM_COMPILED_EXEC_TEST(WasmRemoveBreakPoint) {
+  WasmRunner<int> runner(execution_tier);
+  Isolate* isolate = runner.main_isolate();
+
+  BUILD(runner, WASM_NOP, WASM_NOP, WASM_NOP, WASM_NOP, WASM_NOP,
+        WASM_I32V_1(14));
+
+  Handle<JSFunction> main_fun_wrapper =
+      runner.builder().WrapCode(runner.function_index());
+
+  SetBreakpoint(&runner, runner.function_index(), 1, 1);
+  SetBreakpoint(&runner, runner.function_index(), 2, 2);
+  Handle<BreakPoint> to_delete =
+      SetBreakpoint(&runner, runner.function_index(), 3, 3);
+  SetBreakpoint(&runner, runner.function_index(), 4, 4);
+
+  BreakHandler count_breaks(isolate, {{1, BreakHandler::Continue},
+                                      {2, BreakHandler::Continue,
+                                       [&runner, &to_delete]() {
+                                         ClearBreakpoint(
+                                             &runner, runner.function_index(),
+                                             3, to_delete);
+                                       }},
+                                      {4, BreakHandler::Continue}});
+
+  Handle<Object> global(isolate->context().global_object(), isolate);
+  MaybeHandle<Object> retval =
+      Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
+  CHECK(!retval.is_null());
+  int result;
+  CHECK(retval.ToHandleChecked()->ToInt32(&result));
+  CHECK_EQ(14, result);
+}
+
+WASM_COMPILED_EXEC_TEST(WasmRemoveLastBreakPoint) {
+  WasmRunner<int> runner(execution_tier);
+  Isolate* isolate = runner.main_isolate();
+
+  BUILD(runner, WASM_NOP, WASM_NOP, WASM_NOP, WASM_NOP, WASM_NOP,
+        WASM_I32V_1(14));
+
+  Handle<JSFunction> main_fun_wrapper =
+      runner.builder().WrapCode(runner.function_index());
+
+  SetBreakpoint(&runner, runner.function_index(), 1, 1);
+  SetBreakpoint(&runner, runner.function_index(), 2, 2);
+  Handle<BreakPoint> to_delete =
+      SetBreakpoint(&runner, runner.function_index(), 3, 3);
+
+  BreakHandler count_breaks(
+      isolate, {{1, BreakHandler::Continue},
+                {2, BreakHandler::Continue, [&runner, &to_delete]() {
+                   ClearBreakpoint(&runner, runner.function_index(), 3,
+                                   to_delete);
+                 }}});
+
+  Handle<Object> global(isolate->context().global_object(), isolate);
+  MaybeHandle<Object> retval =
+      Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
+  CHECK(!retval.is_null());
+  int result;
+  CHECK(retval.ToHandleChecked()->ToInt32(&result));
+  CHECK_EQ(14, result);
+}
+
+WASM_COMPILED_EXEC_TEST(WasmRemoveAllBreakPoint) {
+  WasmRunner<int> runner(execution_tier);
+  Isolate* isolate = runner.main_isolate();
+
+  BUILD(runner, WASM_NOP, WASM_NOP, WASM_NOP, WASM_NOP, WASM_NOP,
+        WASM_I32V_1(14));
+
+  Handle<JSFunction> main_fun_wrapper =
+      runner.builder().WrapCode(runner.function_index());
+
+  Handle<BreakPoint> bp1 =
+      SetBreakpoint(&runner, runner.function_index(), 1, 1);
+  Handle<BreakPoint> bp2 =
+      SetBreakpoint(&runner, runner.function_index(), 2, 2);
+  Handle<BreakPoint> bp3 =
+      SetBreakpoint(&runner, runner.function_index(), 3, 3);
+
+  BreakHandler count_breaks(
+      isolate, {{1, BreakHandler::Continue, [&runner, &bp1, &bp2, &bp3]() {
+                   ClearBreakpoint(&runner, runner.function_index(), 1, bp1);
+                   ClearBreakpoint(&runner, runner.function_index(), 3, bp3);
+                   ClearBreakpoint(&runner, runner.function_index(), 2, bp2);
+                 }}});
+
+  Handle<Object> global(isolate->context().global_object(), isolate);
+  MaybeHandle<Object> retval =
+      Execution::Call(isolate, main_fun_wrapper, global, 0, nullptr);
+  CHECK(!retval.is_null());
+  int result;
+  CHECK(retval.ToHandleChecked()->ToInt32(&result));
+  CHECK_EQ(14, result);
+}
+
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8