diff options
| author | bors <bors@rust-lang.org> | 2020-11-29 23:14:40 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2020-11-29 23:14:40 +0000 |
| commit | cf9bfdb8726c07974c8db93f70ee7213ee20c563 (patch) | |
| tree | 899569465dd46c8710ec951d9718a626bbd9188a | |
| parent | 349b3b324dade7ca638091db93ba08bbc443c63d (diff) | |
| parent | 1da5780303b5df0505f20d7653778454225bb3cc (diff) | |
| download | rust-cf9bfdb8726c07974c8db93f70ee7213ee20c563.tar.gz rust-cf9bfdb8726c07974c8db93f70ee7213ee20c563.zip | |
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
```
| -rw-r--r-- | library/core/src/fmt/mod.rs | 28 | ||||
| -rw-r--r-- | src/test/run-make/fmt-write-bloat/Makefile | 25 | ||||
| -rw-r--r-- | src/test/run-make/fmt-write-bloat/main.rs | 32 |
3 files changed, 78 insertions, 7 deletions
diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 23e8d1d856a..4ed62a620c4 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -1084,7 +1084,9 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { // a string piece. for (arg, piece) in fmt.iter().zip(args.pieces.iter()) { formatter.buf.write_str(*piece)?; - run(&mut formatter, arg, &args.args)?; + // SAFETY: arg and args.args come from the same Arguments, + // which guarantees the indexes are always within bounds. + unsafe { run(&mut formatter, arg, &args.args) }?; idx += 1; } } @@ -1098,25 +1100,37 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { Ok(()) } -fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument, args: &[ArgumentV1<'_>]) -> Result { +unsafe fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument, args: &[ArgumentV1<'_>]) -> Result { fmt.fill = arg.format.fill; fmt.align = arg.format.align; fmt.flags = arg.format.flags; - fmt.width = getcount(args, &arg.format.width); - fmt.precision = getcount(args, &arg.format.precision); + // SAFETY: arg and args come from the same Arguments, + // which guarantees the indexes are always within bounds. + unsafe { + fmt.width = getcount(args, &arg.format.width); + fmt.precision = getcount(args, &arg.format.precision); + } // Extract the correct argument - let value = args[arg.position]; + debug_assert!(arg.position < args.len()); + // SAFETY: arg and args come from the same Arguments, + // which guarantees its index is always within bounds. + let value = unsafe { args.get_unchecked(arg.position) }; // Then actually do some printing (value.formatter)(value.value, fmt) } -fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option<usize> { +unsafe fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option<usize> { match *cnt { rt::v1::Count::Is(n) => Some(n), rt::v1::Count::Implied => None, - rt::v1::Count::Param(i) => args[i].as_usize(), + rt::v1::Count::Param(i) => { + debug_assert!(i < args.len()); + // SAFETY: cnt and args come from the same Arguments, + // which guarantees this index is always within bounds. + unsafe { args.get_unchecked(i).as_usize() } + } } } diff --git a/src/test/run-make/fmt-write-bloat/Makefile b/src/test/run-make/fmt-write-bloat/Makefile new file mode 100644 index 00000000000..26e08086a72 --- /dev/null +++ b/src/test/run-make/fmt-write-bloat/Makefile @@ -0,0 +1,25 @@ +-include ../../run-make-fulldeps/tools.mk + +# ignore-windows + +ifeq ($(shell $(RUSTC) -vV | grep 'host: $(TARGET)'),) + +# Don't run this test when cross compiling. +all: + +else + +NM = nm + +PANIC_SYMS = panic_bounds_check pad_integral Display Debug + +# Allow for debug_assert!() in debug builds of std. +ifdef NO_DEBUG_ASSERTIONS +PANIC_SYMS += panicking panic_fmt +endif + +all: main.rs + $(RUSTC) $< -O + $(NM) $(call RUN_BINFILE,main) | $(CGREP) -v $(PANIC_SYMS) + +endif diff --git a/src/test/run-make/fmt-write-bloat/main.rs b/src/test/run-make/fmt-write-bloat/main.rs new file mode 100644 index 00000000000..e86c48014c3 --- /dev/null +++ b/src/test/run-make/fmt-write-bloat/main.rs @@ -0,0 +1,32 @@ +#![feature(lang_items)] +#![feature(start)] +#![no_std] + +use core::fmt; +use core::fmt::Write; + +#[link(name = "c")] +extern "C" {} + +struct Dummy; + +impl fmt::Write for Dummy { + #[inline(never)] + fn write_str(&mut self, _: &str) -> fmt::Result { + Ok(()) + } +} + +#[start] +fn main(_: isize, _: *const *const u8) -> isize { + let _ = writeln!(Dummy, "Hello World"); + 0 +} + +#[lang = "eh_personality"] +fn eh_personality() {} + +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { + loop {} +} |
