VM: Fix receiver type propagation in presence of try-catch.

runtime/vm/flow_graph.cc

Index: runtime/vm/flow_graph.cc
diff --git a/runtime/vm/flow_graph.cc b/runtime/vm/flow_graph.cc
index a5cadf04173fc9749c9b3e9e511986c453d7b3db..599fce04311de948c0aa5ab34877a8cc61bdb6b5 100644
--- a/runtime/vm/flow_graph.cc
+++ b/runtime/vm/flow_graph.cc
@@ -5,6 +5,7 @@
 #include "vm/flow_graph.h"
 
 #include "vm/bit_vector.h"
+#include "vm/cha.h"
 #include "vm/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
 #include "vm/flow_graph_range_analysis.h"
@@ -372,6 +373,75 @@ static void VerifyUseListsInInstruction(Instruction* instr) {
 }
 
 
+void FlowGraph::ComputeIsReceiverRecursive(PhiInstr* phi,
+                                           BitVector* processed) const {
+  if (phi->is_receiver() != PhiInstr::kUnknownReceiver) return;
+  if (processed->Contains(phi->ssa_temp_index())) return;
+  processed->Add(phi->ssa_temp_index());
+  for (intptr_t i = 0; i < phi->InputCount(); ++i) {
+    Definition* def = phi->InputAt(i)->definition();
+    if (def->IsParameter() && (def->AsParameter()->index() == 0)) continue;
+    if (!def->IsPhi()) {
+      phi->set_is_receiver(PhiInstr::kNotReceiver);
+      return;
+    }
+    ComputeIsReceiverRecursive(def->AsPhi(), processed);

[Vyacheslav Egorov (Google), 2016/03/31 05:00:58]

Something is not right here. Imagine situation:

x <- phi (y, z)
y <- phi (x, x)
z <- not-receiver 

in this case we will mark y as a receiver phi as far as I can see. 

I think the right way to do it is to not use bitvector at all, but instead mark
phi optimistically PhiInstr::kReceiver before the loop and then when it proves
to be wrong walk all uses of the phi and mark phis among them as kNotReceiver. 

If possible it would be good to have a test case covering this corner case too. 

[Florian Schneider, 2016/03/31 17:13:54]

You're right. The current way does not the compute the property correctly for
the phis visited. I could also only compute the property for the phi of the
initial call to ComputeRecursive, but I think I'll do a pass over all phis.

+    if (def->AsPhi()->is_receiver() == PhiInstr::kNotReceiver) {
+      phi->set_is_receiver(PhiInstr::kNotReceiver);
+      return;
+    }
+  }
+  phi->set_is_receiver(PhiInstr::kReceiver);
+}
+
+
+bool FlowGraph::IsReceiver(Definition* def) const {
+  if (def->IsParameter()) return (def->AsParameter()->index() == 0);
+  if (!def->IsPhi()) return false;
+  PhiInstr* phi = def->AsPhi();
+  if (phi->is_receiver() != PhiInstr::kUnknownReceiver) {
+    return (phi->is_receiver() == PhiInstr::kReceiver);
+  }
+  // Not known if this phi is the receiver yet. Compute it now.
+  BitVector* processed =
+      new(zone()) BitVector(zone(), max_virtual_register_number());
+  ComputeIsReceiverRecursive(phi, processed);
+  return (phi->is_receiver() == PhiInstr::kReceiver);
+}
+
+
+// Use CHA to determine if the call needs a class check: if the callee's
+// receiver is the same as the caller's receiver and there are no overriden
+// callee functions, then no class check is needed.
+bool FlowGraph::InstanceCallNeedsClassCheck(InstanceCallInstr* call,
+                                            RawFunction::Kind kind) const {
+  if (!FLAG_use_cha_deopt && !isolate()->all_classes_finalized()) {
+    // Even if class or function are private, lazy class finalization
+    // may later add overriding methods.
+    return true;
+  }
+  Definition* callee_receiver = call->ArgumentAt(0);
+  ASSERT(callee_receiver != NULL);
+  if (function().IsDynamicFunction() && IsReceiver(callee_receiver)) {
+    const String& name = (kind == RawFunction::kMethodExtractor)
+        ? String::Handle(zone(), Field::NameFromGetter(call->function_name()))
+        : call->function_name();
+    const Class& cls = Class::Handle(zone(), function().Owner());
+    if (!thread()->cha()->HasOverride(cls, name)) {
+      if (FLAG_trace_cha) {
+        THR_Print("  **(CHA) Instance call needs no check, "
+            "no overrides of '%s' '%s'\n",
+            name.ToCString(), cls.ToCString());
+      }
+      thread()->cha()->AddToLeafClasses(cls);
+      return false;
+    }
+  }
+  return true;
+}
+
+
+
 bool FlowGraph::VerifyUseLists() {
   // Verify the initial definitions.
   for (intptr_t i = 0; i < graph_entry_->initial_definitions()->length(); ++i) {