Ensure the debugger considers parameters to be in scope at the method entry debug step.

runtime/vm/flow_graph_builder.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/flow_graph_builder.h"
 
 #include "lib/invocation_mirror.h"
 #include "vm/ast_printer.h"
 #include "vm/bit_vector.h"
 #include "vm/class_finalizer.h"
@@ skipping 4061 matching lines @@
   LocalScope* scope = node->scope();
   const Function& function = owner()->function();
   const intptr_t num_context_variables =
       (scope != NULL) ? scope->num_context_variables() : 0;
   const bool is_top_level_sequence =
       node == owner()->parsed_function().node_sequence();
   // The outermost function sequence cannot contain a label.
   ASSERT((node->label() == NULL) || !is_top_level_sequence);
   NestedBlock nested_block(owner(), node);
 
-  if (FLAG_support_debugger && is_top_level_sequence) {
-    AddInstruction(new(Z) DebugStepCheckInstr(function.token_pos(),
-                                              RawPcDescriptors::kRuntimeCall));
-  }
-
   if (num_context_variables > 0) {
     // The local scope declares variables that are captured.
     // Allocate and chain a new context (Except don't chain when at the function
     // entry if the function does not capture any variables from outer scopes).
     Value* allocated_context =
         Bind(new(Z) AllocateContextInstr(node->token_pos(),
                                          num_context_variables));
     { LocalVariable* tmp_var = EnterTempLocalScope(allocated_context);
       if (!is_top_level_sequence || HasContextScope()) {
         ASSERT(is_top_level_sequence ||
@@ skipping 42 matching lines @@
           // allow GC of passed value if it gets overwritten by a new value in
           // the function.
           Value* null_constant = Bind(new(Z) ConstantInstr(
               Object::ZoneHandle(Z, Object::null())));
           Do(BuildStoreLocal(*temp_local, null_constant));
         }
       }
     }
   }
 
+  if (FLAG_support_debugger && is_top_level_sequence) {
+    // Place a debug check at method entry to ensure breaking on a method always
+    // happens, even if there are no assignments/calls/runtimecalls in the first
+    // basic block. Place this check at the last parameter to ensure parameters
+    // are in scope in the debugger at method entry.
+    const int num_params = function.NumParameters();
+    intptr_t check_pos = 0;
+    if (num_params > 0) {
+      const LocalVariable& parameter = *scope->VariableAt(num_params - 1);
+      check_pos = parameter.token_pos();
+    }
+    if (check_pos == 0) {
+      // No parameters or synthetic parameters.
+      check_pos = node->token_pos();

[hausner, 2015/10/07 18:13:29]

Are there ever sequence nodes that have a kNoSourcePos token position, e.g. in
generated/synthetic functions?
Also, I think no debug check should be inserted for functions that are not
debuggable.

[rmacnak, 2015/10/07 19:27:48]

Attempted assert says yes, but these seem to all be unit tests where the
function name is at token 0. i.e., kNoSourcePos is in fact a valid source
position.
Done.

+    }
+    AddInstruction(new(Z) DebugStepCheckInstr(check_pos,
+                                              RawPcDescriptors::kRuntimeCall));
+  }
+
   // This check may be deleted if the generated code is leaf.
   // Native functions don't need a stack check at entry.
   if (is_top_level_sequence && !function.is_native()) {
     // Always allocate CheckOverflowInstr so that deopt-ids match regardless
     // if we inline or not.
     if (!function.IsImplicitGetterFunction() &&
         !function.IsImplicitSetterFunction()) {
       CheckStackOverflowInstr* check =
           new(Z) CheckStackOverflowInstr(function.token_pos(), 0);
       // If we are inlining don't actually attach the stack check. We must still
@@ skipping 519 matching lines @@
   Report::MessageF(Report::kBailout,
                    Script::Handle(function.script()),
                    function.token_pos(),
                    "FlowGraphBuilder Bailout: %s %s",
                    String::Handle(function.name()).ToCString(),
                    reason);
   UNREACHABLE();
 }
 
 }  // namespace dart