-
Notifications
You must be signed in to change notification settings - Fork 20
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
Considering redefining jboolean
as either an bool
or our own #[repr(u8)]
enum
#19
Comments
JNI C99 specPDF sections 6.2.5 and 6.3.1.2 imply that 0 and 1 are the only valid bit patterns for a C The definition of |
hehe, I'm definitely not so sure about that :) I'd be willing to bet the opposite I think - that most will interpret anything except zero as true. cpu instructions are often pretty good at telling you when a result was zero or not zero so I'd imagine it could be common to take advantage of that and implement true as "not So it seems like it would be a perfect opportunity for undefined behaviour in the sense that maybe some implementations really do only consider |
For reference here, the |
Before really looking to switch Some useful links:
I certainly haven't seen any hint of there being any kind of roundtrip guarantees for other values, more the opposite in fact since it looks like different JVM implementations will do their own 0/1 normalization of booleans at different times - which don't appear to be well specified. Overall there is no strong consensus on whether values besides 0 and 1 can be stored in a In practice, because JVMs have tried to account for how C treats non-zero values as truthy then other values are quite likely to be considered truthy and/or get normalized to 1 but that's not guaranteed. It does still look to me like |
JNI only strictly defines two valid values for a `jboolean` and there's no consensus on whether other values greater than one will be interpreted as TRUE in all situations. The safest interpretation is to say that it will lead to undefined behaviour to pass any value besides zero or one as a `jboolean`. Addresses jni-rs/jni-rs#400 Closes #19
While looking at jni-rs/jni-rs#400 it was noted that there's a risk that code can easily cause undefined behaviour for the JVM by setting boolean array elements to values other than
0
or1
.This isn't something that's specific to array elements though; anywhere that Rust returns a
jboolean
to Java it needs to ensure the byte value is either0
or1
.Since
jboolean
is currently simply an alias foru8
(pub type jboolean = u8;
) then it's very easy to return arbitrary invalid boolean values to the JVM.My initial thought was that we could represent a
jboolean
as:and it was then also noted by @argv-minus-one that this is essentially how the Rust Reference defines
bool
: https://doc.rust-lang.org/reference/types/boolean.htmlMy initial reservation with simply defining
jboolean
as an alias forbool
is that Rust doesn't generally guarantee much in terms of stable ABI and I wouldn't like to potentially depending on an implementation detail. e.g. the Rust Reference says "this book is not normative. It may include details that are specific to rustc itself, and should not be taken as a specification for the Rust language."It looks like historically
bool
was assumed to be defined in terms of the C_Bool
type (and several FFI projects have made that assumption) which wouldn't technically guarantee thatbool
were one byte on all platforms, ref: https://internals.rust-lang.org/t/rusts-stability-story-should-apply-to-bool-in-ffi/6305/8Considering the follow up discussion here: rust-lang/rust#46176 it looks like the consensus was that
bool
is defined to be the same as C's_Bool
but it was also noted that that means it's one byte one all platforms supported by Rust.A
bool
is also explicitly documented as being one byte here: https://doc.rust-lang.org/std/mem/fn.size_of.htmlI think it's fair to conclude that Rust's bool type is basically guaranteed to always be 1 byte and to also use the values
1
and0
for true and false - which is what we want forjboolean
The text was updated successfully, but these errors were encountered: