Auto merge of #76241 - RalfJung:flt2dec, r=Mark-Simulacrum

flt2dec: properly handle uninitialized memory 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-lang · Sep 2, 2020 · 95815c9 · 95815c9
2 parents da897df + 56129d3
commit 95815c9
Show file tree

Hide file tree

Showing 8 changed files with 387 additions and 293 deletions.
diff --git a/library/core/benches/num/flt2dec/strategy/dragon.rs b/library/core/benches/num/flt2dec/strategy/dragon.rs
@@ -1,59 +1,76 @@
 use super::super::*;
 use core::num::flt2dec::strategy::dragon::*;
+use std::mem::MaybeUninit;
 use test::Bencher;
 
 #[bench]
 fn bench_small_shortest(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; MAX_SIG_DIGITS];
-    b.iter(|| format_shortest(&decoded, &mut buf));
+    let mut buf = [MaybeUninit::new(0); MAX_SIG_DIGITS];
+    b.iter(|| {
+        format_shortest(&decoded, &mut buf);
+    });
 }
 
 #[bench]
 fn bench_big_shortest(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; MAX_SIG_DIGITS];
-    b.iter(|| format_shortest(&decoded, &mut buf));
+    let mut buf = [MaybeUninit::new(0); MAX_SIG_DIGITS];
+    b.iter(|| {
+        format_shortest(&decoded, &mut buf);
+    });
 }
 
 #[bench]
 fn bench_small_exact_3(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; 3];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 3];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_big_exact_3(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; 3];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 3];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_small_exact_12(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; 12];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 12];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_big_exact_12(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; 12];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 12];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_small_exact_inf(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; 1024];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 1024];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_big_exact_inf(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; 1024];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 1024];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
diff --git a/library/core/benches/num/flt2dec/strategy/grisu.rs b/library/core/benches/num/flt2dec/strategy/grisu.rs
@@ -1,5 +1,6 @@
 use super::super::*;
 use core::num::flt2dec::strategy::grisu::*;
+use std::mem::MaybeUninit;
 use test::Bencher;
 
 pub fn decode_finite<T: DecodableFloat>(v: T) -> Decoded {
@@ -12,55 +13,71 @@ pub fn decode_finite<T: DecodableFloat>(v: T) -> Decoded {
 #[bench]
 fn bench_small_shortest(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; MAX_SIG_DIGITS];
-    b.iter(|| format_shortest(&decoded, &mut buf));
+    let mut buf = [MaybeUninit::new(0); MAX_SIG_DIGITS];
+    b.iter(|| {
+        format_shortest(&decoded, &mut buf);
+    });
 }
 
 #[bench]
 fn bench_big_shortest(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; MAX_SIG_DIGITS];
-    b.iter(|| format_shortest(&decoded, &mut buf));
+    let mut buf = [MaybeUninit::new(0); MAX_SIG_DIGITS];
+    b.iter(|| {
+        format_shortest(&decoded, &mut buf);
+    });
 }
 
 #[bench]
 fn bench_small_exact_3(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; 3];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 3];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_big_exact_3(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; 3];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 3];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_small_exact_12(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; 12];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 12];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_big_exact_12(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; 12];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 12];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_small_exact_inf(b: &mut Bencher) {
     let decoded = decode_finite(3.141592f64);
-    let mut buf = [0; 1024];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 1024];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
 
 #[bench]
 fn bench_big_exact_inf(b: &mut Bencher) {
     let decoded = decode_finite(f64::MAX);
-    let mut buf = [0; 1024];
-    b.iter(|| format_exact(&decoded, &mut buf, i16::MIN));
+    let mut buf = [MaybeUninit::new(0); 1024];
+    b.iter(|| {
+        format_exact(&decoded, &mut buf, i16::MIN);
+    });
 }
