From dff831dbfc64aef548caed79829a62c4aae13f17 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 5 Feb 2024 03:19:01 +0000 Subject: [PATCH] [ffi] Fix callback subtyping test Previously we were reusing the ensureNativeTypeToDartType function to check that Dart function passed as a callback matches the native type. This works if the types exactly match, but the subtyping test is backwards, so it doesn't allow certain cases that should be allowed. The main case is that when the native function type returns void, the Dart function should be allowed to return anything. So I added ensureDartTypeToNativeType, which reverses the subtype test. As well as making the return types more permissive, this has also changed what parameters are allowed to be passed to callbacks. For example, in tests/ffi/vmspecific_static_checks_typeddata_test.dart:80, passing a Handle to a function expecting an Int8List used to work, but is now a compile error. I think this change is an improvement, because previously it would have been possible to pass any type of object to that callback. So this change turns some potential runtime type errors into compile errors. But technically I think this is a breaking change. Fixes: https://github.com/dart-lang/sdk/issues/53659 Bug: https://github.com/dart-lang/sdk/issues/53659 Change-Id: I6846a59fc309ec897cba8f985d7dd0a63b912b42 TEST=tests/ffi/function_callbacks_subtype_test.dart and others Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346440 Commit-Queue: Liam Appelbe Reviewed-by: Daco Harkes Reviewed-by: Brian Wilkerson --- .../lib/src/generated/ffi_verifier.dart | 98 ++++++++----- .../diagnostics/ffi_async_callback_test.dart | 20 +++ .../diagnostics/must_be_a_subtype_test.dart | 11 ++ pkg/vm/lib/transformations/ffi/common.dart | 49 ++++--- pkg/vm/lib/transformations/ffi/native.dart | 9 +- pkg/vm/lib/transformations/ffi/use_sites.dart | 31 ++-- runtime/bin/ffi_test/ffi_test_functions.cc | 24 +++- .../async_void_function_callbacks_test.dart | 7 +- .../ffi/function_callbacks_subtype_test.dart | 132 ++++++++++++++++++ ...isolate_local_function_callbacks_test.dart | 27 ++-- tests/ffi/variance_function_test.dart | 6 +- ...pecific_variance_function_checks_test.dart | 14 +- 12 files changed, 329 insertions(+), 99 deletions(-) create mode 100644 tests/ffi/function_callbacks_subtype_test.dart diff --git a/pkg/analyzer/lib/src/generated/ffi_verifier.dart b/pkg/analyzer/lib/src/generated/ffi_verifier.dart index 301adb7918e..e50a82b6d1e 100644 --- a/pkg/analyzer/lib/src/generated/ffi_verifier.dart +++ b/pkg/analyzer/lib/src/generated/ffi_verifier.dart @@ -514,6 +514,7 @@ class FfiVerifier extends RecursiveAstVisitor { } if (!_validateCompatibleNativeType( + _FfiTypeCheckDirection.nativeToDart, type, ffiSignature, // Functions are not allowed in native fields, but allowing them in the @@ -631,8 +632,9 @@ class FfiVerifier extends RecursiveAstVisitor { ); return; } - if (!_validateCompatibleFunctionTypes(dartType, nativeType, - nativeFieldWrappersAsPointer: true, allowStricterReturn: true)) { + if (!_validateCompatibleFunctionTypes( + _FfiTypeCheckDirection.nativeToDart, dartType, nativeType, + nativeFieldWrappersAsPointer: true, permissiveReturnType: true)) { _errorReporter.atNode( errorNode, FfiCode.MUST_BE_A_SUBTYPE, @@ -1084,7 +1086,8 @@ class FfiVerifier extends RecursiveAstVisitor { final DartType TPrime = T.typeArguments[0]; final DartType F = node.typeArgumentTypes![0]; final isLeaf = _isLeaf(node.argumentList.arguments); - if (!_validateCompatibleFunctionTypes(F, TPrime)) { + if (!_validateCompatibleFunctionTypes( + _FfiTypeCheckDirection.nativeToDart, F, TPrime)) { _errorReporter.atNode( node, FfiCode.MUST_BE_A_SUBTYPE, @@ -1100,11 +1103,14 @@ class FfiVerifier extends RecursiveAstVisitor { /// Validates that the given [nativeType] is, when native types are converted /// to their Dart equivalent, a subtype of [dartType]. + /// [permissiveReturnType] means that the [direction] is ignored for return + /// types, and subtyping is allowed in either direction. bool _validateCompatibleFunctionTypes( + _FfiTypeCheckDirection direction, DartType dartType, DartType nativeType, { bool nativeFieldWrappersAsPointer = false, - bool allowStricterReturn = false, + bool permissiveReturnType = false, }) { // We require both to be valid function types. if (dartType is! FunctionType || @@ -1134,25 +1140,26 @@ class FfiVerifier extends RecursiveAstVisitor { } // Validate that the return types are compatible. - if (!_validateCompatibleNativeType( - dartType.returnType, nativeType.returnType)) { - // TODO(dacoharkes): Fix inconsistency between `FfiNative` and `asFunction`. - // http://dartbug.com/49518 - if (!allowStricterReturn) { - return false; - } else if (!_validateCompatibleNativeType( - dartType.returnType, nativeType.returnType, - checkCovariance: true)) { + if (permissiveReturnType) { + // TODO(dacoharkes): Fix inconsistency between `FfiNative` and + // `asFunction`. http://dartbug.com/49518. + if (!(_validateCompatibleNativeType(_FfiTypeCheckDirection.nativeToDart, + dartType.returnType, nativeType.returnType) || + _validateCompatibleNativeType(_FfiTypeCheckDirection.dartToNative, + dartType.returnType, nativeType.returnType))) { return false; } + } else if (!_validateCompatibleNativeType( + direction, dartType.returnType, nativeType.returnType)) { + return false; } // Validate that the parameter types are compatible. for (int i = 0; i < parameterCount; ++i) { if (!_validateCompatibleNativeType( + direction.reverse, dartType.normalParameterTypes[i], nativeTypeNormalParameterTypes[i], - checkCovariance: true, nativeFieldWrappersAsPointer: nativeFieldWrappersAsPointer, )) { return false; @@ -1163,13 +1170,13 @@ class FfiVerifier extends RecursiveAstVisitor { return true; } - /// Validates that, if we convert [nativeType] to it's corresponding - /// [dartType] the latter is a subtype of the former if - /// [checkCovariance]. + /// Validates that the [nativeType] can be converted to the [dartType] if + /// [direction] is [_FfiTypeCheckDirection.nativeToDart], or the reverse for + /// [_FfiTypeCheckDirection.dartToNative]. bool _validateCompatibleNativeType( + _FfiTypeCheckDirection direction, DartType dartType, DartType nativeType, { - bool checkCovariance = false, bool nativeFieldWrappersAsPointer = false, bool allowFunctions = false, }) { @@ -1184,13 +1191,15 @@ class FfiVerifier extends RecursiveAstVisitor { } else if (nativeReturnType == _PrimitiveDartType.bool) { return dartType.isDartCoreBool; } else if (nativeReturnType == _PrimitiveDartType.void_) { - return dartType is VoidType; + return direction == _FfiTypeCheckDirection.dartToNative + ? true + : dartType is VoidType; } else if (dartType is VoidType) { // Don't allow other native subtypes if the Dart return type is void. return nativeReturnType == _PrimitiveDartType.void_; } else if (nativeReturnType == _PrimitiveDartType.handle) { InterfaceType objectType = typeSystem.objectStar; - return checkCovariance + return direction == _FfiTypeCheckDirection.dartToNative ? /* everything is subtype of objectStar */ true : typeSystem.isSubtypeOf(objectType, dartType); } else if (dartType is InterfaceType && nativeType is InterfaceType) { @@ -1206,7 +1215,7 @@ class FfiVerifier extends RecursiveAstVisitor { if (_isValidTypedData(nativeType, dartType)) { return true; } - return checkCovariance + return direction == _FfiTypeCheckDirection.dartToNative ? typeSystem.isSubtypeOf(dartType, nativeType) : typeSystem.isSubtypeOf(nativeType, dartType); } else if (dartType is FunctionType && @@ -1214,7 +1223,8 @@ class FfiVerifier extends RecursiveAstVisitor { nativeType is InterfaceType && nativeType.isNativeFunction) { final nativeFunction = nativeType.typeArguments[0]; - return _validateCompatibleFunctionTypes(dartType, nativeFunction, + return _validateCompatibleFunctionTypes( + direction, dartType, nativeFunction, nativeFieldWrappersAsPointer: nativeFieldWrappersAsPointer); } else { // If the [nativeType] is not a primitive int/double type then it has to @@ -1382,7 +1392,8 @@ class FfiVerifier extends RecursiveAstVisitor { Expression f = node.argumentList.arguments[0]; DartType FT = f.typeOrThrow; - if (!_validateCompatibleFunctionTypes(FT, T)) { + if (!_validateCompatibleFunctionTypes( + _FfiTypeCheckDirection.dartToNative, FT, T)) { _errorReporter.atNode( f, FfiCode.MUST_BE_A_SUBTYPE, @@ -1393,7 +1404,7 @@ class FfiVerifier extends RecursiveAstVisitor { // TODO(brianwilkerson): Validate that `f` is a top-level function. final DartType R = (T as FunctionType).returnType; - if ((FT as FunctionType).returnType is VoidType || + if (_primitiveNativeType(R) == _PrimitiveDartType.void_ || R.isPointer || R.isHandle || R.isCompoundSubtype) { @@ -1413,7 +1424,8 @@ class FfiVerifier extends RecursiveAstVisitor { } else { Expression e = node.argumentList.arguments[1]; var eType = e.typeOrThrow; - if (!_validateCompatibleNativeType(eType, R, checkCovariance: true)) { + if (!_validateCompatibleNativeType( + _FfiTypeCheckDirection.dartToNative, eType, R)) { _errorReporter.atNode( e, FfiCode.MUST_BE_A_SUBTYPE, @@ -1474,7 +1486,8 @@ class FfiVerifier extends RecursiveAstVisitor { return; } final isLeaf = _isLeaf(node.argumentList.arguments); - if (!_validateCompatibleFunctionTypes(F, S)) { + if (!_validateCompatibleFunctionTypes( + _FfiTypeCheckDirection.nativeToDart, F, S)) { final AstNode errorNode = typeArguments[1]; _errorReporter.atNode( errorNode, @@ -1605,7 +1618,8 @@ class FfiVerifier extends RecursiveAstVisitor { var f = node.argumentList.arguments[0]; var funcType = f.typeOrThrow; - if (!_validateCompatibleFunctionTypes(funcType, typeArg)) { + if (!_validateCompatibleFunctionTypes( + _FfiTypeCheckDirection.dartToNative, funcType, typeArg)) { _errorReporter.atNode( f, FfiCode.MUST_BE_A_SUBTYPE, @@ -1614,10 +1628,9 @@ class FfiVerifier extends RecursiveAstVisitor { return; } - var retType = (funcType as FunctionType).returnType; var natRetType = (typeArg as FunctionType).returnType; if (isolateLocal) { - if (retType is VoidType || + if (_primitiveNativeType(natRetType) == _PrimitiveDartType.void_ || natRetType.isPointer || natRetType.isHandle || natRetType.isCompoundSubtype) { @@ -1637,8 +1650,8 @@ class FfiVerifier extends RecursiveAstVisitor { } else { var e = (node.argumentList.arguments[1] as NamedExpression).expression; var eType = e.typeOrThrow; - if (!_validateCompatibleNativeType(eType, natRetType, - checkCovariance: true)) { + if (!_validateCompatibleNativeType( + _FfiTypeCheckDirection.dartToNative, eType, natRetType)) { _errorReporter.atNode( e, FfiCode.MUST_BE_A_SUBTYPE, @@ -1654,11 +1667,11 @@ class FfiVerifier extends RecursiveAstVisitor { } } } else { - if (retType is! VoidType) { + if (_primitiveNativeType(natRetType) != _PrimitiveDartType.void_) { _errorReporter.atNode( f, FfiCode.MUST_RETURN_VOID, - arguments: [retType], + arguments: [natRetType], ); } } @@ -1847,6 +1860,25 @@ class FfiVerifier extends RecursiveAstVisitor { } } +enum _FfiTypeCheckDirection { + // Passing a value from native code to Dart code. For example, the return type + // of a loaded native function, or the arguments of a native callback. + nativeToDart, + + // Passing a value from Dart code to native code. For example, the arguments + // of a loaded native function, or the return type of a native callback. + dartToNative; + + _FfiTypeCheckDirection get reverse { + switch (this) { + case nativeToDart: + return dartToNative; + case dartToNative: + return nativeToDart; + } + } +} + enum _PrimitiveDartType { double, int, diff --git a/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart b/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart index 0d7bf94c395..9f2c2760a82 100644 --- a/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart +++ b/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart @@ -133,6 +133,16 @@ void g() { ''', []); } + test_NativeCallable_isolateLocal_voidReturnPermissive() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +int f(int i) => i * 2; +void g() { + NativeCallable.isolateLocal(f); +} +''', []); + } + test_NativeCallable_listener_inferred() async { await assertErrorsInCode(r''' import 'dart:ffi'; @@ -200,6 +210,16 @@ void f(int i) => i * 2; void g() { NativeCallable.listener(f); } +''', []); + } + + test_NativeCallable_listener_voidReturnPermissive() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +int f(int i) => i * 2; +void g() { + NativeCallable.listener(f); +} ''', []); } } diff --git a/pkg/analyzer/test/src/diagnostics/must_be_a_subtype_test.dart b/pkg/analyzer/test/src/diagnostics/must_be_a_subtype_test.dart index 551c7ec5135..c60c7fb541f 100644 --- a/pkg/analyzer/test/src/diagnostics/must_be_a_subtype_test.dart +++ b/pkg/analyzer/test/src/diagnostics/must_be_a_subtype_test.dart @@ -63,6 +63,17 @@ void g() { '''); } + test_fromFunction_valid_voidReturnPermissive() async { + await assertNoErrorsInCode(r''' +import 'dart:ffi'; +typedef T = Void Function(Int8); +int f(int i) => i * 2; +void g() { + Pointer.fromFunction(f); +} +'''); + } + test_lookupFunction_F() async { await assertErrorsInCode(r''' import 'dart:ffi'; diff --git a/pkg/vm/lib/transformations/ffi/common.dart b/pkg/vm/lib/transformations/ffi/common.dart index 5f93772919f..037ecd638fc 100644 --- a/pkg/vm/lib/transformations/ffi/common.dart +++ b/pkg/vm/lib/transformations/ffi/common.dart @@ -137,6 +137,14 @@ const List unalignedLoadsStores = [ NativeType.kDouble, ]; +enum FfiTypeCheckDirection { + // Passing a value from native code to Dart code. + nativeToDart, + + // Passing a value from Dart code to native code. + dartToNative, +} + /// [FfiTransformer] contains logic which is shared between /// _FfiUseSiteTransformer and _FfiDefinitionTransformer. class FfiTransformer extends Transformer { @@ -1396,7 +1404,8 @@ class FfiTransformer extends Transformer { } } - void ensureNativeTypeToDartType( + DartType ensureNativeTypeMatch( + FfiTypeCheckDirection direction, DartType nativeType, DartType dartType, TreeNode reportErrorOn, { @@ -1411,23 +1420,31 @@ class FfiTransformer extends Transformer { allowInlineArray: allowArray, allowVoid: allowVoid, )!; - if (dartType == correspondingDartType) return; - if (env.isSubtypeOf(correspondingDartType, dartType, - SubtypeCheckMode.ignoringNullabilities)) { - // If subtype, manually check the return type is not void. - if (correspondingDartType is FunctionType) { - if (dartType is FunctionType) { - if ((dartType.returnType is VoidType) == - (correspondingDartType.returnType is VoidType)) { - return; + if (dartType == correspondingDartType) return correspondingDartType; + switch (direction) { + case FfiTypeCheckDirection.nativeToDart: + if (env.isSubtypeOf(correspondingDartType, dartType, + SubtypeCheckMode.ignoringNullabilities)) { + // If subtype, manually check the return type is not void. + if (correspondingDartType is FunctionType) { + if (dartType is FunctionType) { + if ((dartType.returnType is VoidType) == + (correspondingDartType.returnType is VoidType)) { + return correspondingDartType; + } + // One of the return types is void, the other isn't, report error. + } else { + // One is a function type, the other isn't, report error. + } + } else { + return correspondingDartType; } - // One of the return types is void, the other isn't, report error. - } else { - // One is a function type, the other isn't, report error. } - } else { - return; - } + case FfiTypeCheckDirection.dartToNative: + if (env.isSubtypeOf(dartType, correspondingDartType, + SubtypeCheckMode.ignoringNullabilities)) { + return correspondingDartType; + } } diagnosticReporter.report( templateFfiTypeMismatch.withArguments(dartType, correspondingDartType, diff --git a/pkg/vm/lib/transformations/ffi/native.dart b/pkg/vm/lib/transformations/ffi/native.dart index 3762b42b2b2..e32025f1bee 100644 --- a/pkg/vm/lib/transformations/ffi/native.dart +++ b/pkg/vm/lib/transformations/ffi/native.dart @@ -25,7 +25,8 @@ import 'package:kernel/reference_from_index.dart' show ReferenceFromIndex; import 'package:kernel/target/targets.dart' show DiagnosticReporter; import 'package:kernel/type_environment.dart'; -import 'common.dart' show FfiStaticTypeError, FfiTransformer, NativeType; +import 'common.dart' + show FfiStaticTypeError, FfiTransformer, NativeType, FfiTypeCheckDirection; import 'native_type_cfe.dart'; /// Transform @Native annotated functions into FFI native function pointer @@ -765,7 +766,8 @@ class FfiNativeTransformer extends FfiTransformer { allowHandle: true, allowInlineArray: true, ); - ensureNativeTypeToDartType(ffiType, dartType, node, + ensureNativeTypeMatch( + FfiTypeCheckDirection.nativeToDart, ffiType, dartType, node, allowHandle: true, allowArray: true); // Array types must have an @Array annotation denoting its size. @@ -955,7 +957,8 @@ class FfiNativeTransformer extends FfiTransformer { try { ensureNativeTypeValid(nativeType, node); - ensureNativeTypeToDartType( + ensureNativeTypeMatch( + FfiTypeCheckDirection.nativeToDart, nativeType, wrappedDartFunctionType, node, diff --git a/pkg/vm/lib/transformations/ffi/use_sites.dart b/pkg/vm/lib/transformations/ffi/use_sites.dart index 08cb5035bec..f63fb2124d4 100644 --- a/pkg/vm/lib/transformations/ffi/use_sites.dart +++ b/pkg/vm/lib/transformations/ffi/use_sites.dart @@ -32,7 +32,8 @@ import 'package:kernel/type_environment.dart'; import 'definitions.dart' as definitions; import 'native_type_cfe.dart'; import 'native.dart' as native; -import 'common.dart' show FfiStaticTypeError, FfiTransformer, NativeType; +import 'common.dart' + show FfiStaticTypeError, FfiTransformer, NativeType, FfiTypeCheckDirection; import 'finalizable.dart'; /// Checks and replaces calls to dart:ffi compound fields and methods. @@ -402,7 +403,8 @@ mixin _FfiUseSiteTransformer on FfiTransformer { _ensureIsLeafIsConst(node); final isLeaf = getIsLeafBoolean(node) ?? false; ensureNativeTypeValid(nativeType, node); - ensureNativeTypeToDartType( + ensureNativeTypeMatch( + FfiTypeCheckDirection.nativeToDart, nativeType, dartType, node, @@ -423,7 +425,8 @@ mixin _FfiUseSiteTransformer on FfiTransformer { final isLeaf = getIsLeafBoolean(node) ?? false; ensureNativeTypeValid(nativeType, node); - ensureNativeTypeToDartType( + ensureNativeTypeMatch( + FfiTypeCheckDirection.nativeToDart, nativeType, dartType, node, @@ -457,15 +460,18 @@ mixin _FfiUseSiteTransformer on FfiTransformer { final DartType dartType = func.getStaticType(staticTypeContext!); ensureNativeTypeValid(nativeType, node); - ensureNativeTypeToDartType(nativeType, dartType, node); + final ffiFuncType = ensureNativeTypeMatch( + FfiTypeCheckDirection.dartToNative, nativeType, dartType, node) + as FunctionType; final funcType = dartType as FunctionType; // Check return type. - if (funcType.returnType != VoidType()) { + if (ffiFuncType.returnType != VoidType()) { diagnosticReporter.report( templateFfiNativeCallableListenerReturnVoid.withArguments( - funcType.returnType, currentLibrary.isNonNullableByDefault), + ffiFuncType.returnType, + currentLibrary.isNonNullableByDefault), func.fileOffset, 1, func.location?.file); @@ -839,11 +845,12 @@ mixin _FfiUseSiteTransformer on FfiTransformer { } ensureNativeTypeValid(nativeType, node); - ensureNativeTypeToDartType( + final ffiFuncType = ensureNativeTypeMatch( + FfiTypeCheckDirection.dartToNative, nativeType, dartType, node, - ); + ) as FunctionType; final funcType = dartType as FunctionType; @@ -875,7 +882,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer { if (hasExceptionalReturn) { diagnosticReporter.report( templateFfiExpectedNoExceptionalReturn.withArguments( - funcType.returnType, currentLibrary.isNonNullableByDefault), + ffiFuncType.returnType, currentLibrary.isNonNullableByDefault), node.fileOffset, 1, node.location?.file); @@ -886,7 +893,7 @@ mixin _FfiUseSiteTransformer on FfiTransformer { if (!hasExceptionalReturn) { diagnosticReporter.report( templateFfiExpectedExceptionalReturn.withArguments( - funcType.returnType, currentLibrary.isNonNullableByDefault), + ffiFuncType.returnType, currentLibrary.isNonNullableByDefault), node.fileOffset, 1, node.location?.file); @@ -1307,8 +1314,8 @@ mixin _FfiUseSiteTransformer on FfiTransformer { ensureNativeTypeValid(nativeType, node, allowCompounds: true, allowInlineArray: true); - ensureNativeTypeToDartType( - nativeType, arg.getStaticType(staticTypeContext!), node, + ensureNativeTypeMatch(FfiTypeCheckDirection.nativeToDart, nativeType, + arg.getStaticType(staticTypeContext!), node, allowArray: true); return StaticInvocation( diff --git a/runtime/bin/ffi_test/ffi_test_functions.cc b/runtime/bin/ffi_test/ffi_test_functions.cc index d2e3f7a8d46..9a84fea423f 100644 --- a/runtime/bin/ffi_test/ffi_test_functions.cc +++ b/runtime/bin/ffi_test/ffi_test_functions.cc @@ -1307,10 +1307,28 @@ DART_EXPORT void CallFunctionOnNewThreadNonBlocking(int64_t response_id, //////////////////////////////////////////////////////////////////////////////// // Tests for isolate local callbacks. -DART_EXPORT void CallTwoIntFunction(int32_t (*fn)(int32_t, int32_t), - int32_t a, - int32_t b) { +DART_EXPORT int32_t CallTwoIntFunction(int32_t (*fn)(int32_t, int32_t), + int32_t a, + int32_t b) { + return fn(a, b); +} + +DART_EXPORT void CallTwoIntVoidFunction(void (*fn)(int32_t, int32_t), + int32_t a, + int32_t b) { fn(a, b); } +DART_EXPORT void* CallTwoIntPointerFunction(void* (*fn)(int32_t, int32_t), + int32_t a, + int32_t b) { + return fn(a, b); +} + +DART_EXPORT int32_t CallTwoPointerIntFunction(int32_t (*fn)(void*, void*), + void* a, + void* b) { + return fn(a, b); +} + } // namespace dart diff --git a/tests/ffi/async_void_function_callbacks_test.dart b/tests/ffi/async_void_function_callbacks_test.dart index 37db22158c4..d58bb6ffcc0 100644 --- a/tests/ffi/async_void_function_callbacks_test.dart +++ b/tests/ffi/async_void_function_callbacks_test.dart @@ -189,11 +189,10 @@ testNativeCallableKeepAliveGetter() { Future testNativeCallableClosure() async { final lib = NativeLibrary(); int c = 70000; - void foo(int a, int b) { - simpleFunctionResult.complete(a + b + c); - } - final callback = NativeCallable.listener(foo); + final callback = NativeCallable.listener((int a, int b) { + simpleFunctionResult.complete(a + b + c); + }); simpleFunctionResult = Completer(); lib.callFunctionOnSameThread(1000, callback.nativeFunction); diff --git a/tests/ffi/function_callbacks_subtype_test.dart b/tests/ffi/function_callbacks_subtype_test.dart new file mode 100644 index 00000000000..b8120878ead --- /dev/null +++ b/tests/ffi/function_callbacks_subtype_test.dart @@ -0,0 +1,132 @@ +// Copyright (c) 2024, 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 test program for testing dart:ffi callbacks that take advantage of +// subtyping rules. +// +// VMOptions= +// VMOptions=--stacktrace-every=100 +// VMOptions=--use-slow-path +// VMOptions=--use-slow-path --stacktrace-every=100 +// VMOptions=--dwarf_stack_traces --no-retain_function_objects --no-retain_code_objects +// VMOptions=--test_il_serialization +// VMOptions=--profiler +// SharedObjects=ffi_test_functions + +import 'dart:async'; +import 'dart:ffi'; + +import "package:expect/expect.dart"; + +import 'dylib_utils.dart'; + +final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions"); + +typedef TwoIntVoidFnNativeType = Void Function(Pointer, Int32, Int32); +typedef TwoIntVoidFnType = void Function(Pointer, int, int); +final callTwoIntVoidFunction = + ffiTestFunctions.lookupFunction( + "CallTwoIntVoidFunction"); + +typedef TwoIntPointerFnNativeType = Pointer Function( + Pointer, Int32, Int32); +typedef TwoIntPointerFnType = Pointer Function(Pointer, int, int); +final callTwoIntPointerFunction = ffiTestFunctions.lookupFunction< + TwoIntPointerFnNativeType, + TwoIntPointerFnType>("CallTwoIntPointerFunction"); + +typedef TwoPointerIntFnNativeType = Int32 Function( + Pointer, Pointer, Pointer); +typedef TwoPointerIntFnType = int Function( + Pointer, Pointer, Pointer); +final callTwoPointerIntFunction = ffiTestFunctions.lookupFunction< + TwoPointerIntFnNativeType, + TwoPointerIntFnType>("CallTwoPointerIntFunction"); + +typedef VoidReturnFunction = Void Function(Int32, Int32); +int addVoidResult = 0; +int addVoid(int x, int y) { + print("addVoid($x, $y)"); + addVoidResult = x + y; + return addVoidResult; +} + +final addVoidAsyncResult = Completer(); +int addVoidAsync(int x, int y) { + print("addVoidAsync($x, $y)"); + final result = x + y; + addVoidAsyncResult.complete(result); + return result; +} + +typedef NaTyPtrReturnFunction = Pointer Function(Int32, Int32); +Pointer addInt64PtrReturn(int x, int y) { + print("addInt64PtrReturn($x, $y)"); + return Pointer.fromAddress(x + y); +} + +typedef Int64PtrParamFunction = Int32 Function(Pointer, Pointer); +int addNaTyPtrParam(Pointer x, Pointer y) { + print("addNaTyPtrParam($x, $y)"); + return x.address + y.address; +} + +Future main() async { + await testReturnVoid(); + testReturnSubtype(); + testParamSubtype(); + print("Done! :)"); +} + +Future testReturnVoid() async { + // If the native function type returns void, the Dart function can return + // anything. + final legacyCallback = Pointer.fromFunction(addVoid); + callTwoIntVoidFunction(legacyCallback, 100, 23); + Expect.equals(123, addVoidResult); + + final isolateLocal = NativeCallable.isolateLocal(addVoid) + ..keepIsolateAlive = false; + callTwoIntVoidFunction(isolateLocal.nativeFunction, 400, 56); + Expect.equals(456, addVoidResult); + + final listener = NativeCallable.listener(addVoidAsync) + ..keepIsolateAlive = false; + callTwoIntVoidFunction(listener.nativeFunction, 700, 89); + Expect.equals(789, await addVoidAsyncResult.future); +} + +void testReturnSubtype() { + // The Dart function is allowed to return a subtype of the native return type. + final legacyCallback = + Pointer.fromFunction(addInt64PtrReturn); + Expect.equals( + 123, callTwoIntPointerFunction(legacyCallback, 100, 23).address); + + final isolateLocal = + NativeCallable.isolateLocal(addInt64PtrReturn) + ..keepIsolateAlive = false; + Expect.equals(456, + callTwoIntPointerFunction(isolateLocal.nativeFunction, 400, 56).address); +} + +void testParamSubtype() { + // The Dart function is allowed to accept params that are a supertype of the + // native type's params. + final legacyCallback = + Pointer.fromFunction(addNaTyPtrParam, 0); + Expect.equals( + 123, + callTwoPointerIntFunction(legacyCallback, Pointer.fromAddress(100), + Pointer.fromAddress(23))); + + final isolateLocal = NativeCallable.isolateLocal( + addNaTyPtrParam, + exceptionalReturn: 0) + ..keepIsolateAlive = false; + Expect.equals( + 456, + callTwoPointerIntFunction(isolateLocal.nativeFunction, + Pointer.fromAddress(400), Pointer.fromAddress(56))); +} diff --git a/tests/ffi/isolate_local_function_callbacks_test.dart b/tests/ffi/isolate_local_function_callbacks_test.dart index ad4d5c0e6c2..0ea85636876 100644 --- a/tests/ffi/isolate_local_function_callbacks_test.dart +++ b/tests/ffi/isolate_local_function_callbacks_test.dart @@ -61,11 +61,9 @@ testNativeCallableStatic() { testNativeCallableClosure() { int c = 70000; - int closure(int a, int b) { - return a + b + c; - } - final callback = NativeCallable.isolateLocal(closure, + final callback = NativeCallable.isolateLocal( + (int a, int b) => a + b + c, exceptionalReturn: 0); Expect.equals(71234, callTwoIntFunction(callback.nativeFunction, 1000, 234)); @@ -115,13 +113,10 @@ testNativeCallableNestedCloseCallStatic() { testNativeCallableNestedCloseCallClosure() { late NativeCallable callback; - int selfClosing(int a, int b) { + callback = NativeCallable.isolateLocal((int a, int b) { callback.close(); return a + b; - } - - callback = NativeCallable.isolateLocal(selfClosing, - exceptionalReturn: 0); + }, exceptionalReturn: 0); Expect.equals(1234, callTwoIntFunction(callback.nativeFunction, 1000, 234)); @@ -150,15 +145,13 @@ testNativeCallableExceptionalReturnStatic() { } testNativeCallableExceptionalReturnClosure() { - int thrower(int a, int b) { + final callback = + NativeCallable.isolateLocal((int a, int b) { if (a != 1000) { throw "Oh no!"; } return a + b; - } - - final callback = NativeCallable.isolateLocal(thrower, - exceptionalReturn: 5678); + }, exceptionalReturn: 5678); Expect.equals(1234, callTwoIntFunction(callback.nativeFunction, 1000, 234)); Expect.equals(5678, callTwoIntFunction(callback.nativeFunction, 0, 0)); @@ -182,13 +175,11 @@ Future testNativeCallableDontKeepAliveStatic() async { Future testNativeCallableDontKeepAliveClosure() async { int c = 70000; - int closure(int a, int b) { - return a + b + c; - } final exitPort = ReceivePort(); await Isolate.spawn((_) async { - final callback = NativeCallable.isolateLocal(closure, + final callback = NativeCallable.isolateLocal( + (int a, int b) => a + b + c, exceptionalReturn: 0); Expect.equals( diff --git a/tests/ffi/variance_function_test.dart b/tests/ffi/variance_function_test.dart index bd3f0d1af8f..704019f7937 100644 --- a/tests/ffi/variance_function_test.dart +++ b/tests/ffi/variance_function_test.dart @@ -198,9 +198,9 @@ void callbackParamImplicitDowncast1() { } void callbackParamSubtype1() { - final callback = ffiTestFunctions.lookupFunction(callbackParamOpName); - final fp = Pointer.fromFunction(int64PointerParamOp); + final callback = ffiTestFunctions.lookupFunction(callbackParamOpName); + final fp = Pointer.fromFunction(naTyPointerParamOp); callback(fp); } diff --git a/tests/ffi/vmspecific_variance_function_checks_test.dart b/tests/ffi/vmspecific_variance_function_checks_test.dart index ad1f7e56e35..964ab1fec24 100644 --- a/tests/ffi/vmspecific_variance_function_checks_test.dart +++ b/tests/ffi/vmspecific_variance_function_checks_test.dart @@ -14,6 +14,7 @@ import 'dylib_utils.dart'; typedef Int64PointerParamOpDart = void Function(Pointer); typedef Int64PointerParamOp = Void Function(Pointer); +typedef NaTyPointerParamOp = Void Function(Pointer); typedef Int64PointerReturnOp = Pointer Function(); typedef NaTyPointerReturnOp = Pointer Function(); @@ -48,20 +49,19 @@ final f3 = p3 //# 10: compile-time error // Test check on callbacks from native to Dart (fromFunction). // =========================================================== -void naTyPointerParamOp(Pointer p) { - final Pointer asInt8 = p.cast(); - asInt8.value = 42; +void int64PointerParamOp(Pointer p) { + p.value = 42; } -Pointer int64PointerReturnOp() { +Pointer naTyPointerReturnOp() { return Pointer.fromAddress(0x13370000); } final implicitDowncast1 = //# 3: compile-time error - Pointer.fromFunction(//# 3: continued + Pointer.fromFunction(//# 3: continued naTyPointerParamOp); //# 3: continued final implicitDowncast2 = //# 4: compile-time error - Pointer.fromFunction(//# 4: continued - int64PointerReturnOp); //# 4: continued + Pointer.fromFunction(//# 4: continued + naTyPointerReturnOp); //# 4: continued void main() {}