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

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
diff --git a/runtime/vm/flow_graph_builder.cc b/runtime/vm/flow_graph_builder.cc
index 9bbd9e083764eb564c0cd36bf693eccea4db1e20..947049d044363d1413b326699e1df5796cf9ee57 100644
--- a/runtime/vm/flow_graph_builder.cc
+++ b/runtime/vm/flow_graph_builder.cc
@@ -4079,11 +4079,6 @@ void EffectGraphVisitor::VisitSequenceNode(SequenceNode* node) {
   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
@@ -4146,6 +4141,25 @@ void EffectGraphVisitor::VisitSequenceNode(SequenceNode* node) {
     }
   }
 
+  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()) {