ART: Delete catch phis for undefined vregs.

SSA builder assumed that (eagerly created) catch phis for undefined
vregs must be dead and will be deleted by dead phi elimination. This,
however, does not hold for --debuggable because the catch phis might
be revived for their environment uses. This patch modifies the builder
to delete the phis itself.

Bug: 24054676
Change-Id: Iaa5f2487ff1c38353b44cb89c709bbff1ecd73cc

diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index af8aa23..583da30 100644 (file)
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -624,8 +624,31 @@ void SSAChecker::VisitPhi(HPhi* phi) {
   }
 
   if (phi->IsCatchPhi()) {
-    // The number of inputs of a catch phi corresponds to the total number of
-    // throwing instructions caught by this catch block.
+    // The number of inputs of a catch phi should be the total number of throwing
+    // instructions caught by this catch block. We do not enforce this, however,
+    // because we do not remove the corresponding inputs when we prove that an
+    // instruction cannot throw. Instead, we at least test that all phis have the
+    // same, non-zero number of inputs (b/24054676).
+    size_t input_count_this = phi->InputCount();
+    if (input_count_this == 0u) {
+      AddError(StringPrintf("Phi %d in catch block %d has zero inputs.",
+                            phi->GetId(),
+                            phi->GetBlock()->GetBlockId()));
+    } else {
+      HInstruction* next_phi = phi->GetNext();
+      if (next_phi != nullptr) {
+        size_t input_count_next = next_phi->InputCount();
+        if (input_count_this != input_count_next) {
+          AddError(StringPrintf("Phi %d in catch block %d has %zu inputs, "
+                                "but phi %d has %zu inputs.",
+                                phi->GetId(),
+                                phi->GetBlock()->GetBlockId(),
+                                input_count_this,
+                                next_phi->GetId(),
+                                input_count_next));
+        }
+      }
+    }
   } else {
     // Ensure the number of inputs of a non-catch phi is the same as the number
     // of its predecessors.

diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index 9dcbea0..6f71ea3 100644 (file)
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -577,12 +577,28 @@ void SsaBuilder::VisitInstruction(HInstruction* instruction) {
     const HTryBoundary& try_entry =
         instruction->GetBlock()->GetTryCatchInformation()->GetTryEntry();
     for (HExceptionHandlerIterator it(try_entry); !it.Done(); it.Advance()) {
-      ArenaVector<HInstruction*>* handler_locals = GetLocalsFor(it.Current());
+      HBasicBlock* catch_block = it.Current();
+      ArenaVector<HInstruction*>* handler_locals = GetLocalsFor(catch_block);
       DCHECK_EQ(handler_locals->size(), current_locals_->size());
-      for (size_t i = 0, e = current_locals_->size(); i < e; ++i) {
-        HInstruction* local_value = (*current_locals_)[i];
-        if (local_value != nullptr) {
-          (*handler_locals)[i]->AsPhi()->AddInput(local_value);
+      for (size_t vreg = 0, e = current_locals_->size(); vreg < e; ++vreg) {
+        HInstruction* handler_value = (*handler_locals)[vreg];
+        if (handler_value == nullptr) {
+          // Vreg was undefined at a previously encountered throwing instruction
+          // and the catch phi was deleted. Do not record the local value.
+          continue;
+        }
+        DCHECK(handler_value->IsPhi());
+
+        HInstruction* local_value = (*current_locals_)[vreg];
+        if (local_value == nullptr) {
+          // This is the first instruction throwing into `catch_block` where
+          // `vreg` is undefined. Delete the catch phi.
+          catch_block->RemovePhi(handler_value->AsPhi());
+          (*handler_locals)[vreg] = nullptr;
+        } else {
+          // Vreg has been defined at all instructions throwing into `catch_block`
+          // encountered so far. Record the local value in the catch phi.
+          handler_value->AsPhi()->AddInput(local_value);
         }
       }
     }