Fix a bug in graph construction for for loops.

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/code_descriptors.h"
@@ skipping 1693 matching lines @@
 
   owner()->IncrementLoopDepth();
   // Compose body to set any jump labels.
   EffectGraphVisitor for_body(owner(), temp_index());
   node->body()->Visit(&for_body);
 
   EffectGraphVisitor for_increment(owner(), temp_index());
   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(),
                               node->condition()->token_pos());
     node->condition()->Visit(&for_test);
     Append(for_test);
 
     BlockEntryInstr* body_entry = for_test.CreateTrueSuccessor();
     AppendFragment(body_entry, for_body);
 
@@ skipping 1759 matching lines @@
   intptr_t len = OS::SNPrint(NULL, 0, kFormat, function_name, reason) + 1;
   char* chars = Isolate::Current()->current_zone()->Alloc<char>(len);
   OS::SNPrint(chars, len, kFormat, function_name, reason);
   const Error& error = Error::Handle(
       LanguageError::New(String::Handle(String::New(chars))));
   Isolate::Current()->long_jump_base()->Jump(1, error);
 }
 
 
 }  // namespace dart