[wasm][debug] Remove --debug-in-liftoff flag

The flag is on by default, and tests already rely on the new behaviour.
This CL removes the flag and immediately affected code. More code will
be removed component by component in follow-up CLs.

Drive-by: Inline {RemoveBreakpointFromInfo} into {ClearBreakPoint},
which only redirected to that method after this CL.

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

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

src/debug/debug.cc

@@ -872,20 +872,6 @@ void Debug::PrepareStepIn(Handle<JSFunction> function) {
   if (in_debug_scope()) return;
   if (break_disabled()) return;
   Handle<SharedFunctionInfo> shared(function->shared(), isolate_);
-  // If stepping from JS into Wasm, and we are using the wasm interpreter for
-  // debugging, prepare the interpreter for step in.
-  if (shared->HasWasmExportedFunctionData() && !FLAG_debug_in_liftoff) {
-    auto imported_function = Handle<WasmExportedFunction>::cast(function);
-    Handle<WasmInstanceObject> wasm_instance(imported_function->instance(),
-                                             isolate_);
-    Handle<WasmDebugInfo> wasm_debug_info =
-        WasmInstanceObject::GetOrCreateDebugInfo(wasm_instance);
-    int func_index = shared->wasm_exported_function_data().function_index();
-    WasmDebugInfo::PrepareStepIn(wasm_debug_info, func_index);
-    // We need to reset all of this since break would be
-    // handled in Wasm Interpreter now. Otherwise it would be a loop here.
-    ClearStepping();
-  }
   if (IsBlackboxed(shared)) return;
   if (*function == thread_local_.ignore_step_into_function_) return;
   thread_local_.ignore_step_into_function_ = Smi::zero();
@@ -1048,7 +1034,7 @@ void Debug::PrepareStep(StepAction step_action) {
       wasm_frame->debug_info().PrepareStep(step_action);
       return;
     }
-  } else if (FLAG_debug_in_liftoff && frame->is_wasm_compiled()) {
+  } else if (frame->is_wasm_compiled()) {
     // Handle stepping in Liftoff code.
     WasmCompiledFrame* wasm_frame = WasmCompiledFrame::cast(frame);
     wasm::WasmCodeRefScope code_ref_scope;

src/flags/flag-definitions.h

@@ -721,10 +721,6 @@ DEFINE_BOOL(trace_wasm_memory, false,
 DEFINE_INT(wasm_tier_mask_for_testing, 0,
            "bitmask of functions to compile with TurboFan instead of Liftoff")
 
-DEFINE_BOOL(
-    debug_in_liftoff, true,
-    "use Liftoff instead of the C++ interpreter for debugging WebAssembly")
-
 DEFINE_BOOL(validate_asm, true, "validate asm.js modules before compiling")
 DEFINE_BOOL(suppress_asm_messages, false,
             "don't emit asm.js related messages (for golden file testing)")

src/wasm/wasm-debug.cc

@@ -1318,63 +1318,71 @@ bool WasmScript::SetBreakPointForFunction(Handle<Script> script, int func_index,
   WasmScript::AddBreakpointToInfo(script, func.code.offset() + offset,
                                   break_point);
 
-  if (FLAG_debug_in_liftoff) {
-    native_module->GetDebugInfo()->SetBreakpoint(func_index, offset, isolate);
-  } else {
-    // Iterate over all instances and tell them to set this new breakpoint.
-    // We do this using the weak list of all instances from the script.
-    Handle<WeakArrayList> weak_instance_list(script->wasm_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::SetBreakpoint(debug_info, func_index, offset);
-      }
+  native_module->GetDebugInfo()->SetBreakpoint(func_index, offset, isolate);
+
+  return true;
+}
+
+namespace {
+
+int GetBreakpointPos(Isolate* isolate, Object break_point_info_or_undef) {
+  if (break_point_info_or_undef.IsUndefined(isolate)) return kMaxInt;
+  return BreakPointInfo::cast(break_point_info_or_undef).source_position();
+}
+
+int FindBreakpointInfoInsertPos(Isolate* isolate,
+                                Handle<FixedArray> breakpoint_infos,
+                                int position) {
+  // Find insert location via binary search, taking care of undefined values on
+  // the right. Position is always greater than zero.
+  DCHECK_LT(0, position);
+
+  int left = 0;                            // inclusive
+  int right = breakpoint_infos->length();  // exclusive
+  while (right - left > 1) {
+    int mid = left + (right - left) / 2;
+    Object mid_obj = breakpoint_infos->get(mid);
+    if (GetBreakpointPos(isolate, mid_obj) <= position) {
+      left = mid;
+    } else {
+      right = mid;
     }
   }
 
-  return true;
+  int left_pos = GetBreakpointPos(isolate, breakpoint_infos->get(left));
+  return left_pos < position ? left + 1 : left;
 }
 
+}  // namespace
+
 // static
 bool WasmScript::ClearBreakPoint(Handle<Script> script, int position,
                                  Handle<BreakPoint> break_point) {
+  if (!script->has_wasm_breakpoint_infos()) return false;
+
   Isolate* isolate = script->GetIsolate();
+  Handle<FixedArray> breakpoint_infos(script->wasm_breakpoint_infos(), isolate);
 
-  // Find the function for this breakpoint.
-  const wasm::WasmModule* module = script->wasm_native_module()->module();
-  int func_index = GetContainingWasmFunction(module, position);
-  if (func_index < 0) return false;
-  const wasm::WasmFunction& func = module->functions[func_index];
-  int offset_in_func = position - func.code.offset();
+  int pos = FindBreakpointInfoInsertPos(isolate, breakpoint_infos, position);
 
-  if (!WasmScript::RemoveBreakpointFromInfo(script, position, break_point)) {
-    return false;
-  }
+  // Does a BreakPointInfo object already exist for this position?
+  if (pos == breakpoint_infos->length()) return false;
 
-  if (!FLAG_debug_in_liftoff) {
-    // Iterate over all instances and tell them to remove this breakpoint.
-    // We do this using the weak list of all instances from the script.
-    Handle<WeakArrayList> weak_instance_list(script->wasm_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);
-      }
+  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;
 }
 
@@ -1406,38 +1414,6 @@ bool WasmScript::ClearBreakPointById(Handle<Script> script, int breakpoint_id) {
   return false;
 }
 
-namespace {
-
-int GetBreakpointPos(Isolate* isolate, Object break_point_info_or_undef) {
-  if (break_point_info_or_undef.IsUndefined(isolate)) return kMaxInt;
-  return BreakPointInfo::cast(break_point_info_or_undef).source_position();
-}
-
-int FindBreakpointInfoInsertPos(Isolate* isolate,
-                                Handle<FixedArray> breakpoint_infos,
-                                int position) {
-  // Find insert location via binary search, taking care of undefined values on
-  // the right. Position is always greater than zero.
-  DCHECK_LT(0, position);
-
-  int left = 0;                            // inclusive
-  int right = breakpoint_infos->length();  // exclusive
-  while (right - left > 1) {
-    int mid = left + (right - left) / 2;
-    Object mid_obj = breakpoint_infos->get(mid);
-    if (GetBreakpointPos(isolate, mid_obj) <= position) {
-      left = mid;
-    } else {
-      right = mid;
-    }
-  }
-
-  int left_pos = GetBreakpointPos(isolate, breakpoint_infos->get(left));
-  return left_pos < position ? left + 1 : left;
-}
-
-}  // namespace
-
 // static
 void WasmScript::AddBreakpointToInfo(Handle<Script> script, int position,
                                      Handle<BreakPoint> break_point) {
@@ -1494,37 +1470,6 @@ void WasmScript::AddBreakpointToInfo(Handle<Script> script, int position,
   new_breakpoint_infos->set(insert_pos, *breakpoint_info);
 }
 
-// static
-bool WasmScript::RemoveBreakpointFromInfo(Handle<Script> script, int position,
-                                          Handle<BreakPoint> break_point) {
-  if (!script->has_wasm_breakpoint_infos()) return false;
-
-  Isolate* isolate = script->GetIsolate();
-  Handle<FixedArray> breakpoint_infos(script->wasm_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 WasmScript::SetBreakpointsOnNewInstance(
     Handle<Script> script, Handle<WasmInstanceObject> instance) {
   if (!script->has_wasm_breakpoint_infos()) return;

src/wasm/wasm-objects.h

@@ -953,9 +953,6 @@ class WasmScript : public AllStatic {
   // Helper functions that update the breakpoint info list.
   static void AddBreakpointToInfo(Handle<Script>, int position,
                                   Handle<BreakPoint> break_point);
-
-  static bool RemoveBreakpointFromInfo(Handle<Script>, int position,
-                                       Handle<BreakPoint> break_point);
 };
 
 // Tags provide an object identity for each exception defined in a wasm module