diff --git a/library/core/src/fmt/float.rs b/library/core/src/fmt/float.rs
@@ -14,25 +14,17 @@ fn float_to_decimal_common_exact<T>(
 where
     T: flt2dec::DecodableFloat,
 {
-    // SAFETY: Possible undefined behavior, see FIXME(#76092)
-    unsafe {
-        let mut buf = MaybeUninit::<[u8; 1024]>::uninit(); // enough for f32 and f64
-        let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 4]>::uninit();
-        // FIXME(#76092): This is calling `assume_init_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!)
-        let formatted = flt2dec::to_exact_fixed_str(
-            flt2dec::strategy::grisu::format_exact,
-            *num,
-            sign,
-            precision,
-            buf.assume_init_mut(),
-            parts.assume_init_mut(),
-        );
-        fmt.pad_formatted_parts(&formatted)
-    }
+    let mut buf: [MaybeUninit<u8>; 1024] = MaybeUninit::uninit_array(); // enough for f32 and f64
+    let mut parts: [MaybeUninit<flt2dec::Part<'_>>; 4] = MaybeUninit::uninit_array();
+    let formatted = flt2dec::to_exact_fixed_str(
+        flt2dec::strategy::grisu::format_exact,
+        *num,
+        sign,
+        precision,
+        &mut buf,
+        &mut parts,
+    );
+    fmt.pad_formatted_parts(&formatted)
 }
 
 // Don't inline this so callers that call both this and the above won't wind
@@ -47,22 +39,18 @@ fn float_to_decimal_common_shortest<T>(
 where
     T: flt2dec::DecodableFloat,
 {
-    // SAFETY: Possible undefined behavior, see FIXME(#76092)
-    unsafe {
-        // enough for f32 and f64
-        let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninit();
-        let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 4]>::uninit();
-        // FIXME(#76092)
-        let formatted = flt2dec::to_shortest_str(
-            flt2dec::strategy::grisu::format_shortest,
-            *num,
-            sign,
-            precision,
-            buf.assume_init_mut(),
-            parts.assume_init_mut(),
-        );
-        fmt.pad_formatted_parts(&formatted)
-    }
+    // enough for f32 and f64
+    let mut buf: [MaybeUninit<u8>; flt2dec::MAX_SIG_DIGITS] = MaybeUninit::uninit_array();
+    let mut parts: [MaybeUninit<flt2dec::Part<'_>>; 4] = MaybeUninit::uninit_array();
+    let formatted = flt2dec::to_shortest_str(
+        flt2dec::strategy::grisu::format_shortest,
+        *num,
+        sign,
+        precision,
+        &mut buf,
+        &mut parts,
+    );
+    fmt.pad_formatted_parts(&formatted)
 }
 
 // Common code of floating point Debug and Display.
@@ -103,22 +91,18 @@ fn float_to_exponential_common_exact<T>(
 where
     T: flt2dec::DecodableFloat,
 {
-    // SAFETY: Possible undefined behavior, see FIXME(#76092)
-    unsafe {
-        let mut buf = MaybeUninit::<[u8; 1024]>::uninit(); // enough for f32 and f64
-        let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 6]>::uninit();
-        // FIXME(#76092)
-        let formatted = flt2dec::to_exact_exp_str(
-            flt2dec::strategy::grisu::format_exact,
-            *num,
-            sign,
-            precision,
-            upper,
-            buf.assume_init_mut(),
-            parts.assume_init_mut(),
-        );
-        fmt.pad_formatted_parts(&formatted)
-    }
+    let mut buf: [MaybeUninit<u8>; 1024] = MaybeUninit::uninit_array(); // enough for f32 and f64
+    let mut parts: [MaybeUninit<flt2dec::Part<'_>>; 6] = MaybeUninit::uninit_array();
+    let formatted = flt2dec::to_exact_exp_str(
+        flt2dec::strategy::grisu::format_exact,
+        *num,
+        sign,
+        precision,
+        upper,
+        &mut buf,
+        &mut parts,
+    );
+    fmt.pad_formatted_parts(&formatted)
 }
 
 // Don't inline this so callers that call both this and the above won't wind
@@ -133,23 +117,19 @@ fn float_to_exponential_common_shortest<T>(
 where
     T: flt2dec::DecodableFloat,
 {
-    // SAFETY: Possible undefined behavior, see FIXME(#76092)
-    unsafe {
-        // enough for f32 and f64
-        let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninit();
-        let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 6]>::uninit();
-        // FIXME(#76092)
-        let formatted = flt2dec::to_shortest_exp_str(
-            flt2dec::strategy::grisu::format_shortest,
-            *num,
-            sign,
-            (0, 0),
-            upper,
-            buf.assume_init_mut(),
-            parts.assume_init_mut(),
-        );
-        fmt.pad_formatted_parts(&formatted)
-    }
+    // enough for f32 and f64
+    let mut buf: [MaybeUninit<u8>; flt2dec::MAX_SIG_DIGITS] = MaybeUninit::uninit_array();
+    let mut parts: [MaybeUninit<flt2dec::Part<'_>>; 6] = MaybeUninit::uninit_array();
+    let formatted = flt2dec::to_shortest_exp_str(
+        flt2dec::strategy::grisu::format_shortest,
+        *num,
+        sign,
+        (0, 0),
+        upper,
+        &mut buf,
+        &mut parts,
+    );
+    fmt.pad_formatted_parts(&formatted)
 }
 
 // Common code of floating point LowerExp and UpperExp.