Skip to content

Commit

Permalink
[vm/compiler] Fix handling of captured try_finally_return_value variable
Browse files Browse the repository at this point in the history
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 #47610

Change-Id: Id15ea719ddda892eaff0b06f6450b1a8de36e8da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219283
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
  • Loading branch information
alexmarkov authored and commit-bot@chromium.org committed Nov 5, 2021
1 parent f8df8ca commit 9b7caf3
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 4 deletions.
19 changes: 15 additions & 4 deletions runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
32 changes: 32 additions & 0 deletions tests/language/async_star/regression_47610_test.dart
Original file line number Diff line number Diff line change
@@ -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<int> 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);
}
34 changes: 34 additions & 0 deletions tests/language_2/async_star/regression_47610_test.dart
Original file line number Diff line number Diff line change
@@ -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<int> 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);
}

0 comments on commit 9b7caf3

Please sign in to comment.