Skip to content
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

Fix incorrect use of MaybeUninit::assume_init_mut in flt2dec #76092

Closed
Tracked by #63566
Dylan-DPC-zz opened this issue Aug 30, 2020 · 5 comments · Fixed by #76241
Closed
Tracked by #63566

Fix incorrect use of MaybeUninit::assume_init_mut in flt2dec #76092

Dylan-DPC-zz opened this issue Aug 30, 2020 · 5 comments · Fixed by #76241
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@Dylan-DPC-zz
Copy link

Dylan-DPC-zz commented Aug 30, 2020

Creating this issue to track this FIXME note in float formatting:

This is calling get_mut on an uninitialized
MaybeUninit (here and elsewhere in this file). Revisit this once
we decided whether that is valid or not.
We can do this only because we are libstd and coupled to the compiler.
(FWIW, using freeze would not be enough; flt2dec::Part is an enum!)

The issue being mentioned when the fixme was closed without addressing this concern, and the new tracking issue doesn't raise it, so created this so that we can discuss it here and link it to the tracking issue

cc @RalfJung

@RalfJung
Copy link
Member

Besides the float code, there's also the Read code doing something similar for allowing reads into uninitialized buffers. However, rust-lang/rfcs#2930 should resolve that I think.

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Aug 30, 2020

Could we rename it to validity of .assume_init_mut() on uninitialized float(s)? or &mut on uninitialized floats?


Regarding the issue at hand, obviously only libstd and other internal crates can rely on the internals of the rustc it is released with, so technically, we could go ahead with #76047 and "ignore" the FIXME, as long as we have this issue to remind us of fixing that.

Doing so is not that bad as when I attempted to tackle #66174, since by now we have initiatives such as the one I suggested there having been implemented in a library crate, using a library version of out references, and we also have the aforementioned RFC getting some progress. So this time, letting the FIXME without fixing it yet is not just a "useless procrastination of the problem", it is a meaningful "wait for the required language feature", I'd say 🙂

@RalfJung
Copy link
Member

The issue is not really specific to floats, it is tight to whether references are allowed to refer to uninitialized data.

So this is basically the rustc side of (a part of) rust-lang/unsafe-code-guidelines#77.

@RalfJung RalfJung changed the title Validity of get_mut/assume_init_mut on MaybeUninit Validity of MaybeUninit::get_mut/assume_init_mut on invalid data Aug 30, 2020
@RalfJung
Copy link
Member

So this time, letting the FIXME without fixing it yet is not just a "useless procrastination of the problem", it is a meaningful "wait for the required language feature", I'd say slightly_smiling_face

Agreed for the Read side. For the float usage, really that code should be converted to raw pointers.

@RalfJung RalfJung changed the title Validity of MaybeUninit::get_mut/assume_init_mut on invalid data Fix incorrect use of MaybeUninit::assume_init_mut in standard library Sep 2, 2020
@RalfJung RalfJung changed the title Fix incorrect use of MaybeUninit::assume_init_mut in standard library Fix incorrect use of MaybeUninit::assume_init_mut in flt2dec Sep 2, 2020
@RalfJung
Copy link
Member

RalfJung commented Sep 2, 2020

Actually, let's use #42788 for the Read part (there's another FIXME in the code referencing that already).

This is just for flt2dec, and will be fixed by #76241.

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 2, 2020
@bors bors closed this as completed in 95815c9 Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants