Skip to content

Commit

Permalink
feat(ext/ffi): Make op_ffi_ptr_of fast (#16297)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aapoalas committed Oct 20, 2022
1 parent 722ea20 commit e2be70b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 35 deletions.
2 changes: 1 addition & 1 deletion cli/tests/testdata/run/ffi/unstable_ffi_4.js
Original file line number Diff line number Diff line change
@@ -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));
9 changes: 8 additions & 1 deletion ext/ffi/00_ffi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
}

Expand Down
48 changes: 16 additions & 32 deletions ext/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2063,47 +2063,31 @@ 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,
{
check_unstable(state, "Deno.UnsafePointer#of");
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(
Expand Down
2 changes: 1 addition & 1 deletion test_ffi/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn basic() {
false\n\
true\n\
false\n\
false\n\
true\n\
false\n\
579\n\
true\n\
Expand Down

0 comments on commit e2be70b

Please sign in to comment.