Skip to content

Commit 18e150a

Browse files
committed
Use unchecked indexing in integer _fmt_inner to keep bounds checks elided
The `_fmt_inner` helper in `core::fmt::num` wrote into its `MaybeUninit<u8>` buffer through regular `[]` indexing and then relied on surrounding `core::hint::assert_unchecked` hints to convince LLVM to elide the resulting bounds checks. Under `-Copt-level=z` with fat LTO (and other configurations where the range information fails to propagate) the hints were dropped and the optimizer left a `panic_bounds_check` call on the hot integer-formatting path. Switch the per-pair buffer writes to `get_unchecked_mut` / `get_unchecked` on both `buf` and `DECIMAL_PAIRS`, with SAFETY comments that reestablish the invariants from the preceding `assert_unchecked` hints. The existing `assert_unchecked`s are retained because they still document the `offset`/`buf.len()` invariants and help downstream codegen. Also add a codegen-llvm regression test that checks the `Display` path for `usize` and `u64` does not contain any `panic_bounds_check`, with a paired sanity check to make sure the symbol itself is still emitted by the compiler for a real out-of-bounds index.
1 parent 12f35ad commit 18e150a

2 files changed

Lines changed: 73 additions & 9 deletions

File tree

library/core/src/fmt/num.rs

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,18 @@ macro_rules! impl_Display {
210210
remain /= scale;
211211
let pair1 = (quad / 100) as usize;
212212
let pair2 = (quad % 100) as usize;
213-
buf[offset + 0].write(DECIMAL_PAIRS[pair1 * 2 + 0]);
214-
buf[offset + 1].write(DECIMAL_PAIRS[pair1 * 2 + 1]);
215-
buf[offset + 2].write(DECIMAL_PAIRS[pair2 * 2 + 0]);
216-
buf[offset + 3].write(DECIMAL_PAIRS[pair2 * 2 + 1]);
213+
// SAFETY: `offset + 4 <= buf.len()` by the asserts above, and
214+
// `pair1, pair2 < 100` so every `DECIMAL_PAIRS` index is `< 200`
215+
// which is the exact length of `DECIMAL_PAIRS`. Using unchecked
216+
// indexing here avoids relying on LLVM to elide the bounds
217+
// checks, which can regress under `opt-level=z` + fat LTO
218+
// (see https://github.com/rust-lang/rust/issues/152061).
219+
unsafe {
220+
buf.get_unchecked_mut(offset).write(*DECIMAL_PAIRS.get_unchecked(pair1 * 2));
221+
buf.get_unchecked_mut(offset + 1).write(*DECIMAL_PAIRS.get_unchecked(pair1 * 2 + 1));
222+
buf.get_unchecked_mut(offset + 2).write(*DECIMAL_PAIRS.get_unchecked(pair2 * 2));
223+
buf.get_unchecked_mut(offset + 3).write(*DECIMAL_PAIRS.get_unchecked(pair2 * 2 + 1));
224+
}
217225
}
218226

219227
// Format per two digits from the lookup table.
@@ -228,8 +236,14 @@ macro_rules! impl_Display {
228236

229237
let pair = (remain % 100) as usize;
230238
remain /= 100;
231-
buf[offset + 0].write(DECIMAL_PAIRS[pair * 2 + 0]);
232-
buf[offset + 1].write(DECIMAL_PAIRS[pair * 2 + 1]);
239+
// SAFETY: `offset + 2 <= buf.len()` by the asserts above, and
240+
// `pair < 100` so the `DECIMAL_PAIRS` indices are `< 200`.
241+
// See the comment in the outer loop for why this uses unchecked
242+
// indexing instead of regular `[]` (rust-lang/rust#152061).
243+
unsafe {
244+
buf.get_unchecked_mut(offset).write(*DECIMAL_PAIRS.get_unchecked(pair * 2));
245+
buf.get_unchecked_mut(offset + 1).write(*DECIMAL_PAIRS.get_unchecked(pair * 2 + 1));
246+
}
233247
}
234248

235249
// Format the last remaining digit, if any.
@@ -242,10 +256,14 @@ macro_rules! impl_Display {
242256
unsafe { core::hint::assert_unchecked(offset <= buf.len()) }
243257
offset -= 1;
244258

245-
// Either the compiler sees that remain < 10, or it prevents
246-
// a boundary check up next.
247259
let last = (remain & 15) as usize;
248-
buf[offset].write(DECIMAL_PAIRS[last * 2 + 1]);
260+
// SAFETY: `offset < buf.len()` by the asserts above, and
261+
// `last < 16` so `last * 2 + 1 < 32 < 200 == DECIMAL_PAIRS.len()`.
262+
// See the comment in the outer loop for why this uses unchecked
263+
// indexing instead of regular `[]` (rust-lang/rust#152061).
264+
unsafe {
265+
buf.get_unchecked_mut(offset).write(*DECIMAL_PAIRS.get_unchecked(last * 2 + 1));
266+
}
249267
// not used: remain = 0;
250268
}
251269

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
//@ compile-flags: -Copt-level=3
2+
// Regression test for https://github.com/rust-lang/rust/issues/152061.
3+
//
4+
// `impl fmt::Display` for integers lowered through `_fmt_inner` in
5+
// `library/core/src/fmt/num.rs` used to leave a `panic_bounds_check`
6+
// path in optimized LLVM IR when LLVM failed to propagate the
7+
// `assume`-based range information across LTO / `opt-level=z`
8+
// boundaries. The implementation was rewritten to use
9+
// `get_unchecked{_mut}` for the buffer writes, so the `panic_bounds_check`
10+
// path must not appear regardless of the optimizer's propagation.
11+
12+
#![crate_type = "lib"]
13+
14+
use std::fmt::Write;
15+
16+
pub struct NoopWriter;
17+
impl Write for NoopWriter {
18+
fn write_str(&mut self, _s: &str) -> std::fmt::Result {
19+
Ok(())
20+
}
21+
}
22+
23+
// CHECK-LABEL: @format_usize
24+
#[no_mangle]
25+
pub fn format_usize(w: &mut NoopWriter, x: usize) {
26+
// The Display path through `_fmt_inner` must not emit a bounds check.
27+
// CHECK-NOT: panic_bounds_check
28+
let _ = write!(w, "{}", x);
29+
}
30+
31+
// CHECK-LABEL: @format_u64
32+
#[no_mangle]
33+
pub fn format_u64(w: &mut NoopWriter, x: u64) {
34+
// CHECK-NOT: panic_bounds_check
35+
let _ = write!(w, "{}", x);
36+
}
37+
38+
// Sanity check: make sure `panic_bounds_check` is still the symbol LLVM
39+
// emits for a non-elidable out-of-bounds index, so the `CHECK-NOT`s
40+
// above are guarding against something real and cannot pass vacuously.
41+
// CHECK-LABEL: @test_check
42+
#[no_mangle]
43+
pub fn test_check(arr: &[u8], i: usize) -> u8 {
44+
// CHECK: panic_bounds_check
45+
arr[i]
46+
}

0 commit comments

Comments
 (0)