[vm/compiler] Do not use call->ArgumentAt(0) to access receiver.
If call can have type arguments we can't use call->ArgumentAt(0) to
access receiver, instead we need to use call->ArgumentAt(call->FirstArgIndex()).
This lead to a bug in polymorphic inlining of functions with type
arguments - instead of loading class id from the receiver we would be
loading it from the first argument which contains type arguments.
This CL also cleans up other parts of the compiler that used ArgumentAt(0)
when they needed receiver.
Fixes https://github.com/dart-lang/protobuf/issues/95
Change-Id: I5504f7aff714894ff9fe580710c376f1d1933bfa
Reviewed-on: https://dart-review.googlesource.com/54411
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
diff --git a/runtime/vm/compiler/backend/flow_graph.cc b/runtime/vm/compiler/backend/flow_graph.cc
index 499bfc7..5411f90 100644
--- a/runtime/vm/compiler/backend/flow_graph.cc
+++ b/runtime/vm/compiler/backend/flow_graph.cc
@@ -426,7 +426,7 @@
// may later add overriding methods.
return true;
}
- Definition* callee_receiver = call->ArgumentAt(0);
+ Definition* callee_receiver = call->Receiver()->definition();
ASSERT(callee_receiver != NULL);
if (function().IsDynamicFunction() && IsReceiver(callee_receiver)) {
const String& name =
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
index c0d51f7..247a75b 100644
--- a/runtime/vm/compiler/backend/il.cc
+++ b/runtime/vm/compiler/backend/il.cc
@@ -3699,7 +3699,7 @@
}
Definition* InstanceCallInstr::Canonicalize(FlowGraph* flow_graph) {
- const intptr_t receiver_cid = PushArgumentAt(0)->value()->Type()->ToCid();
+ const intptr_t receiver_cid = Receiver()->Type()->ToCid();
// TODO(erikcorry): Even for cold call sites we could still try to look up
// methods when we know the receiver cid. We don't currently do this because
diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h
index 2fee1fc..a957568 100644
--- a/runtime/vm/compiler/backend/il.h
+++ b/runtime/vm/compiler/backend/il.h
@@ -3077,6 +3077,9 @@
}
intptr_t FirstArgIndex() const { return type_args_len_ > 0 ? 1 : 0; }
+ Value* Receiver() const {
+ return this->PushArgumentAt(FirstArgIndex())->value();
+ }
intptr_t ArgumentCountWithoutTypeArgs() const {
return arguments_->length() - FirstArgIndex();
}
@@ -3324,6 +3327,8 @@
}
intptr_t type_args_len() const { return instance_call()->type_args_len(); }
+ Value* Receiver() const { return instance_call()->Receiver(); }
+
bool HasOnlyDispatcherOrImplicitAccessorTargets() const;
const CallTargets& targets() const { return targets_; }
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
index e137fad..9e2bd30 100644
--- a/runtime/vm/compiler/backend/inliner.cc
+++ b/runtime/vm/compiler/backend/inliner.cc
@@ -1388,16 +1388,17 @@
// Find the closure of the callee.
ASSERT(call->ArgumentCount() > 0);
Function& target = Function::ZoneHandle();
- AllocateObjectInstr* alloc =
- call->ArgumentAt(0)->OriginalDefinition()->AsAllocateObject();
- if ((alloc != NULL) && !alloc->closure_function().IsNull()) {
- target ^= alloc->closure_function().raw();
- ASSERT(alloc->cls().IsClosureClass());
- }
- ConstantInstr* constant =
- call->ArgumentAt(0)->OriginalDefinition()->AsConstant();
- if ((constant != NULL) && constant->value().IsClosure()) {
- target ^= Closure::Cast(constant->value()).function();
+ Definition* receiver =
+ call->Receiver()->definition()->OriginalDefinition();
+ if (AllocateObjectInstr* alloc = receiver->AsAllocateObject()) {
+ if (!alloc->closure_function().IsNull()) {
+ target ^= alloc->closure_function().raw();
+ ASSERT(alloc->cls().IsClosureClass());
+ }
+ } else if (ConstantInstr* constant = receiver->AsConstant()) {
+ if (constant->value().IsClosure()) {
+ target ^= Closure::Cast(constant->value()).function();
+ }
}
if (target.IsNull()) {
@@ -1712,7 +1713,7 @@
// Replace the receiver argument with a redefinition to prevent code from
// the inlined body from being hoisted above the inlined entry.
GrowableArray<Definition*> arguments(call_->ArgumentCount());
- Definition* receiver = call_->ArgumentAt(0);
+ Definition* receiver = call_->Receiver()->definition();
RedefinitionInstr* redefinition =
new (Z) RedefinitionInstr(new (Z) Value(receiver));
redefinition->set_ssa_temp_index(
@@ -1768,7 +1769,7 @@
BlockEntryInstr* current_block = entry;
Instruction* cursor = entry;
- Definition* receiver = call_->ArgumentAt(0);
+ Definition* receiver = call_->Receiver()->definition();
// There are at least two variants including non-inlined ones, so we have
// at least one branch on the class id.
LoadClassIdInstr* load_cid =
@@ -2930,13 +2931,14 @@
TargetEntryInstr* entry;
Definition* last;
if (FlowGraphInliner::TryInlineRecognizedMethod(
- flow_graph, receiver_cid, target, call, call->ArgumentAt(0),
- call->token_pos(), call->ic_data(), &entry, &last, policy)) {
+ flow_graph, receiver_cid, target, call,
+ call->Receiver()->definition(), call->token_pos(), call->ic_data(),
+ &entry, &last, policy)) {
// Insert receiver class check if needed.
if (MethodRecognizer::PolymorphicTarget(target) ||
flow_graph->InstanceCallNeedsClassCheck(call, target.kind())) {
Instruction* check = flow_graph->CreateCheckClass(
- call->ArgumentAt(0), *Cids::Create(Z, *call->ic_data(), 0),
+ call->Receiver()->definition(), *Cids::Create(Z, *call->ic_data(), 0),
call->deopt_id(), call->token_pos());
flow_graph->InsertBefore(call, check, call->env(), FlowGraph::kEffect);
}
@@ -2980,9 +2982,8 @@
Definition* receiver = NULL;
intptr_t receiver_cid = kIllegalCid;
if (!call->function().is_static()) {
- receiver = call->ArgumentAt(call->FirstArgIndex());
- receiver_cid =
- call->PushArgumentAt(call->FirstArgIndex())->value()->Type()->ToCid();
+ receiver = call->Receiver()->definition();
+ receiver_cid = call->Receiver()->Type()->ToCid();
}
if (FlowGraphInliner::TryInlineRecognizedMethod(
flow_graph, receiver_cid, call->function(), call, receiver,
diff --git a/runtime/vm/compiler/backend/type_propagator.cc b/runtime/vm/compiler/backend/type_propagator.cc
index 76cf248..6b9c5bb 100644
--- a/runtime/vm/compiler/backend/type_propagator.cc
+++ b/runtime/vm/compiler/backend/type_propagator.cc
@@ -298,19 +298,22 @@
void FlowGraphTypePropagator::VisitInstanceCall(InstanceCallInstr* instr) {
if (instr->has_unique_selector()) {
- SetCid(instr->ArgumentAt(0), instr->ic_data()->GetReceiverClassIdAt(0));
+ SetCid(instr->Receiver()->definition(),
+ instr->ic_data()->GetReceiverClassIdAt(0));
return;
}
- CheckNonNullSelector(instr, instr->ArgumentAt(0), instr->function_name());
+ CheckNonNullSelector(instr, instr->Receiver()->definition(),
+ instr->function_name());
}
void FlowGraphTypePropagator::VisitPolymorphicInstanceCall(
PolymorphicInstanceCallInstr* instr) {
if (instr->instance_call()->has_unique_selector()) {
- SetCid(instr->ArgumentAt(0), instr->targets().MonomorphicReceiverCid());
+ SetCid(instr->Receiver()->definition(),
+ instr->targets().MonomorphicReceiverCid());
return;
}
- CheckNonNullSelector(instr, instr->ArgumentAt(0),
+ CheckNonNullSelector(instr, instr->Receiver()->definition(),
instr->instance_call()->function_name());
}
diff --git a/runtime/vm/compiler/call_specializer.cc b/runtime/vm/compiler/call_specializer.cc
index adfdeb3..637823d 100644
--- a/runtime/vm/compiler/call_specializer.cc
+++ b/runtime/vm/compiler/call_specializer.cc
@@ -284,8 +284,7 @@
return;
}
- const intptr_t receiver_cid =
- call->PushArgumentAt(0)->value()->Type()->ToCid();
+ const intptr_t receiver_cid = call->Receiver()->Type()->ToCid();
if (receiver_cid == kDynamicCid) {
return; // No information about receiver was infered.
}
diff --git a/tests/language_2/vm/regress_protobuf_95_test.dart b/tests/language_2/vm/regress_protobuf_95_test.dart
new file mode 100644
index 0000000..623aba3
--- /dev/null
+++ b/tests/language_2/vm/regress_protobuf_95_test.dart
@@ -0,0 +1,27 @@
+// Copyright (c) 2018, 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.
+
+// Verify that compiler loads class id from the receiver and not from type
+// arguments when performing polymorphic inlining in the AOT mode.
+
+import "package:expect/expect.dart";
+
+abstract class Base {
+ Type rareMethod<T>();
+}
+
+class A extends Base {
+ Type rareMethod<T>() => A;
+}
+
+class B extends Base {
+ Type rareMethod<T>() => B;
+}
+
+Type trampoline<T>(Base v) => v.rareMethod<T>();
+
+void main() {
+ Expect.equals(A, trampoline(new A()));
+ Expect.equals(B, trampoline(new B()));
+}
