Skip to content

Fix integer overflow in format!("{:.0?}", Duration::MAX) #102484

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

Merged
merged 1 commit into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions library/core/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,7 @@ impl fmt::Debug for Duration {
/// to the formatter's `width`, if specified.
fn fmt_decimal(
f: &mut fmt::Formatter<'_>,
mut integer_part: u64,
integer_part: u64,
mut fractional_part: u32,
mut divisor: u32,
prefix: &str,
Expand Down Expand Up @@ -1075,7 +1075,7 @@ impl fmt::Debug for Duration {
// normal floating point numbers. However, we only need to do work
// when rounding up. This happens if the first digit of the
// remaining ones is >= 5.
if fractional_part > 0 && fractional_part >= divisor * 5 {
let integer_part = if fractional_part > 0 && fractional_part >= divisor * 5 {
// Round up the number contained in the buffer. We go through
// the buffer backwards and keep track of the carry.
let mut rev_pos = pos;
Expand All @@ -1099,9 +1099,18 @@ impl fmt::Debug for Duration {
// the whole buffer to '0's and need to increment the integer
// part.
if carry {
integer_part += 1;
// If `integer_part == u64::MAX` and precision < 9, any
// carry of the overflow during rounding of the
// `fractional_part` into the `integer_part` will cause the
// `integer_part` itself to overflow. Avoid this by using an
// `Option<u64>`, with `None` representing `u64::MAX + 1`.
integer_part.checked_add(1)
} else {
Some(integer_part)
}
}
} else {
Some(integer_part)
};

// Determine the end of the buffer: if precision is set, we just
// use as many digits from the buffer (capped to 9). If it isn't
Expand All @@ -1111,7 +1120,12 @@ impl fmt::Debug for Duration {
// This closure emits the formatted duration without emitting any
// padding (padding is calculated below).
let emit_without_padding = |f: &mut fmt::Formatter<'_>| {
write!(f, "{}{}", prefix, integer_part)?;
if let Some(integer_part) = integer_part {
write!(f, "{}{}", prefix, integer_part)?;
} else {
// u64::MAX + 1 == 18446744073709551616
write!(f, "{}18446744073709551616", prefix)?;
}

// Write the decimal point and the fractional part (if any).
if end > 0 {
Expand Down Expand Up @@ -1141,12 +1155,17 @@ impl fmt::Debug for Duration {
// 2. The postfix: can be "µs" so we have to count UTF8 characters.
let mut actual_w = prefix.len() + postfix.chars().count();
// 3. The integer part:
if let Some(log) = integer_part.checked_ilog10() {
// integer_part is > 0, so has length log10(x)+1
actual_w += 1 + log as usize;
if let Some(integer_part) = integer_part {
if let Some(log) = integer_part.checked_ilog10() {
// integer_part is > 0, so has length log10(x)+1
actual_w += 1 + log as usize;
} else {
// integer_part is 0, so has length 1.
actual_w += 1;
}
} else {
// integer_part is 0, so has length 1.
actual_w += 1;
// integer_part is u64::MAX + 1, so has length 20
actual_w += 20;
}
// 4. The fractional part (if any):
if end > 0 {
Expand Down
24 changes: 23 additions & 1 deletion library/core/tests/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,31 @@ fn correct_sum() {
#[test]
fn debug_formatting_extreme_values() {
assert_eq!(
format!("{:?}", Duration::new(18_446_744_073_709_551_615, 123_456_789)),
format!("{:?}", Duration::new(u64::MAX, 123_456_789)),
"18446744073709551615.123456789s"
);
assert_eq!(format!("{:.0?}", Duration::MAX), "18446744073709551616s");
assert_eq!(format!("{:.0?}", Duration::new(u64::MAX, 500_000_000)), "18446744073709551616s");
assert_eq!(format!("{:.0?}", Duration::new(u64::MAX, 499_999_999)), "18446744073709551615s");
assert_eq!(
format!("{:.3?}", Duration::new(u64::MAX, 999_500_000)),
"18446744073709551616.000s"
);
assert_eq!(
format!("{:.3?}", Duration::new(u64::MAX, 999_499_999)),
"18446744073709551615.999s"
);
assert_eq!(
format!("{:.8?}", Duration::new(u64::MAX, 999_999_995)),
"18446744073709551616.00000000s"
);
assert_eq!(
format!("{:.8?}", Duration::new(u64::MAX, 999_999_994)),
"18446744073709551615.99999999s"
);
assert_eq!(format!("{:21.0?}", Duration::MAX), "18446744073709551616s");
assert_eq!(format!("{:22.0?}", Duration::MAX), "18446744073709551616s ");
assert_eq!(format!("{:24.0?}", Duration::MAX), "18446744073709551616s ");
}

#[test]
Expand Down