feat(ext/ffi): Safe number pointers #15173
Conversation
aapoalas
force-pushed
the
feat-ext-ffi-32-bit-support
branch
from
58b055c
to
c59a522
Compare
Turns out these are quite well handled by the existing type tests. The tests had just turned off with |
| NativeType::I64 => "int64_t", | ||
| NativeType::ISize => "intptr_t", | ||
| NativeType::USize => "uintptr_t", | ||
| NativeType::Pointer | NativeType::Function => "void*", |
There was a problem hiding this comment.
C does not mandate that function pointers and data pointers are the same.
A quick search yielded this: https://stackoverflow.com/questions/1473935/can-the-size-of-pointers-vary-between-data-and-function-pointers.
See the documents that they reference: https://c-faq.com/ptrs/generic.html; you should use void(*)(), but because of C's irregular syntax regarding some types (such as functions), you should emit a type alias near the top of the file (typedef void (*function_pointer_t)()), and then use that here (Function => "function_pointer_t").
There was a problem hiding this comment.
I opened a new issue for this: #15292
It seems that the main case where this is true is Harvard architecture, which is not supported by Deno either. These seem to usually be lower-bitness environments, eg. 16 bit data pointers, which would mean that compiling V8 for those might not be possible in the first place. Compiling Rust might not work either.
There was a problem hiding this comment.
I opened a new issue for this: #15292
It seems that the main case where this is true is Harvard architecture, which is not supported by Deno either.
These seem to usually be lower-bitness environments, eg. 16 bit data pointers, which would mean that compiling V8 for those might not be possible in the first place.
Indeed, this is the usual case, that sizeof::<fn()>() >= size_of::<usize>().
Compiling Rust might not work either.
I believe Rust could still work on such platforms, as it requests passing fn() as FFI functions.
https://doc.rust-lang.org/std/primitive.fn.html
And Rust acknowledges that function pointers may not be pointer-sized.
https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html
But if we have confirmation that V8 can't support such platforms, then it is perfectly understandable to disregard this situation as not relevant.
(I'm not sure if V8's JiT compiler would work on such platforms, as Harvard platforms are more complicated to do so on.)
Although, another case may be CHERI and such...
There was a problem hiding this comment.
Perhaps a const assert assert!(sizeof::<fn()>() == size_of::<usize>()) is fine.
aapoalas
changed the title
This PR changes the JS representation of pointers returned by the FFI API to use plain JS numbers on 64 bit systems when the pointer value can be safely represented as a JS number. (Note: There would be another option as well, which is to always turn pointers into f64 as they are, bitwise, and save that into a JS
Number. Whether or not that actually works depends entirely on if V8 does any kind of changes on the f64 it's given, eg. unifying any NaN values to be the same NaN.)