From 52cace81609c7d706ea91c5335d43e962ce4e89c Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Mon, 15 Jul 2019 16:57:36 +0000 Subject: [PATCH] Reland "[vm/ffi] Support structs on 32bit architectures" Fixed Flutter iOS build. Fixes: https://github.com/dart-lang/sdk/issues/36334 Change-Id: Idee38671cf0f33797824b37f08a92f32f931d8e0 Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try, app-kernel-linux-debug-x64-try, vm-kernel-linux-debug-simdbc64-try,vm-kernel-linux-debug-ia32-try,vm-dartkb-linux-debug-simarm64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-dartkb-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-ffi-android-product-arm-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108818 Reviewed-by: Samir Jindel Commit-Queue: Daco Harkes --- pkg/vm/lib/transformations/ffi.dart | 72 ++++++ .../lib/transformations/ffi_definitions.dart | 219 +++++++++--------- runtime/bin/ffi_test/ffi_test_functions.cc | 37 +-- runtime/lib/ffi_patch.dart | 5 + runtime/vm/compiler/ffi.cc | 54 +++++ runtime/vm/compiler/ffi.h | 10 + .../vm/compiler/frontend/bytecode_reader.cc | 1 + runtime/vm/compiler/frontend/kernel_to_il.cc | 11 +- runtime/vm/compiler/method_recognizer.cc | 1 + runtime/vm/compiler/recognized_methods_list.h | 10 +- runtime/vm/interpreter.cc | 10 +- runtime/vm/object.cc | 6 +- tests/ffi/enable_structs_test.dart | 22 -- tests/ffi/ffi.status | 8 - tests/ffi/structs_test.dart | 16 +- tests/ffi/very_large_struct.dart | 4 +- 16 files changed, 315 insertions(+), 171 deletions(-) delete mode 100644 tests/ffi/enable_structs_test.dart diff --git a/pkg/vm/lib/transformations/ffi.dart b/pkg/vm/lib/transformations/ffi.dart index c90cd641fda..219a0c2214a 100644 --- a/pkg/vm/lib/transformations/ffi.dart +++ b/pkg/vm/lib/transformations/ffi.dart @@ -86,6 +86,74 @@ const List nativeTypeSizes = [ UNKNOWN, // Struct ]; +/// The struct layout in various ABIs. +/// +/// ABIs differ per architectures and with different compilers. +/// We pick the default struct layout based on the architecture and OS. +/// +/// Compilers _can_ deviate from the default layout, but this prevents +/// executables from making system calls. So this seems rather uncommon. +/// +/// In the future, we might support custom struct layouts. For more info see +/// https://github.com/dart-lang/sdk/issues/35768. +enum Abi { + /// Layout in all 64bit ABIs (x64 and arm64). + wordSize64, + + /// Layout in System V ABI for x386 (ia32 on Linux) and in iOS Arm 32 bit. + wordSize32Align32, + + /// Layout in both the Arm 32 bit ABI and the Windows ia32 ABI. + wordSize32Align64, +} + +/// WORD_SIZE in bytes. +const wordSize = { + Abi.wordSize64: 8, + Abi.wordSize32Align32: 4, + Abi.wordSize32Align64: 4, +}; + +/// Elements that are not aligned to their size. +/// +/// Has an entry for all Abis. Empty entries document that every native +/// type is aligned to it's own size in this ABI. +/// +/// See runtime/vm/compiler/ffi.cc for asserts in the VM that verify these +/// alignments. +/// +/// TODO(37470): Add uncommon primitive data types when we want to support them. +const nonSizeAlignment = >{ + Abi.wordSize64: {}, + + // x86 System V ABI: + // > uint64_t | size 8 | alignment 4 + // > double | size 8 | alignment 4 + // https://github.com/hjl-tools/x86-psABI/wiki/intel386-psABI-1.1.pdf page 8. + // + // iOS 32 bit alignment: + // https://developer.apple.com/documentation/uikit/app_and_environment/updating_your_app_from_32-bit_to_64-bit_architecture/updating_data_structures + Abi.wordSize32Align32: {NativeType.kDouble: 4, NativeType.kInt64: 4}, + + // The default for MSVC x86: + // > The alignment-requirement for all data except structures, unions, and + // > arrays is either the size of the object or the current packing size + // > (specified with either /Zp or the pack pragma, whichever is less). + // https://docs.microsoft.com/en-us/cpp/c-language/padding-and-alignment-of-structure-members?view=vs-2019 + // + // GCC _can_ compile on Linux to this alignment with -malign-double, but does + // not do so by default: + // > Warning: if you use the -malign-double switch, structures containing the + // > above types are aligned differently than the published application + // > binary interface specifications for the x86-32 and are not binary + // > compatible with structures in code compiled without that switch. + // https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html + // + // Arm always requires 8 byte alignment for 8 byte values: + // http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/IHI0042D_aapcs.pdf 4.1 Fundamental Data Types + Abi.wordSize32Align64: {}, +}; + /// [FfiTransformer] contains logic which is shared between /// _FfiUseSiteTransformer and _FfiDefinitionTransformer. class FfiTransformer extends Transformer { @@ -98,6 +166,7 @@ class FfiTransformer extends Transformer { final Class intClass; final Class doubleClass; final Constructor pragmaConstructor; + final Procedure listElementAt; final Library ffiLibrary; final Class nativeFunctionClass; @@ -114,6 +183,7 @@ class FfiTransformer extends Transformer { final Field addressOfField; final Constructor structFromPointer; final Procedure libraryLookupMethod; + final Procedure abiMethod; /// Classes corresponding to [NativeType], indexed by [NativeType]. final List nativeTypesClasses; @@ -124,6 +194,7 @@ class FfiTransformer extends Transformer { intClass = coreTypes.intClass, doubleClass = coreTypes.doubleClass, pragmaConstructor = coreTypes.pragmaConstructor, + listElementAt = coreTypes.index.getMember('dart:core', 'List', '[]'), ffiLibrary = index.getLibrary('dart:ffi'), nativeFunctionClass = index.getClass('dart:ffi', 'NativeFunction'), pointerClass = index.getClass('dart:ffi', 'Pointer'), @@ -144,6 +215,7 @@ class FfiTransformer extends Transformer { index.getMember('dart:ffi', 'Pointer', 'fromFunction'), libraryLookupMethod = index.getMember('dart:ffi', 'DynamicLibrary', 'lookup'), + abiMethod = index.getTopLevelMember('dart:ffi', '_abi'), nativeTypesClasses = nativeTypeClassNames .map((name) => index.getClass('dart:ffi', name)) .toList(); diff --git a/pkg/vm/lib/transformations/ffi_definitions.dart b/pkg/vm/lib/transformations/ffi_definitions.dart index eb4b5cea4b2..ccbe253cba3 100644 --- a/pkg/vm/lib/transformations/ffi_definitions.dart +++ b/pkg/vm/lib/transformations/ffi_definitions.dart @@ -15,19 +15,13 @@ import 'package:front_end/src/api_unstable/vm.dart' templateFfiStructGeneric, templateFfiWrongStructInheritance; -import 'package:kernel/ast.dart'; +import 'package:kernel/ast.dart' hide MapEntry; import 'package:kernel/class_hierarchy.dart' show ClassHierarchy; import 'package:kernel/core_types.dart'; import 'package:kernel/library_index.dart' show LibraryIndex; import 'package:kernel/target/targets.dart' show DiagnosticReporter; -import 'ffi.dart' - show - ReplacedMembers, - NativeType, - FfiTransformer, - nativeTypeSizes, - WORD_SIZE; +import 'ffi.dart'; /// Checks and elaborates the dart:ffi structs and fields. /// @@ -66,8 +60,8 @@ ReplacedMembers transformLibraries( ClassHierarchy hierarchy, List libraries, DiagnosticReporter diagnosticReporter) { - final LibraryIndex index = LibraryIndex( - component, const ["dart:ffi", "dart:_internal", "dart:core"]); + final LibraryIndex index = + LibraryIndex(component, const ["dart:ffi", "dart:core"]); if (!index.containsLibrary("dart:ffi")) { // If dart:ffi is not loaded, do not do the transformation. return ReplacedMembers({}, {}); @@ -82,27 +76,13 @@ ReplacedMembers transformLibraries( /// Checks and elaborates the dart:ffi structs and fields. class _FfiDefinitionTransformer extends FfiTransformer { final LibraryIndex index; - final Field _internalIs64Bit; - final Constructor _unimplementedErrorCtor; - static const String _errorOn32BitMessage = - "Code-gen for FFI structs is not supported on 32-bit platforms."; Map replacedGetters = {}; Map replacedSetters = {}; _FfiDefinitionTransformer(this.index, CoreTypes coreTypes, ClassHierarchy hierarchy, DiagnosticReporter diagnosticReporter) - : _internalIs64Bit = index.getTopLevelMember('dart:_internal', 'is64Bit'), - _unimplementedErrorCtor = - index.getMember('dart:core', 'UnimplementedError', ''), - super(index, coreTypes, hierarchy, diagnosticReporter) {} - - Statement guardOn32Bit(Statement body) { - final Throw error = Throw(ConstructorInvocation(_unimplementedErrorCtor, - Arguments([StringLiteral(_errorOn32BitMessage)]))); - return IfStatement( - StaticGet(_internalIs64Bit), body, ExpressionStatement(error)); - } + : super(index, coreTypes, hierarchy, diagnosticReporter) {} @override visitClass(Class node) { @@ -119,8 +99,8 @@ class _FfiDefinitionTransformer extends FfiTransformer { final bool fieldsValid = _checkFieldAnnotations(node); if (fieldsValid) { - int size = _replaceFields(node); - _replaceSizeOfMethod(node, size); + final structSize = _replaceFields(node); + _replaceSizeOfMethod(node, structSize); } return node; @@ -169,27 +149,27 @@ class _FfiDefinitionTransformer extends FfiTransformer { f.name.name.length, f.fileUri); } - List annos = _getAnnotations(f).toList(); + final nativeTypeAnnos = _getNativeTypeAnnotations(f).toList(); if (_isPointerType(f)) { - if (annos.length != 0) { + if (nativeTypeAnnos.length != 0) { diagnosticReporter.report( templateFfiFieldNoAnnotation.withArguments(f.name.name), f.fileOffset, f.name.name.length, f.fileUri); } - } else if (annos.length != 1) { + } else if (nativeTypeAnnos.length != 1) { diagnosticReporter.report( templateFfiFieldAnnotation.withArguments(f.name.name), f.fileOffset, f.name.name.length, f.fileUri); } else { - DartType dartType = f.type; - DartType nativeType = - InterfaceType(nativeTypesClasses[annos.first.index]); + final DartType dartType = f.type; + final DartType nativeType = + InterfaceType(nativeTypesClasses[nativeTypeAnnos.first.index]); // TODO(36730): Support structs inside structs. - DartType shouldBeDartType = + final DartType shouldBeDartType = convertNativeTypeToDartType(nativeType, /*allowStructs=*/ false); if (shouldBeDartType == null || !env.isSubtypeOf(dartType, shouldBeDartType)) { @@ -207,7 +187,7 @@ class _FfiDefinitionTransformer extends FfiTransformer { } void _checkConstructors(Class node) { - List toRemove = []; + final toRemove = []; // Constructors cannot have initializers because initializers refer to // fields, and the fields were replaced with getter/setter pairs. @@ -241,44 +221,62 @@ class _FfiDefinitionTransformer extends FfiTransformer { node.addMember(ctor); } - /// Computes the field offsets in the struct and replaces the fields with - /// getters and setters using these offsets. + /// Computes the field offsets (for all ABIs) in the struct and replaces the + /// fields with getters and setters using these offsets. /// - /// Returns the total size of the struct. - int _replaceFields(Class node) { - List fields = []; - List types = []; + /// Returns the total size of the struct (for all ABIs). + Map _replaceFields(Class node) { + final fields = []; + final types = []; for (Field f in node.fields) { if (_isPointerType(f)) { fields.add(f); types.add(NativeType.kPointer); } else { - List annos = _getAnnotations(f).toList(); - if (annos.length == 1) { - NativeType t = annos.first; + final nativeTypeAnnos = _getNativeTypeAnnotations(f).toList(); + if (nativeTypeAnnos.length == 1) { + NativeType t = nativeTypeAnnos.first; fields.add(f); types.add(t); } } } - List offsets = _calculateOffsets(types); - int size = _calculateSize(offsets, types); + final sizeAndOffsets = {}; + for (Abi abi in Abi.values) { + sizeAndOffsets[abi] = _calculateSizeAndOffsets(types, abi); + } for (int i = 0; i < fields.length; i++) { - List methods = - _generateMethodsForField(fields[i], types[i], offsets[i]); - for (Procedure p in methods) { - node.addMember(p); - } + final fieldOffsets = sizeAndOffsets + .map((Abi abi, SizeAndOffsets v) => MapEntry(abi, v.offsets[i])); + final methods = + _generateMethodsForField(fields[i], types[i], fieldOffsets); + methods.forEach((p) => node.addMember(p)); } for (Field f in fields) { f.remove(); } - return size; + return sizeAndOffsets.map((k, v) => MapEntry(k, v.size)); + } + + /// Expression that queries VM internals at runtime to figure out on which ABI + /// we are. + Expression _runtimeBranchOnLayout(Map values) { + return MethodInvocation( + ConstantExpression( + ListConstant(InterfaceType(intClass), [ + IntConstant(values[Abi.wordSize64]), + IntConstant(values[Abi.wordSize32Align32]), + IntConstant(values[Abi.wordSize32Align64]) + ]), + InterfaceType(intClass)), + Name("[]"), + Arguments([StaticInvocation(abiMethod, Arguments([]))]), + listElementAt); } /// Sample output: @@ -286,60 +284,58 @@ class _FfiDefinitionTransformer extends FfiTransformer { /// double get x => _xPtr.load(); /// set x(double v) => _xPtr.store(v); List _generateMethodsForField( - Field field, NativeType type, int offset) { - DartType nativeType = type == NativeType.kPointer + Field field, NativeType type, Map offsets) { + final DartType nativeType = type == NativeType.kPointer ? field.type : InterfaceType(nativeTypesClasses[type.index]); - DartType pointerType = InterfaceType(pointerClass, [nativeType]); - Name pointerName = Name('#_ptr_${field.name.name}'); + final DartType pointerType = InterfaceType(pointerClass, [nativeType]); + final Name pointerName = Name('#_ptr_${field.name.name}'); // Sample output: // ffi.Pointer get _xPtr => addressOf.offsetBy(...).cast>(); Expression pointer = PropertyGet(ThisExpression(), addressOfField.name, addressOfField); - if (offset != 0) { + final hasNonZero = offsets.values.skipWhile((i) => i == 0).isNotEmpty; + if (hasNonZero) { pointer = MethodInvocation(pointer, offsetByMethod.name, - Arguments([IntLiteral(offset)]), offsetByMethod); + Arguments([_runtimeBranchOnLayout(offsets)]), offsetByMethod); } - Procedure pointerGetter = Procedure( + final Procedure pointerGetter = Procedure( pointerName, ProcedureKind.Getter, FunctionNode( - guardOn32Bit(ReturnStatement(MethodInvocation( - pointer, - castMethod.name, - Arguments([], types: [nativeType]), - castMethod))), + ReturnStatement(MethodInvocation(pointer, castMethod.name, + Arguments([], types: [nativeType]), castMethod)), returnType: pointerType)); // Sample output: // double get x => _xPtr.load(); - Procedure getter = Procedure( + final Procedure getter = Procedure( field.name, ProcedureKind.Getter, FunctionNode( - guardOn32Bit(ReturnStatement(MethodInvocation( + ReturnStatement(MethodInvocation( PropertyGet(ThisExpression(), pointerName, pointerGetter), loadMethod.name, Arguments([], types: [field.type]), - loadMethod))), + loadMethod)), returnType: field.type)); // Sample output: // set x(double v) => _xPtr.store(v); Procedure setter = null; if (!field.isFinal) { - VariableDeclaration argument = + final VariableDeclaration argument = VariableDeclaration('#v', type: field.type); setter = Procedure( field.name, ProcedureKind.Setter, FunctionNode( - guardOn32Bit(ReturnStatement(MethodInvocation( + ReturnStatement(MethodInvocation( PropertyGet(ThisExpression(), pointerName, pointerGetter), storeMethod.name, Arguments([VariableGet(argument)]), - storeMethod))), + storeMethod)), returnType: VoidType(), positionalParameters: [argument])); } @@ -347,66 +343,61 @@ class _FfiDefinitionTransformer extends FfiTransformer { replacedGetters[field] = getter; replacedSetters[field] = setter; - if (setter != null) { - return [pointerGetter, getter, setter]; - } else { - return [pointerGetter, getter]; - } + return [pointerGetter, getter, if (setter != null) setter]; } /// Sample output: /// static int #sizeOf() => 24; - void _replaceSizeOfMethod(Class struct, int size) { + void _replaceSizeOfMethod(Class struct, Map sizes) { final Field sizeOf = Field(Name("#sizeOf"), - isStatic: true, isFinal: true, initializer: IntLiteral(size)); + isStatic: true, + isFinal: true, + initializer: _runtimeBranchOnLayout(sizes), + type: InterfaceType(intClass)); _makeEntryPoint(sizeOf); struct.addMember(sizeOf); } - // TODO(dacoharkes): move to VM, take into account architecture - // https://github.com/dart-lang/sdk/issues/35768 - int _sizeInBytes(NativeType t) { - int size = nativeTypeSizes[t.index]; + int _sizeInBytes(NativeType type, Abi abi) { + final int size = nativeTypeSizes[type.index]; if (size == WORD_SIZE) { - size = 8; + return wordSize[abi]; } return size; } - int _align(int offset, int size) { - int remainder = offset % size; + int _alignmentOf(NativeType type, Abi abi) { + final int alignment = nonSizeAlignment[abi][type]; + if (alignment != null) return alignment; + return _sizeInBytes(type, abi); + } + + int _alignOffset(int offset, int alignment) { + final int remainder = offset % alignment; if (remainder != 0) { offset -= remainder; - offset += size; + offset += alignment; } return offset; } - // TODO(dacoharkes): move to VM, take into account architecture - // https://github.com/dart-lang/sdk/issues/35768 - List _calculateOffsets(List types) { + // TODO(37271): Support nested structs. + SizeAndOffsets _calculateSizeAndOffsets(List types, Abi abi) { int offset = 0; - List offsets = []; + final offsets = []; for (NativeType t in types) { - int size = _sizeInBytes(t); - offset = _align(offset, size); + final int size = _sizeInBytes(t, abi); + final int alignment = _alignmentOf(t, abi); + offset = _alignOffset(offset, alignment); offsets.add(offset); offset += size; } - return offsets; - } - - // TODO(dacoharkes): move to VM, take into account architecture - // https://github.com/dart-lang/sdk/issues/35768 - int _calculateSize(List offsets, List types) { - if (offsets.isEmpty) { - return 0; - } - int largestElement = types.map((e) => _sizeInBytes(e)).reduce(math.max); - int highestOffsetIndex = types.length - 1; - int highestOffset = offsets[highestOffsetIndex]; - int highestOffsetSize = _sizeInBytes(types[highestOffsetIndex]); - return _align(highestOffset + highestOffsetSize, largestElement); + final int minimumAlignment = 1; + final sizeAlignment = types + .map((t) => _alignmentOf(t, abi)) + .followedBy([minimumAlignment]).reduce(math.max); + final int size = _alignOffset(offset, sizeAlignment); + return SizeAndOffsets(size, offsets); } void _makeEntryPoint(Annotatable node) { @@ -415,7 +406,7 @@ class _FfiDefinitionTransformer extends FfiTransformer { } NativeType _getFieldType(Class c) { - NativeType fieldType = getType(c); + final fieldType = getType(c); if (fieldType == NativeType.kVoid) { // Fields cannot have Void types. @@ -424,13 +415,13 @@ class _FfiDefinitionTransformer extends FfiTransformer { return fieldType; } - Iterable _getAnnotations(Field node) { - Iterable preConstant2018 = node.annotations + Iterable _getNativeTypeAnnotations(Field node) { + final Iterable preConstant2018 = node.annotations .whereType() .map((expr) => expr.target.parent) .map((klass) => _getFieldType(klass)) .where((type) => type != null); - Iterable postConstant2018 = node.annotations + final Iterable postConstant2018 = node.annotations .whereType() .map((expr) => expr.constant) .whereType() @@ -441,3 +432,13 @@ class _FfiDefinitionTransformer extends FfiTransformer { return postConstant2018.followedBy(preConstant2018); } } + +class SizeAndOffsets { + /// Size of the entire struct. + final int size; + + /// Offset in bytes for each field, indexed by field number. + final List offsets; + + SizeAndOffsets(this.size, this.offsets); +} diff --git a/runtime/bin/ffi_test/ffi_test_functions.cc b/runtime/bin/ffi_test/ffi_test_functions.cc index 33500881361..4ec885b0b1e 100644 --- a/runtime/bin/ffi_test/ffi_test_functions.cc +++ b/runtime/bin/ffi_test/ffi_test_functions.cc @@ -343,22 +343,29 @@ DART_EXPORT int64_t* NullableInt64ElemAt1(int64_t* a) { return retval; } +// A struct designed to exercise all kinds of alignment rules. +// Note that offset32A (System V ia32) aligns doubles on 4 bytes while offset32B +// (Arm 32 bit and MSVC ia32) aligns on 8 bytes. +// TODO(37271): Support nested structs. +// TODO(37470): Add uncommon primitive data types when we want to support them. struct VeryLargeStruct { - int8_t a; - int16_t b; - int32_t c; - int64_t d; - uint8_t e; - uint16_t f; - uint32_t g; - uint64_t h; - intptr_t i; - float j; - double k; - VeryLargeStruct* parent; - intptr_t numChildren; - VeryLargeStruct* children; - int8_t smallLastField; + // size32 size64 offset32A offset32B offset64 + int8_t a; // 1 0 0 0 + int16_t b; // 2 2 2 2 + int32_t c; // 4 4 4 4 + int64_t d; // 8 8 8 8 + uint8_t e; // 1 16 16 16 + uint16_t f; // 2 18 18 18 + uint32_t g; // 4 20 20 20 + uint64_t h; // 8 24 24 24 + intptr_t i; // 4 8 32 32 32 + double j; // 8 36 40 40 + float k; // 4 44 48 48 + VeryLargeStruct* parent; // 4 8 48 52 56 + intptr_t numChildren; // 4 8 52 56 64 + VeryLargeStruct* children; // 4 8 56 60 72 + int8_t smallLastField; // 1 60 64 80 + // sizeof 64 72 88 }; // Sums the fields of a very large struct, including the first field (a) from diff --git a/runtime/lib/ffi_patch.dart b/runtime/lib/ffi_patch.dart index 4f586cd8f89..e2c40754953 100644 --- a/runtime/lib/ffi_patch.dart +++ b/runtime/lib/ffi_patch.dart @@ -68,3 +68,8 @@ class Pointer { @patch void free() native "Ffi_free"; } + +// Returns the ABI used for size and alignment calculations. +// See pkg/vm/lib/transformations/ffi.dart. +int _abi() + native "Recognized method: method is directly interpreted by the bytecode interpreter or IR graph is built in the flow graph builder."; diff --git a/runtime/vm/compiler/ffi.cc b/runtime/vm/compiler/ffi.cc index 1ad36052905..484406b6ab5 100644 --- a/runtime/vm/compiler/ffi.cc +++ b/runtime/vm/compiler/ffi.cc @@ -52,6 +52,60 @@ size_t ElementSizeInBytes(intptr_t class_id) { return element_size_table[index]; } +// See pkg/vm/lib/transformations/ffi.dart, which makes these assumptions. +#if defined(HOST_ARCH_X64) || defined(HOST_ARCH_ARM64) +static_assert(alignof(double) == 8, "FFI transformation alignment"); +static_assert(alignof(uint64_t) == 8, "FFI transformation alignment"); +#elif defined(HOST_ARCH_IA32) && \ + (defined(HOST_OS_LINUX) || defined(HOST_OS_MACOS) || \ + defined(HOST_OS_ANDROID)) || \ + defined(HOST_ARCH_ARM) && defined(HOST_OS_IOS) +static_assert(alignof(double) == 4, "FFI transformation alignment"); +static_assert(alignof(uint64_t) == 4, "FFI transformation alignment"); +#elif defined(HOST_ARCH_IA32) && defined(HOST_OS_WINDOWS) || \ + defined(HOST_ARCH_ARM) +static_assert(alignof(double) == 8, "FFI transformation alignment"); +static_assert(alignof(uint64_t) == 8, "FFI transformation alignment"); +#else +#error "Unknown platform. Please add alignment requirements for ABI." +#endif + +#if defined(TARGET_ARCH_DBC) +static Abi HostAbi() { +#if defined(HOST_ARCH_X64) || defined(HOST_ARCH_ARM64) + return Abi::kWordSize64; +#elif defined(HOST_ARCH_IA32) && \ + (defined(HOST_OS_LINUX) || defined(HOST_OS_MACOS) || \ + defined(HOST_OS_ANDROID)) || \ + defined(HOST_ARCH_ARM) && defined(HOST_OS_IOS) + return Abi::kWordSize32Align32; +#elif defined(HOST_ARCH_IA32) && defined(HOST_OS_WINDOWS) || \ + defined(HOST_ARCH_ARM) + return Abi::kWordSize32Align64; +#else +#error "Unknown platform. Please add alignment requirements for ABI." +#endif +} +#endif // defined(TARGET_ARCH_DBC) + +Abi TargetAbi() { +#if defined(TARGET_ARCH_DBC) + return HostAbi(); +#elif defined(TARGET_ARCH_X64) || defined(TARGET_ARCH_ARM64) + return Abi::kWordSize64; +#elif defined(TARGET_ARCH_IA32) && \ + (defined(TARGET_OS_LINUX) || defined(TARGET_OS_MACOS) || \ + defined(TARGET_OS_ANDROID)) || \ + defined(TARGET_ARCH_ARM) && defined(TARGET_OS_IOS) + return Abi::kWordSize32Align32; +#elif defined(TARGET_ARCH_IA32) && defined(TARGET_OS_WINDOWS) || \ + defined(TARGET_ARCH_ARM) + return Abi::kWordSize32Align64; +#else +#error "Unknown platform. Please add alignment requirements for ABI." +#endif +} + #if !defined(DART_PRECOMPILED_RUNTIME) Representation TypeRepresentation(const AbstractType& result_type) { diff --git a/runtime/vm/compiler/ffi.h b/runtime/vm/compiler/ffi.h index 9c9810af18b..0e5ff05e3e8 100644 --- a/runtime/vm/compiler/ffi.h +++ b/runtime/vm/compiler/ffi.h @@ -25,6 +25,16 @@ constexpr intptr_t kMinimumArgumentWidth = 4; // Storage size for an FFI type (extends 'ffi.NativeType'). size_t ElementSizeInBytes(intptr_t class_id); +// These ABIs should be kept in sync with pkg/vm/lib/transformations/ffi.dart. +enum class Abi { + kWordSize64 = 0, + kWordSize32Align32 = 1, + kWordSize32Align64 = 2 +}; + +// The target ABI. Defines sizes and alignment of native types. +Abi TargetAbi(); + // Unboxed representation of an FFI type (extends 'ffi.NativeType'). Representation TypeRepresentation(const AbstractType& result_type); diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc index 6dbbfc35fd6..a54192bb747 100644 --- a/runtime/vm/compiler/frontend/bytecode_reader.cc +++ b/runtime/vm/compiler/frontend/bytecode_reader.cc @@ -1036,6 +1036,7 @@ RawTypedData* BytecodeReaderHelper::NativeEntry(const Function& function, case MethodRecognizer::kLinkedHashMap_setUsedData: case MethodRecognizer::kLinkedHashMap_getDeletedKeys: case MethodRecognizer::kLinkedHashMap_setDeletedKeys: + case MethodRecognizer::kFfiAbi: break; default: kind = MethodRecognizer::kUnknown; diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index 8469d753b29..55ee9eaac96 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -755,7 +755,7 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph( const MethodRecognizer::Kind kind = MethodRecognizer::RecognizeKind(function); switch (kind) { -// On simdbc we fall back to natives. +// On simdbc and the bytecode interpreter we fall back to natives. #if !defined(TARGET_ARCH_DBC) case MethodRecognizer::kTypedData_ByteDataView_factory: case MethodRecognizer::kTypedData_Int8ArrayView_factory: @@ -773,6 +773,11 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph( case MethodRecognizer::kTypedData_Int32x4ArrayView_factory: case MethodRecognizer::kTypedData_Float64x2ArrayView_factory: #endif // !defined(TARGET_ARCH_DBC) + // This list must be kept in sync with BytecodeReaderHelper::NativeEntry in + // runtime/vm/compiler/frontend/bytecode_reader.cc and implemented in the + // bytecode interpreter in runtime/vm/interpreter.cc. Alternatively, these + // methods must work in their original form (a Dart body or native entry) in + // the bytecode interpreter. case MethodRecognizer::kObjectEquals: case MethodRecognizer::kStringBaseLength: case MethodRecognizer::kStringBaseIsEmpty: @@ -800,6 +805,7 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph( case MethodRecognizer::kLinkedHashMap_setUsedData: case MethodRecognizer::kLinkedHashMap_getDeletedKeys: case MethodRecognizer::kLinkedHashMap_setDeletedKeys: + case MethodRecognizer::kFfiAbi: return true; default: return false; @@ -1062,6 +1068,9 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( kNoStoreBarrier); body += NullConstant(); break; + case MethodRecognizer::kFfiAbi: + body += IntConstant(static_cast(compiler::ffi::TargetAbi())); + break; default: { UNREACHABLE(); break; diff --git a/runtime/vm/compiler/method_recognizer.cc b/runtime/vm/compiler/method_recognizer.cc index e36397c6d34..112f19463c5 100644 --- a/runtime/vm/compiler/method_recognizer.cc +++ b/runtime/vm/compiler/method_recognizer.cc @@ -247,6 +247,7 @@ void MethodRecognizer::Libraries(GrowableArray* libs) { libs->Add(&Library::ZoneHandle(Library::InternalLibrary())); libs->Add(&Library::ZoneHandle(Library::DeveloperLibrary())); libs->Add(&Library::ZoneHandle(Library::AsyncLibrary())); + libs->Add(&Library::ZoneHandle(Library::FfiLibrary())); } RawGrowableObjectArray* MethodRecognizer::QueryRecognizedMethods(Zone* zone) { diff --git a/runtime/vm/compiler/recognized_methods_list.h b/runtime/vm/compiler/recognized_methods_list.h index f9f380b823e..bc9e549857d 100644 --- a/runtime/vm/compiler/recognized_methods_list.h +++ b/runtime/vm/compiler/recognized_methods_list.h @@ -9,9 +9,11 @@ namespace dart { // clang-format off // (class-name, function-name, recognized enum, fingerprint). -// When adding a new function add a 0 as fingerprint, build and run to get the -// correct fingerprint from the mismatch error (or use Library::GetFunction() -// and print func.SourceFingerprint()). +// When adding a new function add a 0 as fingerprint, build and run with +// `tools/test.py vm/dart/reused_instructions_test` to get the correct +// fingerprint from the mismatch error (or use Library::GetFunction() and print +// func.SourceFingerprint()). +// TODO(36376): Restore checking fingerprints of recognized methods. #define OTHER_RECOGNIZED_LIST(V) \ V(::, identical, ObjectIdentical, 0x49c6e96a) \ V(ClassID, getID, ClassIDgetID, 0x7b18b257) \ @@ -141,6 +143,7 @@ namespace dart { V(_HashVMBase, get:_deletedKeys, LinkedHashMap_getDeletedKeys, 0x558481c2) \ V(_HashVMBase, set:_deletedKeys, LinkedHashMap_setDeletedKeys, 0x5aa9888d) \ V(::, _classRangeCheck, ClassRangeCheck, 0x2ae76b84) \ + V(::, _abi, FfiAbi, 0x0) \ // List of intrinsics: // (class-name, function-name, intrinsification method, fingerprint). @@ -465,6 +468,7 @@ namespace dart { V(_HashVMBase, set:_hashMask, LinkedHashMap_setHashMask, 0x7219c45b) \ V(_HashVMBase, get:_deletedKeys, LinkedHashMap_getDeletedKeys, 0x558481c2) \ V(_HashVMBase, set:_deletedKeys, LinkedHashMap_setDeletedKeys, 0x5aa9888d) \ + V(::, _abi, FfiAbi, 0x0) \ // A list of core function that should never be inlined. #define INLINE_BLACK_LIST(V) \ diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc index 6e4290f3921..b437ea46326 100644 --- a/runtime/vm/interpreter.cc +++ b/runtime/vm/interpreter.cc @@ -5,6 +5,7 @@ #include // NOLINT #include +#include "vm/compiler/ffi.h" #include "vm/globals.h" #if !defined(DART_PRECOMPILED_RUNTIME) @@ -1386,7 +1387,7 @@ DART_NOINLINE bool Interpreter::AllocateMint(Thread* thread, } else { SP[0] = 0; // Space for the result. SP[1] = thread->isolate()->object_store()->mint_class(); // Class object. - SP[2] = Object::null(); // Type arguments. + SP[2] = Object::null(); // Type arguments. Exit(thread, FP, SP + 3, pc); NativeArguments args(thread, 2, SP + 1, SP); if (!InvokeRuntime(thread, this, DRT_AllocateObject, args)) { @@ -1574,8 +1575,8 @@ RawObject* Interpreter::Call(RawFunction* function, Thread* thread) { // Interpreter state (see constants_kbc.h for high-level overview). const KBCInstr* pc; // Program Counter: points to the next op to execute. - RawObject** FP; // Frame Pointer. - RawObject** SP; // Stack Pointer. + RawObject** FP; // Frame Pointer. + RawObject** SP; // Stack Pointer. uint32_t op; // Currently executing op. @@ -2204,6 +2205,9 @@ SwitchDispatch: SP[0]; *--SP = null_value; } break; + case MethodRecognizer::kFfiAbi: { + *++SP = Smi::New(static_cast(compiler::ffi::TargetAbi())); + } break; default: { NativeEntryData::Payload* payload = NativeEntryData::FromTypedArray(data); diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 6740d4e92f5..3346ae0cd16 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -8065,8 +8065,9 @@ void Function::SetDeoptReasonForAll(intptr_t deopt_id, } bool Function::CheckSourceFingerprint(const char* prefix, int32_t fp) const { - // TODO(alexmarkov): '(kernel_offset() <= 0)' looks like an impossible - // condition, fix this and re-enable fingerprints checking. + // TODO(36376): Restore checking fingerprints of recognized methods. + // '(kernel_offset() <= 0)' looks like an impossible condition, fix this and + // re-enable fingerprints checking. if (!Isolate::Current()->obfuscate() && !is_declared_in_bytecode() && (kernel_offset() <= 0) && (SourceFingerprint() != fp)) { const bool recalculatingFingerprints = false; @@ -12494,6 +12495,7 @@ void Library::CheckFunctionFingerprints() { all_libs.Add(&Library::ZoneHandle(Library::TypedDataLibrary())); all_libs.Add(&Library::ZoneHandle(Library::CollectionLibrary())); all_libs.Add(&Library::ZoneHandle(Library::InternalLibrary())); + all_libs.Add(&Library::ZoneHandle(Library::FfiLibrary())); OTHER_RECOGNIZED_LIST(CHECK_FINGERPRINTS2); INLINE_WHITE_LIST(CHECK_FINGERPRINTS); INLINE_BLACK_LIST(CHECK_FINGERPRINTS); diff --git a/tests/ffi/enable_structs_test.dart b/tests/ffi/enable_structs_test.dart deleted file mode 100644 index 21f48d089f7..00000000000 --- a/tests/ffi/enable_structs_test.dart +++ /dev/null @@ -1,22 +0,0 @@ -// 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. -// -// Dart test program for testing that structs are locked out on 32-bit platforms. - -library FfiTest; - -import 'dart:ffi'; - -import "package:expect/expect.dart"; - -class C extends Struct { - @IntPtr() - int x; -} - -void main() { - final C c = nullptr.cast().load(); - Expect.throws(() => c.x); - Expect.throws(() => c.x = 0); -} diff --git a/tests/ffi/ffi.status b/tests/ffi/ffi.status index 6bfee889e4b..87499fc6906 100644 --- a/tests/ffi/ffi.status +++ b/tests/ffi/ffi.status @@ -33,13 +33,5 @@ function_callbacks_test: Skip # Issue dartbug.com/37295 [ $system != android && $system != linux && $system != macos && $system != windows ] *: Skip # FFI not yet supported on other OSes. -# dartbug.com/35768: Structs not supported on 32-bit. -[ $arch == arm || $arch == ia32 || $arch == simdbc ] -function_structs_test: Skip -structs_test: Skip - -[ $arch == arm64 || $arch == simdbc64 || $arch == x64 ] -enable_structs_test: SkipByDesign # Tests that structs don't work on 32-bit systems. - [ $arch == simarm || $arch == simarm64 ] *: Skip # FFI not yet supported on the arm simulator. diff --git a/tests/ffi/structs_test.dart b/tests/ffi/structs_test.dart index 359d162308e..ef6c86ec0ff 100644 --- a/tests/ffi/structs_test.dart +++ b/tests/ffi/structs_test.dart @@ -3,6 +3,8 @@ // BSD-style license that can be found in the LICENSE file. // // Dart test program for testing dart:ffi struct pointers. +// +// VMOptions=--deterministic --optimization-counter-threshold=50 --enable-inlining-annotations library FfiTest; @@ -15,12 +17,14 @@ import 'coordinate.dart'; import 'utf8.dart'; void main() { - testStructAllocate(); - testStructFromAddress(); - testStructWithNulls(); - testBareStruct(); - testTypeTest(); - testUtf8(); + for (int i = 0; i < 100; i++) { + testStructAllocate(); + testStructFromAddress(); + testStructWithNulls(); + testBareStruct(); + testTypeTest(); + testUtf8(); + } } /// allocates each coordinate separately in c memory diff --git a/tests/ffi/very_large_struct.dart b/tests/ffi/very_large_struct.dart index 71d3095a42f..c7bb261b353 100644 --- a/tests/ffi/very_large_struct.dart +++ b/tests/ffi/very_large_struct.dart @@ -35,10 +35,10 @@ class VeryLargeStruct extends Struct { @IntPtr() int i; - @Float() + @Double() double j; - @Double() + @Float() double k; Pointer parent;