[vm/ffi] Support passing structs by value

This CL adds passing structs by value in FFI trampolines.
Nested structs and inline arrays are future work.
C defines passing empty structs as undefined behavior, so that is not
supported in this CL.

Suggested review order:
1) commit message
2) ffi/marshaller (decisions for what is done in IL and what in MC)
3) frontend/kernel_to_il (IL construction)
4) backend/il (MC generation from IL)
5) rest in VM

Overall architecture is that structs are split up into word-size chunks
in IL when this is possible: 1 definition in IL per chunk, 1 Location in
IL per chunk, and 1 NativeLocation for the backend per chunk.
In some cases it is not possible or less convenient to split into
chunks. In these cases TypedDataBase objects are stored into and loaded
from directly in machine code.
The various cases:
- FFI call arguments which are not passed as pointers: pass individual
  chunks to FFI call which already have the right location.
- FFI call arguments which are passed as pointers: Pass in TypedDataBase
  to FFI call, allocate space on the stack, and make a copy on the stack
  and pass the copies' address to the callee.
- FFI call return value: pass in TypedData to FFI call, and copy result
  in machine code.
- FFI callback arguments which are not passed as pointers: IL definition
  for each chunk, and populate a new TypedData with those chunks.
- FFI callback arguments which are passed as pointer: IL definition for
  the pointer, and copying of contents in IL.
- FFI return value when location is pointer: Copy data to callee result
  location in IL.
- FFI return value when location is not a pointer: Copy data in machine
  code to the right registers.

Some other notes about the implementation:
- Due to Store/LoadIndexed loading doubles from float arrays, we use
  a int32 instead and use the BitCastInstr.
- Linux ia32 uses `ret 4` when returning structs by value. This requires
  special casing in the FFI callback trampolines to either use `ret` or
  `ret 4` when returning.
- The 1 IL definition, 1 Location, and 1 NativeLocation approach does
  not remove the need for special casing PairLocations in the machine
  code generation because they are 1 Location belonging to 1 definition.

Because of the amount of corner cases in the calling conventions that
need to be covered, the tests are generated, rather than hand-written.

ABIs tested on CQ: x64 (Linux, MacOS, Windows), ia32 (Linux, Windows),
arm (Android softFP, Linux hardFP), arm64 Android.
ABIs tested locally through Flutter: ia32 Android (emulator), x64 iOS
(simulator), arm64 iOS.
ABIs not tested: arm iOS.
TEST=runtime/bin/ffi_test/ffi_test_functions_generated.cc
TEST=runtime/bin/ffi_test/ffi_test_functions.cc
TEST=tests/{ffi,ffi_2}/function_structs_by_value_generated_test.dart
TEST=tests/{ffi,ffi_2}/function_callbacks_structs_by_value_generated_tes
TEST=tests/{ffi,ffi_2}/function_callbacks_structs_by_value_test.dart
TEST=tests/{ffi,ffi_2}/vmspecific_static_checks_test.dart

Closes #36730.

Change-Id: I474d3a4ee1faadbe767ddadd1b696e24d8dc364c
Cq-Include-Trybots: luci.dart.try:dart-sdk-linux-try,dart-sdk-mac-try,dart-sdk-win-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-mac-release-x64-try,vm-kernel-nnbd-win-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-precomp-android-release-arm_x64-try,analyzer-analysis-server-linux-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/140290
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: dcharkes

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -3789,6 +3789,31 @@ const MessageCode messageFastaUsageShort =
   -o <file> Generate the output into <file>.
   -h        Display this message (add -v for information about all options).""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(String name)> templateFfiEmptyStruct = const Template<
+        Message Function(String name)>(
+    messageTemplate:
+        r"""Struct '#name' is empty. Empty structs are undefined behavior.""",
+    withArguments: _withArgumentsFfiEmptyStruct);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name)> codeFfiEmptyStruct =
+    const Code<Message Function(String name)>(
+  "FfiEmptyStruct",
+  templateFfiEmptyStruct,
+);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsFfiEmptyStruct(String name) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  return new Message(codeFfiEmptyStruct,
+      message:
+          """Struct '${name}' is empty. Empty structs are undefined behavior.""",
+      arguments: {'name': name});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeFfiExceptionalReturnNull = messageFfiExceptionalReturnNull;
 

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

