Skip to content

Commit

Permalink
[vm] Implement NativeFinalizer
Browse files Browse the repository at this point in the history
This CL implements `NativeFinalizer` in the GC.

`FinalizerEntry`s are extended to track `external_size` and in which
`Heap::Space` the finalizable value is.

On attaching a native finalizer, the external size is added to the
relevant heap. When the finalizable value is promoted from new to old
space, the external size is promoted as well. And when a native
finalizer is run or is detached, the external size is removed from the
relevant heap again.

In contrast to Dart `Finalizer`s, `NativeFinalizer`s are run on isolate
shutdown.

When the `NativeFinalizer`s themselves are collected, the finalizers are
not run. Users should stick the native finalizer in a global variable to
ensure finalization. We will revisit this design when we add send and
exit support, because there is a design space to explore what to do in
that case. This current solution promises the least to users.

In this implementation native finalizers have a Dart entry to clean up
the entries from the `all_entries` field of the finalizer. We should
consider using another data structure that avoids the need for this Dart
entry. See the TODO left in the code.

Bug: #47777

TEST=runtime/tests/vm/dart(_2)/isolates/fast_object_copy_test.dart
TEST=runtime/vm/object_test.cc
TEST=tests/ffi(_2)/vmspecific_native_finalizer_*

Change-Id: I8f594c80c3c344ad83e1f2de10de028eb8456121
Cq-Include-Trybots: luci.dart.try:vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-ffi-android-debug-arm64c-try,dart-sdk-mac-arm64-try,vm-kernel-mac-release-arm64-try,pkg-mac-release-arm64-try,vm-kernel-precomp-nnbd-mac-release-arm64-try,vm-kernel-win-debug-x64c-try,vm-kernel-win-debug-x64-try,vm-kernel-precomp-win-debug-x64c-try,vm-kernel-nnbd-win-release-ia32-try,vm-ffi-android-debug-arm-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-mac-debug-x64-try,vm-kernel-nnbd-mac-debug-x64-try,vm-kernel-nnbd-linux-debug-ia32-try,benchmark-linux-try,flutter-frontend-try,pkg-linux-debug-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-gcc-linux-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64c-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/236320
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
  • Loading branch information
