Incorrect use of `assume_init` about ntex HOT 12 CLOSED

@RalfJung Are you serious? I wish you were a bot.

from ntex.

ntex has a critical bug. (I should note that the bytes crate this code is forked from is fine; the bug was introduced in the ntex fork or was fixed upstream in the mean time.) I told you guys about it because I care deeply about making unsafe Rust as reliable as possible (e.g. by writing and maintaining tools like Miri, which can help you to find bugs like this). It's entirely up to you what you want to do about that. You can be angry at the messenger or try to fix the problem.

Please don't ping me again.

I appreciate your effort. It's sad the community can be toxic at times.

from ntex.

@RalfJung Seriously, dedicate yourselves to making software and stop bothering those who have the detail to offer us something as good as ntex.

from ntex.

ntex has a critical bug. (I should note that the bytes crate this code is forked from is fine; the bug was introduced in the ntex fork or was fixed upstream in the mean time.) I told you guys about it because I care deeply about making unsafe Rust as reliable as possible (e.g. by writing and maintaining tools like Miri, which can help you to find bugs like this). It's entirely up to you what you want to do about that. You can be angry at the messenger or try to fix the problem.

I think main reason why some people are "angry at the messenger" is that this issue is created outside of feature context, just with reliance on output of Miri. This seems common for rust community, context does not matter, formal compliance with the tooling is everything.

p.s. I don't want to be rude or angry, this is just my observations on rust community. In any case, I removed assume_init. let's use more cpu cycles, lets heat the world :)

from ntex.

uninit memory is never used, implementation control this invariant.

btw is this issue created by bot?

from ntex.

No I'm a person. :) I am a leader of the Unsafe Code Guidelines WG.
A bot (but not mine) found this bug but I verified it before creating the issue.

uninit memory is never used, implementation control this invariant.

It doesn't matter whether the memory is used or not. Please read the assume_init documentation.
You can easily check that this code has Undefined Behavior (UB) by testing it with Miri. If you run this code (click "Tools - Miri") it will print

error: Undefined Behavior: type validation failed at .value.arc.pointer: encountered uninitialized raw pointer
  [--> src/main.rs:14:34
](https://play.rust-lang.org/#)   |
14 |     let _inner: Inner = unsafe { mem::MaybeUninit::uninit().assume_init() };
   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .value.arc.pointer: encountered uninitialized raw pointer
   |
   = 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

from ntex.

Also note the lint being printed:

warning: the type `Inner` does not permit being left uninitialized
  [--> src/main.rs:14:34
](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3c86550e07dda3f122b0c0f3af1f9ad5#)   |
14 |     let _inner: Inner = unsafe { mem::MaybeUninit::uninit().assume_init() };
   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                                  |
   |                                  this code causes undefined behavior when executed
   |                                  help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
   |
   = note: `#[warn(invalid_value)]` on by default
note: `std::ptr::NonNull<Shared>` must be non-null (in this struct field)
  [--> src/main.rs:7:5
](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3c86550e07dda3f122b0c0f3af1f9ad5#)   |
7  |     arc: NonNull<Shared>,
   |     ^^^^^^^^^^^^^^^^^^^^

Nowhere does it say there is an exception for when the uninit memory is "not used", or anything like that. "this code causes undefined behavior when executed", under all conditions.

If you want to learn more about why data needs to be valid even when it is "unused", I wrote a blog post on that subject.

from ntex.

Also note the lint being printed:

warning: the type `Inner` does not permit being left uninitialized
  [--> src/main.rs:14:34
](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3c86550e07dda3f122b0c0f3af1f9ad5#)   |
14 |     let _inner: Inner = unsafe { mem::MaybeUninit::uninit().assume_init() };
   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                                  |
   |                                  this code causes undefined behavior when executed
   |                                  help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
   |
   = note: `#[warn(invalid_value)]` on by default
note: `std::ptr::NonNull<Shared>` must be non-null (in this struct field)
  [--> src/main.rs:7:5
](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3c86550e07dda3f122b0c0f3af1f9ad5#)   |
7  |     arc: NonNull<Shared>,
   |     ^^^^^^^^^^^^^^^^^^^^

Nowhere does it say there is an exception for when the uninit memory is "not used", or anything like that. "this code causes undefined behavior when executed", under all conditions.

If you want to learn more about why data needs to be valid even when it is "unused", I wrote a blog post on that subject.

What about this? Does partial inintialization with transmute cause ub?

let mut arr: [MaybeUninit<u8>; 8] = unsafe { MaybeUninit::uninit().assume_init() };
arr[0].write(1);
arr[1].write(2);
let slice: &[u8] = unsafe { mem::transmute(&arr[..2]) };

BTW do you mind give #107 a review to see if it fix the ub issue?

from ntex.

What about this? Does partial inintialization with transmute cause ub?

As long as you only transmute initialized parts, you are fine. But you can avoid transmute by using slice_assume_init_mut instead.

@RalfJung Are you serious? I wish you were a bot.

Wow. You might want to overthink how you interact with other people on the internet.
I am unsubscribing form this thread.

from ntex.

As long as you only transmute initialized parts, you are fine. But you can avoid transmute by using slice_assume_init_mut instead.

Thanks for the answer. I looked at said api and indeed pointer casting is better.

from ntex.

ntex has a critical bug. (I should note that the bytes crate this code is forked from is fine; the bug was introduced in the ntex fork or was fixed upstream in the mean time.) I told you guys about it because I care deeply about making unsafe Rust as reliable as possible (e.g. by writing and maintaining tools like Miri, which can help you to find bugs like this). It's entirely up to you what you want to do about that. You can be angry at the messenger or try to fix the problem.

Please don't ping me again.

from ntex.

I am not sure how to fix this code, and bytes is not fixed as well.

would be interesting to know how this critical bug manifests itself

from ntex.