[cfe/ffi] asFunction/fromFunction variance in CFE + regression test

Issue: #37385

Change-Id: I0a0704f3513bf8de802e7481d813f72678837e0b
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,dart-sdk-linux-try,analyzer-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107511
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

Author: dcharkes

pkg/vm/lib/transformations/ffi_use_sites.dart

@@ -344,17 +344,18 @@ class _FfiUseSiteTransformer extends FfiTransformer {
   }
 
   void _ensureNativeTypeToDartType(
-      DartType containerTypeArg, DartType elementType, Expression node,
+      DartType nativeType, DartType dartType, Expression node,
       {bool allowStructs: false}) {
-    final DartType shouldBeElementType =
-        convertNativeTypeToDartType(containerTypeArg, allowStructs);
-    if (elementType == shouldBeElementType) return;
-    // We disable implicit downcasts, they will go away when NNBD lands.
-    if (env.isSubtypeOf(elementType, shouldBeElementType,
-        SubtypeCheckMode.ignoringNullabilities)) return;
+    final DartType correspondingDartType =
+        convertNativeTypeToDartType(nativeType, allowStructs);
+    if (dartType == correspondingDartType) return;
+    if (env.isSubtypeOf(correspondingDartType, dartType,
+        SubtypeCheckMode.ignoringNullabilities)) {
+      return;
+    }
     diagnosticReporter.report(
         templateFfiTypeMismatch.withArguments(
-            elementType, shouldBeElementType, containerTypeArg),
+            dartType, correspondingDartType, nativeType),
         node.fileOffset,
         1,
         node.location.file);

runtime/bin/ffi_test/ffi_test_functions.cc

@@ -858,4 +858,31 @@ DART_EXPORT int TestCallbackWrongIsolate(void (*fn)()) {
 
 #endif  // defined(TARGET_OS_LINUX)
 
+// Receives some pointer (Pointer<NativeType> in Dart) and writes some bits.
+DART_EXPORT void NativeTypePointerParam(void* p) {
+  uint8_t* p2 = reinterpret_cast<uint8_t*>(p);
+  p2[0] = 42;
+}
+
+// Manufactures some pointer (Pointer<NativeType> in Dart) with a bogus address.
+DART_EXPORT void* NativeTypePointerReturn() {
+  uint64_t bogus_address = 0x13370000;
+  return reinterpret_cast<void*>(bogus_address);
+}
+
+// Passes some pointer (Pointer<NativeType> in Dart) to Dart as argument.
+DART_EXPORT void CallbackNativeTypePointerParam(void (*f)(void*)) {
+  void* pointer = malloc(sizeof(int64_t));
+  f(pointer);
+  free(pointer);
+}
+
+// Receives some pointer (Pointer<NativeType> in Dart) from Dart as return
+// value.
+DART_EXPORT void CallbackNativeTypePointerReturn(void* (*f)()) {
+  void* p = f();
+  uint8_t* p2 = reinterpret_cast<uint8_t*>(p);
+  p2[0] = 42;
+}
+
 }  // namespace dart

tests/ffi/function_test.dart

@@ -13,8 +13,6 @@
 // VMOptions=--write-protect-code --no-dual-map-code --stacktrace-every=100
 // SharedObjects=ffi_test_functions
 
-library FfiTest;
-
 import 'dart:ffi';
 
 import 'dylib_utils.dart';
@@ -44,6 +42,7 @@ void main() {
     testVoidReturn();
     testNoArgs();
     testException();
+    testLookupFunctionPointerNativeType();
   }
 }
 
@@ -405,6 +404,17 @@ void testNoArgs() {
   Expect.approxEquals(1337.0, result);
 }
 
