-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
c_bool
is not (always?) c_int
, causes miscompilation of extent hook API
#53
Comments
How about defining |
Did you confirm that building on MSVC does indeed use the compat header and not whatever |
No, I don't have any machine that runs Windows. The file is named stdbool.h and get included in the header searching path by configure script, so even MSVC ships one, the former should be used instead. |
Okay. Then yes, the fix would be: #[cfg(target_env = "msvc")]
type c_bool = c_int;
#[cfg(not(target_env = "msvc"))]
type c_bool = bool; |
Cool, can you send it as a PR? |
Fixes #53 Signed-off-by: Arnav Singh <me@arnavion.dev>
jemallocator/jemalloc-sys/src/lib.rs
Line 53 in fd00e12
This leads to a miscompilation when using API that involves
c_bool
, ie the extent hook API. For example, when my implementation of this Rust typedef wrote to the supposedc_bool
pointers, it ended up smashing other variables in the caller's stack. I'm using thex86_64-unknown-linux-musl
andx86_64-unknown-linux-gnu
targets with gcc 11, gcc 12 and gcc 13.jemalloc expects to be using C99
bool
via thestdbool.h
header, and the equivalent of that is Rust'sbool
, notc_int
. jemalloc does have this header for Windows MSVC that defines it to win32'sBOOL
which is apparently anint
. I'm not sure if even that is required, since web search seems to indicate MSVC also gotstdbool.h
in VS 2013, but I don't have Windows to check myself.Workaround: Write the extent hook implementations with the correct signature (using Rust
bool
), then transmute then into the incorrect signatures that tikv-jemalloc-sys requires (usingc_int
) viastd::mem::transmute
.The text was updated successfully, but these errors were encountered: