Prevent downstream miri false positives

[addisoncrump]

The use of mem::uninitialized induces a miri false positive: https://github.com/japaric/hash32/blob/main/src/murmur3.rs#L73

This is problematic as it prevents downstream crates from using miri to check for UB in their own crates.

If you change the example to use Murmur3 instead of FNV, you can see this plainly:

[addisoncrump@addisoncrump-main hash32]$ cargo miri run --example derive
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `/home/addisoncrump/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri target/miri/x86_64-unknown-linux-gnu/debug/examples/derive`
error: Undefined Behavior: constructing invalid value at .value.bytes[0]: encountered uninitialized bytes
  --> /home/addisoncrump/Dokumente/experiments/libafl/hash32/src/murmur3.rs:73:27
   |
73 |             buf: unsafe { mem::uninitialized() },
   |                           ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .value.bytes[0]: encountered uninitialized bytes
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: backtrace:
   = note: inside `<hash32::Murmur3Hasher as std::default::Default>::default` at /home/addisoncrump/Dokumente/experiments/libafl/hash32/src/murmur3.rs:73:27
note: inside `main` at examples/derive.rs:21:19
  --> examples/derive.rs:21:19
   |
21 |     let mut mm3 = Murmur3Hasher::default();
   |                   ^^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

This PR switches out the buffer for a MaybeUninit to remove the false positive in miri.

[japaric]

Thanks!

[japaric]

bors merge

[japaric]

bors cancel
(need to fix bors configuration)

[bors]

Canceled.

[japaric]

bors merge

[bors]

Build succeeded:

• ci

[Manishearth]

The API that replaced this is still UB, I think.

https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html#method.assume_init_mut

You are not allowed to call assume_init_*() until the result is actually initialized. It doesn't matter if it's a u8, this is true for all Rust types (except perhaps MaybeUninit itself). This isn't the same as C's uninit invariant.

[newAM]

The API that replaced this is still UB, I think.

https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html#method.assume_init_mut

You are not allowed to call assume_init_*() until the result is actually initialized. It doesn't matter if it's a u8, this is true for all Rust types (except perhaps MaybeUninit itself). This isn't the same as C's uninit invariant.

Thank you for catching this!

If you have time I would appreciate feedback on the PR I opened as a potential fix: #42

[addisoncrump]

Another day, another UB case that I failed to understand 😔 (though, in fairness, my understanding of the Rust memory model was a lot worse in 2022)

[newAM]

Another day, another UB case that I failed to understand 😔 (though, in fairness, my understanding of the Rust memory model was a lot worse in 2022)

I think this one isn't too bad to understand, at least at the documentation level.

The docstring for MaybeUninit<T>::assume_init states:

It is up to the caller to guarantee that the MaybeUninit really is in an initialized state, i.e., a state that is considered “valid” for type T. Calling this when the content is not yet fully initialized causes immediate undefined behavior

In hash32 this code initialized the MaybeUninit<[u8; 4]>, it calls assume_init_mut len-1 times to initialize each element.
For a 4 element array the first 3 times this gets called not all of the elements in the inner [u8; 4] type have been initialized, violating the safety requirement.

        for i in 0..len {
            unsafe {
                *self
                    .buf
                    .bytes
                    .assume_init_mut()
                    .get_unchecked_mut(start + i) = *buf.get_unchecked(i);
            }
        }

The solution is to change the type from MaybeUninit<[u8; 4]> to [MaybeUninit<u8>; 4], such that each element of the array can be initialized separately without violating the safety requirements.