[vm/concurrency/debugger] Fix pool patching synchronization for lightweight isolates breakpoints.

TEST=service/break_on_function_many_child_isolates_test

Issue https://github.com/dart-lang/sdk/issues/36097

Change-Id: I15b9d9db938042c89f21700876bf75d932f044a6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/191781
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>

runtime/observatory/tests/service/break_on_function_child_isolate_test.dart

@@ -2,83 +2,9 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 // VMOptions=--verbose_debug --enable-isolate-groups --experimental-enable-isolate-groups-jit
-import 'dart:async';
-import 'dart:isolate' as dart_isolate;
 
-import 'package:observatory/service_io.dart';
-import 'package:test/test.dart';
-import 'service_test_common.dart';
-import 'test_helper.dart';
-import 'dart:developer';
-
-const int LINE_A = 18;
-const int LINE_B = 25;
-const int LINE_C = 29;
-
-foo(args) { // LINE_A
-  final dart_isolate.SendPort sendPort = args[0] as dart_isolate.SendPort;
-  sendPort.send('reply from foo');
-}
-
-testMain() async {
-  final rpResponse = dart_isolate.ReceivePort();
-  debugger(); // LINE_B
-  await dart_isolate.Isolate.spawn(foo, [rpResponse.sendPort]);
-  await rpResponse.first;
-  rpResponse.close();
-  debugger(); // LINE_C
-}
-
-final completerAtFoo = Completer();
-
-final tests = <IsolateTest>[
-  hasPausedAtStart,
-  resumeIsolate,
-  hasStoppedAtBreakpoint,
-  stoppedAtLine(LINE_B + 1),
-  (Isolate isolate) async {
-    // Set up a listener to wait for child isolate launch and breakpoint events.
-    final stream = await isolate.vm.getEventStream(VM.kDebugStream);
-    var childIsolate;
-    var subscription;
-    subscription = stream.listen((ServiceEvent event) async {
-      switch (event.kind) {
-        case ServiceEvent.kPauseStart:
-          childIsolate = event.isolate!;
-          await childIsolate.reload();
-
-          Library rootLib = await childIsolate.rootLibrary.load() as Library;
-          final foo = rootLib.functions.singleWhere((f) => f.name == 'foo');
-          final bpt = await childIsolate.addBreakpointAtEntry(foo);
-
-          expect(bpt is Breakpoint, isTrue);
-          childIsolate.resume();
-          break;
-        case ServiceEvent.kPauseBreakpoint:
-          if (childIsolate == event.isolate) {
-            ServiceMap stack = await childIsolate.getStack();
-            Frame top = stack['frames'][0];
-            Script script = await top.location!.script.load() as Script;
-            expect(script.tokenToLine(top.location!.tokenPos), equals(LINE_A));
-
-            childIsolate.resume();
-            subscription.cancel();
-            completerAtFoo.complete();
-          }
-          break;
-      }
-    });
-  },
-  resumeIsolate,
-  (Isolate isolate) async {
-    await completerAtFoo.future;
-  },
-  hasStoppedAtBreakpoint,
-  stoppedAtLine(LINE_C + 1),
-  resumeIsolate,
-];
+import 'break_on_function_many_child_isolates_test.dart';
 
 main(args) async {
-  runIsolateTests(args, tests,
-      testeeConcurrent: testMain, pause_on_start: true);
+  await runIsolateBreakpointPauseTest(args, /*nIsolates=*/ 1);
 }

runtime/observatory/tests/service/break_on_function_many_child_isolates_test.dart

