Fix a VM bug in the handling of try/catch/finally.

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 3226 matching lines @@
   BuildLoadContext(node->context_var());
 
   EffectGraphVisitor for_catch(owner(), temp_index());
   node->VisitChildren(&for_catch);
   Append(for_catch);
 }
 
 
 void EffectGraphVisitor::VisitTryCatchNode(TryCatchNode* node) {
   InlineBailout("EffectGraphVisitor::VisitTryCatchNode (exception)");
-  intptr_t old_try_index = owner()->try_index();

[Kevin Millikin (Google), 2013/06/26 14:27:04]

The name 'try_index' isn't quite right.  There can be more than one handler per
try (i.e., a catch and a finally) so numbering each try doesn't serve to
identify the handler.

I propose changing the name to 'handler_index', but I'd make that as a separate,
cosmetic-only, change.

-  intptr_t try_index = owner()->AllocateTryIndex();
-  owner()->set_try_index(try_index);
+  intptr_t original_handler_index = owner()->try_index();
+  intptr_t try_handler_index = owner()->AllocateTryIndex();
+  owner()->set_try_index(try_handler_index);
 
   // Preserve CTX into local variable '%saved_context'.
   BuildStoreContext(node->context_var());
 
-  EffectGraphVisitor for_try_block(owner(), temp_index());
-  node->try_block()->Visit(&for_try_block);
+  EffectGraphVisitor for_try(owner(), temp_index());
+  node->try_block()->Visit(&for_try);
 
-  if (for_try_block.is_open()) {
+  if (for_try.is_open()) {
     JoinEntryInstr* after_try =
-        new JoinEntryInstr(owner()->AllocateBlockId(), old_try_index);
-    for_try_block.Goto(after_try);
-    for_try_block.exit_ = after_try;
+        new JoinEntryInstr(owner()->AllocateBlockId(), original_handler_index);
+    for_try.Goto(after_try);
+    for_try.exit_ = after_try;
   }
 
   JoinEntryInstr* try_entry =
-      new JoinEntryInstr(owner()->AllocateBlockId(), try_index);
+      new JoinEntryInstr(owner()->AllocateBlockId(), try_handler_index);
 
   Goto(try_entry);
-  AppendFragment(try_entry, for_try_block);
-  exit_ = for_try_block.exit_;
+  AppendFragment(try_entry, for_try);
+  exit_ = for_try.exit_;
 
   // We are done generating code for the try block.
-  owner()->set_try_index(old_try_index);
+  owner()->set_try_index(original_handler_index);
 
   CatchClauseNode* catch_block = node->catch_block();
+  SequenceNode* finally_block = node->finally_block();
   if (catch_block != NULL) {
-    EffectGraphVisitor for_catch_block(owner(), temp_index());
-    catch_block->Visit(&for_catch_block);
+    // If there is a finally block, it is the handler for code in the catch
+    // block.
+    intptr_t catch_handler_index = (finally_block == NULL)
+        ? original_handler_index
+        : owner()->AllocateTryIndex();
+    owner()->set_try_index(catch_handler_index);
+    EffectGraphVisitor for_catch(owner(), temp_index());
+    catch_block->Visit(&for_catch);
     CatchBlockEntryInstr* catch_entry =
         new CatchBlockEntryInstr(owner()->AllocateBlockId(),
-                                 old_try_index,
+                                 catch_handler_index,
                                  catch_block->handler_types(),
-                                 try_index);
+                                 try_handler_index);
     owner()->AddCatchEntry(catch_entry);
-    ASSERT(!for_catch_block.is_open());
-    AppendFragment(catch_entry, for_catch_block);
+    ASSERT(!for_catch.is_open());
+    AppendFragment(catch_entry, for_catch);
     if (node->end_catch_label() != NULL) {
       JoinEntryInstr* join = node->end_catch_label()->join_for_continue();
       if (join != NULL) {
         if (is_open()) Goto(join);
         exit_ = join;
       }
     }
+
+    if (finally_block != NULL) {
+      // Create a handler for the code in the catch block, containing the
+      // code in the finally block.
+      owner()->set_try_index(original_handler_index);
+      EffectGraphVisitor for_finally(owner(), temp_index());
+      for_finally.AddInstruction(
+          new CatchEntryInstr(catch_block->exception_var(),
+                              catch_block->stacktrace_var()));
+      for_finally.BuildLoadContext(catch_block->context_var());
+
+      finally_block->Visit(&for_finally);
+      if (for_finally.is_open()) {
+        // Rethrow the exception.  Manually build the graph for rethrow.
+        Value* exception = for_finally.Bind(
+            for_finally.BuildLoadLocal(catch_block->exception_var()));
+        for_finally.PushArgument(exception);
+        Value* stacktrace = for_finally.Bind(
+            for_finally.BuildLoadLocal(catch_block->stacktrace_var()));
+        for_finally.PushArgument(stacktrace);
+        for_finally.AddInstruction(new ReThrowInstr(catch_block->token_pos()));
+        for_finally.CloseFragment();
+      }
+      ASSERT(!for_finally.is_open());
+
+      const Array& types = Array::ZoneHandle(Array::New(1, Heap::kOld));
+      types.SetAt(0, Type::Handle(Type::DynamicType()));
+      CatchBlockEntryInstr* finally_entry =
+          new CatchBlockEntryInstr(owner()->AllocateBlockId(),
+                                   original_handler_index,
+                                   types,
+                                   catch_handler_index);
+      owner()->AddCatchEntry(finally_entry);
+      AppendFragment(finally_entry, for_finally);
+    }
   }
 
   // Generate code for the finally block if one exists.
   if ((node->finally_block() != NULL) && is_open()) {

[Florian Schneider, 2013/06/27 14:05:59]

s/node->finally_block()/finally_block/

[Kevin Millikin (Google), 2013/06/28 11:59:08]

Well spotted.  Done.

     EffectGraphVisitor for_finally_block(owner(), temp_index());
     node->finally_block()->Visit(&for_finally_block);

[Florian Schneider, 2013/06/27 14:05:59]

s/node->finally_block()/finally_block/

     Append(for_finally_block);
   }
 }
 
 
 // Looks up dynamic method noSuchMethod in target_class
 // (including its super class chain) and builds a static call to it.
 StaticCallInstr* EffectGraphVisitor::BuildStaticNoSuchMethodCall(
     const Class& target_class,
     AstNode* receiver,
@@ skipping 201 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