diff --git a/runtime/tests/vm/dart/regress_flutter76919_test.dart b/runtime/tests/vm/dart/regress_flutter76919_test.dart new file mode 100644 index 00000000000..669641a9346 --- /dev/null +++ b/runtime/tests/vm/dart/regress_flutter76919_test.dart @@ -0,0 +1,32 @@ +// Copyright (c) 2021, 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. + +// Regression test for https://github.com/flutter/flutter/issues/76919 +// Verifies that we don't try to strengthen an environmentless assertion +// with a class check in AOT mode which crashes AOT compiler. + +import 'dart:typed_data'; +import 'package:expect/expect.dart'; + +class C> { + final L list; + + C(this.list); + + @pragma('vm:never-inline') + E operator [](int index) => list[index]; + + @pragma('vm:never-inline') + void operator []=(int index, E value) { + // We emit AssertAssignable(value, E) on entry. + // Speculative compilation of this line produces CheckSmi(value) + list[index] = value; + } +} + +void main(List args) { + final v = C(Uint8List(1)); + v[0] = 1; + Expect.equals(1, v[0]); +} diff --git a/runtime/tests/vm/dart_2/regress_flutter76919_test.dart b/runtime/tests/vm/dart_2/regress_flutter76919_test.dart new file mode 100644 index 00000000000..669641a9346 --- /dev/null +++ b/runtime/tests/vm/dart_2/regress_flutter76919_test.dart @@ -0,0 +1,32 @@ +// Copyright (c) 2021, 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. + +// Regression test for https://github.com/flutter/flutter/issues/76919 +// Verifies that we don't try to strengthen an environmentless assertion +// with a class check in AOT mode which crashes AOT compiler. + +import 'dart:typed_data'; +import 'package:expect/expect.dart'; + +class C> { + final L list; + + C(this.list); + + @pragma('vm:never-inline') + E operator [](int index) => list[index]; + + @pragma('vm:never-inline') + void operator []=(int index, E value) { + // We emit AssertAssignable(value, E) on entry. + // Speculative compilation of this line produces CheckSmi(value) + list[index] = value; + } +} + +void main(List args) { + final v = C(Uint8List(1)); + v[0] = 1; + Expect.equals(1, v[0]); +} diff --git a/runtime/vm/compiler/backend/type_propagator.cc b/runtime/vm/compiler/backend/type_propagator.cc index fd01d698402..dce6e906514 100644 --- a/runtime/vm/compiler/backend/type_propagator.cc +++ b/runtime/vm/compiler/backend/type_propagator.cc @@ -48,6 +48,7 @@ void FlowGraphTypePropagator::Propagate(FlowGraph* flow_graph) { FlowGraphTypePropagator::FlowGraphTypePropagator(FlowGraph* flow_graph) : FlowGraphVisitor(flow_graph->reverse_postorder()), flow_graph_(flow_graph), + is_aot_(CompilerState::Current().is_aot()), visited_blocks_(new (flow_graph->zone()) BitVector(flow_graph->zone(), flow_graph->reverse_postorder().length())), @@ -119,7 +120,13 @@ void FlowGraphTypePropagator::PropagateRecursive(BlockEntryInstr* block) { const intptr_t rollback_point = rollback_.length(); - StrengthenAsserts(block); + if (!is_aot_) { + // Don't try to strengthen asserts with class checks in AOT mode, this is a + // speculative optimization which only really makes sense in JIT mode. + // It is also written to expect environments to be attached to + // AssertAssignable instructions, which is not always a case in AOT mode. + StrengthenAsserts(block); + } block->Accept(this); diff --git a/runtime/vm/compiler/backend/type_propagator.h b/runtime/vm/compiler/backend/type_propagator.h index 5c274206c20..435ef4a4f1c 100644 --- a/runtime/vm/compiler/backend/type_propagator.h +++ b/runtime/vm/compiler/backend/type_propagator.h @@ -77,6 +77,7 @@ class FlowGraphTypePropagator : public FlowGraphVisitor { Zone* zone() const { return flow_graph_->zone(); } FlowGraph* flow_graph_; + const bool is_aot_; BitVector* visited_blocks_; diff --git a/runtime/vm/compiler/backend/type_propagator_test.cc b/runtime/vm/compiler/backend/type_propagator_test.cc index 1090bc9cd90..05620b4ec36 100644 --- a/runtime/vm/compiler/backend/type_propagator_test.cc +++ b/runtime/vm/compiler/backend/type_propagator_test.cc @@ -491,4 +491,50 @@ class Cset_type_check_mode(LocalVariable::kTypeCheckedByCaller); + H.flow_graph()->parsed_function().scope()->AddVariable(v0_var); + + auto normal_entry = H.flow_graph()->graph_entry()->normal_entry(); + + // We are going to build the following graph: + // + // B0[graph_entry]: + // B1[function_entry]: + // v0 <- Parameter(0) + // AssertAssignable(v0, 'int') + // CheckSmi(v0) + // Return(v0) + + { + BlockBuilder builder(H.flow_graph(), normal_entry); + Definition* v0 = builder.AddParameter(0, 0, /*with_frame=*/true, kTagged); + auto null_value = builder.AddNullDefinition(); + builder.AddDefinition(new AssertAssignableInstr( + InstructionSource(), new Value(v0), + new Value( + H.flow_graph()->GetConstant(Type::ZoneHandle(Type::IntType()))), + new Value(null_value), new Value(null_value), Symbols::Value(), + S.GetNextDeoptId())); + builder.AddInstruction(new CheckSmiInstr(new Value(v0), S.GetNextDeoptId(), + InstructionSource())); + builder.AddReturn(new Value(v0)); + } + + H.FinishGraph(); + + H.flow_graph()->EliminateEnvironments(); + FlowGraphTypePropagator::Propagate(H.flow_graph()); +} + } // namespace dart