rusty_v8 icon indicating copy to clipboard operation
rusty_v8 copied to clipboard

Function pointer types are invalid

Open aapoalas opened this issue 1 year ago • 5 comments

Recent addition of Fast API calls (at least) use the types *const c_void in Rust and void* in C++. This is not strictly correct on all platforms, see f.ex. https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html#representation

It would be better to explicitly use extern "C" fn() type (or just fn()) in Rust and void (*)() in C++ as the function pointer type.

aapoalas avatar Jul 26 '22 10:07 aapoalas

@aapoalas could you provide some pointers of what needs to be changed for anyone willing to take up this issue?

bartlomieju avatar Nov 28 '22 22:11 bartlomieju

So, anywhere that a function pointer is passed it would be technically more correct to use the type extern "C" fn() or fn() (depending on externality) as the type, as opposed to using a *const c_void data pointer.

The difference is that on some architectures (none officially supported, or perhaps even likely to ever work) a function pointer may have a different width than a data pointer (usually smaller). All function pointers on the other hand always have the same size, regardless of their actual parameters so using a parameterless function type is fine.

This issue is essentially only about technical code correctness. Though, it is of course better to mark things by what they are: A function is a function, and data is data.

aapoalas avatar Nov 29 '22 05:11 aapoalas

Also see https://github.com/denoland/deno/issues/15292.

vimirage avatar Nov 29 '22 05:11 vimirage

Practically, this is a non-issue.

I'll try to come around with fixes for this if no one else does in a few months, but really effort ought to be focused on anything more important in the meanwhile, such as.. e.g. 32-bit platform support?

vimirage avatar Nov 29 '22 05:11 vimirage

Yes.

aapoalas avatar Nov 29 '22 05:11 aapoalas