UpperHex formatting might panic

When writing non-panicking code, it is impossible to use the "{:X}" format specifier as the impl core::fmt::UpperHex for usize and similar code might panic. It seems that it is because the GenericRadix::fmt_int method is using slice indexing notation here:

    let buf = &buf[curr..];

This seem to be easily avoidable by replacing it with buf.get(curr..) and some error handling.

curr is initialized to buf.len(), and is only ever decremented, so as long as the decrements do not overflow, &buf[curr..] cannot panic.

buf has length 128, and curr is decremented exactly once for each digit, so for curr to overflow would require that the number to be formatted have more that 128 digits in the particular base (2, 8, or 16), which is currently not possible since u128 is the largest fixed-width integer type, and usize::BITS <= 64 on all currently supported platforms.

I suppose a const _: () = assert!(T::BITS <= 128) or similar could be added to this code to ensure that any future platform with usize::BITS > 128 would fail to compile unless this code was updated to account for it, (but I imagine there are several other places in the standard library which makes a similar assumption).

curr is initialized to buf.len(), and is only ever decremented, so as long as the decrements do not overflow, &buf[curr..] cannot panic.

It's OK that it won't panic in practice, but the compiler is unable to prove it, so it is generating the panic code and linking with panic-related functionality which is desirable to avoid. That is, using crates such as no_panics_whatsoever or similar functionality by other means won't work.

@rustbot

@rustbot label -needs-triage +T-libs +A-panic +C-enhancement

Replace checked slice indexing by unchecked to support panic-free code

Based on minimizing the code to:

pub fn fmt_int(buf: &mut [u8; 128], mut x: u128) -> &[u8] {
    let mut curr = buf.len();
    for byte in buf.iter_mut().rev() {
        if *byte == 0 { break; }
        curr -= 1;
    }
    &buf[curr..]
}

it seems like LLVM should be removing this panic: https://alive2.llvm.org/ce/z/UJHdEr

Based on minimizing the code to:

pub fn fmt_int(buf: &mut [u8; 128], mut x: u128) -> &[u8] {
    let mut curr = buf.len();
    for byte in buf.iter_mut().rev() {
        if *byte == 0 { break; }
        curr -= 1;
    }
    &buf[curr..]
}

it seems like LLVM should be removing this panic: https://alive2.llvm.org/ce/z/UJHdEr

/might/ be related to #127553

@zachs18

Based on minimizing the code to:

pub fn fmt_int(buf: &mut [u8; 128], mut x: u128) -> &[u8] {
let mut curr = buf.len();
for byte in buf.iter_mut().rev() {
if *byte == 0 { break; }
curr -= 1;
}
&buf[curr..]
}
it seems like LLVM should be removing this panic: https://alive2.llvm.org/ce/z/UJHdEr

I think in the original code the compiler might not be able to infer that curr does not underflow, as the assumption is that the max T width is 128 bits, but the compiler does not make such an assumption (as @zachs18 mentions above). I wonder if adding the assertion on the width would give it the necessary hint. Need to test it out, as it might be a better solution than replacing by get_unchecked() whatsoever
Upd:
It looks like the trait T is bound to (DisplayInt) does not provide any type width information, so for compile-time checks more changes will be required.

Replace checked slice indexing by unchecked to support panic-free code

Rollup merge of rust-lang#132473 - ZhekaS:core_fmt_radix_no_panic, r=…

Unrolled build for rust-lang#132473

Navigation Menu

UpperHex formatting might panic #126425

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

UpperHex formatting might panic #126425

Description

Activity

zachs18 commented on Jun 13, 2024

ZhekaS commented on Jun 13, 2024

veera-sivarajan commented on Jun 14, 2024

clubby789 commented on Nov 3, 2024

ericlagergren commented on Nov 3, 2024

ZhekaS commented on Nov 4, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions