Skip to content

Commit

Permalink
Reland "[vm] Cleanup old async/async*/sync* implementation from the VM"
Browse files Browse the repository at this point in the history
This is a reland of commit bc8afad

On top of the original commit, this change fixes incorrect
propagation of async/async*/sync* modifiers from a function to
its dynamic invocation forwarder.

TEST=ci, runtime/tests/vm/dart/regress_b_238653741_test.dart
Fixes b/238653741
Issue: #48378

Original change's description:
> [vm] Cleanup old async/async*/sync* implementation from the VM
>
> TEST=ci
>
> Issue: #48378
> Change-Id: I089ba4ed5613f30eec29f0db4ac6d5d8fbffd185
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249980
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Reviewed-by: Slava Egorov <vegorov@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>

Change-Id: Iaad033d974a23fc6c5880a3d7f41818eb117f839
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251300
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
  • Loading branch information
alexmarkov authored and Commit Bot committed Jul 12, 2022
1 parent 91a9774 commit 61caeca
Show file tree
Hide file tree
Showing 28 changed files with 518 additions and 1,615 deletions.
25 changes: 25 additions & 0 deletions runtime/tests/vm/dart/regress_b_238653741_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) 2022, 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 b/238653741.
//
// Verifies that calling async function through dynamic invocation forwarder
// does not result in a runtime error.

import "package:expect/expect.dart";

class A {
Future<int> foo(int arg) async => arg + 3;
}

class B {
Future<int> foo(String arg) async => int.parse(arg);
}

List<dynamic> objects = [A(), B(), 42];

void main() async {
Expect.equals(7, await objects[0].foo(4));
Expect.equals(8, await objects[1].foo("8"));
}
2 changes: 1 addition & 1 deletion runtime/tests/vm/dart/regress_flutter51298_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// 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.

// VMOptions=--async_igoto_threshold=0 --optimization_counter_threshold=10 --deterministic
// VMOptions=--optimization_counter_threshold=10 --deterministic

// Regression test for https://github.com/flutter/flutter/issues/51298.
// This would cause a crash due to bad offsets causing entry to hit the pre-code
Expand Down
27 changes: 27 additions & 0 deletions runtime/tests/vm/dart_2/regress_b_238653741_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) 2022, 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 b/238653741.
//
// Verifies that calling async function through dynamic invocation forwarder
// does not result in a runtime error.

// @dart=2.9

import "package:expect/expect.dart";

class A {
Future<int> foo(int arg) async => arg + 3;
}

class B {
Future<int> foo(String arg) async => int.parse(arg);
}

List<dynamic> objects = [A(), B(), 42];

void main() async {
Expect.equals(7, await objects[0].foo(4));
Expect.equals(8, await objects[1].foo("8"));
}
2 changes: 1 addition & 1 deletion runtime/tests/vm/dart_2/regress_flutter51298_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// @dart = 2.9

// VMOptions=--async_igoto_threshold=0 --optimization_counter_threshold=10 --deterministic
// VMOptions=--optimization_counter_threshold=10 --deterministic

// Regression test for https://github.com/flutter/flutter/issues/51298.
// This would cause a crash due to bad offsets causing entry to hit the pre-code
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/code_descriptors.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ class ExceptionHandlerList : public ZoneAllocated {

explicit ExceptionHandlerList(const Function& function)
: list_(),
has_async_handler_(function.IsCompactAsyncFunction() ||
function.IsCompactAsyncStarFunction()) {}
has_async_handler_(function.IsAsyncFunction() ||
function.IsAsyncGenerator()) {}

intptr_t Length() const { return list_.length(); }

Expand Down
17 changes: 0 additions & 17 deletions runtime/vm/compiler/aot/precompiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,6 @@ void Precompiler::DoCompileAll() {
IG->object_store()->set_simple_instance_of_true_function(null_function);
IG->object_store()->set_simple_instance_of_false_function(
null_function);
IG->object_store()->set_async_star_move_next_helper(null_function);
IG->object_store()->set_complete_on_async_return(null_function);
IG->object_store()->set_async_star_stream_controller(null_class);
DropMetadata();
DropLibraryEntries();
Expand Down Expand Up @@ -1136,14 +1134,6 @@ void Precompiler::AddTypesOf(const Function& function) {
return;
}

// Preserve parents for generated bodies in async/async*/sync* functions,
// since predicates like Function::IsAsyncClosure(), etc. need that info.
if (function.is_generated_body()) {
AddRetainReason(parent_function, RetainReasons::kIsSyncAsyncFunction);
AddTypesOf(parent_function);
return;
}

// We're not retaining the parent due to this function, so wrap it with
// a weak serialization reference.
const auto& data = ClosureData::CheckedHandle(Z, function.data());
Expand Down Expand Up @@ -2983,10 +2973,6 @@ void Precompiler::DiscardCodeObjects() {
++codes_with_native_function_;
return;
}
if (function_.IsAsyncClosure() || function_.IsAsyncGenClosure()) {
++codes_with_async_closure_function_;
return;
}

// Retain Code objects corresponding to dynamically
// called functions.
Expand Down Expand Up @@ -3040,8 +3026,6 @@ void Precompiler::DiscardCodeObjects() {
codes_with_pc_descriptors_);
THR_Print(" %8" Pd " Codes with native functions\n",
codes_with_native_function_);
THR_Print(" %8" Pd " Codes with async closure functions\n",
codes_with_async_closure_function_);
THR_Print(" %8" Pd " Codes with dynamically called functions\n",
codes_with_dynamically_called_function_);
THR_Print(" %8" Pd " Codes with deferred functions\n",
Expand Down Expand Up @@ -3073,7 +3057,6 @@ void Precompiler::DiscardCodeObjects() {
intptr_t codes_with_exception_handlers_ = 0;
intptr_t codes_with_pc_descriptors_ = 0;
intptr_t codes_with_native_function_ = 0;
intptr_t codes_with_async_closure_function_ = 0;
intptr_t codes_with_dynamically_called_function_ = 0;
intptr_t codes_with_deferred_function_ = 0;
intptr_t codes_with_ffi_trampoline_function_ = 0;
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/compiler/backend/il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6837,7 +6837,7 @@ void RawStoreFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
const Code& ReturnInstr::GetReturnStub(FlowGraphCompiler* compiler) const {
const Function& function = compiler->parsed_function().function();
ASSERT(function.IsSuspendableFunction());
if (function.IsCompactAsyncFunction()) {
if (function.IsAsyncFunction()) {
if (!value()->Type()->CanBeFuture()) {
return Code::ZoneHandle(compiler->zone(),
compiler->isolate_group()
Expand All @@ -6847,11 +6847,11 @@ const Code& ReturnInstr::GetReturnStub(FlowGraphCompiler* compiler) const {
return Code::ZoneHandle(
compiler->zone(),
compiler->isolate_group()->object_store()->return_async_stub());
} else if (function.IsCompactAsyncStarFunction()) {
} else if (function.IsAsyncGenerator()) {
return Code::ZoneHandle(
compiler->zone(),
compiler->isolate_group()->object_store()->return_async_star_stub());
} else if (function.IsCompactSyncStarFunction()) {
} else if (function.IsSyncGenerator()) {
return Code::ZoneHandle(
compiler->zone(),
compiler->isolate_group()->object_store()->return_sync_star_stub());
Expand Down
Loading

0 comments on commit 61caeca

Please sign in to comment.