@@ -163,6 +163,22 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
       element.name == 'DynamicLibraryExtension' &&
       element.library.name == 'dart.ffi';
 
+  bool _isEmptyStruct(ClassElement classElement) {
+    final fields = classElement.fields;
+    var structFieldCount = 0;
+    for (final field in fields) {
+      final declaredType = field.type;
+      if (declaredType.isDartCoreInt) {
+        structFieldCount++;
+      } else if (declaredType.isDartCoreDouble) {
+        structFieldCount++;
+      } else if (_isPointer(declaredType.element)) {
+        structFieldCount++;
+      }
+    }
+    return structFieldCount == 0;
+  }
+
   bool _isHandle(Element element) =>
       element.name == 'Handle' && element.library.name == 'dart.ffi';
 
@@ -246,12 +262,12 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
           nativeType.optionalParameterTypes.isNotEmpty) {
         return false;
       }
-      if (!_isValidFfiNativeType(nativeType.returnType, true)) {
+      if (!_isValidFfiNativeType(nativeType.returnType, true, false)) {
         return false;
       }
 
-      for (final DartType typeArg in nativeType.typeArguments) {
-        if (!_isValidFfiNativeType(typeArg, false)) {
+      for (final DartType typeArg in nativeType.normalParameterTypes) {
+        if (!_isValidFfiNativeType(typeArg, false, false)) {
           return false;
         }
       }
@@ -261,7 +277,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
   }
 
   /// Validates that the given [nativeType] is a valid dart:ffi native type.
-  bool _isValidFfiNativeType(DartType nativeType, bool allowVoid) {
+  bool _isValidFfiNativeType(
+      DartType nativeType, bool allowVoid, bool allowEmptyStruct) {
     if (nativeType is InterfaceType) {
       // Is it a primitive integer/double type (or ffi.Void if we allow it).
       final primitiveType = _primitiveNativeType(nativeType);
@@ -274,10 +291,20 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
       }
       if (_isPointerInterfaceType(nativeType)) {
         final nativeArgumentType = nativeType.typeArguments.single;
-        return _isValidFfiNativeType(nativeArgumentType, true) ||
+        return _isValidFfiNativeType(nativeArgumentType, true, true) ||
             _isStructClass(nativeArgumentType) ||
             _isNativeTypeInterfaceType(nativeArgumentType);
       }
+      if (_isStructClass(nativeType)) {
+        if (!allowEmptyStruct) {
+          if (_isEmptyStruct(nativeType.element)) {
+            // TODO(dartbug.com/36780): This results in an error message not
+            // mentioning empty structs at all.
+            return false;
+          }
+        }
+        return true;
+      }
     } else if (nativeType is FunctionType) {
       return _isValidFfiNativeFunctionType(nativeType);
     }
@@ -533,7 +560,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     final DartType R = (T as FunctionType).returnType;
     if ((FT as FunctionType).returnType.isVoid ||
         _isPointer(R.element) ||
-        _isHandle(R.element)) {
+        _isHandle(R.element) ||
+        _isStructClass(R)) {
       if (argCount != 1) {
         _errorReporter.reportErrorForNode(
             FfiCode.INVALID_EXCEPTION_VALUE, node.argumentList.arguments[1]);

pkg/front_end/lib/src/api_unstable/vm.dart

@@ -47,6 +47,7 @@ export '../fasta/fasta_codes.dart'
         messageFfiExpectedConstant,
         noLength,
         templateFfiDartTypeMismatch,
+        templateFfiEmptyStruct,
         templateFfiExpectedExceptionalReturn,
         templateFfiExpectedNoExceptionalReturn,
         templateFfiExtendsOrImplementsSealedClass,

pkg/front_end/messages.status

@@ -315,6 +315,7 @@ FastaUsageLong/example: Fail
 FastaUsageShort/analyzerCode: Fail
 FastaUsageShort/example: Fail
 FfiDartTypeMismatch/analyzerCode: Fail
+FfiEmptyStruct/analyzerCode: Fail
 FfiExceptionalReturnNull/analyzerCode: Fail
 FfiExpectedConstant/analyzerCode: Fail
 FfiExpectedExceptionalReturn/analyzerCode: Fail

pkg/front_end/messages.yaml

@@ -1914,7 +1914,7 @@ InternalProblemUnsupportedNullability:
 
 IncrementalCompilerIllegalParameter:
   template: "Illegal parameter name '#string' found during expression compilation."
-  
+
 IncrementalCompilerIllegalTypeParameter:
   template: "Illegal type parameter name '#string' found during expression compilation."
 
@@ -4233,6 +4233,11 @@ FfiTypeMismatch:
   template: "Expected type '#type' to be '#type2', which is the Dart type corresponding to '#type3'."
   external: test/ffi_test.dart
 
+FfiEmptyStruct:
+  # Used by dart:ffi
+  template: "Struct '#name' is empty. Empty structs are undefined behavior."
+  external: test/ffi_test.dart
+
 FfiTypeInvalid:
   # Used by dart:ffi
   template: "Expected type '#type' to be a valid and instantiated subtype of 'NativeType'."

pkg/vm/lib/target/vm.dart

@@ -22,7 +22,6 @@ import 'package:kernel/vm/constants_native_effects.dart'
 
 import '../transformations/call_site_annotator.dart' as callSiteAnnotator;
 import '../transformations/lowering.dart' as lowering show transformLibraries;
-import '../transformations/ffi.dart' as transformFfi show ReplacedMembers;
 import '../transformations/ffi_definitions.dart' as transformFfiDefinitions
     show transformLibraries;
 import '../transformations/ffi_use_sites.dart' as transformFfiUseSites
@@ -155,17 +154,16 @@ class VmTarget extends Target {
         this, coreTypes, hierarchy, libraries, referenceFromIndex);
     logger?.call("Transformed mixin applications");
 
-    transformFfi.ReplacedMembers replacedFields =
-        transformFfiDefinitions.transformLibraries(
-            component,
-            coreTypes,
-            hierarchy,
-            libraries,
-            diagnosticReporter,
-            referenceFromIndex,
-            changedStructureNotifier);
+    final ffiTransformerData = transformFfiDefinitions.transformLibraries(
+        component,
+        coreTypes,
+        hierarchy,
+        libraries,
+        diagnosticReporter,
+        referenceFromIndex,
+        changedStructureNotifier);
     transformFfiUseSites.transformLibraries(component, coreTypes, hierarchy,
-        libraries, diagnosticReporter, replacedFields, referenceFromIndex);
+        libraries, diagnosticReporter, ffiTransformerData, referenceFromIndex);
     logger?.call("Transformed ffi annotations");
 
     // TODO(kmillikin): Make this run on a per-method basis.

pkg/vm/lib/transformations/ffi.dart

@@ -310,8 +310,8 @@ class FfiTransformer extends Transformer {
   /// [Handle]                             -> [Object]
   /// [NativeFunction]<T1 Function(T2, T3) -> S1 Function(S2, S3)
   ///    where DartRepresentationOf(Tn) -> Sn
-  DartType convertNativeTypeToDartType(
-      DartType nativeType, bool allowStructs, bool allowHandle) {
+  DartType convertNativeTypeToDartType(DartType nativeType,
+      {bool allowStructs = false, bool allowHandle = false}) {
     if (nativeType is! InterfaceType) {
       return null;
     }
@@ -352,13 +352,13 @@ class FfiTransformer extends Transformer {
       return null;
     }
     if (fun.typeParameters.length != 0) return null;
-    // TODO(36730): Structs cannot appear in native function signatures.
-    final DartType returnType = convertNativeTypeToDartType(
-        fun.returnType, /*allowStructs=*/ false, /*allowHandle=*/ true);
+
+    final DartType returnType = convertNativeTypeToDartType(fun.returnType,
+        allowStructs: allowStructs, allowHandle: true);
     if (returnType == null) return null;
     final List<DartType> argumentTypes = fun.positionalParameters
-        .map((t) => convertNativeTypeToDartType(
-            t, /*allowStructs=*/ false, /*allowHandle=*/ true))
+        .map((t) => convertNativeTypeToDartType(t,
+            allowStructs: allowStructs, allowHandle: true))
         .toList();
     if (argumentTypes.contains(null)) return null;
     return FunctionType(argumentTypes, returnType, Nullability.legacy);
@@ -373,12 +373,12 @@ class FfiTransformer extends Transformer {
   }
 }
 
-/// Contains replaced members, of which all the call sites need to be replaced.
-///
-/// [ReplacedMembers] is populated by _FfiDefinitionTransformer and consumed by
-/// _FfiUseSiteTransformer.
-class ReplacedMembers {
+/// Contains all information collected by _FfiDefinitionTransformer that is
+/// needed in _FfiUseSiteTransformer.
+class FfiTransformerData {
   final Map<Field, Procedure> replacedGetters;
   final Map<Field, Procedure> replacedSetters;
-  ReplacedMembers(this.replacedGetters, this.replacedSetters);
+  final Set<Class> emptyStructs;
+  FfiTransformerData(
+      this.replacedGetters, this.replacedSetters, this.emptyStructs);
 }

pkg/vm/lib/transformations/ffi_definitions.dart

@@ -56,7 +56,7 @@ import 'ffi.dart';
 ///
 ///   static final int #sizeOf = 24;
 /// }
-ReplacedMembers transformLibraries(
+FfiTransformerData transformLibraries(
     Component component,
     CoreTypes coreTypes,
     ClassHierarchy hierarchy,
@@ -70,17 +70,17 @@ ReplacedMembers transformLibraries(
     // TODO: This check doesn't make sense: "dart:ffi" is always loaded/created
     // for the VM target.
     // If dart:ffi is not loaded, do not do the transformation.
-    return ReplacedMembers({}, {});
+    return FfiTransformerData({}, {}, {});
   }
   if (index.tryGetClass('dart:ffi', 'NativeFunction') == null) {
     // If dart:ffi is not loaded (for real): do not do the transformation.
-    return ReplacedMembers({}, {});
+    return FfiTransformerData({}, {}, {});
   }
   final transformer = new _FfiDefinitionTransformer(index, coreTypes, hierarchy,
       diagnosticReporter, referenceFromIndex, changedStructureNotifier);
   libraries.forEach(transformer.visitLibrary);
-  return ReplacedMembers(
-      transformer.replacedGetters, transformer.replacedSetters);
+  return FfiTransformerData(transformer.replacedGetters,
+      transformer.replacedSetters, transformer.emptyStructs);
 }
 
 /// Checks and elaborates the dart:ffi structs and fields.
@@ -89,6 +89,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
 
   Map<Field, Procedure> replacedGetters = {};
   Map<Field, Procedure> replacedSetters = {};
+  Set<Class> emptyStructs = {};
 
   ChangedStructureNotifier changedStructureNotifier;
 
@@ -231,7 +232,9 @@ class _FfiDefinitionTransformer extends FfiTransformer {
             Nullability.legacy);
         // TODO(dartbug.com/37271): Support structs inside structs.
         final DartType shouldBeDartType = convertNativeTypeToDartType(
-            nativeType, /*allowStructs=*/ false, /*allowHandle=*/ false);
+            nativeType,
+            allowStructs: false,
+            allowHandle: false);
         if (shouldBeDartType == null ||
             !env.isSubtypeOf(type, shouldBeDartType,
                 SubtypeCheckMode.ignoringNullabilities)) {
@@ -338,6 +341,9 @@ class _FfiDefinitionTransformer extends FfiTransformer {
     }
 
     _annoteStructWithFields(node, classes);
+    if (classes.isEmpty) {
+      emptyStructs.add(node);
+    }
 
     final sizeAndOffsets = <Abi, SizeAndOffsets>{};
     for (final Abi abi in Abi.values) {
@@ -565,6 +571,9 @@ class _FfiDefinitionTransformer extends FfiTransformer {
     return offset;
   }
 
+  // Keep consistent with runtime/vm/compiler/ffi/native_type.cc
+  // NativeCompoundType::FromNativeTypes.
+  //
   // TODO(37271): Support nested structs.
   SizeAndOffsets _calculateSizeAndOffsets(List<NativeType> types, Abi abi) {
     int offset = 0;