Better breakpoint management

runtime/vm/debugger.cc

 // Copyright (c) 2012, 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.
 
 #include "vm/debugger.h"
 
 #include "vm/code_generator.h"
 #include "vm/code_patcher.h"
 #include "vm/compiler.h"
 #include "vm/dart_entry.h"
@@ skipping 852 matching lines @@
   if (lowest_pc_index >= 0) {
     // We found the the pc descriptor within the given token range that
     // has the lowest execution address. This is the first possible
     // breakpoint on the line. We use this instead of the nearest
     // PC descriptor measured in token index distance.
     best_fit_index = lowest_pc_index;
   }
   if (best_fit_index >= 0) {
     CodeBreakpoint* bpt = GetCodeBreakpoint(desc.PC(best_fit_index));
     // We should only ever have one code breakpoint at the same address.
-    // If we find an existing breakpoint, it must be an internal one which
-    // is used for stepping, or one that was left over from previously
-    // deleting a source breakpoint. Make sure it's enabled.
     if (bpt != NULL) {

[siva, 2012/07/30 22:17:15]

Why is it not necessary to ensure that the breakpoint is enabled in this case?

[hausner, 2012/07/30 22:36:57]

Because the caller will make sure it gets enabled if necessary.

-      ASSERT(bpt->src_bpt() == NULL);
-      bpt->Enable();
       return bpt;
     }
 
     bpt = new CodeBreakpoint(func, best_fit_index);
     if (verbose) {
       OS::Print("Setting breakpoint in function '%s' (%s:%d) (PC %p)\n",
                 String::Handle(func.name()).ToCString(),
                 String::Handle(bpt->SourceUrl()).ToCString(),
                 bpt->LineNumber(),
                 bpt->pc());
     }
     RegisterCodeBreakpoint(bpt);
     return bpt;
   }
   return NULL;
 }
 
 
 SourceBreakpoint* Debugger::SetBreakpoint(const Function& target_function,
                                           intptr_t first_token_pos,
                                           intptr_t last_token_pos) {
   if ((last_token_pos < target_function.token_pos()) ||
       (target_function.end_token_pos() < first_token_pos)) {
     // The given token position is not within the target function.
     return NULL;
   }
   EnsureFunctionIsDeoptimized(target_function);
-  SourceBreakpoint* bpt = GetSourceBreakpoint(target_function, first_token_pos);
-  if (bpt != NULL) {
-    // A breakpoint for this location already exists, return it.
-    return bpt;
+
+  CodeBreakpoint* cbpt = NULL;
+  SourceBreakpoint* bpt = NULL;
+  if (target_function.HasCode()) {
+    cbpt = MakeCodeBreakpoint(target_function, first_token_pos, last_token_pos);
+    if (cbpt != NULL) {
+      if (cbpt->src_bpt() != NULL) {
+        // There is already a source breakpoint for the location.
+        ASSERT(cbpt->src_bpt() ==
+               GetSourceBreakpoint(target_function, cbpt->token_pos()));
+        return cbpt->src_bpt();
+      }
+      // No source breakpoint exists yet that is associated with the code
+      // breakpoint we found. (This is an internal breakpoint.) Adjust
+      // the breakpoint location to the actual position where breakpoint
+      // got set.
+      first_token_pos = cbpt->token_pos();
+    }
+  } else {
+    bpt = GetSourceBreakpoint(target_function, first_token_pos);
+    if (bpt != NULL) {
+      // A source breakpoint for this uncompiled location already
+      // exists.
+      return bpt;
+    }
   }
   bpt = new SourceBreakpoint(nextId(), target_function, first_token_pos);
   RegisterSourceBreakpoint(bpt);
   if (verbose && !target_function.HasCode()) {
     OS::Print("Registering breakpoint for "
               "uncompiled function '%s' at line %d\n",
               target_function.ToFullyQualifiedCString(),
               bpt->LineNumber());
   }
 
-  if (target_function.HasCode()) {
-    CodeBreakpoint* cbpt =
-        MakeCodeBreakpoint(target_function, first_token_pos, last_token_pos);
-    if (cbpt != NULL) {
-      ASSERT(cbpt->src_bpt() == NULL);
-      cbpt->set_src_bpt(bpt);
-      SignalBpResolved(bpt);
-    } else {
-      if (verbose) {
-        OS::Print("Failed to set breakpoint at '%s' line %d\n",
-                  String::Handle(bpt->SourceUrl()).ToCString(),
-                  bpt->LineNumber());
-      }
+  if (cbpt != NULL) {
+    ASSERT(cbpt->src_bpt() == NULL);
+    cbpt->set_src_bpt(bpt);
+    SignalBpResolved(bpt);
+  } else {
+    if (verbose) {
+      OS::Print("Failed to set breakpoint at '%s' line %d\n",
+                String::Handle(bpt->SourceUrl()).ToCString(),
+                bpt->LineNumber());
     }
   }
   bpt->Enable();
   return bpt;
 }
 
 
 // Synchronize the enabled/disabled state of all code breakpoints
 // associated with the source breakpoint bpt.
 void Debugger::SyncBreakpoint(SourceBreakpoint* bpt) {
@@ skipping 615 matching lines @@
 }
 
 
 void Debugger::RegisterCodeBreakpoint(CodeBreakpoint* bpt) {
   ASSERT(bpt->next() == NULL);
   bpt->set_next(code_breakpoints_);
   code_breakpoints_ = bpt;
 }
 
 }  // namespace dart