dcharkes authored and Commit Bot committed Mar 26, 2022
1 parent e5f118d commit 532c116
Show file tree
Hide file tree
Showing 81 changed files with 2,450 additions and 392 deletions.
30 changes: 30 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4083,6 +4083,36 @@ const MessageCode messageFfiAbiSpecificIntegerMappingInvalid = const MessageCode
problemMessage:
r"""Classes extending 'AbiSpecificInteger' must have exactly one 'AbiSpecificIntegerMapping' annotation specifying the mapping from ABI to a NativeType integer with a fixed size.""");

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String string, String name)>
templateFfiCompoundImplementsFinalizable =
const Template<Message Function(String string, String name)>(
problemMessageTemplate:
r"""#string '#name' can't implement Finalizable.""",
correctionMessageTemplate:
r"""Try removing the implements clause from '#name'.""",
withArguments: _withArgumentsFfiCompoundImplementsFinalizable);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String string, String name)>
codeFfiCompoundImplementsFinalizable =
const Code<Message Function(String string, String name)>(
"FfiCompoundImplementsFinalizable",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsFfiCompoundImplementsFinalizable(
String string, String name) {
if (string.isEmpty) throw 'No string provided';
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
return new Message(codeFfiCompoundImplementsFinalizable,
problemMessage: """${string} '${name}' can't implement Finalizable.""",
correctionMessage:
"""Try removing the implements clause from '${name}'.""",
arguments: {'string': string, 'name': name});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<
Message Function(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,8 @@ FfiCode.ANNOTATION_ON_POINTER_FIELD:
status: needsEvaluation
FfiCode.ARGUMENT_MUST_BE_A_CONSTANT:
status: needsEvaluation
FfiCode.COMPOUND_IMPLEMENTS_FINALIZABLE:
status: needsEvaluation
FfiCode.CREATION_OF_STRUCT_OR_UNION:
status: needsEvaluation
since: ~2.15
Expand Down
1 change: 1 addition & 0 deletions pkg/analyzer/lib/error/error.dart
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ const List<ErrorCode> errorCodeValues = [
FfiCode.ABI_SPECIFIC_INTEGER_MAPPING_UNSUPPORTED,
FfiCode.ANNOTATION_ON_POINTER_FIELD,
FfiCode.ARGUMENT_MUST_BE_A_CONSTANT,
FfiCode.COMPOUND_IMPLEMENTS_FINALIZABLE,
FfiCode.CREATION_OF_STRUCT_OR_UNION,
FfiCode.EMPTY_STRUCT,
FfiCode.EXTRA_ANNOTATION_ON_STRUCT_FIELD,
Expand Down
42 changes: 42 additions & 0 deletions pkg/analyzer/lib/src/dart/error/ffi_code.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,48 @@ class FfiCode extends AnalyzerErrorCode {
hasPublishedDocs: true,
);

/**
* Parameters:
* 0: the name of the struct or union class
*/
// #### Description
//
// The analyzer produces this diagnostic when a subclass of either `Struct`
// or `Union` implements `Finalizable`.
//
// For more information about FFI, see [C interop using dart:ffi][].
//
// #### Example
//
// The following code produces this diagnostic because the class `S`
// implements `Finalizable`:
//
// ```dart
// import 'dart:ffi';
//
// class [!S!] extends Struct implements Finalizable {
// external Pointer notEmpty;
// }
// ```
//
// #### Common fixes
//
// Try removing the implements clause from the class:
//
// ```dart
// import 'dart:ffi';
//
// class S extends Struct {
// external Pointer notEmpty;
// }
// ```
static const FfiCode COMPOUND_IMPLEMENTS_FINALIZABLE = FfiCode(
'COMPOUND_IMPLEMENTS_FINALIZABLE',
"The class '{0}' can't implement Finalizable.",
correctionMessage: "Try removing the implements clause from '{0}'.",
hasPublishedDocs: true,
);

/**
* No parameters.
*/
Expand Down
22 changes: 19 additions & 3 deletions pkg/analyzer/lib/src/generated/ffi_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,25 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}
}

if (inCompound && node.declaredElement!.typeParameters.isNotEmpty) {
_errorReporter.reportErrorForNode(
FfiCode.GENERIC_STRUCT_SUBCLASS, node.name, [node.name.name]);
if (inCompound) {
if (node.declaredElement!.typeParameters.isNotEmpty) {
_errorReporter.reportErrorForNode(
FfiCode.GENERIC_STRUCT_SUBCLASS, node.name, [node.name.name]);
}
final implementsClause = node.implementsClause;
if (implementsClause != null) {
final compoundType = node.declaredElement!.thisType;
final structType = compoundType.superclass!;
final ffiLibrary = structType.element.library;
final finalizableElement = ffiLibrary.getType(_finalizableClassName)!;
final finalizableType = finalizableElement.thisType;
if (typeSystem.isSubtypeOf(compoundType, finalizableType)) {
_errorReporter.reportErrorForNode(
FfiCode.COMPOUND_IMPLEMENTS_FINALIZABLE,
node.name,
[node.name.name]);
}
}
}
super.visitClassDeclaration(node);
}
Expand Down
39 changes: 39 additions & 0 deletions pkg/analyzer/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15689,6 +15689,45 @@ FfiCode:
return p.asFunction(isLeaf: true);
}
```
COMPOUND_IMPLEMENTS_FINALIZABLE:
problemMessage: "The class '{0}' can't implement Finalizable."
correctionMessage: "Try removing the implements clause from '{0}'."
comment: |-
Parameters:
0: the name of the struct or union class
hasPublishedDocs: true
documentation: |-
#### Description

The analyzer produces this diagnostic when a subclass of either `Struct`
or `Union` implements `Finalizable`.

For more information about FFI, see [C interop using dart:ffi][].

#### Example

The following code produces this diagnostic because the class `S`
implements `Finalizable`:

```dart
import 'dart:ffi';

class [!S!] extends Struct implements Finalizable {
external Pointer notEmpty;
}
```

#### Common fixes

Try removing the implements clause from the class:

```dart
import 'dart:ffi';

class S extends Struct {
external Pointer notEmpty;
}
```
CREATION_OF_STRUCT_OR_UNION:
problemMessage: "Subclasses of 'Struct' and 'Union' are backed by native memory, and can't be instantiated by a generative constructor."
correctionMessage: "Try allocating it via allocation, or load from a 'Pointer'."
Expand Down
36 changes: 36 additions & 0 deletions pkg/analyzer/tool/diagnostics/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -1992,6 +1992,42 @@ suitable value:
var l = const [0];
{% endprettify %}

### compound_implements_finalizable

_The class '{0}' can't implement Finalizable._

#### Description

The analyzer produces this diagnostic when a subclass of either `Struct`
or `Union` implements `Finalizable`.

For more information about FFI, see [C interop using dart:ffi][].

#### Example

The following code produces this diagnostic because the class `S`
implements `Finalizable`:

{% prettify dart tag=pre+code %}
import 'dart:ffi';

class [!S!] extends Struct implements Finalizable {
external Pointer notEmpty;
}
{% endprettify %}

#### Common fixes

Try removing the implements clause from the class:

{% prettify dart tag=pre+code %}
import 'dart:ffi';

class S extends Struct {
external Pointer notEmpty;
}
{% endprettify %}

### concrete_class_has_enum_superinterface

_Concrete classes can't have 'Enum' as a superinterface._
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/lib/src/api_unstable/vm.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export '../fasta/fasta_codes.dart'
noLength,
templateCantHaveNamedParameters,
templateCantHaveOptionalParameters,
templateFfiCompoundImplementsFinalizable,
templateFfiDartTypeMismatch,
templateFfiEmptyStruct,
templateFfiExpectedConstantArg,
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/messages.status
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ FastaUsageShort/analyzerCode: Fail
FastaUsageShort/example: Fail
FfiAbiSpecificIntegerInvalid/analyzerCode: Fail
FfiAbiSpecificIntegerMappingInvalid/analyzerCode: Fail
FfiCompoundImplementsFinalizable/analyzerCode: Fail
FfiDartTypeMismatch/analyzerCode: Fail
FfiEmptyStruct/analyzerCode: Fail
FfiExceptionalReturnNull/analyzerCode: Fail
Expand Down
6 changes: 6 additions & 0 deletions pkg/front_end/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4690,6 +4690,12 @@ FfiStructGeneric:
problemMessage: "#string '#name' should not be generic."
external: test/ffi_test.dart

FfiCompoundImplementsFinalizable:
# Used by dart:ffi
problemMessage: "#string '#name' can't implement Finalizable."
correctionMessage: "Try removing the implements clause from '#name'."
external: test/ffi_test.dart

FfiDartTypeMismatch:
# Used by dart:ffi
problemMessage: "Expected '#type' to be a subtype of '#type2'."
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/test/spell_checking_list_common.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,7 @@ filter
filtered
filtering
final
finalizable
finalization
finalize
finalized
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ additionalExports = (ffi::nullptr,
ffi::unsized,
ffi::sizeOf,
ffi::Dart_NativeMessageHandler,
ffi::NativeFinalizerFunction,
ffi::Abi,
ffi::AbiSpecificInteger,
ffi::AbiSpecificIntegerArray,
Expand Down Expand Up @@ -81,6 +82,7 @@ additionalExports = (ffi::nullptr,
ffi::Long,
ffi::LongLong,
ffi::NativeApi,
ffi::NativeFinalizer,
ffi::NativeFunction,
ffi::NativeFunctionPointer,
ffi::NativePort,
Expand Down Expand Up @@ -129,6 +131,7 @@ additionalExports = (ffi::nullptr,
ffi::unsized,
ffi::sizeOf,
ffi::Dart_NativeMessageHandler,
ffi::NativeFinalizerFunction,
ffi::Abi,
ffi::AbiSpecificInteger,
ffi::AbiSpecificIntegerArray,
Expand Down Expand Up @@ -171,6 +174,7 @@ additionalExports = (ffi::nullptr,
ffi::Long,
ffi::LongLong,
ffi::NativeApi,
ffi::NativeFinalizer,
ffi::NativeFunction,
ffi::NativeFunctionPointer,
ffi::NativePort,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ additionalExports = (ffi::nullptr,
ffi::unsized,
ffi::sizeOf,
ffi::Dart_NativeMessageHandler,
ffi::NativeFinalizerFunction,
ffi::Abi,
ffi::AbiSpecificInteger,
ffi::AbiSpecificIntegerArray,
Expand Down Expand Up @@ -81,6 +82,7 @@ additionalExports = (ffi::nullptr,
ffi::Long,
ffi::LongLong,
ffi::NativeApi,
ffi::NativeFinalizer,
ffi::NativeFunction,
ffi::NativeFunctionPointer,
ffi::NativePort,
Expand Down Expand Up @@ -129,6 +131,7 @@ additionalExports = (ffi::nullptr,
ffi::unsized,
ffi::sizeOf,
ffi::Dart_NativeMessageHandler,
ffi::NativeFinalizerFunction,
ffi::Abi,
ffi::AbiSpecificInteger,
ffi::AbiSpecificIntegerArray,
Expand Down Expand Up @@ -171,6 +174,7 @@ additionalExports = (ffi::nullptr,
ffi::Long,
ffi::LongLong,
ffi::NativeApi,
ffi::NativeFinalizer,
ffi::NativeFunction,
ffi::NativeFunctionPointer,
ffi::NativePort,
Expand Down
3 changes: 2 additions & 1 deletion pkg/vm/lib/target/vm.dart
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ class VmTarget extends Target {
bool allowPlatformPrivateLibraryAccess(Uri importer, Uri imported) =>
super.allowPlatformPrivateLibraryAccess(importer, imported) ||
importer.path.contains('runtime/tests/vm/dart') ||
importer.path.contains('test-lib');
importer.path.contains('test-lib') ||
importer.path.contains('tests/ffi');

// TODO(sigmund,ahe): limit this to `dart-ext` libraries only (see
// https://github.com/dart-lang/sdk/issues/29763).
Expand Down
14 changes: 14 additions & 0 deletions pkg/vm/lib/transformations/ffi/definitions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:front_end/src/api_unstable/vm.dart'
messageFfiAbiSpecificIntegerMappingInvalid,
messageFfiPackedAnnotationAlignment,
messageNonPositiveArrayDimensions,
templateFfiCompoundImplementsFinalizable,
templateFfiEmptyStruct,
templateFfiFieldAnnotation,
templateFfiFieldNull,
Expand Down Expand Up @@ -321,6 +322,19 @@ class _FfiDefinitionTransformer extends FfiTransformer {
return null;
}

final finalizableType = FutureOrType(
InterfaceType(finalizableClass, Nullability.nullable),
Nullability.nullable);
if (env.isSubtypeOf(InterfaceType(node, Nullability.nonNullable),
finalizableType, SubtypeCheckMode.ignoringNullabilities)) {
diagnosticReporter.report(
templateFfiCompoundImplementsFinalizable.withArguments(
node.superclass!.name, node.name),
node.fileOffset,
1,
node.location!.file);
}

if (node.superclass == structClass) {
final packingAnnotations = _getPackedAnnotations(node);
if (packingAnnotations.length > 1) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/vm/lib/transformations/ffi/finalizable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import 'package:kernel/type_environment.dart';
/// This transformation is not AST-node preserving. [Expression]s and
/// [Statement]s can be replaced by other [Expression]s and [Statement]s
/// respectively. This means one cannot do `visitX() { super.visitX() as X }`.
///
/// This transform must be run on the standard libaries as well. For example
/// `NativeFinalizer`s `attach` implementation depends on it.
mixin FinalizableTransformer on Transformer {
TypeEnvironment get env;
Procedure get reachabilityFenceFunction;
Expand Down
7 changes: 7 additions & 0 deletions runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,13 @@ DART_EXPORT void ReleaseClosureCallback() {
Dart_DeletePersistentHandle_DL(closure_to_callback_);
}

////////////////////////////////////////////////////////////////////////////////
// NativeFinalizer tests

DART_EXPORT void SetArgumentTo42(intptr_t* token) {
*token = 42;
}

////////////////////////////////////////////////////////////////////////////////
// Functions for testing @FfiNative.

Expand Down
Loading

0 comments on commit 532c116

Please sign in to comment.