Fix a bug in graph construction for for loops.

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 bf5d5d11a0d7f217f166038bbb72c58285d60274..ac4e35d1f21fbb58ecfe14f70709ebc76ad40da7 100644
--- a/runtime/vm/flow_graph_builder.cc
+++ b/runtime/vm/flow_graph_builder.cc
@@ -1711,34 +1711,28 @@ void EffectGraphVisitor::VisitForNode(ForNode* node) {
   node->increment()->Visit(&for_increment);
 
   // Join the loop body and increment and then tie the loop.
-  JoinEntryInstr* join = node->label()->join_for_continue();
-  if ((join != NULL) || for_body.is_open()) {
-    JoinEntryInstr* loop_start =
+  JoinEntryInstr* continue_join = node->label()->join_for_continue();
+  if ((continue_join != NULL) || for_body.is_open()) {
+    JoinEntryInstr* loop_entry =
         new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index());
-    if (join != NULL) {
-      if (for_body.is_open()) for_body.Goto(join);
-      AppendFragment(join, for_increment);
-      for_increment.Goto(loop_start);

[Kevin Millikin (Google), 2013/06/20 09:44:24]

The bug is in the order of side effects here.  If 1720 and 1721 are swapped
(connect the empty increment to the loop start before connecting the continue to
the now-not-empty increment) it will also work.

+    if (continue_join != NULL) {
+      if (for_body.is_open()) for_body.Goto(continue_join);
+      Instruction* current = AppendFragment(continue_join, for_increment);
+      current->Goto(loop_entry);
     } else {
       for_body.Append(for_increment);
-      for_body.Goto(loop_start);
+      for_body.Goto(loop_entry);
     }
-    Goto(loop_start);
-    exit_ = loop_start;
+    Goto(loop_entry);
+    exit_ = loop_entry;
     AddInstruction(
         new CheckStackOverflowInstr(node->token_pos(), owner()->loop_depth()));
   }
 
   if (node->condition() == NULL) {
     // Endless loop, no test.
-    JoinEntryInstr* body_entry =
-        new JoinEntryInstr(owner()->AllocateBlockId(), owner()->try_index());
-    AppendFragment(body_entry, for_body);
-    Goto(body_entry);
-    if (node->label()->join_for_break() != NULL) {
-      // Control flow of ForLoop continues into join_for_break.
-      exit_ = node->label()->join_for_break();
-    }
+    Append(for_body);
+    exit_ = node->label()->join_for_break();  // May be NULL.
   } else {
     TestGraphVisitor for_test(owner(),
                               temp_index(),