[wasm][debug] Clear breakpoints when debugger is disabled

Currently WebAssembly breakpoint information survive disabling and
re-enabling the debugger. This is different from JavaScript, where
they are all removed. The frontend is expected to re-set the
breakpoint then.

Thus this CL remembers all wasm scripts where breakpoints have been set
in the Debug object, and clears them all when the debugger gets
disabled.

R=bmeurer@chromium.org

Bug: v8:10403
Change-Id: I5f8a8f3123727c954921920897ee7bf3b73f0ae8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2184969
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67639}

src/debug/debug.cc

@@ -622,6 +622,7 @@ bool Debug::SetBreakPointForScript(Handle<Script> script,
   Handle<BreakPoint> break_point =
       isolate_->factory()->NewBreakPoint(*id, condition);
   if (script->type() == Script::TYPE_WASM) {
+    RecordWasmScriptWithBreakpoints(script);
     return WasmScript::SetBreakPoint(script, source_position, break_point);
   }
 
@@ -777,12 +778,53 @@ void Debug::RemoveBreakpointForWasmScript(Handle<Script> script, int id) {
   }
 }
 
+void Debug::RecordWasmScriptWithBreakpoints(Handle<Script> script) {
+  if (wasm_scripts_with_breakpoints_.is_null()) {
+    Handle<WeakArrayList> new_list = isolate_->factory()->NewWeakArrayList(4);
+    wasm_scripts_with_breakpoints_ =
+        isolate_->global_handles()->Create(*new_list);
+  }
+  {
+    DisallowHeapAllocation no_gc;
+    for (int idx = wasm_scripts_with_breakpoints_->length() - 1; idx >= 0;
+         --idx) {
+      HeapObject wasm_script;
+      if (wasm_scripts_with_breakpoints_->Get(idx).GetHeapObject(
+              &wasm_script) &&
+          wasm_script == *script) {
+        return;
+      }
+    }
+  }
+  Handle<WeakArrayList> new_list = WeakArrayList::Append(
+      isolate_, wasm_scripts_with_breakpoints_, MaybeObjectHandle{script});
+  if (*new_list != *wasm_scripts_with_breakpoints_) {
+    isolate_->global_handles()->Destroy(
+        wasm_scripts_with_breakpoints_.location());
+    wasm_scripts_with_breakpoints_ =
+        isolate_->global_handles()->Create(*new_list);
+  }
+}
+
 // Clear out all the debug break code.
 void Debug::ClearAllBreakPoints() {
   ClearAllDebugInfos([=](Handle<DebugInfo> info) {
     ClearBreakPoints(info);
     info->ClearBreakInfo(isolate_);
   });
+  // Clear all wasm breakpoints.
+  if (!wasm_scripts_with_breakpoints_.is_null()) {
+    DisallowHeapAllocation no_gc;
+    for (int idx = wasm_scripts_with_breakpoints_->length() - 1; idx >= 0;
+         --idx) {
+      HeapObject raw_wasm_script;
+      if (wasm_scripts_with_breakpoints_->Get(idx).GetHeapObject(
+              &raw_wasm_script)) {
+        WasmScript::ClearAllBreakpoints(Script::cast(raw_wasm_script));
+      }
+    }
+    wasm_scripts_with_breakpoints_ = Handle<WeakArrayList>{};
+  }
 }
 
 void Debug::FloodWithOneShot(Handle<SharedFunctionInfo> shared,

src/debug/debug.h

@@ -248,6 +248,8 @@ class V8_EXPORT_PRIVATE Debug {
   void RemoveBreakpoint(int id);
   void RemoveBreakpointForWasmScript(Handle<Script> script, int id);
 
+  void RecordWasmScriptWithBreakpoints(Handle<Script> script);
+
   // Find breakpoints from the debug info and the break location and check
   // whether they are hit. Return an empty handle if not, or a FixedArray with
   // hit BreakPoint objects.
@@ -546,6 +548,9 @@ class V8_EXPORT_PRIVATE Debug {
   // Storage location for registers when handling debug break calls
   ThreadLocal thread_local_;
 
+  // This is a global handle, lazily initialized.
+  Handle<WeakArrayList> wasm_scripts_with_breakpoints_;
+
   Isolate* isolate_;
 
   friend class Isolate;

src/wasm/wasm-debug.cc

@@ -1117,6 +1117,12 @@ bool WasmScript::ClearBreakPointById(Handle<Script> script, int breakpoint_id) {
   return false;
 }
 
+// static
+void WasmScript::ClearAllBreakpoints(Script script) {
+  script.set_wasm_breakpoint_infos(
+      ReadOnlyRoots(script.GetIsolate()).empty_fixed_array());
+}
+
 // static
 void WasmScript::AddBreakpointToInfo(Handle<Script> script, int position,
                                      Handle<BreakPoint> break_point) {

src/wasm/wasm-objects.h

@@ -882,6 +882,9 @@ class WasmScript : public AllStatic {
   V8_EXPORT_PRIVATE static bool ClearBreakPointById(Handle<Script>,
                                                     int breakpoint_id);
 
+  // Remove all set breakpoints.
+  static void ClearAllBreakpoints(Script);
+
   // 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,

test/inspector/debugger/wasm-breakpoint-reset-on-debugger-restart-expected.txt

@@ -0,0 +1,11 @@
+Test that breakpoints do not survive a restart of the debugger.
+Instantiating.
+Waiting for wasm script (ignoring first non-wasm script).
+Setting breakpoint.
+Calling func.
+Script wasm://wasm/8c388106 byte offset 33: Wasm opcode 0x01
+func returned.
+Restarting debugger.
+Calling func.
+func returned.
+Finished.

test/inspector/debugger/wasm-breakpoint-reset-on-debugger-restart.js

@@ -0,0 +1,63 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const {session, contextGroup, Protocol} = InspectorTest.start(
+    'Test that breakpoints do not survive a restart of the debugger.');
+session.setupScriptMap();
+
+utils.load('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+
+const func =
+    builder.addFunction('func', kSig_v_v).addBody([kExprNop]).exportFunc();
+
+const module_bytes = JSON.stringify(builder.toArray());
+
+function instantiate(bytes) {
+  let buffer = new ArrayBuffer(bytes.length);
+  let view = new Uint8Array(buffer);
+  for (let i = 0; i < bytes.length; ++i) {
+    view[i] = bytes[i] | 0;
+  }
+
+  let module = new WebAssembly.Module(buffer);
+  return new WebAssembly.Instance(module);
+}
+contextGroup.addScript(instantiate.toString());
+
+Protocol.Debugger.onPaused(async msg => {
+  await session.logSourceLocation(msg.params.callFrames[0].location);
+  Protocol.Debugger.resume();
+});
+
+(async function test() {
+  await Protocol.Debugger.enable();
+  InspectorTest.log('Instantiating.');
+  // Spawn asynchronously:
+  Protocol.Runtime.evaluate(
+      {'expression': `const instance = instantiate(${module_bytes});`});
+  InspectorTest.log(
+      'Waiting for wasm script (ignoring first non-wasm script).');
+  const [, {params: wasm_script}] = await Protocol.Debugger.onceScriptParsed(2);
+  InspectorTest.log('Setting breakpoint.');
+  await Protocol.Debugger.setBreakpoint({
+    'location': {
+      'scriptId': wasm_script.scriptId,
+      'lineNumber': 0,
+      'columnNumber': func.body_offset
+    }
+  });
+  for (let run of [0, 1]) {
+    InspectorTest.log('Calling func.');
+    await Protocol.Runtime.evaluate({'expression': 'instance.exports.func()'});
+    InspectorTest.log('func returned.');
+    if (run == 1) continue;
+    InspectorTest.log('Restarting debugger.');
+    await Protocol.Debugger.disable();
+    await Protocol.Debugger.enable();
+  }
+  InspectorTest.log('Finished.');
+  InspectorTest.completeTest();
+})();