@@ -0,0 +1,113 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// VMOptions=--verbose_debug --enable-isolate-groups --experimental-enable-isolate-groups-jit
+//
+// Tests breakpoint pausing and resuming with many isolates running and pausing
+// simultaneously.
+
+import 'dart:async';
+import 'dart:developer';
+import 'dart:isolate' as dart_isolate;
+
+import 'package:observatory/service_io.dart';
+import 'package:test/test.dart';
+
+import 'service_test_common.dart';
+import 'test_helper.dart';
+
+const int LINE_A = 23;
+const int LINE_B = 35;
+const int LINE_C = 41;
+
+foo(args) { // LINE_A
+  print('${dart_isolate.Isolate.current.debugName}: $args');
+  final sendPort = args[0] as dart_isolate.SendPort;
+  final int i = args[1] as int;
+  sendPort.send('reply from foo: $i');
+}
+
+int nIsolates = -1;
+
+testMain() async {
+  final rps = List<dart_isolate.ReceivePort>.generate(
+      nIsolates, (i) => dart_isolate.ReceivePort());
+  debugger(); // LINE_B
+  for (int i = 0; i < nIsolates; i++) {
+    await dart_isolate.Isolate.spawn(foo, [rps[i].sendPort, i],
+        debugName: "foo$i");
+  }
+  print(await Future.wait(rps.map((rp) => rp.first)));
+  debugger(); // LINE_C
+}
+
+final completerAtFoo = List<Completer>.generate(nIsolates, (_) => Completer());
+int completerCount = 0;
+
+final tests = <IsolateTest>[
+  hasPausedAtStart,
+  resumeIsolate,
+  hasStoppedAtBreakpoint,
+  stoppedAtLine(LINE_B + 1),
+  (Isolate isolate) async {
+    // Set up a listener to wait for child isolate launch and breakpoint events.
+    final stream = await isolate.vm.getEventStream(VM.kDebugStream);
+    var subscription;
+    subscription = stream.listen((ServiceEvent event) async {
+      switch (event.kind) {
+        case ServiceEvent.kPauseStart:
+          final childIsolate = event.isolate!;
+          await childIsolate.reload();
+
+          for (Library lib in childIsolate.libraries) {
+            await lib.load();
+            if (lib.uri!
+                .endsWith('break_on_function_many_child_isolates_test.dart')) {
+              final foo = lib.functions.singleWhere((f) => f.name == 'foo');
+              final bpt = await childIsolate.addBreakpointAtEntry(foo);
+
+              expect(bpt is Breakpoint, isTrue);
+              break;
+            }
+          }
+          childIsolate.resume();
+          break;
+        case ServiceEvent.kPauseBreakpoint:
+          final name = event.isolate!.name!;
+          if (!name.startsWith('foo')) {
+            break;
+          }
+          final childIsolate = event.isolate;
+          final ndx = int.parse(name.substring('foo'.length));
+          final stack = await childIsolate!.getStack();
+          final top = stack['frames'][0];
+          final script = await top.location.script.load() as Script;
+          expect(script.tokenToLine(top.location.tokenPos), equals(LINE_A));
+
+          childIsolate.resume();
+          if ((++completerCount) == nIsolates) {
+            subscription.cancel();
+          }
+          completerAtFoo[ndx].complete();
+          break;
+      }
+    });
+  },
+  resumeIsolate,
+  (Isolate isolate) async {
+    await Future.wait(completerAtFoo.map((c) => c.future));
+  },
+  hasStoppedAtBreakpoint,
+  stoppedAtLine(LINE_C + 1),
+  resumeIsolate,
+];
+
+Future runIsolateBreakpointPauseTest(args, nIsolates_) {
+  nIsolates = nIsolates_;
+  return runIsolateTests(args, tests,
+      testeeConcurrent: testMain, pause_on_start: true);
+}
+
+main(args) async {
+  await runIsolateBreakpointPauseTest(args, /*nIsolates=*/ 30);
+}

runtime/observatory/tests/service/service_kernel.status

@@ -59,6 +59,7 @@ break_on_activation_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_async_function_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_default_constructor_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_function_child_isolate_test: SkipByDesign # Debugger is disabled in AOT mode.
+break_on_function_many_child_isolates_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_function_test: SkipByDesign # Debugger is disabled in AOT mode.
 breakpoint_async_break_test: SkipByDesign # Debugger is disabled in AOT mode.
 breakpoint_in_package_parts_class_file_uri_test: SkipByDesign # Debugger is disabled in AOT mode.

runtime/observatory_2/tests/service_2/break_on_function_child_isolate_test.dart

