Skip to content

Commit

Permalink
Document unsafe blocks in core::fmt
Browse files Browse the repository at this point in the history
  • Loading branch information
foeb committed Mar 6, 2020
1 parent d3c7934 commit 3d146a3
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
6 changes: 4 additions & 2 deletions src/libcore/fmt/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -16,6 +14,7 @@ fn float_to_decimal_common_exact<T>(
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();
Expand Down Expand Up @@ -48,6 +47,7 @@ fn float_to_decimal_common_shortest<T>(
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();
Expand Down Expand Up @@ -103,6 +103,7 @@ fn float_to_exponential_common_exact<T>(
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();
Expand Down Expand Up @@ -132,6 +133,7 @@ fn float_to_exponential_common_shortest<T>(
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();
Expand Down
18 changes: 16 additions & 2 deletions src/libcore/fmt/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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) } }
}

Expand Down Expand Up @@ -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) })
}

Expand Down
27 changes: 25 additions & 2 deletions src/libcore/fmt/num.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()))
};
Expand Down Expand Up @@ -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::<u8>::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);
Expand All @@ -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);
}
Expand All @@ -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))
Expand Down Expand Up @@ -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::<u8>::uninit(); 40];
let mut curr = buf.len() as isize; //index for buf
let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf);
Expand All @@ -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);
}
Expand All @@ -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';
}
Expand All @@ -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;
Expand All @@ -350,6 +371,8 @@ macro_rules! impl_Exp {
// stores 'e' (or 'E') and the up to 2-digit exponent
let mut exp_buf = [MaybeUninit::<u8>::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 {
Expand Down

0 comments on commit 3d146a3

Please sign in to comment.