Skip to content

Commit

Permalink
[vm/ffi] Fix constant Finalizables
Browse files Browse the repository at this point in the history
The kernel builder relied on only seeing expressions emitted by the
finalizable transform (variable get and this) as arguments to the fence.

However, other transforms can run later. In this case the expression
was turned into a constant.

This CL changes the implementation to accept any expression rather than
accepting a subset of expressions.

TEST=tests/ffi/regress_49402_test.dart

Closes: #49402
Change-Id: I1a962a5b7a38099eb5c3bbf5a5a8145b16727d97
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250744
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
  • Loading branch information
dcharkes authored and Commit Bot committed Jul 7, 2022
1 parent ae36599 commit d3ea8bf
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 22 deletions.
30 changes: 9 additions & 21 deletions runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1169,17 +1169,19 @@ Fragment StreamingFlowGraphBuilder::BuildStatementAt(intptr_t kernel_offset) {
return BuildStatement(); // read statement.
}

Fragment StreamingFlowGraphBuilder::BuildExpression(TokenPosition* position) {
Fragment StreamingFlowGraphBuilder::BuildExpression(
TokenPosition* position,
bool allow_late_uninitialized) {
++num_ast_nodes_;
uint8_t payload = 0;
Tag tag = ReadTag(&payload); // read tag.
switch (tag) {
case kInvalidExpression:
return BuildInvalidExpression(position);
case kVariableGet:
return BuildVariableGet(position);
return BuildVariableGet(position, allow_late_uninitialized);
case kSpecializedVariableGet:
return BuildVariableGet(payload, position);
return BuildVariableGet(payload, position, allow_late_uninitialized);
case kVariableSet:
return BuildVariableSet(position);
case kSpecializedVariableSet:
Expand Down Expand Up @@ -5818,26 +5820,12 @@ Fragment StreamingFlowGraphBuilder::BuildReachabilityFence() {
ASSERT(positional_count == 1);

// The CFE transform only generates a subset of argument expressions:
// either variable get or `this`.
uint8_t payload = 0;
Tag tag = ReadTag(&payload);
// either variable get or `this`. However, subsequent transforms can
// generate different expressions, including: constant expressions.
// So, build an arbitrary expression here instead.
TokenPosition* position = nullptr;
const bool allow_late_uninitialized = true;
Fragment code;
switch (tag) {
case kVariableGet:
code = BuildVariableGet(position, allow_late_uninitialized);
break;
case kSpecializedVariableGet:
code = BuildVariableGet(payload, position, allow_late_uninitialized);
break;
case kThisExpression:
code = BuildThisExpression(position);
break;
default:
// The transformation should not be generating anything else.
FATAL1("Unexpected tag %i", tag);
}
Fragment code = BuildExpression(position, allow_late_uninitialized);

const intptr_t named_args_len = ReadListLength();
ASSERT(named_args_len == 0);
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/compiler/frontend/kernel_binary_flowgraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
Fragment BuildInitializers(const Class& parent_class);
FlowGraph* BuildGraphOfFunction(bool constructor);

Fragment BuildExpression(TokenPosition* position = nullptr);
Fragment BuildExpression(TokenPosition* position = nullptr,
bool allow_late_uninitialized = false);
Fragment BuildStatement(TokenPosition* position = nullptr);
Fragment BuildStatementWithBranchCoverage(TokenPosition* position = nullptr);

Expand Down
13 changes: 13 additions & 0 deletions tests/ffi/regress_49402_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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';

class A implements Finalizable {}

void b([A? a]) {}

void main() {
b();
}
15 changes: 15 additions & 0 deletions tests/ffi_2/regress_49402_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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.

// @dart=2.9

import 'dart:ffi';

class A implements Finalizable {}

void b([A a]) {}

void main() {
b();
}

0 comments on commit d3ea8bf

Please sign in to comment.