From a2900a5f1665508da93fe88a9119ee5d77cf2190 Mon Sep 17 00:00:00 2001
From: Alexander Markov <alexmarkov@google.com>
Date: Wed, 17 Mar 2021 19:17:05 +0000
Subject: [PATCH] [vm] Fix access to parameters in async closure body inside
 field initializer

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 https://github.com/dart-lang/sdk/issues/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>
---
 runtime/tests/vm/dart/regress_45306_test.dart | 33 +++++++++++++++++++
 .../tests/vm/dart_2/regress_45306_test.dart   | 33 +++++++++++++++++++
 runtime/vm/compiler/frontend/scope_builder.cc | 15 +++------
 3 files changed, 71 insertions(+), 10 deletions(-)
 create mode 100644 runtime/tests/vm/dart/regress_45306_test.dart
 create mode 100644 runtime/tests/vm/dart_2/regress_45306_test.dart

diff --git a/runtime/tests/vm/dart/regress_45306_test.dart b/runtime/tests/vm/dart/regress_45306_test.dart
new file mode 100644
index 000000000000..2b5e3c04e72e
--- /dev/null
+++ b/runtime/tests/vm/dart/regress_45306_test.dart
@@ -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();
+}
diff --git a/runtime/tests/vm/dart_2/regress_45306_test.dart b/runtime/tests/vm/dart_2/regress_45306_test.dart
new file mode 100644
index 000000000000..36af440b1fa3
--- /dev/null
+++ b/runtime/tests/vm/dart_2/regress_45306_test.dart
@@ -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();
+}
diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc
index 11f7de3c6f6f..ce41bb4cb691 100644
--- a/runtime/vm/compiler/frontend/scope_builder.cc
+++ b/runtime/vm/compiler/frontend/scope_builder.cc
@@ -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);
@@ -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();
   }