[vm/ffi] Convert Objects to Dart_Handles in FFI calls

This includes support for calling Dart_PropagateError in native code
when doing FFI calls, and catching uncaught exceptions with Dart_IsError
when doing FFI callbacks.

The support for Dart_PropagateError adds a catch entry to the FFI
trampoline, which prevents inlining these trampolines in AOT. This
regresses the FfiCall benchmarks by 1-2% in AOT.

In addition, Dart_PropagateError requires maintaining a bit whether we
entered native/VM code from generated code through FFI or not. That way
we can do the proper transition on the exception path. When entering
generated code, we store this bit on the stack, right after the entry
frame.

Design: http://go/dart-ffi-handles

Issue: #36858
Issue: #41319

Change-Id: Idfd7ff69132fb29cc730931a4113d914d4437396
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,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-nnbd-linux-debug-x64-try,analyzer-nnbd-linux-release-try,front-end-nnbd-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/145591
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>

Author: dcharkes

pkg/analyzer/lib/src/dart/error/ffi_code.dart

@@ -61,7 +61,7 @@ class FfiCode extends AnalyzerErrorCode {
       message:
           "The method 'Pointer.fromFunction' must not have an exceptional return "
           "value (the second argument) when the return type of the function is "
-          "either 'void' or 'Pointer'.",
+          "either 'void', 'Handle' or 'Pointer'.",
       correction: "Try removing the exceptional return value.");
 
   /**
@@ -102,7 +102,7 @@ class FfiCode extends AnalyzerErrorCode {
       message:
           "The method 'Pointer.fromFunction' must have an exceptional return "
           "value (the second argument) when the return type of the function is "
-          "neither 'void' or 'Pointer'.",
+          "neither 'void', 'Handle' or 'Pointer'.",
       correction: "Try adding an exceptional return value.");
 
   /**

pkg/analyzer/lib/src/generated/ffi_verifier.dart

@@ -190,6 +190,9 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
   bool _isPointer(Element element) =>
       element.name == 'Pointer' && element.library.name == 'dart.ffi';
 
+  bool _isHandle(Element element) =>
+      element.name == 'Handle' && element.library.name == 'dart.ffi';
+
   bool _isNativeFunctionPointerExtension(Element element) =>
       element.name == 'NativeFunctionPointer' &&
       element.library.name == 'dart.ffi';
@@ -295,6 +298,9 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
         if (name == 'Void') {
           return _PrimitiveDartType.void_;
         }
+        if (name == 'Handle') {
+          return _PrimitiveDartType.handle;
+        }
       }
     }
     return _PrimitiveDartType.none;
@@ -449,6 +455,11 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
       return dartType.isDartCoreDouble;
     } else if (nativeReturnType == _PrimitiveDartType.void_) {
       return dartType.isVoid;
+    } else if (nativeReturnType == _PrimitiveDartType.handle) {
+      InterfaceType objectType = typeSystem.objectStar;
+      return checkCovariance
+          ? /* everything is subtype of objectStar */ true
+          : typeSystem.isSubtypeOf2(objectType, dartType);
     } else if (dartType is InterfaceType && nativeType is InterfaceType) {
       return checkCovariance
           ? typeSystem.isSubtypeOf2(dartType, nativeType)
@@ -520,7 +531,9 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
 
     // TODO(brianwilkerson) Validate that `f` is a top-level function.
     final DartType R = (T as FunctionType).returnType;
-    if ((FT as FunctionType).returnType.isVoid || _isPointer(R.element)) {
+    if ((FT as FunctionType).returnType.isVoid ||
+        _isPointer(R.element) ||
+        _isHandle(R.element)) {
       if (argCount != 1) {
         _errorReporter.reportErrorForNode(
             FfiCode.INVALID_EXCEPTION_VALUE, node.argumentList.arguments[1]);
@@ -591,5 +604,6 @@ enum _PrimitiveDartType {
   double,
   int,
   void_,
+  handle,
   none,
 }

pkg/vm/lib/transformations/ffi.dart

@@ -34,7 +34,8 @@ enum NativeType {
   kFloat,
   kDouble,
   kVoid,
-  kStruct
+  kStruct,
+  kHandle,
 }
 
 const NativeType kNativeTypeIntStart = NativeType.kInt8;
@@ -59,7 +60,8 @@ const List<String> nativeTypeClassNames = [
   'Float',
   'Double',
   'Void',
-  'Struct'
+  'Struct',
+  'Handle'
 ];
 
 const int UNKNOWN = 0;
@@ -85,6 +87,7 @@ const List<int> nativeTypeSizes = [
   8, // Double
   UNKNOWN, // Void
   UNKNOWN, // Struct
+  WORD_SIZE, // Handle
 ];
 
 /// The struct layout in various ABIs.
@@ -181,6 +184,7 @@ class FfiTransformer extends Transformer {
   final DiagnosticReporter diagnosticReporter;
   final ReferenceFromIndex referenceFromIndex;
 
+  final Class objectClass;
   final Class intClass;
   final Class doubleClass;
   final Class listClass;
@@ -219,6 +223,7 @@ class FfiTransformer extends Transformer {
   FfiTransformer(this.index, this.coreTypes, this.hierarchy,
       this.diagnosticReporter, this.referenceFromIndex)
       : env = new TypeEnvironment(coreTypes, hierarchy),
+        objectClass = coreTypes.objectClass,
         intClass = coreTypes.intClass,
         doubleClass = coreTypes.doubleClass,
         listClass = coreTypes.listClass,
@@ -288,9 +293,11 @@ class FfiTransformer extends Transformer {
   /// [Void]                               -> [void]
   /// [Pointer]<T>                         -> [Pointer]<T>
   /// T extends [Pointer]                  -> T
+  /// [Handle]                             -> [Object]
   /// [NativeFunction]<T1 Function(T2, T3) -> S1 Function(S2, S3)
   ///    where DartRepresentationOf(Tn) -> Sn
-  DartType convertNativeTypeToDartType(DartType nativeType, bool allowStructs) {
+  DartType convertNativeTypeToDartType(
+      DartType nativeType, bool allowStructs, bool allowHandle) {
     if (nativeType is! InterfaceType) {
       return null;
     }
@@ -317,6 +324,9 @@ class FfiTransformer extends Transformer {
     if (nativeType_ == NativeType.kVoid) {
       return VoidType();
     }
+    if (nativeType_ == NativeType.kHandle && allowHandle) {
+      return InterfaceType(objectClass, Nullability.legacy);
+    }
     if (nativeType_ != NativeType.kNativeFunction ||
         native.typeArguments[0] is! FunctionType) {
       return null;
@@ -329,11 +339,12 @@ class FfiTransformer extends Transformer {
     }
     if (fun.typeParameters.length != 0) return null;
     // TODO(36730): Structs cannot appear in native function signatures.
-    final DartType returnType =
-        convertNativeTypeToDartType(fun.returnType, /*allowStructs=*/ false);
+    final DartType returnType = convertNativeTypeToDartType(
+        fun.returnType, /*allowStructs=*/ false, /*allowHandle=*/ true);
     if (returnType == null) return null;
     final List<DartType> argumentTypes = fun.positionalParameters
-        .map((t) => convertNativeTypeToDartType(t, /*allowStructs=*/ false))
+        .map((t) => convertNativeTypeToDartType(
+            t, /*allowStructs=*/ false, /*allowHandle=*/ true))
         .toList();
     if (argumentTypes.contains(null)) return null;
     return FunctionType(argumentTypes, returnType, Nullability.legacy);

pkg/vm/lib/transformations/ffi_definitions.dart

@@ -230,8 +230,8 @@ class _FfiDefinitionTransformer extends FfiTransformer {
             nativeTypesClasses[nativeTypeAnnos.first.index],
             Nullability.legacy);
         // TODO(36730): Support structs inside structs.
-        final DartType shouldBeDartType =
-            convertNativeTypeToDartType(nativeType, /*allowStructs=*/ false);
+        final DartType shouldBeDartType = convertNativeTypeToDartType(
+            nativeType, /*allowStructs=*/ false, /*allowHandle=*/ false);
         if (shouldBeDartType == null ||
             !env.isSubtypeOf(type, shouldBeDartType,
                 SubtypeCheckMode.ignoringNullabilities)) {

pkg/vm/lib/transformations/ffi_use_sites.dart

@@ -202,7 +202,8 @@ class _FfiUseSiteTransformer extends FfiTransformer {
                 .classNode);
 
         if (expectedReturn == NativeType.kVoid ||
-            expectedReturn == NativeType.kPointer) {
+            expectedReturn == NativeType.kPointer ||
+            expectedReturn == NativeType.kHandle) {
           if (node.arguments.positional.length > 1) {
             diagnosticReporter.report(
                 templateFfiExpectedNoExceptionalReturn.withArguments(
@@ -380,9 +381,9 @@ class _FfiUseSiteTransformer extends FfiTransformer {
 
   void _ensureNativeTypeToDartType(
       DartType nativeType, DartType dartType, Expression node,
-      {bool allowStructs: false}) {
+      {bool allowStructs: false, bool allowHandle: false}) {
     final DartType correspondingDartType =
-        convertNativeTypeToDartType(nativeType, allowStructs);
+        convertNativeTypeToDartType(nativeType, allowStructs, allowHandle);
     if (dartType == correspondingDartType) return;
     if (env.isSubtypeOf(correspondingDartType, dartType,
         SubtypeCheckMode.ignoringNullabilities)) {
@@ -398,8 +399,9 @@ class _FfiUseSiteTransformer extends FfiTransformer {
   }
 
   void _ensureNativeTypeValid(DartType nativeType, Expression node,
-      {bool allowStructs: false}) {
-    if (!_nativeTypeValid(nativeType, allowStructs: allowStructs)) {
+      {bool allowStructs: false, bool allowHandle: false}) {
+    if (!_nativeTypeValid(nativeType,
+        allowStructs: allowStructs, allowHandle: allowHandle)) {
       diagnosticReporter.report(
           templateFfiTypeInvalid.withArguments(
               nativeType, currentLibrary.isNonNullableByDefault),
@@ -412,8 +414,10 @@ class _FfiUseSiteTransformer extends FfiTransformer {
 
   /// The Dart type system does not enforce that NativeFunction return and
   /// parameter types are only NativeTypes, so we need to check this.
-  bool _nativeTypeValid(DartType nativeType, {bool allowStructs: false}) {
-    return convertNativeTypeToDartType(nativeType, allowStructs) != null;
+  bool _nativeTypeValid(DartType nativeType,
+      {bool allowStructs: false, allowHandle: false}) {
+    return convertNativeTypeToDartType(nativeType, allowStructs, allowHandle) !=
+        null;
   }
 
   void _ensureIsStaticFunction(Expression node) {

runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc

@@ -790,4 +790,126 @@ DART_EXPORT void ThreadPoolTest_BarrierSync(
   dart_enter_isolate(isolate);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Functions for handle tests.
+//
+// vmspecific_handle_test.dart
+
+static void RunFinalizer(void* isolate_callback_data,
+                         Dart_WeakPersistentHandle handle,
+                         void* peer) {
+  printf("Running finalizer for weak handle.\n");
+}
+
+// Tests that passing handles through FFI calls works, and that the FFI call
+// sets up the VM state etc. correctly so that the handle API calls work.
+DART_EXPORT Dart_Handle PassObjectToC(Dart_Handle h) {
+  // Can use "h" until this function returns.
+
+  // A persistent handle which outlives this call. Lifetime managed in C.
+  auto persistent_handle = Dart_NewPersistentHandle(h);
+
+  Dart_Handle handle_2 = Dart_HandleFromPersistent(persistent_handle);
+  Dart_DeletePersistentHandle(persistent_handle);
+  if (Dart_IsError(handle_2)) {
+    Dart_PropagateError(handle_2);
+  }
+
+  Dart_Handle return_value;
+  if (!Dart_IsNull(h)) {
+    // A weak handle which outlives this call. Lifetime managed in C.
+    auto weak_handle = Dart_NewWeakPersistentHandle(
+        h, reinterpret_cast<void*>(0x1234), 64, RunFinalizer);
+    return_value = Dart_HandleFromWeakPersistent(weak_handle);
+
+    // Deleting a weak handle is not required, it deletes itself on
+    // finalization.
+    // Deleting a weak handle cancels the finalizer.
+    Dart_DeleteWeakPersistentHandle(weak_handle);
+  } else {
+    return_value = h;
+  }
+
+  return return_value;
+}
+
+DART_EXPORT void ClosureCallbackThroughHandle(void (*callback)(Dart_Handle),
+                                              Dart_Handle closureHandle) {
+  printf("ClosureCallbackThroughHandle %p %p\n", callback, closureHandle);
+  callback(closureHandle);
+}
+
+DART_EXPORT Dart_Handle ReturnHandleInCallback(Dart_Handle (*callback)()) {
+  printf("ReturnHandleInCallback %p\n", callback);
+  Dart_Handle handle = callback();
+  if (Dart_IsError(handle)) {
+    printf("callback() returned an error, propagating error\n");
+    // Do C/C++ resource cleanup if needed, before propagating error.
+    Dart_PropagateError(handle);
+  }
+  return handle;
+}
+
+// Recurses til `i` reaches 0. Throws some Dart_Invoke in there as well.
+DART_EXPORT Dart_Handle HandleRecursion(Dart_Handle object,
+                                        Dart_Handle (*callback)(int64_t),
+                                        int64_t i) {
+  printf("HandleRecursion %" Pd64 "\n", i);
+  const bool do_invoke = i % 3 == 0;
+  const bool do_gc = i % 7 == 3;
+  if (do_gc) {
+    Dart_ExecuteInternalCommand("gc-now", nullptr);
+  }
+  Dart_Handle result;
+  if (do_invoke) {
+    Dart_Handle method_name = Dart_NewStringFromCString("a");
+    if (Dart_IsError(method_name)) {
+      Dart_PropagateError(method_name);
+    }
+    Dart_Handle arg = Dart_NewInteger(i - 1);
+    if (Dart_IsError(arg)) {
+      Dart_PropagateError(arg);
+    }
+    printf("Dart_Invoke\n");
+    result = Dart_Invoke(object, method_name, 1, &arg);
+  } else {
+    printf("callback\n");
+    result = callback(i - 1);
+  }
+  if (do_gc) {
+    Dart_ExecuteInternalCommand("gc-now", nullptr);
+  }
+  if (Dart_IsError(result)) {
+    // Do C/C++ resource cleanup if needed, before propagating error.
+    printf("Dart_PropagateError %" Pd64 "\n", i);
+    Dart_PropagateError(result);
+  }
+  printf("return %" Pd64 "\n", i);
+  return result;
+}
+
+DART_EXPORT int64_t HandleReadFieldValue(Dart_Handle handle) {
+  printf("HandleReadFieldValue\n");
+  Dart_Handle field_name = Dart_NewStringFromCString("a");
+  if (Dart_IsError(field_name)) {
+    printf("Dart_PropagateError(field_name)\n");
+    Dart_PropagateError(field_name);
+  }
+  Dart_Handle field_value = Dart_GetField(handle, field_name);
+  if (Dart_IsError(field_value)) {
+    printf("Dart_PropagateError(field_value)\n");
+    Dart_PropagateError(field_value);
+  }
+  int64_t value;
+  Dart_Handle err = Dart_IntegerToInt64(field_value, &value);
+  if (Dart_IsError(err)) {
+    Dart_PropagateError(err);
+  }
+  return value;
+}
+
+DART_EXPORT Dart_Handle TrueHandle() {
+  return Dart_True();
+}
+
 }  // namespace dart

runtime/docs/gc.md

@@ -67,7 +67,7 @@ With the mutator and marker running concurrently, the mutator could write a poin
 
 The barrier is equivalent to
 
-```
+```c++
 StorePoint(RawObject* source, RawObject** slot, RawObject* target) {
   *slot = target;
   if (target->IsSmi()) return;
@@ -84,7 +84,7 @@ StorePoint(RawObject* source, RawObject** slot, RawObject* target) {
 
 But we combine the generational and incremental checks with a shift-and-mask.
 
-```
+```c++
 enum HeaderBits {
   ...
   kOldAndNotMarkedBit,      // Incremental barrier target.

runtime/vm/class_id.h

@@ -131,7 +131,8 @@ namespace dart {
 
 #define CLASS_LIST_FFI_TYPE_MARKER(V)                                          \
   CLASS_LIST_FFI_NUMERIC(V)                                                    \
-  V(Void)
+  V(Void)                                                                      \
+  V(Handle)
 
 #define CLASS_LIST_FFI(V)                                                      \
   V(Pointer)                                                                   \