Skip to content

Commit cf9bfdb

Browse files
committed
Auto merge of #78122 - fusion-engineering-forks:fmt-write-bounds-check, r=Mark-Simulacrum
Avoid panic_bounds_check in fmt::write. Writing any fmt::Arguments would trigger the inclusion of usize formatting and padding code in the resulting binary, because indexing used in fmt::write would generate code using panic_bounds_check, which prints the index and length. These bounds checks are not necessary, as fmt::Arguments never contains any out-of-bounds indexes. This change replaces them with unsafe get_unchecked, to reduce the amount of generated code, which is especially important for embedded targets. --- Demonstration of the size of and the symbols in a 'hello world' no_std binary: <details> <summary>Source code</summary> ```rust #![feature(lang_items)] #![feature(start)] #![no_std] use core::fmt; use core::fmt::Write; #[link(name = "c")] extern "C" { #[allow(improper_ctypes)] fn write(fd: i32, s: &str) -> isize; fn exit(code: i32) -> !; } struct Stdout; impl fmt::Write for Stdout { fn write_str(&mut self, s: &str) -> fmt::Result { unsafe { write(1, s) }; Ok(()) } } #[start] fn main(_argc: isize, _argv: *const *const u8) -> isize { let _ = writeln!(Stdout, "Hello World"); 0 } #[lang = "eh_personality"] fn eh_personality() {} #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { unsafe { exit(1) }; } ``` </details> Before: ``` text data bss dec hex filename 6059 736 8 6803 1a93 before ``` ``` 0000000000001e00 T <T as core::any::Any>::type_id 0000000000003dd0 D core::fmt::num::DEC_DIGITS_LUT 0000000000001ce0 T core::fmt::num::imp::<impl core::fmt::Display for u64>::fmt 0000000000001ce0 T core::fmt::num::imp::<impl core::fmt::Display for usize>::fmt 0000000000001370 T core::fmt::write 0000000000001b30 t core::fmt::Formatter::pad_integral::write_prefix 0000000000001660 T core::fmt::Formatter::pad_integral 0000000000001350 T core::ops::function::FnOnce::call_once 0000000000001b80 t core::ptr::drop_in_place 0000000000001120 t core::ptr::drop_in_place 0000000000001c50 t core::iter::adapters::zip::Zip<A,B>::new 0000000000001c90 t core::iter::adapters::zip::Zip<A,B>::new 0000000000001b90 T core::panicking::panic_bounds_check 0000000000001c10 T core::panicking::panic_fmt 0000000000001130 t <&mut W as core::fmt::Write>::write_char 0000000000001200 t <&mut W as core::fmt::Write>::write_fmt 0000000000001250 t <&mut W as core::fmt::Write>::write_str ``` After: ``` text data bss dec hex filename 3068 600 8 3676 e5c after ``` ``` 0000000000001360 T core::fmt::write 0000000000001340 T core::ops::function::FnOnce::call_once 0000000000001120 t core::ptr::drop_in_place 0000000000001620 t core::iter::adapters::zip::Zip<A,B>::new 0000000000001660 t core::iter::adapters::zip::Zip<A,B>::new 0000000000001130 t <&mut W as core::fmt::Write>::write_char 0000000000001200 t <&mut W as core::fmt::Write>::write_fmt 0000000000001250 t <&mut W as core::fmt::Write>::write_str ```
2 parents 349b3b3 + 1da5780 commit cf9bfdb

File tree

3 files changed

+78
-7
lines changed

3 files changed

+78
-7
lines changed

library/core/src/fmt/mod.rs

+21-7
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,9 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result {
10841084
// a string piece.
10851085
for (arg, piece) in fmt.iter().zip(args.pieces.iter()) {
10861086
formatter.buf.write_str(*piece)?;
1087-
run(&mut formatter, arg, &args.args)?;
1087+
// SAFETY: arg and args.args come from the same Arguments,
1088+
// which guarantees the indexes are always within bounds.
1089+
unsafe { run(&mut formatter, arg, &args.args) }?;
10881090
idx += 1;
10891091
}
10901092
}
@@ -1098,25 +1100,37 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result {
10981100
Ok(())
10991101
}
11001102

1101-
fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument, args: &[ArgumentV1<'_>]) -> Result {
1103+
unsafe fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument, args: &[ArgumentV1<'_>]) -> Result {
11021104
fmt.fill = arg.format.fill;
11031105
fmt.align = arg.format.align;
11041106
fmt.flags = arg.format.flags;
1105-
fmt.width = getcount(args, &arg.format.width);
1106-
fmt.precision = getcount(args, &arg.format.precision);
1107+
// SAFETY: arg and args come from the same Arguments,
1108+
// which guarantees the indexes are always within bounds.
1109+
unsafe {
1110+
fmt.width = getcount(args, &arg.format.width);
1111+
fmt.precision = getcount(args, &arg.format.precision);
1112+
}
11071113

11081114
// Extract the correct argument
1109-
let value = args[arg.position];
1115+
debug_assert!(arg.position < args.len());
1116+
// SAFETY: arg and args come from the same Arguments,
1117+
// which guarantees its index is always within bounds.
1118+
let value = unsafe { args.get_unchecked(arg.position) };
11101119

11111120
// Then actually do some printing
11121121
(value.formatter)(value.value, fmt)
11131122
}
11141123

1115-
fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option<usize> {
1124+
unsafe fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option<usize> {
11161125
match *cnt {
11171126
rt::v1::Count::Is(n) => Some(n),
11181127
rt::v1::Count::Implied => None,
1119-
rt::v1::Count::Param(i) => args[i].as_usize(),
1128+
rt::v1::Count::Param(i) => {
1129+
debug_assert!(i < args.len());
1130+
// SAFETY: cnt and args come from the same Arguments,
1131+
// which guarantees this index is always within bounds.
1132+
unsafe { args.get_unchecked(i).as_usize() }
1133+
}
11201134
}
11211135
}
11221136

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
-include ../../run-make-fulldeps/tools.mk
2+
3+
# ignore-windows
4+
5+
ifeq ($(shell $(RUSTC) -vV | grep 'host: $(TARGET)'),)
6+
7+
# Don't run this test when cross compiling.
8+
all:
9+
10+
else
11+
12+
NM = nm
13+
14+
PANIC_SYMS = panic_bounds_check pad_integral Display Debug
15+
16+
# Allow for debug_assert!() in debug builds of std.
17+
ifdef NO_DEBUG_ASSERTIONS
18+
PANIC_SYMS += panicking panic_fmt
19+
endif
20+
21+
all: main.rs
22+
$(RUSTC) $< -O
23+
$(NM) $(call RUN_BINFILE,main) | $(CGREP) -v $(PANIC_SYMS)
24+
25+
endif
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![feature(lang_items)]
2+
#![feature(start)]
3+
#![no_std]
4+
5+
use core::fmt;
6+
use core::fmt::Write;
7+
8+
#[link(name = "c")]
9+
extern "C" {}
10+
11+
struct Dummy;
12+
13+
impl fmt::Write for Dummy {
14+
#[inline(never)]
15+
fn write_str(&mut self, _: &str) -> fmt::Result {
16+
Ok(())
17+
}
18+
}
19+
20+
#[start]
21+
fn main(_: isize, _: *const *const u8) -> isize {
22+
let _ = writeln!(Dummy, "Hello World");
23+
0
24+
}
25+
26+
#[lang = "eh_personality"]
27+
fn eh_personality() {}
28+
29+
#[panic_handler]
30+
fn panic(_: &core::panic::PanicInfo) -> ! {
31+
loop {}
32+
}

0 commit comments

Comments
 (0)