@@ -0,0 +1,10 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// VMOptions=--verbose_debug --enable-isolate-groups --experimental-enable-isolate-groups-jit
+
+import 'break_on_function_many_child_isolates_test.dart';
+
+main(args) async {
+  await runIsolateBreakpointPauseTest(args, /*nIsolates=*/ 1);
+}

runtime/observatory_2/tests/service_2/break_on_function_many_child_isolates_test.dart

@@ -0,0 +1,113 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// VMOptions=--verbose_debug --enable-isolate-groups --experimental-enable-isolate-groups-jit
+//
+// Tests breakpoint pausing and resuming with many isolates running and pausing
+// simultaneously.
+
+import 'dart:async';
+import 'dart:developer';
+import 'dart:isolate' as dart_isolate;
+
+import 'package:observatory_2/service_io.dart';
+import 'package:test/test.dart';
+
+import 'service_test_common.dart';
+import 'test_helper.dart';
+
+const int LINE_A = 23;
+const int LINE_B = 35;
+const int LINE_C = 41;
+
+foo(args) { // LINE_A
+  print('${dart_isolate.Isolate.current.debugName}: $args');
+  final sendPort = args[0] as dart_isolate.SendPort;
+  final int i = args[1] as int;
+  sendPort.send('reply from foo: $i');
+}
+
+int nIsolates = -1;
+
+testMain() async {
+  final rps = List<dart_isolate.ReceivePort>.generate(
+      nIsolates, (i) => dart_isolate.ReceivePort());
+  debugger(); // LINE_B
+  for (int i = 0; i < nIsolates; i++) {
+    await dart_isolate.Isolate.spawn(foo, [rps[i].sendPort, i],
+        debugName: "foo$i");
+  }
+  print(await Future.wait(rps.map((rp) => rp.first)));
+  debugger(); // LINE_C
+}
+
+final completerAtFoo = List<Completer>.generate(nIsolates, (_) => Completer());
+int completerCount = 0;
+
+final tests = <IsolateTest>[
+  hasPausedAtStart,
+  resumeIsolate,
+  hasStoppedAtBreakpoint,
+  stoppedAtLine(LINE_B + 1),
+  (Isolate isolate) async {
+    // Set up a listener to wait for child isolate launch and breakpoint events.
+    final stream = await isolate.vm.getEventStream(VM.kDebugStream);
+    var subscription;
+    subscription = stream.listen((ServiceEvent event) async {
+      switch (event.kind) {
+        case ServiceEvent.kPauseStart:
+          final childIsolate = event.isolate;
+          await childIsolate.reload();
+
+          for (Library lib in childIsolate.libraries) {
+            await lib.load();
+            if (lib.uri
+                .endsWith('break_on_function_many_child_isolates_test.dart')) {
+              final foo = lib.functions.singleWhere((f) => f.name == 'foo');
+              final bpt = await childIsolate.addBreakpointAtEntry(foo);
+
+              expect(bpt is Breakpoint, isTrue);
+              break;
+            }
+          }
+          childIsolate.resume();
+          break;
+        case ServiceEvent.kPauseBreakpoint:
+          final name = event.isolate.name;
+          if (!name.startsWith('foo')) {
+            break;
+          }
+          final childIsolate = event.isolate;
+          final ndx = int.parse(name.substring('foo'.length));
+          final stack = await childIsolate.getStack();
+          final top = stack['frames'][0];
+          final script = await top.location.script.load() as Script;
+          expect(script.tokenToLine(top.location.tokenPos), equals(LINE_A));
+
+          childIsolate.resume();
+          if ((++completerCount) == nIsolates) {
+            subscription.cancel();
+          }
+          completerAtFoo[ndx].complete();
+          break;
+      }
+    });
+  },
+  resumeIsolate,
+  (Isolate isolate) async {
+    await Future.wait(completerAtFoo.map((c) => c.future));
+  },
+  hasStoppedAtBreakpoint,
+  stoppedAtLine(LINE_C + 1),
+  resumeIsolate,
+];
+
+Future runIsolateBreakpointPauseTest(args, nIsolates_) {
+  nIsolates = nIsolates_;
+  return runIsolateTests(args, tests,
+      testeeConcurrent: testMain, pause_on_start: true);
+}
+
+main(args) async {
+  await runIsolateBreakpointPauseTest(args, /*nIsolates=*/ 30);
+}

