From 52c485c2e9db72447ed41f9bf6768d708a5dc3cd Mon Sep 17 00:00:00 2001
From: Daco Harkes <dacoharkes@google.com>
Date: Tue, 31 May 2022 08:13:17 +0000
Subject: [PATCH] [cfe/ffi] Only keep `Finalizable`s alive after their
 declaration

We were adding fences around the variable declaration expressions
including the variable that was not declared. This led to verification
errors and crashes when not running the verifier.
`Verification error: Variable 'VariableDeclarationImpl(final MyFinalizable myFinalizable)' used out of scope.`

TEST=pkg/vm/testcases/transformations/ffi/regress_49075.dart

Closes: https://github.com/dart-lang/sdk/issues/49075
Change-Id: I1f07f0343d29c3efb3c63c0aa0e3f20338b5c653
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246520
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
---
 .../lib/transformations/ffi/finalizable.dart  |  5 ++-
 .../transformations/ffi/regress_49075.dart    | 14 ++++++
 .../ffi/regress_49075.dart.expect             | 45 +++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 pkg/vm/testcases/transformations/ffi/regress_49075.dart
 create mode 100644 pkg/vm/testcases/transformations/ffi/regress_49075.dart.expect

diff --git a/pkg/vm/lib/transformations/ffi/finalizable.dart b/pkg/vm/lib/transformations/ffi/finalizable.dart
index 84b146967a3c..4fe96f3b6f9b 100644
--- a/pkg/vm/lib/transformations/ffi/finalizable.dart
+++ b/pkg/vm/lib/transformations/ffi/finalizable.dart
@@ -220,14 +220,15 @@ mixin FinalizableTransformer on Transformer {
 
   @override
   TreeNode visitVariableDeclaration(VariableDeclaration node) {
+    node = super.visitVariableDeclaration(node) as VariableDeclaration;
     if (_currentScope == null) {
       // Global variable.
-      return super.visitVariableDeclaration(node);
+      return node;
     }
     if (_isFinalizable(node.type)) {
       _currentScope!.addDeclaration(node);
     }
-    return super.visitVariableDeclaration(node);
+    return node;
   }
 
   @override
diff --git a/pkg/vm/testcases/transformations/ffi/regress_49075.dart b/pkg/vm/testcases/transformations/ffi/regress_49075.dart
new file mode 100644
index 000000000000..0046fd84be77
--- /dev/null
+++ b/pkg/vm/testcases/transformations/ffi/regress_49075.dart
@@ -0,0 +1,14 @@
+// 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.
+
+import 'dart:ffi';
+
+Future<void> main(List<String> arguments) async {
+  // ignore: unused_local_variable
+  final myFinalizable = await MyFinalizable();
+}
+
+class MyFinalizable implements Finalizable {
+  MyFinalizable();
+}
diff --git a/pkg/vm/testcases/transformations/ffi/regress_49075.dart.expect b/pkg/vm/testcases/transformations/ffi/regress_49075.dart.expect
new file mode 100644
index 000000000000..2974867f0ab0
--- /dev/null
+++ b/pkg/vm/testcases/transformations/ffi/regress_49075.dart.expect
@@ -0,0 +1,45 @@
+library #lib /*isNonNullableByDefault*/;
+import self as self;
+import "dart:core" as core;
+import "dart:ffi" as ffi;
+import "dart:_internal" as _in;
+import "dart:async" as asy;
+
+import "dart:ffi";
+
+class MyFinalizable extends core::Object implements ffi::Finalizable {
+  constructor •() → self::MyFinalizable
+    : super core::Object::•() {
+    ;
+    _in::reachabilityFence(this);
+  }
+}
+static method main(core::List<core::String> arguments) → asy::Future<void> /* futureValueType= void */ /* originally async */ {
+  final asy::_Future<void> :async_future = new asy::_Future::•<void>();
+  core::bool* :is_sync = false;
+  void :return_value;
+  (dynamic) → dynamic :async_op_then;
+  (core::Object, core::StackTrace) → dynamic :async_op_error;
+  core::int :await_jump_var = 0;
+  dynamic :await_ctx_var;
+  dynamic :saved_try_context_var0;
+  function :async_op(dynamic :result_or_exception, dynamic :stack_trace) → dynamic yielding 
+    try {
+      #L1:
+      {
+        [yield] let dynamic #t1 = asy::_awaitHelper(new self::MyFinalizable::•(), :async_op_then, :async_op_error) in null;
+        final self::MyFinalizable myFinalizable = _in::unsafeCast<self::MyFinalizable>(:result_or_exception);
+        _in::reachabilityFence(myFinalizable);
+      }
+      asy::_completeWithNoFutureOnAsyncReturn(:async_future, :return_value, :is_sync);
+      return;
+    }
+    on dynamic catch(dynamic exception, core::StackTrace stack_trace) {
+      asy::_completeOnAsyncError(:async_future, exception, stack_trace, :is_sync);
+    }
+  :async_op_then = asy::_asyncThenWrapperHelper(:async_op);
+  :async_op_error = asy::_asyncErrorWrapperHelper(:async_op);
+  :async_op(null, null){() → dynamic};
+  :is_sync = true;
+  return :async_future;
+}