[vm/aot] Disable assert strengthening optimization in AOT

This optimization does not make sense in AOT and can actually cause
AOT compiler to segfault because it expects environment to be
attached to AssertAssignable instructions which is not always
the case on AOT compiler.

Fixes https://github.com/flutter/flutter/issues/76919

TEST=vm/cc/TypePropagator_RegressFlutter76919,vm/dart{,_2}/regress_flutter76919

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: I8f4314d7dac276833a9050bba835616b670a88d5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/188281
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
This commit is contained in:
Vyacheslav Egorov
2021-03-01 13:13:19 +00:00
committed by commit-bot@chromium.org
parent 1f828c7cb2
commit afa22dd1ab
5 changed files with 119 additions and 1 deletions
@@ -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<E, L extends List<E>> {
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<String> args) {
final v = C<int, Uint8List>(Uint8List(1));
v[0] = 1;
Expect.equals(1, v[0]);
}
@@ -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<E, L extends List<E>> {
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<String> args) {
final v = C<int, Uint8List>(Uint8List(1));
v[0] = 1;
Expect.equals(1, v[0]);
}
@@ -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);
@@ -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_;
@@ -491,4 +491,50 @@ class C<NoBound,
}
}
ISOLATE_UNIT_TEST_CASE(TypePropagator_RegressFlutter76919) {
CompilerState S(thread, /*is_aot=*/true, /*is_optimizing=*/true);
FlowGraphBuilderHelper H;
// Add a variable into the scope which would provide static type for the
// parameter.
LocalVariable* v0_var =
new LocalVariable(TokenPosition::kNoSource, TokenPosition::kNoSource,
String::Handle(Symbols::New(thread, "v0")),
AbstractType::ZoneHandle(Type::DynamicType()));
v0_var->set_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