Auto merge of #102484 - beetrees:duration-debug-bug-fix, r=scottmcm

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

Currently `format!("{:.0?}", Duration::MAX)` causes an integer overflow in the `Duration` `Debug` impl ([playground link](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=67675c6895bdb2e37ee727f0ed7622b2)). This is because the carry from the rounding of the fractional_part into the integer_part will cause the integer_part to overflow as it is already `u64::MAX`. This PR uses a larger integer type to avoid that issue, and adds a test for the correct behaviour.
This commit is contained in:
bors 2022-09-30 14:16:57 +00:00
commit 75d3027fb5
2 changed files with 52 additions and 11 deletions

View File

@ -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,
@ -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;
@ -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
@ -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 {
@ -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 {

View File

@ -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]