VM: [Kernel] Ensure we have the correct try-index when translating finally blocks

runtime/vm/kernel_to_il.cc

 // Copyright (c) 2016, 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 <map>
 #include <set>
 #include <string>
 
 #include "vm/kernel_to_il.h"
 
@@ skipping 827 matching lines @@
 class TryFinallyBlock {
  public:
   TryFinallyBlock(FlowGraphBuilder* builder, Statement* finalizer)
       : builder_(builder),
         outer_(builder->try_finally_block_),
         finalizer_(finalizer),
         context_depth_(builder->context_depth_),
         // Finalizers are executed outside of the try block hence
         // try depth of finalizers are one less than current try
         // depth.
-        try_depth_(builder->try_depth_ - 1) {
+        try_depth_(builder->try_depth_ - 1),
+        try_index_(builder_->CurrentTryIndex()) {
     builder_->try_finally_block_ = this;
   }
   ~TryFinallyBlock() { builder_->try_finally_block_ = outer_; }
 
   Statement* finalizer() const { return finalizer_; }
   intptr_t context_depth() const { return context_depth_; }
   intptr_t try_depth() const { return try_depth_; }
+  intptr_t try_index() const { return try_depth_; }

[Kevin Millikin (Google), 2016/11/09 12:30:02]

return try_index_

[kustermann, 2016/11/11 19:41:35]

Thanks for catching this! I've modified the test to also hit this part.

   TryFinallyBlock* outer() const { return outer_; }
 
  private:
   FlowGraphBuilder* const builder_;
   TryFinallyBlock* const outer_;
   Statement* const finalizer_;
   const intptr_t context_depth_;
   const intptr_t try_depth_;
+  const intptr_t try_index_;
 };
 
 
 class TryCatchBlock {
  public:
   explicit TryCatchBlock(FlowGraphBuilder* builder,
                          intptr_t try_handler_index = -1)
       : builder_(builder),
         outer_(builder->try_catch_block_),
         try_index_(try_handler_index) {
     if (try_index_ == -1) try_index_ = builder->AllocateTryIndex();
     builder->try_catch_block_ = this;
   }
   ~TryCatchBlock() { builder_->try_catch_block_ = outer_; }
 
-  intptr_t TryIndex() { return try_index_; }
+  intptr_t try_index() { return try_index_; }
+  TryCatchBlock* outer() const { return outer_; }
 
  private:
   FlowGraphBuilder* builder_;
   TryCatchBlock* outer_;
   intptr_t try_index_;
 };
 
 
 class CatchBlock {
  public:
@@ skipping 947 matching lines @@
       constant_evaluator_(this, zone_, &translation_helper_,
                           &type_translator_) {}
 
 
 FlowGraphBuilder::~FlowGraphBuilder() {}
 
 
 Fragment FlowGraphBuilder::TranslateFinallyFinalizers(
     TryFinallyBlock* outer_finally, intptr_t target_context_depth) {
   TryFinallyBlock* const saved_block = try_finally_block_;
+  TryCatchBlock* const saved_try_catch_block = try_catch_block_;
   const intptr_t saved_depth = context_depth_;
   const intptr_t saved_try_depth = try_depth_;
 
   Fragment instructions;
 
   // While translating the body of a finalizer we need to set the try-finally
   // block which is active when translating the body.
   while (try_finally_block_ != outer_finally) {
     // Set correct try depth (in case there are nested try statements).
     try_depth_ = try_finally_block_->try_depth();
 
     // Potentially restore the context to what is expected for the finally
     // block.
     instructions += AdjustContextTo(try_finally_block_->context_depth());
 
     Statement* finalizer = try_finally_block_->finalizer();
     try_finally_block_ = try_finally_block_->outer();
 
+    // The to-be-translated finalizer has to have the correct try-index (namely
+    // the one outside the try-finally block).
+    bool changed_try_index = false;
+    intptr_t target_try_index = try_finally_block_ == NULL
+        ? CatchClauseNode::kInvalidTryIndex
+        : try_finally_block_->try_index();
+    while (CurrentTryIndex() != target_try_index) {
+      try_catch_block_ = try_catch_block_->outer();
+      changed_try_index = true;
+    }
+    if (changed_try_index) {
+      JoinEntryInstr* entry = BuildJoinEntry();
+      instructions += Goto(entry);
+      instructions = Fragment(instructions.entry, entry);
+    }
+
     // This will potentially have exceptional cases as described in
     // [VisitTryFinally] and will handle them.
     instructions += TranslateStatement(finalizer);
 
     // We only need to make sure that if the finalizer ended normally, we
     // continue towards the next outer try-finally.
     if (!instructions.is_open()) break;
   }
 
   if (instructions.is_open() && target_context_depth != -1) {
-    // A target context depth of -1 indicates that we the code after this
+    // A target context depth of -1 indicates that the code after this
     // will not care about the context chain so we can leave it any way we
     // want after the last finalizer.  That is used when returning.
     instructions += AdjustContextTo(target_context_depth);
   }
 
   try_finally_block_ = saved_block;
+  try_catch_block_ = saved_try_catch_block;
   context_depth_ = saved_depth;
   try_depth_ = saved_try_depth;
 
   return instructions;
 }
 
 
 Fragment FlowGraphBuilder::EnterScope(TreeNode* node, bool* new_context) {
   Fragment instructions;
   const intptr_t context_size =
@@ skipping 760 matching lines @@
   stack_->definition()->set_ssa_temp_index(0);
 
   return variable;
 }
 
 
 intptr_t FlowGraphBuilder::CurrentTryIndex() {
   if (try_catch_block_ == NULL) {
     return CatchClauseNode::kInvalidTryIndex;
   } else {
-    return try_catch_block_->TryIndex();
+    return try_catch_block_->try_index();
   }
 }
 
 
 dart::LocalVariable* FlowGraphBuilder::LookupVariable(
     VariableDeclaration* var) {
   LocalVariable* local = scopes_->locals.Lookup(var);
   ASSERT(local != NULL);
   return local;
 }
@@ skipping 2740 matching lines @@
   //
   //   => We are responsible for catching it, executing the finally block and
   //      rethrowing the exception.
   intptr_t try_handler_index = AllocateTryIndex();
   Fragment try_body = TryCatch(try_handler_index);
   JoinEntryInstr* after_try = BuildJoinEntry();
 
   // Fill in the body of the try.
   ++try_depth_;
   {
+    TryFinallyBlock tfb(this, node->finalizer());
     TryCatchBlock tcb(this, try_handler_index);
-    TryFinallyBlock tfb(this, node->finalizer());
     try_body += TranslateStatement(node->body());
   }
   --try_depth_;
 
   if (try_body.is_open()) {
     // Please note: The try index will be on level out of this block,
     // thereby ensuring if there's an exception in the finally block we
     // won't run it twice.
     JoinEntryInstr* finally_entry = BuildJoinEntry();
 
@@ skipping 271 matching lines @@
   instructions += LoadLocal(closure);
   instructions += LoadLocal(parsed_function_->current_context_var());
   instructions += StoreInstanceField(Closure::context_offset());
 
   return instructions;
 }
 
 
 }  // namespace kernel
 }  // namespace dart