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

flt2dec: properly handle uninitialized memory #76241

Merged
merged 1 commit into from
Sep 2, 2020

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Sep 2, 2020

The float-to-str code currently uses uninitialized memory incorrectly (see #76092). This PR fixes that.

Specifically, that code used &mut [T] as "out references", but it would be incorrect for the caller to actually pass uninitialized memory. So the PR changes this to &mut [MaybeUninit<T>], and then functions return a &[T] to the part of the buffer that they initialized (some functions already did that, indirectly via &Formatted, others were adjusted to return that buffer instead of just the initialized length).

What I particularly like about this is that it moves unsafe to the right place: previously, the outermost caller had to use unsafe to assert that things are initialized; now it is the functions that do the actual initializing which have the corresponding unsafe block when they call MaybeUninit::slice_get_ref (renamed in #76217 to slice_assume_init_ref).

Reviewers please be aware that I have no idea how any of this code actually works. My changes were purely mechanical and type-driven. The test suite passes so I guess I didn't screw up badly...

Cc @sfackler this is somewhat related to your RFC, and possibly some of this code could benefit from (a generalized version of) the API you describe there. But for now I think what I did is "good enough".

Fixes #76092.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2020
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 2, 2020

📌 Commit 56129d3 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 2, 2020
@bors
Copy link
Contributor

bors commented Sep 2, 2020

⌛ Testing commit 56129d3 with merge 95815c9...

@bors
Copy link
Contributor

bors commented Sep 2, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 95815c9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 2, 2020
@bors bors merged commit 95815c9 into rust-lang:master Sep 2, 2020
@RalfJung RalfJung deleted the flt2dec branch September 3, 2020 10:40
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2021
… r=RalfJung

Stabilize `maybe_uninit_ref`

This stabilizes `assume_init_{ref,mut}`. FCP is complete: rust-lang#63568 (comment)
The renaming was done by rust-lang#76047 and FIXME was resolved by rust-lang#76241, so I think we can now stabilize them finally 🎉
Still, it's const-unstable as `assert_inhabited` is unstable.

Closes rust-lang#63568
@cuviper cuviper added this to the 1.48.0 milestone Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix incorrect use of MaybeUninit::assume_init_mut in flt2dec
6 participants