lokathor / bytemuck Goto Github PK

The published bytemuck_derive crates don't contain license files. For most licenses (including MIT), it is usually necessary to include the text of the license itself.

Please consider adding those files to the _derive crate as well. If you're cargo publishing from UNIX-like systems, adding symlinks like LICENSE-APACHE → ../LICENSE-APACHE in the derive folder should do the trick.

PS: If you will add back the .md extensions at some point (as hinted at in the PR that added/renamed the files), keep in mind that you will have to update those symlinks.

Hello, I'm crossposting here because i'm not really sure if it's a bytemuck or glam issue (see bitshifter/glam-rs#266)

Consider the following:

fn main() {
    dbg!(bytemuck::cast_slice::<glam::Quat, u8>(&[glam::Quat::IDENTITY]));
    dbg!(bytemuck::cast_slice::<u8, glam::Quat>(&[0u8; 16]));
}

Works fine, but this one will panic

fn main() {
    dbg!(bytemuck::cast_slice::<u8, glam::Quat>(&[0u8; 16]));
}

thread 'main' panicked at 'cast_slice>TargetAlignmentGreaterAndInputNotAligned', /home/samuel/.cargo/registry/src/github.com-1ecc6299db9ec823/bytemuck-1.7.3/src/lib.rs:119:3
stack backtrace:
   0: rust_begin_unwind
             at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/panicking.rs:100:14
   2: bytemuck::something_went_wrong
             at /home/samuel/.cargo/registry/src/github.com-1ecc6299db9ec823/bytemuck-1.7.3/src/lib.rs:119:3
   3: bytemuck::cast_slice
             at /home/samuel/.cargo/registry/src/github.com-1ecc6299db9ec823/bytemuck-1.7.3/src/lib.rs:319:15
   4: glam_bytemuck_fail::main
             at ./src/main.rs:6:10
   5: core::ops::function::FnOnce::call_once
             at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

This is really strange, why having a first unrelated cast change the behavior of the second one ?

rustc 1.57.0 (f1edd0429 2021-11-29)
simple reproduction: https://github.com/IcanDivideBy0/glam_bytemuck_fail

Basically, is it possible to impl Pod for &[T] where T: Pod? IIRC, slices must not include padding, so this SEEMS okay, and it would be nice to be able to turn &[u64] into &[u8].

Basically I'm wondering if it would be useful to have derive macros for Pod and Zeroable. They could be hidden behind a feature flag so as to not make things slower in the simple case (similar to serde_derive), but could also automatically check that the various constraints for these traits are upheld.

it errors when the allocation fails, but the docs need to explain that

If you only use TransparentWrapper for Copy types, this is completely fine, and will not cause issues

[playground]

use bytemuck::TransparentWrapper; // 1.7.0

#[derive(TransparentWrapper)]
#[repr(transparent)]
struct UhOh(Box<usize>);

fn main() {
    let b = Box::new(5);
    UhOh::wrap(b);
}

     Running `target/debug/playground`
free(): double free detected in tcache 2
timeout: the monitored command dumped core

The issue is in the definition of wrap/peel:

This uses transmute_copy to create a copy of the input value without forgetting it, causing the value to be dropped and then a duplicate returned to the caller. The fix is to forget the input value by wrapping it in ManuallyDrop.

It already implements Zeroable, and the main draw of MaybeUninit is that it can contain theoretically any bit pattern.

With a debug build, I'm consistently getting a stack overflow when using zeroed_box(). Minimal example:

const PAGE_SIZE: usize = 4096;
type BlockChunk = [u8; PAGE_SIZE];

const MIB_16: usize = 16 * 1024 * 1024;
const SUPER_SIZE: usize = MIB_16 / std::mem::size_of::<BlockChunk>();
type SuperPage = [BlockChunk; SUPER_SIZE];

#[test]
fn test_alloc() {
    // ABORT: thread '[...]::test_alloc' has overflowed its stack
    let _: Box<SuperPage> = bytemuck::zeroed_box();
}

EDIT: This happens on windows, have not checked anything else yet.

Reproducible on playpen: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=547372389428bf450c9bf885cf7ea6b8

we might want to have feature gated track_caller attributes on anything that can panic.

This is probably not going to work, but I thought I would ask.

It would be nice if Pod were implemented for num::Complex<T> where T: Pod. It fits all the criteria. But I doubt the num crate would be willing to depend on bytemuck. As an alternative, maybe you could add an optional dependency and feature for num? It's just an idea. It may not work.

num also defines BigInt and BigUint, but I don't know if they would qualify for Pod.

Ref: rust-lang/unsafe-code-guidelines#286

