diff --git a/src/libcore/fmt/float.rs b/src/libcore/fmt/float.rs index 5ef673009bb6d..52d8349bc9a87 100644 --- a/src/libcore/fmt/float.rs +++ b/src/libcore/fmt/float.rs @@ -2,8 +2,6 @@ use crate::fmt::{Debug, Display, Formatter, LowerExp, Result, UpperExp}; use crate::mem::MaybeUninit; use crate::num::flt2dec; -// ignore-tidy-undocumented-unsafe - // Don't inline this so callers don't use the stack space this function // requires unless they have to. #[inline(never)] @@ -16,6 +14,7 @@ fn float_to_decimal_common_exact( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninit(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 4]>::uninit(); @@ -48,6 +47,7 @@ fn float_to_decimal_common_shortest( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninit(); @@ -103,6 +103,7 @@ fn float_to_exponential_common_exact( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { let mut buf = MaybeUninit::<[u8; 1024]>::uninit(); // enough for f32 and f64 let mut parts = MaybeUninit::<[flt2dec::Part<'_>; 6]>::uninit(); @@ -132,6 +133,7 @@ fn float_to_exponential_common_shortest( where T: flt2dec::DecodableFloat, { + // SAFETY: Possible undefined behavior, see FIXME(#53491) unsafe { // enough for f32 and f64 let mut buf = MaybeUninit::<[u8; flt2dec::MAX_SIG_DIGITS]>::uninit(); diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 993b1073493e9..a7b34e5f2f1c5 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1,7 +1,5 @@ //! Utilities for formatting and printing strings. -// ignore-tidy-undocumented-unsafe - #![stable(feature = "rust1", since = "1.0.0")] use crate::cell::{Cell, Ref, RefCell, RefMut, UnsafeCell}; @@ -271,6 +269,14 @@ impl<'a> ArgumentV1<'a> { #[doc(hidden)] #[unstable(feature = "fmt_internals", reason = "internal to format_args!", issue = "none")] pub fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> ArgumentV1<'b> { + // SAFETY: `mem::transmute(x)` is safe because + // 1. `&'b T` keeps the lifetime it originated with `'b` + // (so as to not have an unbounded lifetime) + // 2. `&'b T` and `&'b Void` have the same memory layout + // (when `T` is `Sized`, as it is here) + // `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result` + // and `fn(&Void, &mut Formatter<'_>) -> Result` have the same ABI + // (as long as `T` is `Sized`) unsafe { ArgumentV1 { formatter: mem::transmute(f), value: mem::transmute(x) } } } @@ -1389,6 +1395,14 @@ impl<'a> Formatter<'a> { fn write_formatted_parts(&mut self, formatted: &flt2dec::Formatted<'_>) -> Result { fn write_bytes(buf: &mut dyn Write, s: &[u8]) -> Result { + // SAFETY: This is used for `flt2dec::Part::Num` and `flt2dec::Part::Copy`. + // It's safe to use for `flt2dec::Part::Num` since every char `c` is between + // `b'0'` and `b'9'`, which means `s` is valid UTF-8. + // It's also probably safe in practice to use for `flt2dec::Part::Copy(buf)` + // since `buf` should be plain ASCII, but it's possible for someone to pass + // in a bad value for `buf` into `flt2dec::to_shortest_str` since it is a + // public function. + // FIXME: Determine whether this could result in UB. buf.write_str(unsafe { str::from_utf8_unchecked(s) }) } diff --git a/src/libcore/fmt/num.rs b/src/libcore/fmt/num.rs index 5dfd3a8ecdbd6..7d77e33d74378 100644 --- a/src/libcore/fmt/num.rs +++ b/src/libcore/fmt/num.rs @@ -1,7 +1,5 @@ //! Integer and floating-point number formatting -// ignore-tidy-undocumented-unsafe - use crate::fmt; use crate::mem::MaybeUninit; use crate::num::flt2dec; @@ -84,6 +82,8 @@ trait GenericRadix { } } let buf = &buf[curr..]; + // SAFETY: The only chars in `buf` are created by `Self::digit` which are assumed to be + // valid UTF-8 let buf = unsafe { str::from_utf8_unchecked(slice::from_raw_parts(MaybeUninit::first_ptr(buf), buf.len())) }; @@ -189,11 +189,19 @@ static DEC_DIGITS_LUT: &[u8; 200] = b"0001020304050607080910111213141516171819\ macro_rules! impl_Display { ($($t:ident),* as $u:ident via $conv_fn:ident named $name:ident) => { fn $name(mut n: $u, is_nonnegative: bool, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // 2^128 is about 3*10^38, so 39 gives an extra byte of space let mut buf = [MaybeUninit::::uninit(); 39]; let mut curr = buf.len() as isize; let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); let lut_ptr = DEC_DIGITS_LUT.as_ptr(); + // SAFETY: Since `d1` and `d2` are always less than or equal to `198`, we + // can copy from `lut_ptr[d1..d1 + 1]` and `lut_ptr[d2..d2 + 1]`. To show + // that it's OK to copy into `buf_ptr`, notice that at the beginning + // `curr == buf.len() == 39 > log(n)` since `n < 2^128 < 10^39`, and at + // each step this is kept the same as `n` is divided. Since `n` is always + // non-negative, this means that `curr > 0` so `buf_ptr[curr..curr + 1]` + // is safe to access. unsafe { // need at least 16 bits for the 4-characters-at-a-time to work. assert!(crate::mem::size_of::<$u>() >= 2); @@ -206,6 +214,10 @@ macro_rules! impl_Display { let d1 = (rem / 100) << 1; let d2 = (rem % 100) << 1; curr -= 4; + + // We are allowed to copy to `buf_ptr[curr..curr + 3]` here since + // otherwise `curr < 0`. But then `n` was originally at least `10000^10` + // which is `10^40 > 2^128 > n`. ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); ptr::copy_nonoverlapping(lut_ptr.offset(d2), buf_ptr.offset(curr + 2), 2); } @@ -232,6 +244,8 @@ macro_rules! impl_Display { } } + // SAFETY: `curr` > 0 (since we made `buf` large enough), and all the chars are valid + // UTF-8 since `DEC_DIGITS_LUT` is let buf_slice = unsafe { str::from_utf8_unchecked( slice::from_raw_parts(buf_ptr.offset(curr), buf.len() - curr as usize)) @@ -304,6 +318,8 @@ macro_rules! impl_Exp { }; // 39 digits (worst case u128) + . = 40 + // Since `curr` always decreases by the number of digits copied, this means + // that `curr >= 0`. let mut buf = [MaybeUninit::::uninit(); 40]; let mut curr = buf.len() as isize; //index for buf let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); @@ -313,6 +329,8 @@ macro_rules! impl_Exp { while n >= 100 { let d1 = ((n % 100) as isize) << 1; curr -= 2; + // SAFETY: `d1 <= 198`, so we can copy from `lut_ptr[d1..d1 + 2]` since + // `DEC_DIGITS_LUT` has a length of 200. unsafe { ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); } @@ -324,6 +342,7 @@ macro_rules! impl_Exp { // decode second-to-last character if n >= 10 { curr -= 1; + // SAFETY: Safe since `40 > curr >= 0` (see comment) unsafe { *buf_ptr.offset(curr) = (n as u8 % 10_u8) + b'0'; } @@ -333,11 +352,13 @@ macro_rules! impl_Exp { // add decimal point iff >1 mantissa digit will be printed if exponent != trailing_zeros || added_precision != 0 { curr -= 1; + // SAFETY: Safe since `40 > curr >= 0` unsafe { *buf_ptr.offset(curr) = b'.'; } } + // SAFETY: Safe since `40 > curr >= 0` let buf_slice = unsafe { // decode last character curr -= 1; @@ -350,6 +371,8 @@ macro_rules! impl_Exp { // stores 'e' (or 'E') and the up to 2-digit exponent let mut exp_buf = [MaybeUninit::::uninit(); 3]; let exp_ptr = MaybeUninit::first_ptr_mut(&mut exp_buf); + // SAFETY: In either case, `exp_buf` is written within bounds and `exp_ptr[..len]` + // is contained within `exp_buf` since `len <= 3`. let exp_slice = unsafe { *exp_ptr.offset(0) = if upper {b'E'} else {b'e'}; let len = if exponent < 10 {