Repository: denoland/deno_core
Pull request #419 (2024-01-05 22:23:47)

fix(core): disable fastcalls for typed buffers
With the upcoming v8 upgrade, we cannot support fastcalls for typed buffers.

Fix for #417 


Patch:
diff --git a/core/benches/ops/sync.rs b/core/benches/ops/sync.rs
index f063f2b..992a390 100644
--- a/core/benches/ops/sync.rs
+++ b/core/benches/ops/sync.rs
@@ -133,7 +133,7 @@ pub fn op_external_nofast(_input: *const c_void) {}
 #[op2(fast)]
 pub fn op_buffer(#[buffer] _buffer: &[u8]) {}
 
-#[op2(fast)]
+#[op2]
 pub fn op_buffer_jsbuffer(#[buffer] _buffer: JsBuffer) {}
 
 #[op2(nofast)]
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index 36e9ac9..550ed7a 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -1604,7 +1604,7 @@ mod tests {
     Ok(())
   }
 
-  #[op2(fast)]
+  #[op2]
   pub fn op_buffer_jsbuffer(
     #[buffer] input: JsBuffer,
     #[number] inlen: usize,
@@ -1635,7 +1635,7 @@ mod tests {
     Ok(())
   }
 
-  #[op2(fast)]
+  #[op2]
   pub fn op_buffer_any(#[anybuffer] buffer: &[u8]) -> u32 {
     let mut sum: u32 = 0;
     for i in buffer {
@@ -1689,7 +1689,7 @@ mod tests {
     Ok(())
   }
 
-  #[op2(fast)]
+  #[op2]
   pub fn op_buffer_any_length(#[anybuffer] buffer: &[u8]) -> u32 {
     buffer.len() as _
   }
diff --git a/ops/op2/README.md b/ops/op2/README.md
index 4eac6df..2d0bf8e 100644
--- a/ops/op2/README.md
+++ b/ops/op2/README.md
@@ -588,62 +588,6 @@ any
 <tr>
 <td>
 
-```text
-#[anybuffer] &mut [u8]
-```
-
-</td><td>
-✅
-</td><td>
-ArrayBuffer, ArrayBufferView (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.
-</td></tr>
-<tr>
-<td>
-
-```text
-#[anybuffer] &[u8]
-```
-
-</td><td>
-✅
-</td><td>
-ArrayBuffer, ArrayBufferView (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.
-</td></tr>
-<tr>
-<td>
-
-```text
-#[anybuffer] *mut u8
-```
-
-</td><td>
-✅
-</td><td>
-ArrayBuffer, ArrayBufferView (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.
-</td></tr>
-<tr>
-<td>
-
-```text
-#[anybuffer] *const u8
-```
-
-</td><td>
-✅
-</td><td>
-ArrayBuffer, ArrayBufferView (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.
-</td></tr>
-<tr>
-<td>
-
 ```text
 #[arraybuffer] &mut [u8]
 ```
@@ -742,62 +686,6 @@ Safe, but forces a copy.
 <tr>
 <td>
 
-```text
-#[buffer] &mut [u8]
-```
-
-</td><td>
-✅
-</td><td>
-UInt8Array (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.
-</td></tr>
-<tr>
-<td>
-
-```text
-#[buffer] &[u8]
-```
-
-</td><td>
-✅
-</td><td>
-UInt8Array (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.
-</td></tr>
-<tr>
-<td>
-
-```text
-#[buffer] *mut u8
-```
-
-</td><td>
-✅
-</td><td>
-UInt8Array (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.
-</td></tr>
-<tr>
-<td>
-
-```text
-#[buffer] *const u8
-```
-
-</td><td>
-✅
-</td><td>
-UInt8Array (resizable=true,false)
-</td><td>
-⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.
-</td></tr>
-<tr>
-<td>
-
 ```text
 #[buffer(copy)] Vec<u8>
 ```
@@ -896,20 +784,6 @@ Safe, but forces a copy.
 <tr>
 <td>
 
-```text
-#[buffer] JsBuffer
-```
-
-</td><td>
-✅
-</td><td>
-ArrayBufferView (resizable=false)
-</td><td>
-⚠️ JS may modify the contents of slices obtained from buffer.
-</td></tr>
-<tr>
-<td>
-
 ```text
 #[buffer(detach)] JsBuffer
 ```
diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs
index e497cbc..df46d73 100644
--- a/ops/op2/dispatch_fast.rs
+++ b/ops/op2/dispatch_fast.rs
@@ -730,13 +730,21 @@ fn map_arg_to_v8_fastcall_type(
       BufferType::JsBuffer | BufferType::V8Slice(..),
       _,
       BufferSource::TypedArray,
-    ) => V8FastCallType::V8Value,
+      // This cannot be fast at this time as we have no way of accessing
+      // the shared pointer to the backing store in a fastcall.
+      // https://github.com/denoland/deno_core/issues/417
+      // ) => V8FastCallType::V8Value,
+    ) => return Ok(None),
     Arg::Buffer(
       BufferType::Slice(.., NumericArg::u8)
       | BufferType::Ptr(.., NumericArg::u8),
       _,
       BufferSource::Any,
-    ) => V8FastCallType::AnyArray,
+      // This cannot be fast at this time as we have no way of accessing
+      // the shared pointer to the backing store in a fastcall.
+      // https://github.com/denoland/deno_core/issues/417
+      // ) => V8FastCallType::AnyArray,
+    ) => return Ok(None),
     // TODO(mmastrac): implement fast for any Any-typed buffer
     Arg::Buffer(_, _, BufferSource::Any) => return Ok(None),
     Arg::Buffer(_, _, BufferSource::ArrayBuffer) => V8FastCallType::ArrayBuffer,
diff --git a/ops/op2/valid_args.md b/ops/op2/valid_args.md
index 3f35974..058c0d5 100644
--- a/ops/op2/valid_args.md
+++ b/ops/op2/valid_args.md
@@ -26,10 +26,10 @@
 | X         | #[global] v8::Global<v8::**V8**>    |          | **V8**                                              | ⚠️ Slower than `v8::Local`.                                                                                                                                                                                     |
 | X         | #[serde] SerdeType                  |          | any                                                 | ⚠️ May be slow.                                                                                                                                                                                                 |
 | X         | #[serde] (Tuple, Tuple)             |          | any                                                 | ⚠️ May be slow.                                                                                                                                                                                                 |
-| X         | #[anybuffer] &mut [u8]              | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
-| X         | #[anybuffer] &[u8]                  | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
-| X         | #[anybuffer] *mut u8                | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
-| X         | #[anybuffer] *const u8              | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
+|           | #[anybuffer] &mut [u8]              | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
+|           | #[anybuffer] &[u8]                  | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
+|           | #[anybuffer] *mut u8                | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
+|           | #[anybuffer] *const u8              | X        | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
 | X         | #[arraybuffer] &mut [u8]            | X        | ArrayBuffer (resizable=true,false)                  | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
 | X         | #[arraybuffer] &[u8]                | X        | ArrayBuffer (resizable=true,false)                  | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
 | X         | #[arraybuffer] *mut u8              | X        | ArrayBuffer (resizable=true,false)                  | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
@@ -37,10 +37,10 @@
 | X         | #[arraybuffer(copy)] Vec<u8>        | X        | ArrayBuffer (resizable=true,false)                  | Safe, but forces a copy.                                                                                                                                                                                       |
 | X         | #[arraybuffer(copy)] Box<[u8]>      | X        | ArrayBuffer (resizable=true,false)                  | Safe, but forces a copy.                                                                                                                                                                                       |
 | X         | #[arraybuffer(copy)] bytes::Bytes   | X        | ArrayBuffer (resizable=true,false)                  | Safe, but forces a copy.                                                                                                                                                                                       |
-| X         | #[buffer] &mut [u8]                 | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
-| X         | #[buffer] &[u8]                     | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
-| X         | #[buffer] *mut u8                   | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
-| X         | #[buffer] *const u8                 | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
+|           | #[buffer] &mut [u8]                 | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
+|           | #[buffer] &[u8]                     | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly.                                                                                                                                        |
+|           | #[buffer] *mut u8                   | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
+|           | #[buffer] *const u8                 | X        | UInt8Array (resizable=true,false)                   | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null.                                                |
 | X         | #[buffer(copy)] Vec<u8>             | X        | UInt8Array (resizable=true,false)                   | Safe, but forces a copy.                                                                                                                                                                                       |
 | X         | #[buffer(copy)] Box<[u8]>           | X        | UInt8Array (resizable=true,false)                   | Safe, but forces a copy.                                                                                                                                                                                       |
 | X         | #[buffer(copy)] bytes::Bytes        | X        | UInt8Array (resizable=true,false)                   | Safe, but forces a copy.                                                                                                                                                                                       |
@@ -51,7 +51,7 @@
 |           | #[buffer] V8Slice                   | X        | ArrayBufferView (resizable=false)                   | ⚠️ JS may modify the contents of slices obtained from buffer.                                                                                                                                                   |
 |           | #[buffer(detach)] V8Slice           | X        | ArrayBufferView (resizable=true,false)              | Safe.                                                                                                                                                                                                          |
 |           | #[buffer] V8ResizableSlice          | X        | ArrayBufferView (resizable=true)                    | ⚠️ JS may modify the contents of slices obtained from buffer.                                                                                                                                                   |
-| X         | #[buffer] JsBuffer                  | X        | ArrayBufferView (resizable=false)                   | ⚠️ JS may modify the contents of slices obtained from buffer.                                                                                                                                                   |
+|           | #[buffer] JsBuffer                  | X        | ArrayBufferView (resizable=false)                   | ⚠️ JS may modify the contents of slices obtained from buffer.                                                                                                                                                   |
 | X         | #[buffer(detach)] JsBuffer          |          | ArrayBufferView (resizable=true,false)              | Safe.                                                                                                                                                                                                          |
 |           | #[buffer(unsafe)] bytes::Bytes      | X        | ArrayBufferView (resizable=false)                   | ⚠️ JS may modify the contents of the buffer.                                                                                                                                                                    |
 |           | #[buffer(detach)] bytes::Bytes      | X        | ArrayBufferView (resizable=true,false)              | Safe.                                                                                                                                                                                                          |


Test patch:
diff --git a/testing/checkin/runner/ops_buffer.rs b/testing/checkin/runner/ops_buffer.rs
index b70fe85..87464dc 100644
--- a/testing/checkin/runner/ops_buffer.rs
+++ b/testing/checkin/runner/ops_buffer.rs
@@ -4,7 +4,7 @@ use deno_core::JsBuffer;
 
 use super::testing::TestData;
 
-#[op2(fast)]
+#[op2]
 pub fn op_v8slice_store(
   #[state] test_data: &mut TestData,
   #[string] name: String,