From 9b7caf3b93856838dd51053effec3ff37d2c69a4 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Fri, 5 Nov 2021 16:24:27 +0000 Subject: [PATCH] [vm/compiler] Fix handling of captured try_finally_return_value variable In async/async* methods synthetic :try_finally_return_value variable (which is used to hold return value across finally) could be captured, so flow graph builder needs to have correct context_depth_ in order to access it using LoadLocal(finally_return_variable) when building flow graph for return statement. Previously, flow graph builder left context in an unspecified state and depth after TranslateFinallyFinalizers(NULL, -1), which caused incorrect code being generated for LoadLocal(finally_return_variable). This change fixes this problem by * passing correct target_context_depth to TranslateFinallyFinalizers so context is adjusted to a known depth regardless of context depth which is used by finally; * setting context_depth_ for LoadLocal(finally_return_variable) and then restoring it (to be able to continue building flow graph for the enclosing AST nodes). TEST=tests/language/async_star/regression_47610_test.dart Fixes https://github.com/dart-lang/sdk/issues/47610 Change-Id: Id15ea719ddda892eaff0b06f6450b1a8de36e8da Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219283 Reviewed-by: Martin Kustermann Reviewed-by: Slava Egorov Commit-Queue: Alexander Markov --- .../frontend/kernel_binary_flowgraph.cc | 19 ++++++++--- .../async_star/regression_47610_test.dart | 32 +++++++++++++++++ .../async_star/regression_47610_test.dart | 34 +++++++++++++++++++ 3 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/language/async_star/regression_47610_test.dart create mode 100644 tests/language_2/async_star/regression_47610_test.dart diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index 798174df4d7..6bf2d87a0ba 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -4869,17 +4869,28 @@ Fragment StreamingFlowGraphBuilder::BuildReturnStatement() { if (instructions.is_open()) { if (inside_try_finally) { - ASSERT(scopes()->finally_return_variable != NULL); + LocalVariable* const finally_return_variable = + scopes()->finally_return_variable; + ASSERT(finally_return_variable != nullptr); const Function& function = parsed_function()->function(); if (NeedsDebugStepCheck(function, position)) { instructions += DebugStepCheck(position); } - instructions += StoreLocal(position, scopes()->finally_return_variable); + instructions += StoreLocal(position, finally_return_variable); instructions += Drop(); - instructions += TranslateFinallyFinalizers(NULL, -1); + const intptr_t target_context_depth = + finally_return_variable->is_captured() + ? finally_return_variable->owner()->context_level() + : -1; + instructions += TranslateFinallyFinalizers(nullptr, target_context_depth); if (instructions.is_open()) { - instructions += LoadLocal(scopes()->finally_return_variable); + const intptr_t saved_context_depth = B->context_depth_; + if (finally_return_variable->is_captured()) { + B->context_depth_ = target_context_depth; + } + instructions += LoadLocal(finally_return_variable); instructions += Return(TokenPosition::kNoSource); + B->context_depth_ = saved_context_depth; } } else { instructions += Return(position); diff --git a/tests/language/async_star/regression_47610_test.dart b/tests/language/async_star/regression_47610_test.dart new file mode 100644 index 00000000000..9bb4ce23587 --- /dev/null +++ b/tests/language/async_star/regression_47610_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/dart-lang/sdk/issues/47610. +// Tests returning value from a deep context depth along with +// breaking from 'await for'. + +import "dart:async"; +import "package:expect/expect.dart"; +import "package:async_helper/async_helper.dart"; + +Stream foo() async* { + for (int i = 0; i < 2; ++i) { + for (int j = 0; j < 2; ++j) { + for (int k = 0; k < 2; ++k) { + yield i + j + k; + } + } + } +} + +void test() async { + await for (var x in foo()) { + Expect.equals(0, x); + break; + } +} + +void main() { + asyncTest(test); +} diff --git a/tests/language_2/async_star/regression_47610_test.dart b/tests/language_2/async_star/regression_47610_test.dart new file mode 100644 index 00000000000..4fd78150f90 --- /dev/null +++ b/tests/language_2/async_star/regression_47610_test.dart @@ -0,0 +1,34 @@ +// 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. + +// @dart = 2.9 + +// Regression test for https://github.com/dart-lang/sdk/issues/47610. +// Tests returning value from a deep context depth along with +// breaking from 'await for'. + +import "dart:async"; +import "package:expect/expect.dart"; +import "package:async_helper/async_helper.dart"; + +Stream foo() async* { + for (int i = 0; i < 2; ++i) { + for (int j = 0; j < 2; ++j) { + for (int k = 0; k < 2; ++k) { + yield i + j + k; + } + } + } +} + +void test() async { + await for (var x in foo()) { + Expect.equals(0, x); + break; + } +} + +void main() { + asyncTest(test); +}