It seems we'll have to remove the Pod impls for pointers.

• this is a breaking change
• this is "allowed" as a break because it's a Soundness fix.

The library itself provides no implementors for the trait trait. This is odd as there are several transparent wrappers within the standard library. Some of them might be valid, some less so.

• core::mem::ManuallyDrop<T>
• core::num::Wrapping

Invalid ones:

• core::cell::Cell: This would allow turning a &T into a &Cell<T> and subsequently write to memory that was assumed to be read-only. But Cell::from_mut exists and so a purely mutable variant would be valid. Unclear about the wrapping of niches.
• core::mem::MaybeUninit: Would allow writing uninitialized memory to the referree. Here only a non-mutable variant would be valid. Unclear about the wrapping of niches.
• core::pin::Pin: This basically constructs a Pin and would thus violate some of its safety assumptions of the unsafe constructor. However, it might be possible to gate such an impl with the same bounds as Pin::new. There is no way for a purely mutable or immutable variant that does not adhere to the same rules as even cloning the Pin is a safety critical operation.
• core::task::Waker: Another example of a type that is unsafe to construct.

Unclear:

• core::cell::UnsafeCell: Highly dubious for the reasons of Cell. But itself it does not expose any safe method for writing so it is arguably a pure wrapper on its own that only affects the languages treatment of references and pointers (in stacked borrowed) themselves. Unclear about the wrapping of niches.
• std::io::IoSlice{,Mut}: This is actually a transparent wrapper around a target dependent underlying type such as libc::iovec on unix.

Traits NoPadding and AnyBitPattern that would allow "casting" like this

fn cast<T, U>(T) -> U
where
    T: NoPadding,
    U: AnyBitPattern,
{
   ...
}

Then Pod is simply both.

The NonZero* types can't implement Pod so the blanket implementation of NoUninit for T: Pod doesn't cover them, which means that you can't use bytemuck::cast_slice::<NonZeroU32, u32>() and similar calls

It probably makes sense to provide derives for Eq (and PartialEq) and Hash in bytemuck. On a Pod type Eq can be a simple memcmp / bcmp. Hashing is also much more efficient as you can provide all bytes at once.

I'm confused by this part of the README:

There is experimental support for the Zeroable trait being derived through a proc-macro. I'm not the author of that crate, please file bugs with that crate in the other repo.

https://github.com/rodrimati1992/zeroable_crates

To me it looks like the Zeroable derive is implemented in this repo. And I also don't see any dependencies to that crate anywhere. Is that note just outdated?

Is there any interest in providing something that could, for example, take an unaligned [u8] slice and convert it to a u32?

it seems to be the more stable github action going forward, maybe?

#[repr(transparent)] guaranties that the type has the same layout that the inner one, so I think that the following examples should work :

#[repr(tansparent)]
#[derive(Zeroable, Pod)]
struct TypedUsize<T>(usize, PhantomData<T>);

#[repr(transparent)]
#[derive(Zeroable, Pod)] // with `T: Pod` or `T: Zeroable` bound
struct Unsync<T>(T, PhantomData<*const ()>);

There is try_ version of it, but not the one that unwraps

Because it does not make sense to implement many TransparentWrappers for one struct I suggest changing the definition to

pub unsafe trait TransparentWrapper {
    type Wrapped: ?Sized;

    fn wrap_ref(s: &Wrapped) -> &Self { ... }
    fn wrap_mut(s: &mut Wrapped) -> &mut Self { ... }
}

The atomic types are all bytemuck-compatible, and in fact as far as I know all bytemuck traits can be implemented for Atomic* that can be implemented for the underlying integer type as the operations available on the atomic types are a strict superset (i.e. setting their value via an immutable reference). Even casting between a non-atomic int and an atomic int should be safe, although from this discussion here rust-lang/rust#76314 it may be invalid to cast an &usize (or other scalar) to an &AtomicUsize (or other atomic scalar). This sadly means that Pod cannot be implemented without adding new traits to account for types where owned types can be cast but references cannot.

The Pod trait gives an example of padding in the middle of a struct, but we should also explain that there can be padding on the end too.

Apparently align_to is spec'd such that you can just fail to do the conversion, and so MIRI for example choose to just fail and return the starting slice every time.

That's crap, and we need to implement a "by hand" version that doesn't fail so that MIRI can't try to give us the run-around.

The requirements for this is very similar to Pod, except that it doesn’t require that the type contains no uninit (or padding) bytes. This limits what you can do with a type of this kind, but also broadens the included types to repr(C) structs that contain padding as well as unions. Notably, you can only cast immutable references and owned values into AnyBitPattern types, not mutable references.

