commit 93d5c4eeeff269d798fe66df78aafc28ed3ab605
author Clement Skau <cskau@google.com> Tue Jul 20 13:01:40 2021 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Tue Jul 20 13:01:40 2021 +0000
tree e2aa671940f8e08d915edf41d653108f82dfd341
parent 8674c7da89b05efdfae513705e2e1142df934782

[VM] Adds error for leaf FfiNative w. Handle

This adds an analyzer compile-time error for using Handles
with leaf call FfiNatives.
This same error already exists in the transforms.
(See `_ensureLeafCallDoesNotUseHandles(..)`)

Bug: https://github.com/dart-lang/sdk/issues/46625
Change-Id: Iec52d5df6dd346a25ef2976e552cf1cb58c17ff6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207460
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Clement Skau <cskau@google.com>

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

diff --git a/pkg/analyzer/lib/src/generated/ffi_verifier.dart b/pkg/analyzer/lib/src/generated/ffi_verifier.dart
index ae2d2bc7..73cc7f3 100644
--- a/pkg/analyzer/lib/src/generated/ffi_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/ffi_verifier.dart
@@ -277,12 +277,26 @@
 
     for (Annotation annotation in annotations) {
       if (annotation.name.name == _ffiNativeName) {
+        // All FFI Natives must be static.
         final isStatic = (node is FunctionDeclaration) ||
             ((node is MethodDeclaration) && node.isStatic);
         if (!isStatic) {
           _errorReporter.reportErrorForNode(
               FfiCode.FFI_NATIVE_ONLY_STATIC, node);
         }
+        // Leaf call FFI Natives can't use Handles.
+        ArgumentList? argumentList = annotation.arguments;
+        if (argumentList != null) {
+          NodeList<Expression> arguments = argumentList.arguments;
+          TypeArgumentList? typeArgumentList = annotation.typeArguments;
+          if (typeArgumentList != null) {
+            NodeList<TypeAnnotation> typeArguments = typeArgumentList.arguments;
+            if (typeArguments.isNotEmpty && typeArguments[0].type != null) {
+              _validateFfiLeafCallUsesNoHandles(
+                  arguments, typeArguments[0].type!, node);
+            }
+          }
+        }
       }
     }
   }
@@ -530,7 +544,8 @@
         _errorReporter.reportErrorForNode(
             FfiCode.MUST_BE_A_SUBTYPE, node, [TPrime, F, 'asFunction']);
       }
-      _validateFfiLeafCallUsesNoHandles(node, TPrime, node);
+      _validateFfiLeafCallUsesNoHandles(node.argumentList.arguments, TPrime,
+          node);
     }
     _validateIsLeafIsConst(node);
   }
@@ -622,9 +637,8 @@
     }
   }
 
-  void _validateFfiLeafCallUsesNoHandles(
-      MethodInvocation node, DartType nativeType, AstNode errorNode) {
-    final args = node.argumentList.arguments;
+  void _validateFfiLeafCallUsesNoHandles(NodeList<Expression> args,
+      DartType nativeType, AstNode errorNode) {
     if (args.isNotEmpty) {
       for (final arg in args) {
         if (arg is NamedExpression) {
@@ -817,7 +831,8 @@
           FfiCode.MUST_BE_A_SUBTYPE, errorNode, [S, F, 'lookupFunction']);
     }
     _validateIsLeafIsConst(node);
-    _validateFfiLeafCallUsesNoHandles(node, S, typeArguments![0]);
+    _validateFfiLeafCallUsesNoHandles(node.argumentList.arguments, S,
+        typeArguments![0]);
   }
 
   /// Validate that none of the [annotations] are from `dart:ffi`.

pkg/analyzer/test/src/diagnostics/ffi_native_test.dart

diff --git a/pkg/analyzer/test/src/diagnostics/ffi_native_test.dart b/pkg/analyzer/test/src/diagnostics/ffi_native_test.dart
index e90a157..a11172c 100644
--- a/pkg/analyzer/test/src/diagnostics/ffi_native_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/ffi_native_test.dart
@@ -26,4 +26,32 @@
       error(FfiCode.FFI_NATIVE_ONLY_STATIC, 31, 75),
     ]);
   }
+
+  test_FfiNativeCanUseHandles() async {
+    await assertErrorsInCode(r'''
+import 'dart:ffi';
+@FfiNative<Handle Function(Handle)>('DoesntMatter')
+external Object doesntMatter(Object);
+''', []);
+  }
+
+  test_FfiNativeLeafMustNotReturnHandle() async {
+    await assertErrorsInCode(r'''
+import 'dart:ffi';
+@FfiNative<Handle Function()>('DoesntMatter', isLeaf:true)
+external Object doesntMatter();
+''', [
+      error(FfiCode.LEAF_CALL_MUST_NOT_RETURN_HANDLE, 19, 90),
+    ]);
+  }
+
+  test_FfiNativeLeafMustNotTakeHandles() async {
+    await assertErrorsInCode(r'''
+import 'dart:ffi';
+@FfiNative<Void Function(Handle)>('DoesntMatter', isLeaf:true)
+external void doesntMatter(Object o);
+''', [
+      error(FfiCode.LEAF_CALL_MUST_NOT_TAKE_HANDLE, 19, 100),
+    ]);
+  }
 }