Deoptimize functions before setting breakpoints

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_index_table.h"
 #include "vm/code_generator.h"
 #include "vm/code_patcher.h"
 #include "vm/compiler.h"
 #include "vm/dart_entry.h"
 #include "vm/flags.h"
 #include "vm/globals.h"
 #include "vm/longjump.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
 #include "vm/os.h"
 #include "vm/stack_frame.h"
 #include "vm/stub_code.h"
 #include "vm/visitor.h"
 
 
 namespace dart {
 
 static const bool verbose = false;

[srdjan, 2012/03/13 17:11:35]

Why not add a debugger_verbose flag?

[hausner, 2012/03/13 20:45:03]

Soon I'll give in and do so. So for I intend to remove this flag once the
debugger is fairly stable.

 
 
 SourceBreakpoint::SourceBreakpoint(const Function& func, intptr_t token_index)
     : function_(func.raw()),
       token_index_(token_index),
       line_number_(-1),
       is_enabled_(false),
       next_(NULL) {
   ASSERT(!func.IsNull());
   ASSERT((func.token_index() <= token_index_) &&
@@ skipping 442 matching lines @@
   if (!cls.IsNull()) {
     function = cls.LookupStaticFunction(function_name);
     if (function.IsNull()) {
       function = cls.LookupDynamicFunction(function_name);
     }
   }
   return function.raw();
 }
 
 
-void Debugger::InstrumentForStepping(const Function &target_function) {
+// Deoptimize function if necessary. Does not patch return addresses on the
+// stack. If there are activation frames of this function on the stack,
+// the optimized code will be executed when the callee returns.
+void Debugger::EnsureFunctionIsDeoptimized(const Function& func) {
+  if (func.HasOptimizedCode()) {
+    if (verbose) {
+      printf("Deoptimizing function %s\n",

[srdjan, 2012/03/13 17:11:35]

OS::Print

[hausner, 2012/03/13 20:45:03]

Done.

+          String::Handle(func.name()).ToCString());
+    }
+    func.set_usage_counter(0);
+    func.set_deoptimization_counter(func.deoptimization_counter() + 1);

[srdjan, 2012/03/13 17:11:35]

Don't increase the counter otherwise setting and clearing the breakpoint would
prevent future optimizations. The deopt counter is here to stop endless
deoptimizations of certain bad-behaving methods,

[hausner, 2012/03/13 20:45:03]

Done.

+    Compiler::CompileFunction(func);
+    ASSERT(!func.HasOptimizedCode());
+  }
+}
+
+
+void Debugger::InstrumentForStepping(const Function& target_function) {
   if (!target_function.HasCode()) {
     Compiler::CompileFunction(target_function);
     // If there were any errors, ignore them silently and return without
     // adding breakpoints to target.
     if (!target_function.HasCode()) {
       return;
     }
+  } else {
+    EnsureFunctionIsDeoptimized(target_function);

[srdjan, 2012/03/13 17:11:35]

I rather think positive:

if (target.HasCode()) {
  EnsureFunctionIsDeoptimized(target_function);
} else {
  ...
}

[hausner, 2012/03/13 20:45:03]

The glass is now half full.

On 2012/03/13 17:11:35, srdjan wrote:

   }
-  ASSERT(!target_function.HasOptimizedCode());
   Code& code = Code::Handle(target_function.unoptimized_code());
   ASSERT(!code.IsNull());
   PcDescriptors& desc = PcDescriptors::Handle(code.pc_descriptors());
   for (int i = 0; i < desc.Length(); i++) {
     CodeBreakpoint* bpt = GetCodeBreakpoint(desc.PC(i));
     if (bpt != NULL) {
       // There is already a breakpoint for this address. Leave it alone.
       continue;
     }
     PcDescriptors::Kind kind = desc.DescriptorKind(i);
@@ skipping 48 matching lines @@
 }
 
 
 SourceBreakpoint* Debugger::SetBreakpoint(const Function& target_function,
                                           intptr_t token_index) {
   if ((token_index < target_function.token_index()) ||
       (target_function.end_token_index() <= token_index)) {
     // The given token position is not within the target function.
     return NULL;
   }
+  EnsureFunctionIsDeoptimized(target_function);
   SourceBreakpoint* bpt = GetSourceBreakpoint(target_function, token_index);
   if (bpt != NULL) {
     // A breakpoint for this location already exists, return it.
     return bpt;
   }
   bpt = new SourceBreakpoint(target_function, token_index);
   RegisterSourceBreakpoint(bpt);
   if (verbose && !target_function.HasCode()) {
     OS::Print("Registering breakpoint for uncompiled function '%s'"
               " (%s:%d)\n",
@@ skipping 445 matching lines @@
 }
 
 
 void Debugger::RegisterCodeBreakpoint(CodeBreakpoint* bpt) {
   ASSERT(bpt->next() == NULL);
   bpt->set_next(code_breakpoints_);
   code_breakpoints_ = bpt;
 }
 
 }  // namespace dart