From e2be70b035459cb534360b72ece4ff2abaa09cd7 Mon Sep 17 00:00:00 2001
From: Aapo Alasuutari <aapo.alasuutari@gmail.com>
Date: Thu, 20 Oct 2022 07:07:37 +0300
Subject: [PATCH] feat(ext/ffi): Make op_ffi_ptr_of fast (#16297)

Makes `op_ffi_ptr_of` fast. One of the tests changed from printing
`false` to `true` as the fast `&[u8]` slice path creates the slice with
a null pointer. Thus the `op_ffi_ptr_of` will now return a null pointer
value whereas previously it returned a dangling pointer value.
---
 cli/tests/testdata/run/ffi/unstable_ffi_4.js |  2 +-
 ext/ffi/00_ffi.js                            |  9 +++-
 ext/ffi/lib.rs                               | 48 +++++++-------------
 test_ffi/tests/integration_tests.rs          |  2 +-
 4 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/cli/tests/testdata/run/ffi/unstable_ffi_4.js b/cli/tests/testdata/run/ffi/unstable_ffi_4.js
index 8b4f3d75ae..150791233c 100644
--- a/cli/tests/testdata/run/ffi/unstable_ffi_4.js
+++ b/cli/tests/testdata/run/ffi/unstable_ffi_4.js
@@ -1 +1 @@
-Deno.core.ops.op_ffi_ptr_of(new Uint8Array(0));
+Deno.core.ops.op_ffi_ptr_of(new Uint8Array(0), new Uint32Array(2));
diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js
index f5b0fb4c30..1b340a7662 100644
--- a/ext/ffi/00_ffi.js
+++ b/ext/ffi/00_ffi.js
@@ -160,12 +160,19 @@
     }
   }
 
+  const OUT_BUFFER = new Uint32Array(2);
+  const OUT_BUFFER_64 = new BigInt64Array(OUT_BUFFER.buffer);
   class UnsafePointer {
     static of(value) {
       if (ObjectPrototypeIsPrototypeOf(UnsafeCallbackPrototype, value)) {
         return value.pointer;
       }
-      return ops.op_ffi_ptr_of(value);
+      ops.op_ffi_ptr_of(value, OUT_BUFFER);
+      const result = OUT_BUFFER[0] + 2 ** 32 * OUT_BUFFER[1];
+      if (NumberIsSafeInteger(result)) {
+        return result;
+      }
+      return OUT_BUFFER_64[0];
     }
   }
 
diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs
index ae51db34c0..3ad75e245e 100644
--- a/ext/ffi/lib.rs
+++ b/ext/ffi/lib.rs
@@ -2063,12 +2063,12 @@ fn op_ffi_call_nonblocking<'scope>(
   })
 }
 
-#[op(v8)]
-fn op_ffi_ptr_of<FP, 'scope>(
-  scope: &mut v8::HandleScope<'scope>,
+#[op(fast)]
+fn op_ffi_ptr_of<FP>(
   state: &mut deno_core::OpState,
-  buf: serde_v8::Value<'scope>,
-) -> Result<serde_v8::Value<'scope>, AnyError>
+  buf: &[u8],
+  out: &mut [u32],
+) -> Result<(), AnyError>
 where
   FP: FfiPermissions + 'static,
 {
@@ -2076,34 +2076,18 @@ where
   let permissions = state.borrow_mut::<FP>();
   permissions.check(None)?;
 
-  let pointer = if let Ok(value) =
-    v8::Local::<v8::ArrayBufferView>::try_from(buf.v8_value)
-  {
-    let backing_store = value
-      .buffer(scope)
-      .ok_or_else(|| {
-        type_error("Invalid FFI ArrayBufferView, expected data in the buffer")
-      })?
-      .get_backing_store();
-    let byte_offset = value.byte_offset();
-    &backing_store[byte_offset..] as *const _ as *const u8
-  } else if let Ok(value) = v8::Local::<v8::ArrayBuffer>::try_from(buf.v8_value)
-  {
-    let backing_store = value.get_backing_store();
-    &backing_store[..] as *const _ as *const u8
-  } else {
-    return Err(type_error(
-      "Invalid FFI buffer, expected ArrayBuffer, or ArrayBufferView",
-    ));
-  };
+  let outptr = out.as_ptr() as *mut usize;
+  let length = out.len();
+  assert!(
+    length >= (std::mem::size_of::<usize>() / std::mem::size_of::<u32>())
+  );
+  assert_eq!(outptr as usize % std::mem::size_of::<usize>(), 0);
 
-  let integer: v8::Local<v8::Value> =
-    if pointer as usize > MAX_SAFE_INTEGER as usize {
-      v8::BigInt::new_from_u64(scope, pointer as u64).into()
-    } else {
-      v8::Number::new(scope, pointer as usize as f64).into()
-    };
-  Ok(integer.into())
+  // SAFETY: Out buffer was asserted to be at least large enough to hold a usize, and properly aligned.
+  let out = unsafe { &mut *outptr };
+  *out = buf.as_ptr() as usize;
+
+  Ok(())
 }
 
 unsafe extern "C" fn noop_deleter_callback(
diff --git a/test_ffi/tests/integration_tests.rs b/test_ffi/tests/integration_tests.rs
index 6b48534537..38808d2059 100644
--- a/test_ffi/tests/integration_tests.rs
+++ b/test_ffi/tests/integration_tests.rs
@@ -57,7 +57,7 @@ fn basic() {
     false\n\
     true\n\
     false\n\
-    false\n\
+    true\n\
     false\n\
     579\n\
     true\n\