It's not clear to me why mutable references should care about uninitialized padding? I would just expect it to be ignored.

graphics35334:wgpu-rs dmalyshau$ cargo test
    Updating crates.io index
    Updating git repository `https://github.com/gfx-rs/wgpu`
  Downloaded proc-macro2 v1.0.21
  Downloaded bytemuck v1.4.1
  Downloaded syn v1.0.40
  Downloaded bytemuck_derive v1.0.0
   Compiling proc-macro2 v1.0.21
   Compiling syn v1.0.40
   Compiling wgpu-core v0.6.0 (/Users/dmalyshau/Code/wgpu/wgpu-core)
   Compiling wgpu-types v0.6.0 (/Users/dmalyshau/Code/wgpu/wgpu-types)
   Compiling quote v1.0.3
   Compiling thiserror-impl v1.0.20
   Compiling bytemuck_derive v1.0.0
error[E0433]: failed to resolve: use of undeclared type or module `proc_macro`
  --> /Users/dmalyshau/.cargo/registry/src/github.com-1ecc6299db9ec823/bytemuck_derive-1.0.0/src/lib.rs:36:26
   |
36 | pub fn derive_pod(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
   |                          ^^^^^^^^^^ use of undeclared type or module `proc_macro`

Looks like it can't find the proc_macro thing on my machine. I wonder why?

Let's say I have a type:

#[repr(transparent)]
#[derive(Copy, Clone)]
struct MyType(u32);

Is there a way to mark it as Pod without unsafe and proc-macros?

I re-export from one crate like this:

pub use bytemuck;

...and then import Zeroable and Pod from this re-exported location. Then I add

#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]

This triggers a compiler error:

error[E0433]: failed to resolve: could not find `bytemuck` in the list of imported crates
  --> examples/src/lib.rs:14:39
   |
14 | #[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
   |                                       ^^^^^^^^ could not find `bytemuck` in the list of imported crates
   |
   = note: this error originates in the derive macro `Zeroable` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: could not find `bytemuck` in the list of imported crates
  --> examples/src/lib.rs:14:49
   |
14 | #[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
   |                                                 ^^^ could not find `bytemuck` in the list of imported crates
   |
   = note: this error originates in the derive macro `Pod` (in Nightly builds, run with -Z macro-backtrace for more info)

When I add bytemuck as a dependency directly, and use Zeroable and Pod from there, the error goes away. It seems that the internals of the derive macro can't handle an alternative export path?

Maybe we should explain better the difference between TargetAlignmentGreaterAndInputNotAligned and AlignmentMismatch.
At least for me they are the same error

Where AlignmentMismatch is used
try_from_bytes
try_cast_box
try_cast_vec

Where the longer one is used
try_cast_slice
try_cast_ref

(and also their _mut counterparts)

Hi, I noticed that the documentation of both bytes_of and bytes_of_mut says:

Any ZST becomes an empty slice

But instead of returning an empty slice, it actually causes a runtime panic. E.g., when executing:

use bytemuck;
fn main() {
    let zst: [u32; 0] = [];
    let _result = bytemuck::bytes_of(&zst);  // Panic: entered unreachable code
}

Currently Pod requires Copy for T, which isn't too cool for my project. I want to transform some structs (mostly 4x u32 up to 10x u32) into &[u8]. Since those structs are heavily passed along functions, I want to avoid in having accidental copies occurring. For that reason I would rather fall back to manual cloning (which so far can be fully avoided) than implementing copy on the struct. Any way you could implement a feature that disables the copy requirement?

I'm trying to use bytes_of_mut to get &mut Vec<u8> from Vec<u32>, but it seems not supporting that:

error[E0277]: the trait bound `std::vec::Vec<u32>: bytemuck::pod::Pod` is not satisfied
<snip>
    |
72  |         let vec_u8 = bytemuck::bytes_of_mut(&mut vec_u32);
    |                                               ^^^^^^^^^^^ the trait `bytemuck::pod::Pod` is not implemented for `std::vec::Vec<u32>`

Is there a plan to support that?

There are a number of crates similar to this one:

• https://crates.io/crates/zerocopy
• https://crates.io/crates/plain
• https://crates.io/crates/zero
• https://crates.io/crates/pod

Did you evaluate any of these before deciding to write this crate, and if so can you give me your thoughts on why they weren't suitable? I need something like this crate and I'm interesting in learning if this crate solves any problems that the others don't.

#[repr(packed)] guarantee that there's will be no padding between fields.

Hi! Thanks for bytemuck ❤️ It is a pleasure to work with (compared to my previous unsafe versions of bytes_of and from_bytes ).

When switching to it, I managed to trigger https://rust-lang.github.io/rust-clippy/master/index.html#identity_op in the code generated by the derives.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&code=%23%5Brepr(C)%5D%0A%23%5Bderive(Debug%2C%20Clone%2C%20Copy%2C%20PartialEq%2C%20Default%2C%20bytemuck%3A%3AZeroable%2C%20bytemuck%3A%3APod)%5D%0Apub%20struct%20Vec2%20%7B%0A%20%20%20%20x%3A%20f32%2C%0A%20%20%20%20y%3A%20f32%2C%0A%7D%0A%0Afn%20main()%20%7B%7D

(run clippy on tthe play pen)

The culprit seems to be this generated code, emitted as a static check before the unsafe impl is emitted:

const _: fn() =
    ||
        {
            struct TypeWithoutPadding([u8; 0 + ::core::mem::size_of::<f32>() +
                                               ::core::mem::size_of::<f32>()]);
            let _ = ::core::mem::transmute::<Vec2, TypeWithoutPadding>;
        };

Seems to be trivially fixable by checking the number of fields beforehand in https://github.com/Lokathor/bytemuck/blob/main/derive/src/traits.rs#L209

I can submit a fix, if you desired.

When the feature min_const_generics is turned on, of course.

fn wrap_array<const N: usize>(s: [Inner; N]) -> [Self; N]
where
    Self: Sized,
    Inner: Sized;

fn peel_array<const N: usize>(s: [Self; N]) -> [Inner; N]
where
    Self: Sized,
    Inner: Sized;

By the way, TransparentWrapper can be implemented for core::cmp::Reverse.

Is it possible to add casts for Box<[A]> -> Box<[B]> implemented the same way try_cast_vec and cast_vec are? Even better if it was implemented the same way try_cast_slice and cast_slice are, though I assume that may cause other issues since the vec casts have stricter requirements.

Currently it's possible to do this by bytemuck::cast_vec(boxed_slice.into_vec()).into_boxed_slice() or bytemuck::try_cast_vec(boxed_slice.into_vec()).map(|x| x.into_boxed_slice()) which isn't too long but a helper function would be nice.

Currently, there is no documentation for the derive feature, which allows for easy implementations for Pod and Zeroable, which makes it difficult for people to effectively use the crate, especially as the two derive macros show up in the documentation.

Suggested fixes:

• Provide a note in the documentation suggesting use of the derive feature to get derive implementations for Pod and Zeroable (provide code example)
• Make the derive macro a default feature.

https://github.com/dtolnay/trybuild

This would let us declare things that should fail and then check that they really do fail.

Specifically, we could make a test such as

#[derive(Zeroable, Pod)]
pub struct Bad(u16, u8);

And that should fail to derive, of course.

No need for Travis and AppVeyor when there's no OS related code in here. We can probably just use Github Actions (which Rust is slowly moving to in general it seems).

According to this definition of repr(C) in Rust, structs where all fields have the same type are laid out like arrays (no padding anywhere). The reference doesn't directly state this, but it follows from the given layout algorithm (unless I'm wrong).

