Lokathor/bytemuck

Crate makes assumptions about bool values

Manishearth opened this issue · 5 comments

bytemuck/src/contiguous.rs

Lines 179 to 180 in d10fbfc

impl_contiguous! {
bool as u8 in [0, 1];

This assumes that a bool takes values 0=false, 1=true, which is true on all platform Rust currently supports but is not necessarily globally true, see rust-lang/unsafe-code-guidelines#53.

This is quite minor but I did notice this crate goes out of its way to handle spirv correctly so it does seem worth at least documenting in code and perhaps adding additional runtime-optimizeable asserts for.

https://doc.rust-lang.org/reference/types/boolean.html

The rust reference documents it as being size 1 align 1, and also the exact but patterns of false and true.

I think even if the size and alignment were bigger on a new platform (eg: 4 each perhaps, like a u32 instead of like a u8), then bool would still have to use the bit patterns for 0 or 1 just because so much unsafe code everywhere assumes such by now.

I'm open to bytemuck having additional static assertions or something like that if you'd like to add them somewhere.

Aha, good point. Might be worth linking to that spec, but i assumed from skimming that issue that this was still an open issue.

Yeah, I'm aware there's almost no chance rust will change this, this just cropped up during unsafe code review 😄

@MaulingMonkey informed me that on some obscure platforms it really might not be s1a1, in the example he knew about it was OSX for PPC cpus. Not sure if we need to worry or not in the long run.

I'll mention abibool in passing for those who need interop with booleanish values which might not match Rust's ≈ #[repr(u8)] enum bool { false=0, true=1 } ABI. That includes e.g. most C ABIs, which typedef an integer type for booleans for C89 interop, and can easily be passed nonzero values other than 1 which should probably be interpreted as truthy instead of immediate undefined behavior. Even if bytemuck tried to relax it's assumptions about bool, rustc would still make assumptions about bool, making it no more useful for interop with C or C++ code.

https://docs.rs/bool32/latest/bool32/ i made a bool thing for interacting with vulkan and sdl2