Improve breakpoint resolution

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 "include/dart_api.h"
 
 #include "vm/code_generator.h"
 #include "vm/code_patcher.h"
@@ skipping 33 matching lines @@
 
  private:
   GrowableObjectArray* objs_;
 
   DISALLOW_COPY_AND_ASSIGN(RemoteObjectCache);
 };
 
 
 SourceBreakpoint::SourceBreakpoint(intptr_t id,
                                    const Script& script,
-                                   intptr_t token_pos)
+                                   intptr_t token_pos,
+                                   intptr_t end_token_pos)
     : id_(id),
       script_(script.raw()),
       token_pos_(token_pos),
+      end_token_pos_(end_token_pos),
       is_resolved_(false),
       is_enabled_(false),
       next_(NULL),
       function_(Function::null()),
       line_number_(-1) {
   ASSERT(id_ > 0);
   ASSERT(!script.IsNull());
   ASSERT(token_pos_ >= 0);
 }
 
 
 void SourceBreakpoint::Enable() {
   is_enabled_ = true;
   Isolate::Current()->debugger()->SyncBreakpoint(this);
 }
 
 
 void SourceBreakpoint::Disable() {
   is_enabled_ = false;
   Isolate::Current()->debugger()->SyncBreakpoint(this);
 }
 
 
 void SourceBreakpoint::SetResolved(const Function& func, intptr_t token_pos) {
   ASSERT(func.script() == script_);
   ASSERT((func.token_pos() <= token_pos) &&
          (token_pos <= func.end_token_pos()));
   function_ = func.raw();
   token_pos_ = token_pos;
+  end_token_pos_ = token_pos;
   line_number_ = -1;  // Recalcualte lazily.
   is_resolved_ = true;
 }
 
 
 // TODO(hausner): Get rid of library parameter. A source breakpoint location
 // does not imply a library, since the same source code can be included
 // in more than one library, e.g. the text location of mixin functions.
 void SourceBreakpoint::GetCodeLocation(
     Library* lib,
@@ skipping 1198 matching lines @@
   }
   DebuggerEvent event(kExceptionThrown);
   event.exception = &exc;
   ASSERT(stack_trace_ == NULL);
   stack_trace_ = stack_trace;
   Pause(&event);
   stack_trace_ = NULL;
 }
 
 
-// Given a function and a token position, return the best fit
+// Given a function and a token range, return the best fit
 // token position to set a breakpoint. The best fit is the safe point
