Skip to content

Commit

Permalink
[cfe/ffi] Only keep Finalizables alive after their declaration
Browse files Browse the repository at this point in the history
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: #49075
Change-Id: I1f07f0343d29c3efb3c63c0aa0e3f20338b5c653
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246520
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>
  • Loading branch information
dcharkes authored and whesse committed Jun 20, 2022
1 parent 3c6e251 commit 52c485c
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 2 deletions.
5 changes: 3 additions & 2 deletions pkg/vm/lib/transformations/ffi/finalizable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions pkg/vm/testcases/transformations/ffi/regress_49075.dart
Original file line number Diff line number Diff line change
@@ -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();
}
45 changes: 45 additions & 0 deletions pkg/vm/testcases/transformations/ffi/regress_49075.dart.expect
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 52c485c

Please sign in to comment.