runtime/observatory_2/tests/service_2/service_2_kernel.status

@@ -59,6 +59,7 @@ break_on_activation_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_async_function_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_default_constructor_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_function_child_isolate_test: SkipByDesign # Debugger is disabled in AOT mode.
+break_on_function_many_child_isolates_test: SkipByDesign # Debugger is disabled in AOT mode.
 break_on_function_test: SkipByDesign # Debugger is disabled in AOT mode.
 breakpoint_async_break_test: SkipByDesign # Debugger is disabled in AOT mode.
 breakpoint_in_package_parts_class_file_uri_test: SkipByDesign # Debugger is disabled in AOT mode.

runtime/vm/compiler/jit/compiler.cc

@@ -603,6 +603,21 @@ CodePtr CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
         auto install_code_fun = [&]() {
           *result =
               FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
+#if !defined(PRODUCT)
+          // Isolate debuggers need to be notified of compiled function right
+          // away as code is installed because there might be latent breakpoints
+          // in compiled function, which have to be activated before functions
+          // code is executed. Otherwise concurrently running isolates might
+          // execute code before its patched and miss a need to pause at a
+          // breakpoint.
+          if (!result->IsNull()) {
+            if (!function.HasOptimizedCode()) {
+              thread()->isolate_group()->ForEachIsolate([&](Isolate* isolate) {
+                isolate->debugger()->NotifyCompilation(function);
+              });
+            }
+          }
+#endif
         };
 
         // Grab write program_lock outside of potential safepoint, that lock
@@ -632,15 +647,6 @@ CodePtr CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
         // Must be called outside of safepoint.
         Code::NotifyCodeObservers(function, *result, optimized());
 
