Re-land "[vm/ffi] Enable creating an ExternalTypedData from a Pointer."

sjindel-google · commit-bot@chromium.org · commit 9b5abe523f9e · 2019-08-07T13:10:30.000Z
Changes from original: - Finalize the specific ExternalTypedData class before allocating. - Add new import to all FFI patch files to not depend on the order the patch files are applied. Original CL is in patchset 1. Change-Id: I85c5e0d8bce15d8120575b6dbaae311c64aa6ec2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112242 Commit-Queue: Samir Jindel <sjindel@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
diff --git a/runtime/lib/ffi.cc b/runtime/lib/ffi.cc
@@ -568,7 +568,11 @@ DEFINE_NATIVE_ENTRY(Ffi_fromFunction, 1, 2) {
 
   Class& native_function_class =
       Class::Handle(isolate->class_table()->At(kFfiNativeFunctionCid));
-  native_function_class.EnsureIsFinalized(Thread::Current());
+  const auto& error =
+      Error::Handle(native_function_class.EnsureIsFinalized(Thread::Current()));
+  if (!error.IsNull()) {
+    Exceptions::PropagateError(error);
+  }
 
   Type& native_function_type = Type::Handle(
       Type::New(native_function_class, type_args, TokenPosition::kNoSource));
@@ -608,6 +612,90 @@ DEFINE_NATIVE_ENTRY(Ffi_fromFunction, 1, 2) {
 #endif
 }
 
+DEFINE_NATIVE_ENTRY(Ffi_asExternalTypedData, 0, 2) {
+  GET_NON_NULL_NATIVE_ARGUMENT(Pointer, pointer, arguments->NativeArgAt(0));
+  GET_NON_NULL_NATIVE_ARGUMENT(Integer, count, arguments->NativeArgAt(1));
+  const auto& pointer_type_arg = AbstractType::Handle(pointer.type_argument());
+  const classid_t type_cid = pointer_type_arg.type_class_id();
+  classid_t cid = 0;
+
+  switch (type_cid) {
+    case kFfiInt8Cid:
+      cid = kExternalTypedDataInt8ArrayCid;
+      break;
+    case kFfiUint8Cid:
+      cid = kExternalTypedDataUint8ArrayCid;
+      break;
+    case kFfiInt16Cid:
+      cid = kExternalTypedDataInt16ArrayCid;
+      break;
+    case kFfiUint16Cid:
+      cid = kExternalTypedDataUint16ArrayCid;
+      break;
+    case kFfiInt32Cid:
+      cid = kExternalTypedDataInt32ArrayCid;
+      break;
+    case kFfiUint32Cid:
+      cid = kExternalTypedDataUint32ArrayCid;
+      break;
+    case kFfiInt64Cid:
+      cid = kExternalTypedDataInt64ArrayCid;
+      break;
+    case kFfiUint64Cid:
+      cid = kExternalTypedDataUint64ArrayCid;
+      break;
+    case kFfiIntPtrCid:
+      cid = kWordSize == 4 ? kExternalTypedDataInt32ArrayCid
+                           : kExternalTypedDataInt64ArrayCid;
+      break;
+    case kFfiFloatCid:
+      cid = kExternalTypedDataFloat32ArrayCid;
+      break;
+    case kFfiDoubleCid:
+      cid = kExternalTypedDataFloat64ArrayCid;
+      break;
+    default: {
+      const String& error = String::Handle(
+          String::NewFormatted("Cannot create a TypedData from a Pointer to %s",
+                               pointer_type_arg.ToCString()));
+      Exceptions::ThrowArgumentError(error);
+      UNREACHABLE();
+    }
+  }
+
+  const intptr_t element_count = count.AsInt64Value();
+
+  if (element_count < 0 ||
+      element_count > ExternalTypedData::MaxElements(cid)) {
+    const String& error = String::Handle(
+        String::NewFormatted("Count must be in the range [0, %" Pd "].",
+                             ExternalTypedData::MaxElements(cid)));
+    Exceptions::ThrowArgumentError(error);
+  }
+
+  // The address must be aligned by the element size.
+  const intptr_t element_size = ExternalTypedData::ElementSizeFor(cid);
+  if (!Utils::IsAligned(pointer.NativeAddress(), element_size)) {
+    const String& error = String::Handle(
+        String::NewFormatted("Pointer address must be aligned to a multiple of"
+                             "the element size (%" Pd ").",
+                             element_size));
+    Exceptions::ThrowArgumentError(error);
+  }
+
+  const auto& typed_data_class =
+      Class::Handle(zone, isolate->class_table()->At(cid));
+  const auto& error = Error::Handle(
+      zone, typed_data_class.EnsureIsFinalized(Thread::Current()));
+  if (!error.IsNull()) {
+    Exceptions::PropagateError(error);
+  }
+
+  return ExternalTypedData::New(
+      cid, reinterpret_cast<uint8_t*>(pointer.NativeAddress()), element_count,
+      Heap::kNew);
+}
+
 #if defined(TARGET_ARCH_DBC)
 
 void FfiMarshalledArguments::SetFunctionAddress(uint64_t value) const {
diff --git a/runtime/lib/ffi_dynamic_library_patch.dart b/runtime/lib/ffi_dynamic_library_patch.dart
@@ -2,7 +2,10 @@
 // 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.
 
+// All imports must be in all FFI patch files to not depend on the order
+// the patches are applied.
 import "dart:_internal" show patch;
+import 'dart:typed_data' show TypedData;
 
 DynamicLibrary _open(String name) native "Ffi_dl_open";
 
diff --git a/runtime/lib/ffi_native_type_patch.dart b/runtime/lib/ffi_native_type_patch.dart
@@ -2,7 +2,10 @@
 // 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.
 
+// All imports must be in all FFI patch files to not depend on the order
+// the patches are applied.
 import "dart:_internal" show patch;
+import 'dart:typed_data' show TypedData;
 
 @patch
 @pragma("vm:entry-point")
diff --git a/runtime/lib/ffi_patch.dart b/runtime/lib/ffi_patch.dart
@@ -2,7 +2,10 @@
 // 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.
 
+// All imports must be in all FFI patch files to not depend on the order
+// the patches are applied.
 import "dart:_internal" show patch;
+import 'dart:typed_data' show TypedData;
 
 @patch
 int sizeOf<T extends NativeType>() native "Ffi_sizeOf";
@@ -17,6 +20,9 @@ Pointer<T> _fromAddress<T extends NativeType>(int ptr) native "Ffi_fromAddress";
 DS _asFunctionInternal<DS extends Function, NS extends Function>(
     Pointer<NativeFunction<NS>> ptr) native "Ffi_asFunctionInternal";
 
+dynamic _asExternalTypedData(Pointer ptr, int count)
+    native "Ffi_asExternalTypedData";
+
 @patch
 @pragma("vm:entry-point")
 class Pointer<T extends NativeType> {
@@ -67,4 +73,8 @@ class Pointer<T extends NativeType> {
 
   @patch
   void free() native "Ffi_free";
+
+  @patch
+  TypedData asExternalTypedData({int count: 1}) =>
+      _asExternalTypedData(this, count);
 }
diff --git a/runtime/vm/bootstrap_natives.h b/runtime/vm/bootstrap_natives.h
@@ -388,6 +388,7 @@ namespace dart {
   V(Ffi_dl_open, 1)                                                            \
   V(Ffi_dl_lookup, 2)                                                          \
   V(Ffi_dl_getHandle, 1)                                                       \
+  V(Ffi_asExternalTypedData, 2)                                                \
   V(TransferableTypedData_factory, 2)                                          \
   V(TransferableTypedData_materialize, 1)
 
diff --git a/sdk/lib/ffi/ffi.dart b/sdk/lib/ffi/ffi.dart
@@ -6,6 +6,8 @@
 /// {@nodoc}
 library dart.ffi;
 
+import 'dart:typed_data' show TypedData;
+
 part "native_type.dart";
 part "annotations.dart";
 part "dynamic_library.dart";
@@ -91,6 +93,35 @@ class Pointer<T extends NativeType> extends NativeType {
   /// Note that this zeros out the address.
   external void free();
 
+  /// Creates an *external* typed data array backed by this pointer.
+  ///
+  /// The typed data array returned is only valid for as long as the backing
+  /// [Pointer]. Accessing any element of the type data array after this
+  /// [Pointer] has been [Pointer.free()]d will cause undefined behavior.
+  ///
+  /// Since [Pointer]s do not know their length, the size of the typed data is
+  /// controlled by `count`, in units of the size of the native type for this
+  /// [Pointer] (similarly to [Pointer.allocate]).
+  ///
+  /// The kind of TypedData produced depends on the native type:
+  ///
+  ///   Pointer<Int8> -> Int8List
+  ///   Pointer<Uint8> -> Uint8List
+  ///   etc. up to Int64/Uint64
+  ///   Pointer<IntPtr> -> Int32List/Int64List depending on platform word size
+  ///   Pointer<Float> -> Float32List
+  ///   Pointer<Double> -> Float64List
+  ///
+  /// Creation of a [Uint8ClampedList] is not supported. Creation of a typed
+  /// data from a [Pointer] to any other native type is not supported.
+  ///
+  /// The pointer must be aligned to a multiple of the native type's size.
+  //
+  // TODO(37773): Use extension methods to articulate more precise return types.
+  // We should still keep this member though as a generic way to access a
+  // Pointer of unknown type.
+  external TypedData asExternalTypedData({int count: 1});
+
   /// Equality for Pointers only depends on their address.
   bool operator ==(other) {
     if (other == null) return false;
diff --git a/sdk/lib/libraries.json b/sdk/lib/libraries.json
@@ -65,9 +65,9 @@
       },
       "ffi": {
         "patches": [
+          "../../runtime/lib/ffi_patch.dart",
           "../../runtime/lib/ffi_dynamic_library_patch.dart",
-          "../../runtime/lib/ffi_native_type_patch.dart",
-          "../../runtime/lib/ffi_patch.dart"
+          "../../runtime/lib/ffi_native_type_patch.dart"
         ],
         "uri": "ffi/ffi.dart"
       },
diff --git a/sdk/lib/libraries.yaml b/sdk/lib/libraries.yaml
@@ -90,9 +90,9 @@ vm:
     ffi:
       uri: "ffi/ffi.dart"
       patches:
+        - "../../runtime/lib/ffi_patch.dart"
         - "../../runtime/lib/ffi_dynamic_library_patch.dart"
         - "../../runtime/lib/ffi_native_type_patch.dart"
-        - "../../runtime/lib/ffi_patch.dart"
 
     _http:
       uri: "_http/http.dart"
diff --git a/tests/ffi/external_typed_data_test.dart b/tests/ffi/external_typed_data_test.dart
diff --git a/tests/ffi/ffi.status b/tests/ffi/ffi.status
