Skip to content

Commit

Permalink
[vm] Fix access to parameters in async closure body inside field init…
Browse files Browse the repository at this point in the history
…ializer

For async closure parsed_function_ could be an outer function
which is unrelated to the closure. So, ScopeBuilder should not
attempt to mark parameters of parsed_function_ with
set_is_forced_stack() if it sees kSyncYielding FunctionNode.
Parameter variables might not be even allocated if
async closure is used inside an instance field initializer
and parsed_function_ is a constructor.

TEST=runtime/tests/vm/dart/regress_45306_test.dart
Fixes #45306

Change-Id: I1b0082cb0e217039c43f19b35d77190493069edc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/191325
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
  • Loading branch information
alexmarkov authored and athomas committed Apr 15, 2021
1 parent 7c8c6b3 commit a2900a5
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 10 deletions.
33 changes: 33 additions & 0 deletions runtime/tests/vm/dart/regress_45306_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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/45306.
// Verifies that ScopeBuilder doesn't crash on an async closure inside
// instance field initializer.

class X {
late final Y y = Y(
() async {},
);

final double? a;
final double? b;
final String? c;

X({
this.a,
this.b,
this.c,
});
}

typedef Callback = Future<void> Function();

class Y {
Y(Callback? f);
}

void main() {
X();
}
33 changes: 33 additions & 0 deletions runtime/tests/vm/dart_2/regress_45306_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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/45306.
// Verifies that ScopeBuilder doesn't crash on an async closure inside
// instance field initializer.

class X {
final Y y = Y(
() async {},
);

final double a;
final double b;
final String c;

X({
this.a,
this.b,
this.c,
});
}

typedef Callback = Future<void> Function();

class Y {
Y(Callback f);
}

void main() {
X();
}
15 changes: 5 additions & 10 deletions runtime/vm/compiler/frontend/scope_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -546,15 +546,6 @@ void ScopeBuilder::VisitFunctionNode() {
}
function_node_helper.SetJustRead(FunctionNodeHelper::kTypeParameters);

// The :sync_op and :async_op continuations are called multiple times. So we
// don't want the parameters from the first invocation to get stored in the
// context and reused on later invocations with different parameters.
if (function_node_helper.async_marker_ == FunctionNodeHelper::kSyncYielding) {
for (intptr_t i = 0; i < function.NumParameters(); i++) {
parsed_function_->ParameterVariable(i)->set_is_forced_stack();
}
}

// Read (but don't visit) the positional and named parameters, because they've
// already been added to the scope.
function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kBody);
Expand Down Expand Up @@ -1537,7 +1528,11 @@ void ScopeBuilder::AddVariableDeclarationParameter(
if (helper.IsCovariant()) {
variable->set_is_explicit_covariant_parameter();
}
if (variable->name().ptr() == Symbols::IteratorParameter().ptr()) {

// The :sync_op and :async_op continuations are called multiple times. So we
// don't want the parameters from the first invocation to get stored in the
// context and reused on later invocations with different parameters.
if (current_function_async_marker_ == FunctionNodeHelper::kSyncYielding) {
variable->set_is_forced_stack();
}

Expand Down

0 comments on commit a2900a5

Please sign in to comment.