+typedef NativeTypeNFT = Pointer<NativeType> Function(
+    Pointer<Pointer<NativeType>>, Int8);
+typedef NativeTypeFT = Pointer<NativeType> Function(
+    Pointer<Pointer<NativeType>>, int);
+
+void testLookupFunctionPointerNativeType() {
+  // The function signature does not match up, but that does not matter since
+  // this test does not use the trampoline.
+  ffiTestFunctions.lookupFunction<NativeTypeNFT, NativeTypeFT>("LargePointer");
+}
+
 // Throw an exception from within the trampoline and collect a stacktrace
 // include its frame.
 void testException() {

tests/ffi/static_checks_test.dart

@@ -42,7 +42,6 @@ void main() {
   testLookupFunctionGeneric2();
   testLookupFunctionWrongNativeFunctionSignature();
   testLookupFunctionTypeMismatch();
-  testLookupFunctionPointerNativeType();
   testNativeFunctionSignatureInvalidReturn();
   testNativeFunctionSignatureInvalidParam();
   testNativeFunctionSignatureInvalidOptionalNamed();
@@ -289,14 +288,6 @@ void testLookupFunctionTypeMismatch() {
   l.lookupFunction<NativeDoubleUnOp, IntUnOp>("cos"); //# 18: compile-time error
 }
 
-typedef NativeTypeNFT = Pointer<NativeType> Function(Pointer<Pointer<NativeType>>, Int8);
-typedef NativeTypeFT = Pointer<NativeType> Function(Pointer<Pointer<NativeType>>, int);
-
-void testLookupFunctionPointerNativeType() {
-  final DynamicLibrary l = dlopenPlatformSpecific("ffi_test_dynamic_library");
-  l.lookupFunction<NativeTypeNFT, NativeTypeFT>("LargePointer"); //# 19: ok
-}
-
 // TODO(dacoharkes): make the next 4 test compile errors
 typedef Invalid1 = int Function(Int8);
 typedef Invalid2 = Int8 Function(int);

tests/ffi/variance_function_checks_test.dart

@@ -0,0 +1,67 @@
+// Copyright (c) 2019, 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.
+//
+// SharedObjects=ffi_test_functions
+
+import 'dart:ffi';
+
+import 'dylib_utils.dart';
+
+// ============================================
+// Tests checks on Dart to native (asFunction).
+// ============================================
+
+typedef Int64PointerParamOpDart = void Function(Pointer<Int64>);
+typedef Int64PointerParamOp = Void Function(Pointer<Int64>);
+typedef Int64PointerReturnOp = Pointer<Int64> Function();
+typedef NaTyPointerReturnOp = Pointer<NativeType> Function();
+
+final paramOpName = "NativeTypePointerParam";
+final returnOpName = "NativeTypePointerReturn";
+
+final DynamicLibrary ffiTestFunctions =
+    dlopenPlatformSpecific("ffi_test_functions");
+
+final p1 =
+    ffiTestFunctions.lookup<NativeFunction<Int64PointerParamOp>>(paramOpName);
+final nonInvariantBinding1 = //# 1:  compile-time error
+    p1.asFunction<NaTyPointerParamOpDart>(); //# 1:  continued
+
+final p2 =
+    ffiTestFunctions.lookup<NativeFunction<NaTyPointerReturnOp>>(returnOpName);
+final nonInvariantBinding2 = //# 2:  compile-time error
+    p2.asFunction<Int64PointerReturnOp>(); //# 2:  continued
+
+final p3 = Pointer<
+    NativeFunction<
+        Pointer<NativeFunction<Pointer<NativeType> Function()>>
+            Function()>>.fromAddress(0x1234);
+final f3 = p3 //# 10:  compile-time error
+    .asFunction< //# 10:  continued
+        Pointer< //# 10:  continued
+                NativeFunction< //# 10:  continued
+                    Pointer<Int8> Function()>> //# 10:  continued
+            Function()>(); //# 10:  continued
+
+// ===========================================================
+// Test check on callbacks from native to Dart (fromFunction).
+// ===========================================================
+
+void naTyPointerParamOp(Pointer<NativeType> p) {
+  final Pointer<Int8> asInt8 = p.cast();
+  asInt8.value = 42;
+}
+
+Pointer<Int64> int64PointerReturnOp() {
+  return Pointer.fromAddress(0x13370000);
+}
+
+final implictDowncast1 = //# 3:  compile-time error
+    Pointer.fromFunction<Int64PointerParamOp>(//# 3:  continued
+        naTyPointerParamOp); //# 3:  continued
+final implictDowncast2 = //# 4:  compile-time error
+    Pointer.fromFunction<NaTyPointerReturnOp>(//# 4:  continued
+        int64PointerReturnOp); //# 4:  continued
+
+void main() {}