-#if !defined(PRODUCT)
-        if (!function.HasOptimizedCode()) {
-          // TODO(dartbug.com/36097): We might need to adjust this once we start
-          // adding debugging support to --enable-isolate-groups.
-          thread()->isolate_group()->ForEachIsolate([&](Isolate* isolate) {
-            isolate->debugger()->NotifyCompilation(function);
-          });
-        }
-#endif
         if (FLAG_disassemble && FlowGraphPrinter::ShouldPrint(function)) {
           Disassembler::DisassembleCode(function, *result, optimized());
         } else if (FLAG_disassemble_optimized && optimized() &&

runtime/vm/debugger.cc

@@ -1455,7 +1455,7 @@ CodeBreakpoint::CodeBreakpoint(const Code& code,
       token_pos_(token_pos),
       pc_(pc),
       line_number_(-1),
-      is_enabled_(false),
+      enabled_count_(0),
       next_(NULL),
       breakpoint_kind_(kind),
       saved_value_(Code::null()) {
@@ -1501,17 +1501,17 @@ intptr_t CodeBreakpoint::LineNumber() {
 }
 
 void CodeBreakpoint::Enable() {
-  if (!is_enabled_) {
+  if (enabled_count_ == 0) {
     PatchCode();
   }
-  ASSERT(is_enabled_);
+  ++enabled_count_;
 }
 
 void CodeBreakpoint::Disable() {
-  if (is_enabled_) {
+  if (enabled_count_ == 1) {
     RestoreCode();
   }
-  ASSERT(!is_enabled_);
+  --enabled_count_;
 }
 
 bool CodeBreakpoint::HasBreakpointLocation(
@@ -1599,13 +1599,13 @@ void Debugger::Shutdown() {
   }
   while (breakpoint_locations_ != nullptr) {
     BreakpointLocation* loc = breakpoint_locations_;
-    group_debugger()->DisableCodeBreakpointsFor(loc);
+    group_debugger()->UnlinkCodeBreakpoints(loc);
     breakpoint_locations_ = breakpoint_locations_->next();
     delete loc;
   }
   while (latent_locations_ != nullptr) {
     BreakpointLocation* loc = latent_locations_;
-    group_debugger()->DisableCodeBreakpointsFor(loc);
+    group_debugger()->UnlinkCodeBreakpoints(loc);
     latent_locations_ = latent_locations_->next();
     delete loc;
   }
@@ -2510,29 +2510,26 @@ void GroupDebugger::MakeCodeBreakpointAt(const Function& func,
   }
   if (lowest_pc_offset != kUwordMax) {
     uword lowest_pc = code.PayloadStart() + lowest_pc_offset;
+    SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock());
     CodeBreakpoint* code_bpt = GetCodeBreakpoint(lowest_pc);
     if (code_bpt == nullptr) {
-      SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock());
-      code_bpt = GetCodeBreakpoint(lowest_pc);
-      if (code_bpt == nullptr) {
-        // No code breakpoint for this code exists; create one.
-        code_bpt =
-            new CodeBreakpoint(code, loc->token_pos_, lowest_pc, lowest_kind);
-        if (FLAG_verbose_debug) {
-          OS::PrintErr("Setting code breakpoint at pos %s pc %#" Px
-                       " offset %#" Px "\n",
-                       loc->token_pos_.ToCString(), lowest_pc,
-                       lowest_pc - code.PayloadStart());
-        }
-        RegisterCodeBreakpoint(code_bpt);
-      } else {
-        if (FLAG_verbose_debug) {
-          OS::PrintErr(
-              "Adding location to existing code breakpoint at pos %s pc %#" Px
-              " offset %#" Px "\n",
-              loc->token_pos_.ToCString(), lowest_pc,
-              lowest_pc - code.PayloadStart());
-        }
+      // No code breakpoint for this code exists; create one.
+      code_bpt =
+          new CodeBreakpoint(code, loc->token_pos_, lowest_pc, lowest_kind);
+      if (FLAG_verbose_debug) {
+        OS::PrintErr("Setting code breakpoint at pos %s pc %#" Px
+                     " offset %#" Px "\n",
+                     loc->token_pos_.ToCString(), lowest_pc,
+                     lowest_pc - code.PayloadStart());
+      }
+      RegisterCodeBreakpoint(code_bpt);
+    } else {
+      if (FLAG_verbose_debug) {
+        OS::PrintErr(
+            "Adding location to existing code breakpoint at pos %s pc %#" Px
+            " offset %#" Px "\n",
+            loc->token_pos_.ToCString(), lowest_pc,
+            lowest_pc - code.PayloadStart());
       }
     }
     code_bpt->AddBreakpointLocation(loc);
@@ -2913,7 +2910,7 @@ BreakpointLocation* Debugger::SetBreakpoint(const Script& script,
 void GroupDebugger::SyncBreakpointLocation(BreakpointLocation* loc) {
   bool any_enabled = loc->AnyEnabled();
 
-  SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock());
+  SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock());
   CodeBreakpoint* cbpt = code_breakpoints_;
   while (cbpt != NULL) {
     if (cbpt->HasBreakpointLocation(loc)) {
@@ -2927,17 +2924,6 @@ void GroupDebugger::SyncBreakpointLocation(BreakpointLocation* loc) {
   }
 }
 
-void GroupDebugger::DisableCodeBreakpointsFor(BreakpointLocation* location) {
-  SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock());
-  CodeBreakpoint* cbpt = code_breakpoints_;
-  while (cbpt != nullptr) {
-    if (cbpt->HasBreakpointLocation(location)) {
-      cbpt->Disable();
-    }
-    cbpt = cbpt->next();
-  }
-}
-
 Breakpoint* Debugger::SetBreakpointAtEntry(const Function& target_function,
                                            bool single_shot) {
   ASSERT(!target_function.IsNull());
@@ -4174,6 +4160,7 @@ void Debugger::NotifyDoneLoading() {
 // TODO(hausner): Could potentially make this faster by checking
 // whether the call target at pc is a debugger stub.
 bool GroupDebugger::HasActiveBreakpoint(uword pc) {
+  SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock());
   CodeBreakpoint* cbpt = GetCodeBreakpoint(pc);
   return (cbpt != nullptr) && (cbpt->IsEnabled());
 }
@@ -4213,6 +4200,7 @@ void GroupDebugger::RegisterCodeBreakpoint(CodeBreakpoint* cbpt) {
 }
 
 CodePtr GroupDebugger::GetPatchedStubAddress(uword breakpoint_address) {
+  SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock());
   CodeBreakpoint* cbpt = GetCodeBreakpoint(breakpoint_address);
   if (cbpt != NULL) {
     return cbpt->OrigStubAddress();
@@ -4305,14 +4293,12 @@ bool Debugger::RemoveBreakpointFromTheList(intptr_t bp_id,
 // should be hit before it gets deleted.
 void GroupDebugger::UnlinkCodeBreakpoints(BreakpointLocation* bpt_location) {
   ASSERT(bpt_location != nullptr);
-  SafepointReadRwLocker sl(Thread::Current(), code_breakpoints_lock());
+  SafepointWriteRwLocker sl(Thread::Current(), code_breakpoints_lock());
   CodeBreakpoint* curr_bpt = code_breakpoints_;
   while (curr_bpt != nullptr) {
     if (curr_bpt->FindAndDeleteBreakpointLocation(bpt_location)) {
-      if (curr_bpt->HasNoBreakpointLocations()) {
-        curr_bpt->Disable();
-        needs_breakpoint_cleanup_ = true;
-      }
+      curr_bpt->Disable();
+      needs_breakpoint_cleanup_ = true;
     }
     curr_bpt = curr_bpt->next();
   }

runtime/vm/debugger.h

@@ -203,6 +203,12 @@ class BreakpointLocation {
 // There may be more than one BreakpointLocation associated with CodeBreakpoint,
 // one for for every isolate in a group that sets a breakpoint at particular
 // code location represented by the CodeBreakpoint.
+// Each BreakpointLocation might be enabled/disabled based on whether it has
+// any actual breakpoints associated with it.
+// The CodeBreakpoint is enabled if it has any such BreakpointLocations
+// associated with it.
+// The class is not thread-safe - users of this class need to ensure the access
+// is synchronized, guarded by mutexes.
 class CodeBreakpoint {
  public:
   CodeBreakpoint(const Code& code,
@@ -226,7 +232,7 @@ class CodeBreakpoint {
 
   void Enable();
   void Disable();
-  bool IsEnabled() const { return is_enabled_; }
+  bool IsEnabled() const { return enabled_count_ > 0; }
 
   CodePtr OrigStubAddress() const;
 
@@ -248,7 +254,7 @@ class CodeBreakpoint {
   TokenPosition token_pos_;
   uword pc_;
   intptr_t line_number_;
-  bool is_enabled_;
+  int enabled_count_;  // incremented for every enabled breakpoint location
 
   // Breakpoint locations from different debuggers/isolates that
   // point to this code breakpoint.
@@ -521,7 +527,6 @@ class GroupDebugger {
   bool HasBreakpointInCode(const Code& code);
 
   void SyncBreakpointLocation(BreakpointLocation* loc);
-  void DisableCodeBreakpointsFor(BreakpointLocation* loc);
 
   void Pause();
 

runtime/vm/debugger_arm.cc

@@ -20,7 +20,7 @@ CodePtr CodeBreakpoint::OrigStubAddress() const {
 }
 
 void CodeBreakpoint::PatchCode() {
-  ASSERT(!is_enabled_);
+  ASSERT(!IsEnabled());
   Code& stub_target = Code::Handle();
   switch (breakpoint_kind_) {
     case UntaggedPcDescriptors::kIcCall:
@@ -38,11 +38,10 @@ void CodeBreakpoint::PatchCode() {
   const Code& code = Code::Handle(code_);
   saved_value_ = CodePatcher::GetStaticCallTargetAt(pc_, code);
   CodePatcher::PatchStaticCallAt(pc_, code, stub_target);
-  is_enabled_ = true;
 }
 
 void CodeBreakpoint::RestoreCode() {
-  ASSERT(is_enabled_);
+  ASSERT(IsEnabled());
   const Code& code = Code::Handle(code_);
   switch (breakpoint_kind_) {
     case UntaggedPcDescriptors::kIcCall:
@@ -54,7 +53,6 @@ void CodeBreakpoint::RestoreCode() {
     default:
       UNREACHABLE();
   }
-  is_enabled_ = false;
 }
 
 #endif  // !PRODUCT

runtime/vm/debugger_arm64.cc

@@ -20,7 +20,7 @@ CodePtr CodeBreakpoint::OrigStubAddress() const {
 }
 
 void CodeBreakpoint::PatchCode() {
-  ASSERT(!is_enabled_);
+  ASSERT(!IsEnabled());
   const Code& code = Code::Handle(code_);
   switch (breakpoint_kind_) {
     case UntaggedPcDescriptors::kIcCall: {
@@ -45,11 +45,10 @@ void CodeBreakpoint::PatchCode() {
     default:
       UNREACHABLE();
   }
-  is_enabled_ = true;
 }
 
 void CodeBreakpoint::RestoreCode() {
-  ASSERT(is_enabled_);
+  ASSERT(IsEnabled());
   const Code& code = Code::Handle(code_);
   switch (breakpoint_kind_) {
     case UntaggedPcDescriptors::kIcCall: {
@@ -68,7 +67,6 @@ void CodeBreakpoint::RestoreCode() {
     default:
       UNREACHABLE();
   }
-  is_enabled_ = false;
 }
 
 #endif  // !PRODUCT

runtime/vm/debugger_ia32.cc

@@ -24,7 +24,7 @@ CodePtr CodeBreakpoint::OrigStubAddress() const {
 }
 
 void CodeBreakpoint::PatchCode() {
-  ASSERT(!is_enabled_);
+  ASSERT(!IsEnabled());
   auto thread = Thread::Current();
   auto zone = thread->zone();
   const Code& code = Code::Handle(zone, code_);
@@ -52,11 +52,10 @@ void CodeBreakpoint::PatchCode() {
     saved_value_ = CodePatcher::GetStaticCallTargetAt(pc_, code);
     CodePatcher::PatchStaticCallAt(pc_, code, stub_target);
   });
-  is_enabled_ = true;
 }
 
 void CodeBreakpoint::RestoreCode() {
-  ASSERT(is_enabled_);
+  ASSERT(IsEnabled());
   auto thread = Thread::Current();
   auto zone = thread->zone();
   const Code& code = Code::Handle(zone, code_);
@@ -74,7 +73,6 @@ void CodeBreakpoint::RestoreCode() {
         UNREACHABLE();
     }
   });
-  is_enabled_ = false;
 }
 
 #endif  // !PRODUCT

runtime/vm/debugger_x64.cc

@@ -21,7 +21,7 @@ CodePtr CodeBreakpoint::OrigStubAddress() const {
 }
 
 void CodeBreakpoint::PatchCode() {
-  ASSERT(!is_enabled_);
+  ASSERT(!IsEnabled());
   Code& stub_target = Code::Handle();
   switch (breakpoint_kind_) {
     case UntaggedPcDescriptors::kIcCall:
@@ -39,11 +39,10 @@ void CodeBreakpoint::PatchCode() {
   const Code& code = Code::Handle(code_);
   saved_value_ = CodePatcher::GetStaticCallTargetAt(pc_, code);
   CodePatcher::PatchPoolPointerCallAt(pc_, code, stub_target);
-  is_enabled_ = true;
 }
 
 void CodeBreakpoint::RestoreCode() {
-  ASSERT(is_enabled_);
+  ASSERT(IsEnabled());
   const Code& code = Code::Handle(code_);
   switch (breakpoint_kind_) {
     case UntaggedPcDescriptors::kIcCall:
@@ -56,7 +55,6 @@ void CodeBreakpoint::RestoreCode() {
     default:
       UNREACHABLE();
   }
-  is_enabled_ = false;
 }
 
 #endif  // !PRODUCT