The documentation of Pod list five unsafe requirements¹. However, not listed among them is a requirement important for slices of the type. Quoting from the unsafe code guidelines.

The layout of a slice [T] of length N is the same as that of a [T; N] array.

Array types, [T; N], store N values of type T with a constant stride. Here, stride is the distance between each pair of consecutive values within the array.

The stride of the array is constant for all element pairs and it is computed as the size of the element type rounded up to the next multiple of the alignment of the element type.

Note the distinction between the two stride and slice. Since cast_slice and others calculates the length of new array by dividing by the size of elements it is important that the size and the stride are the same for Pod types. In particular for repr(Rust) types there are no guarantees (although highly unlikely to be the case) that the size is not a multiple of the alignment. For repr(C)-types and those who transitively are due to repr(transparent) this property is naturally true and thus I would not exactly call this requirement a breaking change. However, it should be listed more explicitely in the documentation.

Theoretically speaking, it could be valid for Rust to layout #[repr(packed)] struct Foo(u16, u8) such that:

• size_of::<Foo>() == 3
• align_of::<Foo>() == 2
• stride_of::<Foo>() == 4 (theoretical new function)

The example in the documentation uses repr(C) where such a type would have necessarily a size of 4.

¹ Extracted from behind the link click the link:

• The type must be inhabited (eg: no Infallible).
• The type must allow any bit pattern (eg: no bool or char, which have illegal bit patterns).
• The type must not contain any padding bytes, either in the middle or on the end (eg: no #[repr(C)] struct Foo(u8, u16), which has padding in the middle, and also no #[repr(C)] struct Foo(u16, u8), which has padding on the end).
• The type needs to have all fields also be Pod.
• The type needs to be repr(C), repr(transparent), or repr(packed).

That is, when we're using path in Cargo.toml to point at a local checkout.

Specifically when the fields are types from glam. Which is on bytemuck version 1.4. Everything's dandy if the field types are locally definded.

minmal reproduction repo

Intended behavior? Anything I can do?

Currently it looks like pod/zeroable structs do not support having enum fields in them, is this correct? As the derive macros for Pod and Zeroable doesn't support enums.

Such as:

#[derive(Copy, Clone, Pod, Zeroable)]
#[repr(C)]
struct TestWithEnum {
  a: u16,
  b: u16,
  e: ContiguousEnum,
}

#[repr(u8)]
#[derive(Clone, Copy, Contiguous)]
enum ContiguousEnum {
  A = 0,
  B = 1,
  C = 2,
}

Though enums that use a primitive representation could be considered pod types (even without the Contiguous additional requirement), and if such an enum implements a variant with value 0 it could support implement Zeroable trait as well right?

One can implement the traits manually for an enum:

unsafe impl bytemuck::Pod for ContiguousEnum {}
unsafe impl bytemuck::Zeroable for ContiguousEnum {}

That said, the crate does specify this as a restriction for what it calls a pod type which could rule out enums, but is a pretty big constraint:

The type must allow any bit pattern (eg: no bool or char, which have illegal bit patterns).

Is this intended as the restriction of why enums are not supported also, or has it simply not been implemented yet in the derive macros? So one could do this instead of manual implementation:

#[repr(u8)]
#[derive(Clone, Copy, Pod, Zeroable, Contiguous)]
enum ContiguousEnum {
  A = 0,
  B = 1,
  C = 2,
}

We have a bunch of use cases in native and shader code where we have pod structs (that do have enums in) that we want to use bytemuck on for the additional compile-time verification that they use the right representation and some limitations, but we are just casting between typed slices and byte slices so maybe what we are after is just a more relaxed pod type that could handle enums and also do not require types to be zeroable (#85)

Hello @Lokathor,

I wanted to switch from zerocopy to bytemuck for multiple reasons, and one of the missing features for both crates is the ability to realign a slice of type A into a Vec of type B, which is allowed to allocate. What do you think about introducing this to this crate (under the allocation feature) and introducing that for Box?

This function would be preferable not to restrict the size (where the length of the input must be the same as the output Vec). It would make this function useless for when you want to transpose differently sized types (e.g. u8 into u16) like for the try_cast_vec function.

Another solution would be to make the try_cast_vec function change the length and capacity of the Vec. This seems a lot better but does seem to be a breaking change in the sense that more results could return an Ok(Vec) instead of an error and that the Vec is no more of the same length than the input one.

// An idea of the possible function signature
fn try_cast_slice_realign_vec<A: Pod, B: Pod>(input: &[A]) -> Result<Vec<B>, PodCastError>;

// Another even better function signature which will allocate only if alignment is invalid
// Note that Cow is in the alloc Rust library
fn try_cast_slice_realign_cow<A: Pod, B: Pod>(input: &[A]) -> Result<Cow<[B]>, PodCastError>;

Could maybe be implemented this way:

let bytes = bytes_of(input);
let len = input.len();
let elem_size = mem::size_of::<B>();

// ensure that it is the alignment that is wrong
// and the length is valid
if len % elem_size == 0 && align_of::<A>() != align_of::<B>() {
    let elems = len / elem_size;
    let mut vec = Vec::<B>::with_capacity(elems);

    unsafe {
        let dst = vec.as_mut_ptr() as *mut u8;
        ptr::copy_nonoverlapping(bytes.as_ptr(), dst, len);
        vec.set_len(elems);
    }

    Ok(Cow::Owned(vec))
}

There probably is a reason or specific history why Pod derives Zeroable, and I may be missing something but couldn't find a motivation in the docs or code.

Logically to me it feels like all zeroable types would be pod types, but all pod types are not necessarily zeroable. Does that make sense?

Been experimenting with a bunch of use cases in our code (also see #84) and while I can see a use for zeroable types, it is (for us at least) very rare and we do have a lot of pod types that due to this requirement also have to derive and be zeroable and expose ::zereod() to the users without it really making much sense to every crate a zeroable implementation of them - so creates some extra noise and potential confusion with this requirement (for us).

Those two impls are useful and there are not compatibility concerns as the enum is already fixed for this major version and not non-exhaustive. (Edit: Error would require std however).

https://blueoakcouncil.org/license/1.0.0#markdown