[vm/ffi] Change asFunction and lookFunction to extension methods

This prevents them from being called dynamically.
Moreover, it prevents asFunction from being called on a non-NativeFunction type argument, simplifying the amount of manual checks.

Note that this CL had to change the CFE and analzyer, and their tests (including mock_sdk) as well.

This can potentially be a breaking change, as the extension methods are only visible when `dart:ffi` is imported, while methods on objects are always visible.

Issue: https://github.com/dart-lang/sdk/issues/35903

Change-Id: I1e291f154228d5d9a34b21a022088bf493f6557d
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,dart-sdk-linux-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,front-end-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-mac-debug-x64-try,analyzer-nnbd-linux-release-try,dart2js-nnbd-linux-x64-chrome-try,ddc-nnbd-linux-release-chrome-try,front-end-nnbd-linux-release-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-nnbd-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135463
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This commit is contained in:
Daco Harkes 2020-02-14 15:46:37 +00:00 committed by commit-bot@chromium.org
parent eceb249f88
commit c1467ab5d3
17 changed files with 129 additions and 117 deletions

View file

@ -126,12 +126,17 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
Element enclosingElement = element.enclosingElement;
if (enclosingElement is ClassElement) {
if (_isPointer(enclosingElement)) {
if (element.name == 'asFunction') {
_validateAsFunction(node, element);
} else if (element.name == 'fromFunction') {
if (element.name == 'fromFunction') {
_validateFromFunction(node, element);
}
} else if (_isDynamicLibrary(enclosingElement) &&
}
}
if (enclosingElement is ExtensionElement) {
if (_isNativeFunctionPointerExtension(enclosingElement)) {
if (element.name == 'asFunction') {
_validateAsFunction(node, element);
}
} else if (_isDynamicLibraryExtension(enclosingElement) &&
element.name == 'lookupFunction') {
_validateLookupFunction(node);
}
@ -152,10 +157,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
return element is ClassElement && element.library.name == 'dart.ffi';
}
/// Return `true` if the given [element] represents the class
/// `DynamicLibrary`.
bool _isDynamicLibrary(ClassElement element) =>
element.name == 'DynamicLibrary' && element.library.name == 'dart.ffi';
/// Return `true` if the given [element] represents the extension
/// `LibraryExtension`.
bool _isDynamicLibraryExtension(Element element) =>
element.name == 'LibraryExtension' && element.library.name == 'dart.ffi';
/// Returns `true` iff [nativeType] is a `ffi.NativeFunction<???>` type.
bool _isNativeFunctionInterfaceType(DartType nativeType) {
@ -184,6 +189,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
bool _isPointer(Element element) =>
element.name == 'Pointer' && element.library.name == 'dart.ffi';
bool _isNativeFunctionPointerExtension(Element element) =>
element.name == 'NativeFunctionPointer' &&
element.library.name == 'dart.ffi';
/// Returns `true` iff [nativeType] is a `ffi.Pointer<???>` type.
bool _isPointerInterfaceType(DartType nativeType) {
if (nativeType is InterfaceType) {

View file

@ -532,13 +532,19 @@ class Pointer<T extends NativeType> extends NativeType {
static Pointer<NativeFunction<T>> fromFunction<T extends Function>(
@DartRepresentationOf("T") Function f,
[Object exceptionalReturn]);
R asFunction<@DartRepresentationOf("T") R extends Function>();
}
extension NativeFunctionPointer<NF extends Function>
on Pointer<NativeFunction<NF>> {
external DF asFunction<DF extends Function>();
}
class Struct extends NativeType {}
abstract class DynamicLibrary {
F lookupFunction<T extends Function, F extends Function>(String symbolName);
abstract class DynamicLibrary {}
extension LibraryExtension on DynamicLibrary {
external F lookupFunction<T extends Function, F extends Function>(
String symbolName);
}
abstract class NativeFunction<T extends Function> extends NativeType {}
class DartRepresentationOf {

View file

@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/src/dart/error/ffi_code.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
import '../dart/resolution/driver_resolution.dart';
@ -25,7 +26,9 @@ class C {
}
}
''', [
error(FfiCode.NON_NATIVE_FUNCTION_TYPE_ARGUMENT_TO_POINTER, 109, 1),
// This changed from a method to a extension method, uses Dart semantics
// instead of manual check now.
error(StaticTypeWarningCode.UNDEFINED_METHOD, 98, 10),
]);
}

View file

@ -77,9 +77,6 @@ class C extends Int8 {}
import 'dart:ffi';
class C extends Pointer {}
''', [
// TODO(brianwilkerson) The following diagnostic should not be generated.
error(StaticWarningCode.NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_ONE,
25, 1),
error(FfiCode.SUBTYPE_OF_FFI_CLASS_IN_EXTENDS, 35, 7),
]);
}
@ -198,9 +195,6 @@ class C implements Int8 {}
import 'dart:ffi';
class C implements Pointer {}
''', [
// TODO(brianwilkerson) The following diagnostic should not be generated.
error(StaticWarningCode.NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_ONE,
25, 1),
error(FfiCode.SUBTYPE_OF_FFI_CLASS_IN_IMPLEMENTS, 38, 7),
]);
}
@ -333,9 +327,6 @@ class C with Int8 {}
import 'dart:ffi';
class C with Pointer {}
''', [
// TODO(brianwilkerson) The following diagnostic should not be generated.
error(StaticWarningCode.NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_ONE,
25, 1),
error(CompileTimeErrorCode.MIXIN_INHERITS_FROM_NOT_OBJECT, 32, 7),
error(FfiCode.SUBTYPE_OF_FFI_CLASS_IN_WITH, 32, 7),
]);

View file

@ -234,11 +234,12 @@ class FfiTransformer extends Transformer {
addressOfField = index.getMember('dart:ffi', 'Struct', '_addressOf'),
structFromPointer =
index.getMember('dart:ffi', 'Struct', '_fromPointer'),
asFunctionMethod = index.getMember('dart:ffi', 'Pointer', 'asFunction'),
asFunctionMethod =
index.getMember('dart:ffi', 'NativeFunctionPointer', 'asFunction'),
asFunctionInternal =
index.getTopLevelMember('dart:ffi', '_asFunctionInternal'),
lookupFunctionMethod =
index.getMember('dart:ffi', 'DynamicLibrary', 'lookupFunction'),
index.getMember('dart:ffi', 'LibraryExtension', 'lookupFunction'),
fromFunctionMethod =
index.getMember('dart:ffi', 'Pointer', 'fromFunction'),
libraryLookupMethod =

View file

@ -153,7 +153,30 @@ class _FfiUseSiteTransformer extends FfiTransformer {
final Member target = node.target;
try {
if (target == fromFunctionMethod) {
if (target == lookupFunctionMethod && !isFfiLibrary) {
final DartType nativeType = InterfaceType(
nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
final DartType dartType = node.arguments.types[1];
_ensureNativeTypeValid(nativeType, node);
_ensureNativeTypeToDartType(nativeType, dartType, node);
return _replaceLookupFunction(node);
} else if (target == asFunctionMethod && !isFfiLibrary) {
final DartType dartType = node.arguments.types[1];
final DartType nativeType = InterfaceType(
nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
_ensureNativeTypeValid(nativeType, node);
_ensureNativeTypeToDartType(nativeType, dartType, node);
final DartType nativeSignature =
(nativeType as InterfaceType).typeArguments[0];
// Inline function body to make all type arguments instatiated.
return StaticInvocation(
asFunctionInternal,
Arguments([node.arguments.positional[0]],
types: [dartType, nativeSignature]));
} else if (target == fromFunctionMethod) {
final DartType nativeType = InterfaceType(
nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
final Expression func = node.arguments.positional[0];
@ -246,15 +269,10 @@ class _FfiUseSiteTransformer extends FfiTransformer {
// We need to replace calls to 'DynamicLibrary.lookupFunction' with explicit
// Kernel, because we cannot have a generic call to 'asFunction' in its body.
//
// Below, in 'visitMethodInvocation', we ensure that the type arguments to
// Above, in 'visitStaticInvocation', we ensure that the type arguments to
// 'lookupFunction' are constants, so by inlining the call to 'asFunction' at
// the call-site, we ensure that there are no generic calls to 'asFunction'.
//
// We will not detect dynamic invocations of 'asFunction' and
// 'lookupFunction': these are handled by the stubs in 'ffi_patch.dart' and
// 'dynamic_library_patch.dart'. Dynamic invocations of 'lookupFunction' (and
// 'asFunction') are not legal and throw a runtime exception.
Expression _replaceLookupFunction(MethodInvocation node) {
Expression _replaceLookupFunction(StaticInvocation node) {
// The generated code looks like:
//
// _asFunctionInternal<DS, NS>(lookup<NativeFunction<NS>>(symbolName))
@ -263,13 +281,16 @@ class _FfiUseSiteTransformer extends FfiTransformer {
final DartType dartSignature = node.arguments.types[1];
final Arguments args = Arguments([
node.arguments.positional.single
node.arguments.positional[1]
], types: [
InterfaceType(nativeFunctionClass, Nullability.legacy, [nativeSignature])
]);
final Expression lookupResult = MethodInvocation(
node.receiver, Name("lookup"), args, libraryLookupMethod);
node.arguments.positional[0],
Name("lookup"),
args,
libraryLookupMethod);
return StaticInvocation(asFunctionInternal,
Arguments([lookupResult], types: [dartSignature, nativeSignature]));
@ -318,35 +339,7 @@ class _FfiUseSiteTransformer extends FfiTransformer {
final Member target = node.interfaceTarget;
try {
// We will not detect dynamic invocations of 'asFunction' and
// 'lookupFunction' -- these are handled by the 'asFunctionInternal' stub
// in 'dynamic_library_patch.dart'. Dynamic invocations of 'asFunction'
// and 'lookupFunction' are not legal and throw a runtime exception.
if (target == lookupFunctionMethod) {
final DartType nativeType = InterfaceType(
nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
final DartType dartType = node.arguments.types[1];
_ensureNativeTypeValid(nativeType, node);
_ensureNativeTypeToDartType(nativeType, dartType, node);
return _replaceLookupFunction(node);
} else if (target == asFunctionMethod) {
final DartType dartType = node.arguments.types[0];
final DartType pointerType =
node.receiver.getStaticType(_staticTypeContext);
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
_ensureNativeTypeValid(pointerType, node);
_ensureNativeTypeValid(nativeType, node);
_ensureNativeTypeToDartType(nativeType, dartType, node);
final DartType nativeSignature =
(nativeType as InterfaceType).typeArguments[0];
return StaticInvocation(asFunctionInternal,
Arguments([node.receiver], types: [dartType, nativeSignature]));
} else if (target == elementAtMethod) {
// TODO(37773): When moving to extension methods we can get rid of
// this rewiring.
if (target == elementAtMethod) {
final DartType pointerType =
node.receiver.getStaticType(_staticTypeContext);
final DartType nativeType = _pointerTypeGetTypeArg(pointerType);

View file

@ -884,7 +884,8 @@ Fragment BaseFlowGraphBuilder::AllocateObject(TokenPosition position,
Fragment BaseFlowGraphBuilder::BuildFfiAsFunctionInternalCall(
const TypeArguments& signatures) {
ASSERT(signatures.IsInstantiated() && signatures.Length() == 2);
ASSERT(signatures.IsInstantiated());
ASSERT(signatures.Length() == 2);
const AbstractType& dart_type = AbstractType::Handle(signatures.TypeAt(0));
const AbstractType& native_type = AbstractType::Handle(signatures.TypeAt(1));

View file

@ -32,15 +32,6 @@ class DynamicLibrary {
Pointer<T> lookup<T extends NativeType>(String symbolName)
native "Ffi_dl_lookup";
// The real implementation of this function lives in FfiUseSiteTransformer
// for interface calls. Only dynamic calls (which are illegal) reach this
// implementation.
@patch
F lookupFunction<T extends Function, F extends Function>(String symbolName) {
throw UnsupportedError(
"Dynamic invocation of lookupFunction is not supported.");
}
// TODO(dacoharkes): Expose this to users, or extend Pointer?
// https://github.com/dart-lang/sdk/issues/35881
int getHandle() native "Ffi_dl_getHandle";
@ -59,3 +50,10 @@ class DynamicLibrary {
@patch
Pointer<Void> get handle => Pointer.fromAddress(getHandle());
}
extension LibraryExtension on DynamicLibrary {
@patch
DS lookupFunction<NS extends Function, DS extends Function>(
String symbolName) =>
throw UnsupportedError("The body is inlined in the frontend.");
}

View file

@ -106,11 +106,6 @@ class Pointer<T extends NativeType> {
@patch
Pointer<U> cast<U extends NativeType>() => Pointer.fromAddress(address);
@patch
R asFunction<R extends Function>() {
throw UnsupportedError("Pointer.asFunction cannot be called dynamically.");
}
}
/// Returns an integer encoding the ABI used for size and alignment
@ -229,6 +224,13 @@ Pointer<Pointer<S>> _elementAtPointer<S extends NativeType>(
Pointer<Pointer<S>> pointer, int index) =>
Pointer.fromAddress(pointer.address + _intPtrSize * index);
extension NativeFunctionPointer<NF extends Function>
on Pointer<NativeFunction<NF>> {
@patch
DF asFunction<DF extends Function>() =>
throw UnsupportedError("The body is inlined in the frontend.");
}
//
// The following code is generated, do not edit by hand.
//

View file

@ -30,10 +30,6 @@ class DynamicLibrary {
/// Throws an [ArgumentError] if it fails to lookup the symbol.
external Pointer<T> lookup<T extends NativeType>(String symbolName);
/// Helper that combines lookup and cast to a Dart function.
external F lookupFunction<T extends Function, F extends Function>(
String symbolName);
/// Dynamic libraries are equal if they load the same library.
external bool operator ==(other);
@ -43,3 +39,10 @@ class DynamicLibrary {
/// The handle to the dynamic library.
external Pointer<Void> get handle;
}
/// Methods which cannot be invoked dynamically.
extension LibraryExtension on DynamicLibrary {
/// Helper that combines lookup and cast to a Dart function.
external F lookupFunction<T extends Function, F extends Function>(
String symbolName);
}

View file

@ -68,13 +68,6 @@ class Pointer<T extends NativeType> extends NativeType {
/// Cast Pointer<T> to a Pointer<V>.
external Pointer<U> cast<U extends NativeType>();
/// Convert to Dart function, automatically marshalling the arguments
/// and return value.
///
/// Can only be called on [Pointer]<[NativeFunction]>. Does not accept dynamic
/// invocations -- where the type of the receiver is [dynamic].
external R asFunction<@DartRepresentationOf("T") R extends Function>();
/// Equality for Pointers only depends on their address.
bool operator ==(other) {
if (other == null) return false;
@ -87,6 +80,14 @@ class Pointer<T extends NativeType> extends NativeType {
}
}
/// Extension on [Pointer] specialized for the type argument [NativeFunction].
extension NativeFunctionPointer<NF extends Function>
on Pointer<NativeFunction<NF>> {
/// Convert to Dart function, automatically marshalling the arguments
/// and return value.
external DF asFunction<@DartRepresentationOf("NF") DF extends Function>();
}
//
// The following code is generated, do not edit by hand.
//

View file

@ -30,15 +30,6 @@ class DynamicLibrary {
Pointer<T> lookup<T extends NativeType>(String symbolName)
native "Ffi_dl_lookup";
// The real implementation of this function lives in FfiUseSiteTransformer
// for interface calls. Only dynamic calls (which are illegal) reach this
// implementation.
@patch
F lookupFunction<T extends Function, F extends Function>(String symbolName) {
throw UnsupportedError(
"Dynamic invocation of lookupFunction is not supported.");
}
// TODO(dacoharkes): Expose this to users, or extend Pointer?
// https://github.com/dart-lang/sdk/issues/35881
int getHandle() native "Ffi_dl_getHandle";
@ -58,3 +49,10 @@ class DynamicLibrary {
@patch
Pointer<Void> get handle => Pointer.fromAddress(getHandle());
}
extension LibraryExtension on DynamicLibrary {
@patch
DS lookupFunction<NS extends Function, DS extends Function>(
String symbolName) =>
throw UnsupportedError("The body is inlined in the frontend.");
}

View file

@ -104,11 +104,6 @@ class Pointer<T extends NativeType> {
@patch
Pointer<U> cast<U extends NativeType>() => Pointer.fromAddress(address);
@patch
R asFunction<R extends Function>() {
throw UnsupportedError("Pointer.asFunction cannot be called dynamically.");
}
}
/// Returns an integer encoding the ABI used for size and alignment
@ -227,6 +222,13 @@ Pointer<Pointer<S>> _elementAtPointer<S extends NativeType>(
Pointer<Pointer<S>> pointer, int index) =>
Pointer.fromAddress(pointer.address + _intPtrSize * index);
extension NativeFunctionPointer<NF extends Function>
on Pointer<NativeFunction<NF>> {
@patch
DF asFunction<DF extends Function>() =>
throw UnsupportedError("The body is inlined in the frontend.");
}
//
// The following code is generated, do not edit by hand.
//

View file

@ -28,10 +28,6 @@ class DynamicLibrary {
/// Throws an [ArgumentError] if it fails to lookup the symbol.
external Pointer<T> lookup<T extends NativeType>(String symbolName);
/// Helper that combines lookup and cast to a Dart function.
external F lookupFunction<T extends Function, F extends Function>(
String symbolName);
/// Dynamic libraries are equal if they load the same library.
external bool operator ==(Object other);
@ -41,3 +37,10 @@ class DynamicLibrary {
/// The handle to the dynamic library.
external Pointer<Void> get handle;
}
/// Methods which cannot be invoked dynamically.
extension LibraryExtension on DynamicLibrary {
/// Helper that combines lookup and cast to a Dart function.
external F lookupFunction<T extends Function, F extends Function>(
String symbolName);
}

View file

@ -66,13 +66,6 @@ class Pointer<T extends NativeType> extends NativeType {
/// Cast Pointer<T> to a Pointer<V>.
external Pointer<U> cast<U extends NativeType>();
/// Convert to Dart function, automatically marshalling the arguments
/// and return value.
///
/// Can only be called on [Pointer]<[NativeFunction]>. Does not accept dynamic
/// invocations -- where the type of the receiver is [dynamic].
external R asFunction<@DartRepresentationOf("T") R extends Function>();
/// Equality for Pointers only depends on their address.
bool operator ==(Object other) {
if (other is! Pointer) return false;
@ -86,6 +79,14 @@ class Pointer<T extends NativeType> extends NativeType {
}
}
/// Extension on [Pointer] specialized for the type argument [NativeFunction].
extension NativeFunctionPointer<NF extends Function>
on Pointer<NativeFunction<NF>> {
/// Convert to Dart function, automatically marshalling the arguments
/// and return value.
external DF asFunction<@DartRepresentationOf("NF") DF extends Function>();
}
//
// The following code is generated, do not edit by hand.
//

View file

@ -68,7 +68,7 @@ void testWrongTypes() {
// an exception.
void testDynamicAsFunction() {
dynamic x = ffi.nullptr.cast<ffi.NativeFunction<ffi.Void Function()>>();
Expect.throwsUnsupportedError(() {
Expect.throwsNoSuchMethodError(() {
x.asFunction<void Function()>();
});
}
@ -77,7 +77,7 @@ void testDynamicAsFunction() {
// type throws an exception.
void testDynamicLookupFunction() {
dynamic lib = ffiTestFunctions;
Expect.throwsUnsupportedError(() {
Expect.throwsNoSuchMethodError(() {
lib.lookupFunction<ffi.Void Function(), void Function()>("_");
});
}

View file

@ -68,7 +68,7 @@ void testWrongTypes() {
// an exception.
void testDynamicAsFunction() {
dynamic x = ffi.nullptr.cast<ffi.NativeFunction<ffi.Void Function()>>();
Expect.throwsUnsupportedError(() {
Expect.throwsNoSuchMethodError(() {
x.asFunction<void Function()>();
});
}
@ -77,7 +77,7 @@ void testDynamicAsFunction() {
// type throws an exception.
void testDynamicLookupFunction() {
dynamic lib = ffiTestFunctions;
Expect.throwsUnsupportedError(() {
Expect.throwsNoSuchMethodError(() {
lib.lookupFunction<ffi.Void Function(), void Function()>("_");
});
}