-// with the lowest compiled code address that follows the requsted
-// token position.
+// with the lowest compiled code address within the token range.
 intptr_t Debugger::ResolveBreakpointPos(const Function& func,
-                                        intptr_t requested_token_pos) {
+                                        intptr_t requested_token_pos,
+                                        intptr_t last_token_pos) {
   ASSERT(func.HasCode());
   ASSERT(!func.HasOptimizedCode());
+
+  if (requested_token_pos < func.token_pos()) {
+    requested_token_pos = func.token_pos();
+  }
+  if (last_token_pos > func.end_token_pos()) {
+    last_token_pos = func.end_token_pos();
+  }

[srdjan, 2014/04/11 17:12:07]

ASSERT(requested_token_pos <= last_token_pos);

[hausner, 2014/04/11 17:49:05]

Not necessarily. It might be possible that the requested token pos is past the
last token position of the function, e.g. on a one-liner function literal that
is embedded in a line.

+
   Code& code = Code::Handle(func.unoptimized_code());
   ASSERT(!code.IsNull());
   PcDescriptors& desc = PcDescriptors::Handle(code.pc_descriptors());
   intptr_t best_fit_index = -1;
   intptr_t best_fit_pos = INT_MAX;
   uword lowest_pc = kUwordMax;
   intptr_t lowest_pc_index = -1;
+
   for (intptr_t i = 0; i < desc.Length(); i++) {
     intptr_t desc_token_pos = desc.TokenPos(i);
     ASSERT(desc_token_pos >= 0);
     if (IsSafePoint(desc, i)) {
-      if ((desc_token_pos < func.token_pos()) ||
-          (desc_token_pos > func.end_token_pos())) {
-        // The position is outside of the function token range. This can
-        // happen in constructors, for initializer expressions that are
-        // inlined in the field declaration.
-        ASSERT(func.IsConstructor());
-        continue;
-      }
-      if (desc_token_pos < requested_token_pos) {
-        // This descriptor is before the first acceptable token position.
+      if ((desc_token_pos < requested_token_pos) ||
+          (desc_token_pos > last_token_pos)) {
+        // This descriptor is outside the desired token range.
         continue;
       }
       if (desc_token_pos < best_fit_pos) {
         // So far, this descriptor has the lowest token position after
         // the first acceptable token position.
         best_fit_pos = desc_token_pos;
         best_fit_index = i;
       }
       if (desc.PC(i) < lowest_pc) {
         // This descriptor so far has the lowest code address.
@@ skipping 191 matching lines @@
           }
         }
       }
     }
   }
   return best_fit.raw();
 }
 
 
 SourceBreakpoint* Debugger::SetBreakpoint(const Script& script,
-                                          intptr_t token_pos) {
+                                          intptr_t token_pos,
+                                          intptr_t last_token_pos) {
   Function& func = Function::Handle(isolate_);
   func = FindBestFit(script, token_pos);
   if (func.IsNull()) {
     return NULL;
   }
   // There may be more than one function object for a given function
   // in source code. There may be implicit closure functions, and
   // there may be copies of mixin functions. Collect all compiled
   // functions whose source code range matches exactly the best fit
   // function we found.
   GrowableObjectArray& functions =
       GrowableObjectArray::Handle(GrowableObjectArray::New());
   FindCompiledFunctions(script,
                         func.token_pos(),
                         func.end_token_pos(),
                         &functions);
 
   if (functions.Length() > 0) {
     // One or more function object containing this breakpoint location
     // have already been compiled. We can resolve the breakpoint now.
     DeoptimizeWorld();
     func ^= functions.At(0);
-    intptr_t breakpoint_pos = ResolveBreakpointPos(func, token_pos);
+    intptr_t breakpoint_pos =
+        ResolveBreakpointPos(func, token_pos, last_token_pos);
     if (breakpoint_pos >= 0) {
       SourceBreakpoint* bpt = GetSourceBreakpoint(script, breakpoint_pos);
       if (bpt != NULL) {
         // A source breakpoint for this location already exists.
         return bpt;
       }
-      bpt = new SourceBreakpoint(nextId(), script, token_pos);
+      bpt = new SourceBreakpoint(nextId(), script, token_pos, last_token_pos);
       bpt->SetResolved(func, breakpoint_pos);
       RegisterSourceBreakpoint(bpt);
 
       // Create code breakpoints for all compiled functions we found.
       const intptr_t num_functions = functions.Length();
       for (intptr_t i = 0; i < num_functions; i++) {
         func ^= functions.At(i);
         ASSERT(func.HasCode());
         MakeCodeBreakpointsAt(func, bpt);
       }
       bpt->Enable();
+      if (FLAG_verbose_debug) {
+        intptr_t line_number;
+        script.GetTokenLocation(breakpoint_pos, &line_number, NULL);
+        OS::Print("Resolved breakpoint for "
+                  "function '%s' at line %" Pd "\n",
+                  func.ToFullyQualifiedCString(),
+                  line_number);
+      }
       SignalBpResolved(bpt);
       return bpt;
     }
   }
   // There is no compiled function at this token position.
   // Register an unresolved breakpoint.
   if (FLAG_verbose_debug && !func.IsNull()) {
     intptr_t line_number;
     script.GetTokenLocation(token_pos, &line_number, NULL);
     OS::Print("Registering pending breakpoint for "
               "uncompiled function '%s' at line %" Pd "\n",
               func.ToFullyQualifiedCString(),
               line_number);
   }
   SourceBreakpoint* bpt = GetSourceBreakpoint(script, token_pos);
   if (bpt == NULL) {
-    bpt = new SourceBreakpoint(nextId(), script, token_pos);
+    bpt = new SourceBreakpoint(nextId(), script, token_pos, last_token_pos);
   }
   RegisterSourceBreakpoint(bpt);
   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 18 matching lines @@
         Function::Handle(target_function.ImplicitClosureFunction());
     SetInternalBreakpoints(closure_func);
   }
 }
 
 
 SourceBreakpoint* Debugger::SetBreakpointAtEntry(
       const Function& target_function) {
   ASSERT(!target_function.IsNull());
   const Script& script = Script::Handle(target_function.script());
-  return SetBreakpoint(script, target_function.token_pos());
+  return SetBreakpoint(script,
+                       target_function.token_pos(),
+                       target_function.end_token_pos());
 }
 
 
 SourceBreakpoint* Debugger::SetBreakpointAtLine(const String& script_url,
                                                 intptr_t line_number) {
   Library& lib = Library::Handle(isolate_);
   Script& script = Script::Handle(isolate_);
   const GrowableObjectArray& libs =
       GrowableObjectArray::Handle(isolate_->object_store()->libraries());
   for (intptr_t i = 0; i < libs.Length(); i++) {
@@ skipping 24 matching lines @@
     if (FLAG_verbose_debug) {
       OS::Print("No executable code at line %" Pd " in '%s'\n",
                 line_number, script_url.ToCString());
     }
     return NULL;
   }
 
   SourceBreakpoint* bpt = NULL;
   ASSERT(first_token_idx <= last_token_idx);
   while ((bpt == NULL) && (first_token_idx <= last_token_idx)) {
-    bpt = SetBreakpoint(script, first_token_idx);
+    bpt = SetBreakpoint(script, first_token_idx, last_token_idx);
     first_token_idx++;
   }
   if ((bpt == NULL) && FLAG_verbose_debug) {
     OS::Print("No executable code at line %" Pd " in '%s'\n",
                 line_number, script_url.ToCString());
   }
   return bpt;
 }
 
 
@@ skipping 445 matching lines @@
 
       // TODO(hausner): What should we do if function is optimized?
       // Can we deoptimize the function?
       ASSERT(!func.HasOptimizedCode());
 
       // There is no local function within func that contains the
       // breakpoint token position. Resolve the breakpoint if necessary
       // and set the code breakpoints.
       if (!bpt->IsResolved()) {
         // Resolve source breakpoint in the newly compiled function.
-        intptr_t bp_pos = ResolveBreakpointPos(func, bpt->token_pos());
+        intptr_t bp_pos =
+            ResolveBreakpointPos(func, bpt->token_pos(), bpt->end_token_pos());
         if (bp_pos < 0) {
           if (FLAG_verbose_debug) {
             OS::Print("Failed resolving breakpoint for function '%s'\n",
                       String::Handle(func.name()).ToCString());
           }
           continue;
         }
         intptr_t requested_pos = bpt->token_pos();
         bpt->SetResolved(func, bp_pos);
         if (FLAG_verbose_debug) {
@@ skipping 150 matching lines @@
 }
 
 
 void Debugger::RegisterCodeBreakpoint(CodeBreakpoint* bpt) {
   ASSERT(bpt->next() == NULL);
   bpt->set_next(code_breakpoints_);
   code_breakpoints_ = bpt;
 }
 
 }  // namespace dart