diff --git a/runtime/tests/vm/dart/regress_43464_test.dart b/runtime/tests/vm/dart/regress_43464_test.dart new file mode 100644 index 00000000000..43a3c1135bd --- /dev/null +++ b/runtime/tests/vm/dart/regress_43464_test.dart @@ -0,0 +1,21 @@ +// Copyright (c) 2020, 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. + +import 'package:expect/expect.dart'; + +class A {} + +abstract class B { + dynamic foo(T a); +} + +class C extends B { + dynamic foo(A a) { + return () => a; + } +} + +main() { + Expect.throws(() => (C().foo as dynamic)(1)); +} diff --git a/runtime/tests/vm/dart_2/regress_43464_test.dart b/runtime/tests/vm/dart_2/regress_43464_test.dart new file mode 100644 index 00000000000..43a3c1135bd --- /dev/null +++ b/runtime/tests/vm/dart_2/regress_43464_test.dart @@ -0,0 +1,21 @@ +// Copyright (c) 2020, 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. + +import 'package:expect/expect.dart'; + +class A {} + +abstract class B { + dynamic foo(T a); +} + +class C extends B { + dynamic foo(A a) { + return () => a; + } +} + +main() { + Expect.throws(() => (C().foo as dynamic)(1)); +} diff --git a/runtime/vm/compiler/backend/type_propagator.cc b/runtime/vm/compiler/backend/type_propagator.cc index 6c35a83e35b..3877f455625 100644 --- a/runtime/vm/compiler/backend/type_propagator.cc +++ b/runtime/vm/compiler/backend/type_propagator.cc @@ -1405,6 +1405,12 @@ CompileType StaticCallInstr::ComputeType() const { } CompileType LoadLocalInstr::ComputeType() const { + if (local().needs_covariant_check_in_method()) { + // We may not yet have checked the actual type of the parameter value. + // Assuming that the value has the required type can lead to unsound + // optimizations. See dartbug.com/43464. + return CompileType::FromCid(kDynamicCid); + } const AbstractType& local_type = local().type(); TraceStrongModeType(this, local_type); return CompileType::FromAbstractType(local_type); diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc index e736c7ad4be..b859d993580 100644 --- a/runtime/vm/compiler/frontend/scope_builder.cc +++ b/runtime/vm/compiler/frontend/scope_builder.cc @@ -1601,6 +1601,9 @@ void ScopeBuilder::AddVariableDeclarationParameter( helper.IsCovariant() || (helper.IsGenericCovariantImpl() && (attrs.has_non_this_uses || attrs.has_tearoff_uses)); + if (needs_covariant_check_in_method) { + variable->set_needs_covariant_check_in_method(); + } switch (type_check_mode) { case kTypeCheckAllParameters: diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc index 9f873d951ad..9902dc644a6 100644 --- a/runtime/vm/parser.cc +++ b/runtime/vm/parser.cc @@ -211,6 +211,9 @@ void ParsedFunction::AllocateVariables() { if (variable->is_explicit_covariant_parameter()) { raw_parameter->set_is_explicit_covariant_parameter(); } + if (variable->needs_covariant_check_in_method()) { + raw_parameter->set_needs_covariant_check_in_method(); + } raw_parameter->set_type_check_mode(variable->type_check_mode()); if (function().HasOptionalParameters()) { bool ok = scope->AddVariable(raw_parameter); diff --git a/runtime/vm/scopes.h b/runtime/vm/scopes.h index a9761d7b336..b0033246eba 100644 --- a/runtime/vm/scopes.h +++ b/runtime/vm/scopes.h @@ -91,7 +91,7 @@ class LocalVariable : public ZoneAllocated { is_invisible_(false), is_captured_parameter_(false), is_forced_stack_(false), - is_explicit_covariant_parameter_(false), + covariance_mode_(kNotCovariant), is_late_(false), is_chained_future_(false), expected_context_index_(-1), @@ -147,10 +147,17 @@ class LocalVariable : public ZoneAllocated { } bool is_explicit_covariant_parameter() const { - return is_explicit_covariant_parameter_; + return covariance_mode_ == kExplicit; } - void set_is_explicit_covariant_parameter() { - is_explicit_covariant_parameter_ = true; + void set_is_explicit_covariant_parameter() { covariance_mode_ = kExplicit; } + + bool needs_covariant_check_in_method() const { + return covariance_mode_ != kNotCovariant; + } + void set_needs_covariant_check_in_method() { + if (covariance_mode_ == kNotCovariant) { + covariance_mode_ = kImplicit; + } } enum TypeCheckMode { @@ -208,6 +215,12 @@ class LocalVariable : public ZoneAllocated { bool Equals(const LocalVariable& other) const; private: + enum CovarianceMode { + kNotCovariant, + kImplicit, + kExplicit, + }; + static const int kUninitializedIndex = INT_MIN; const TokenPosition declaration_pos_; @@ -228,7 +241,7 @@ class LocalVariable : public ZoneAllocated { bool is_invisible_; bool is_captured_parameter_; bool is_forced_stack_; - bool is_explicit_covariant_parameter_; + CovarianceMode covariance_mode_; bool is_late_; bool is_chained_future_; intptr_t expected_context_index_;