[vm/ffi] Change Pointer<T extends Struct>.ref to use static type

This CL changes the semantics of `Pointer<T extends Struct>.ref` to use
the compile-time `T` rather than the runtime `T`.

This enables tree shaking of subtypes of Struct and optimizing `.ref`.

Issue: #38721

TEST=tests/ffi/vmspecific_static_checks_test.dart
TEST=tests/ffi/*struct*test_.dart

Change-Id: I3f5b08c08ec0799ef8aab3c4177e2ac70d25501c
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,analyzer-nnbd-linux-release-try,front-end-linux-release-x64-try,front-end-nnbd-linux-release-x64-try,benchmark-linux-try,dart-sdk-linux-try,pkg-linux-release-try,vm-ffi-android-release-arm-try,vm-ffi-android-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177862
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Aske Simon Christensen <[email protected]>

Author: dcharkes

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

@@ -152,6 +152,19 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     super.visitFunctionExpressionInvocation(node);
   }
 
+  @override
+  void visitIndexExpression(IndexExpression node) {
+    Element element = node.staticElement;
+    Element enclosingElement = element?.enclosingElement;
+    if (enclosingElement is ExtensionElement) {
+      if (_isNativeStructPointerExtension(enclosingElement)) {
+        if (element.name == '[]') {
+          _validateRefIndexed(node);
+        }
+      }
+    }
+  }
+
   @override
   void visitMethodInvocation(MethodInvocation node) {
     Element element = node.methodName.staticElement;
@@ -178,6 +191,32 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     super.visitMethodInvocation(node);
   }
 
+  @override
+  void visitPrefixedIdentifier(PrefixedIdentifier node) {
+    Element element = node.staticElement;
+    Element enclosingElement = element?.enclosingElement;
+    if (enclosingElement is ExtensionElement) {
+      if (_isNativeStructPointerExtension(enclosingElement)) {
+        if (element.name == 'ref') {
+          _validateRefPrefixedIdentifier(node);
+        }
+      }
+    }
+  }
+
+  @override
+  void visitPropertyAccess(PropertyAccess node) {
+    Element element = node.propertyName.staticElement;
+    Element enclosingElement = element?.enclosingElement;
+    if (enclosingElement is ExtensionElement) {
+      if (_isNativeStructPointerExtension(enclosingElement)) {
+        if (element.name == 'ref') {
+          _validateRefPropertyAccess(node);
+        }
+      }
+    }
+  }
+
   /// Return `true` if the given [element] represents the extension
   /// `AllocatorAlloc`.
   bool _isAllocatorExtension(Element element) =>
@@ -240,6 +279,9 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
       element.name == 'NativeFunctionPointer' &&
       element.library.name == _dartFfiLibraryName;
 
+  bool _isNativeStructPointerExtension(Element element) =>
+      element.name == 'StructPointer' && element.library.name == 'dart.ffi';
+
   /// Returns `true` iff [nativeType] is a `ffi.NativeType` type.
   bool _isNativeTypeInterfaceType(DartType nativeType) {
     if (nativeType is InterfaceType) {
@@ -697,6 +739,35 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  void _validateRefIndexed(IndexExpression node) {
+    DartType targetType = node.target.staticType;
+    if (!_isValidFfiNativeType(targetType, false, true)) {
+      final AstNode errorNode = node;
+      _errorReporter.reportErrorForNode(
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['[]']);
+    }
+  }
+
+  /// Validate the invocation of the extension method
+  /// `Pointer<T extends Struct>.ref`.
+  void _validateRefPrefixedIdentifier(PrefixedIdentifier node) {
+    DartType targetType = node.prefix.staticType;
+    if (!_isValidFfiNativeType(targetType, false, true)) {
+      final AstNode errorNode = node;
+      _errorReporter.reportErrorForNode(
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['ref']);
+    }
+  }
+
+  void _validateRefPropertyAccess(PropertyAccess node) {
+    DartType targetType = node.target.staticType;
+    if (!_isValidFfiNativeType(targetType, false, true)) {
+      final AstNode errorNode = node;
+      _errorReporter.reportErrorForNode(
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['ref']);
+    }
+  }
+
   /// Validate that the given [typeArgument] has a constant value. Return `true`
   /// if a diagnostic was produced because it isn't constant.
   bool _validateTypeArgument(TypeAnnotation typeArgument, String functionName) {

pkg/vm/lib/transformations/ffi.dart

@@ -219,6 +219,8 @@ class FfiTransformer extends Transformer {
   final Procedure offsetByMethod;
   final Procedure elementAtMethod;
   final Procedure addressGetter;
+  final Procedure structPointerRef;
+  final Procedure structPointerElemAt;
   final Procedure asFunctionMethod;
   final Procedure asFunctionInternal;
   final Procedure lookupFunctionMethod;
@@ -282,6 +284,10 @@ class FfiTransformer extends Transformer {
             index.getMember('dart:ffi', 'Struct', '_fromPointer'),
         fromAddressInternal =
             index.getTopLevelMember('dart:ffi', '_fromAddress'),
+        structPointerRef =
+            index.getMember('dart:ffi', 'StructPointer', 'get:ref'),
+        structPointerElemAt =
+            index.getMember('dart:ffi', 'StructPointer', '[]'),
         asFunctionMethod =
             index.getMember('dart:ffi', 'NativeFunctionPointer', 'asFunction'),
         asFunctionInternal =
@@ -344,7 +350,9 @@ class FfiTransformer extends Transformer {
   /// [NativeFunction]<T1 Function(T2, T3) -> S1 Function(S2, S3)
   ///    where DartRepresentationOf(Tn) -> Sn
   DartType convertNativeTypeToDartType(DartType nativeType,
-      {bool allowStructs = false, bool allowHandle = false}) {
+      {bool allowStructs = false,
+      bool allowStructItself = false,
+      bool allowHandle = false}) {
     if (nativeType is! InterfaceType) {
       return null;
     }
@@ -353,6 +361,9 @@ class FfiTransformer extends Transformer {
     final NativeType nativeType_ = getType(nativeClass);
 
     if (hierarchy.isSubclassOf(nativeClass, structClass)) {
+      if (structClass == nativeClass) {
+        return allowStructItself ? nativeType : null;
+      }
       return allowStructs ? nativeType : null;
     }
     if (nativeType_ == null) {

pkg/vm/lib/transformations/ffi_use_sites.dart

@@ -173,7 +173,14 @@ class _FfiUseSiteTransformer extends FfiTransformer {
 
     final Member target = node.target;
     try {
-      if (target == lookupFunctionMethod) {
+      if (target == structPointerRef || target == structPointerElemAt) {
+        final DartType nativeType = node.arguments.types[0];
+
+        _ensureNativeTypeValid(nativeType, node, allowStructItself: false);
+
+        // TODO(http://dartbug.com/38721): Replace calls with direct
+        // constructor invocations.
+      } else if (target == lookupFunctionMethod) {
         final DartType nativeType = InterfaceType(
             nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
         final DartType dartType = node.arguments.types[1];
@@ -427,9 +434,11 @@ class _FfiUseSiteTransformer extends FfiTransformer {
   }
 
   void _ensureNativeTypeValid(DartType nativeType, Expression node,
-      {bool allowHandle: false}) {
+      {bool allowHandle: false, bool allowStructItself = true}) {
     if (!_nativeTypeValid(nativeType,
-        allowStructs: true, allowHandle: allowHandle)) {
+        allowStructs: true,
+        allowStructItself: allowStructItself,
+        allowHandle: allowHandle)) {
       diagnosticReporter.report(
           templateFfiTypeInvalid.withArguments(
               nativeType, currentLibrary.isNonNullableByDefault),
@@ -466,9 +475,13 @@ 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, allowHandle: false}) {
+      {bool allowStructs: false,
+      bool allowStructItself = false,
+      bool allowHandle = false}) {
     return convertNativeTypeToDartType(nativeType,
-            allowStructs: allowStructs, allowHandle: allowHandle) !=
+            allowStructs: allowStructs,
+            allowStructItself: allowStructItself,
+            allowHandle: allowHandle) !=
         null;
   }
 

runtime/lib/ffi.cc

@@ -116,7 +116,7 @@ static ObjectPtr LoadValueStruct(Zone* zone,
 DEFINE_NATIVE_ENTRY(Ffi_loadStruct, 0, 2) {
   GET_NON_NULL_NATIVE_ARGUMENT(Pointer, pointer, arguments->NativeArgAt(0));
   const AbstractType& pointer_type_arg =
-      AbstractType::Handle(pointer.type_argument());
+      AbstractType::Handle(arguments->NativeTypeArgAt(0));
   GET_NON_NULL_NATIVE_ARGUMENT(Integer, index, arguments->NativeArgAt(1));
 
   // TODO(36370): Make representation consistent with kUnboxedFfiIntPtr.

sdk/lib/_internal/vm/lib/ffi_patch.dart

@@ -204,6 +204,13 @@ double _loadDouble(Pointer pointer, int offsetInBytes) native "Ffi_loadDouble";
 Pointer<S> _loadPointer<S extends NativeType>(
     Pointer pointer, int offsetInBytes) native "Ffi_loadPointer";
 
+S _loadStructNoStruct<S extends Struct>(Pointer<S> pointer, int index) {
+  if (S == Struct) {
+    throw ArgumentError("S should be a subtype of Struct.");
+  }
+  return _loadStruct(pointer, index);
+}
+
 S _loadStruct<S extends Struct>(Pointer<S> pointer, int index)
     native "Ffi_loadStruct";
 
@@ -511,10 +518,10 @@ extension PointerPointer<T extends NativeType> on Pointer<Pointer<T>> {
 
 extension StructPointer<T extends Struct> on Pointer<T> {
   @patch
-  T get ref => _loadStruct(this, 0);
+  T get ref => _loadStructNoStruct(this, 0);
 
   @patch
-  T operator [](int index) => _loadStruct(this, index);
+  T operator [](int index) => _loadStructNoStruct(this, index);
 }
 
 extension NativePort on SendPort {

sdk/lib/ffi/ffi.dart

@@ -538,13 +538,17 @@ extension StructPointer<T extends Struct> on Pointer<T> {
   ///
   /// The [address] must be aligned according to the struct alignment rules of
   /// the platform.
+  ///
+  /// This extension method must be invoked with a compile-time constant [T].
   external T get ref;
 
   /// Creates a reference to access the fields of this struct backed by native
   /// memory at `address + sizeOf<T>() * index`.
   ///
   /// The [address] must be aligned according to the struct alignment rules of
   /// the platform.
+  ///
+  /// This extension method must be invoked with a compile-time constant [T].
   external T operator [](int index);
 }
 

tests/ffi/vmspecific_static_checks_test.dart

@@ -49,6 +49,7 @@ void main() {
   testHandleVariance();
   testAllocateGeneric();
   testAllocateNativeType();
+  testRefStruct();
 }
 
 typedef Int8UnOp = Int8 Function(Int8);
@@ -540,3 +541,19 @@ void testAllocateGeneric() {
 void testAllocateNativeType() {
   calloc(); //# 1321: compile-time error
 }
+
+void testRefStruct() {
+  final myStructPointer = malloc<TestStruct13>();
+  Pointer<Struct> structPointer = myStructPointer;
+  structPointer.ref; //# 1330: compile-time error
+  malloc.free(myStructPointer);
+}
+
+T genericRef<T extends Struct>(Pointer<T> p) => //# 1200: compile-time error
+    p.ref; //# 1200: compile-time error
+
+T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: compile-time error
+    p.cast<T>().ref; //# 1201: compile-time error
+
+T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: compile-time error
+    p[0]; //# 1202: compile-time error

tests/ffi_2/vmspecific_static_checks_test.dart

@@ -49,6 +49,7 @@ void main() {
   testHandleVariance();
   testAllocateGeneric();
   testAllocateNativeType();
+  testRefStruct();
 }
 
 typedef Int8UnOp = Int8 Function(Int8);
@@ -538,3 +539,19 @@ void testAllocateGeneric() {
 void testAllocateNativeType() {
   calloc(); //# 1321: compile-time error
 }
+
+void testRefStruct() {
+  final myStructPointer = malloc<TestStruct13>();
+  Pointer<Struct> structPointer = myStructPointer;
+  structPointer.ref; //# 1330: compile-time error
+  malloc.free(myStructPointer);
+}
+
+T genericRef<T extends Struct>(Pointer<T> p) => //# 1200: compile-time error
+    p.ref; //# 1200: compile-time error
+
+T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: compile-time error
+    p.cast<T>().ref; //# 1201: compile-time error
+
+T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: compile-time error
+    p[0]; //# 1202: compile-time error