Skip to content

Commit

Permalink
Rollup merge of rust-lang#78122 - fusion-engineering-forks:fmt-write-…
Browse files Browse the repository at this point in the history
…bounds-check, r=dtolnay

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
```
  • Loading branch information
Dylan-DPC authored Oct 22, 2020
2 parents 4b223d7 + 484d9eb commit 4816d30
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 7 deletions.
28 changes: 21 additions & 7 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,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;
}
}
Expand All @@ -1096,25 +1098,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() }
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/test/run-make-fulldeps/fmt-write-bloat/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-include ../tools.mk

NM=nm

all: main.rs
$(RUSTC) $< -O
$(NM) $(call RUN_BINFILE,main) | $(CGREP) -v panicking panic_fmt panic_bounds_check pad_integral Display Debug
32 changes: 32 additions & 0 deletions src/test/run-make-fulldeps/fmt-write-bloat/main.rs
Original file line number Diff line number Diff line change
@@ -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 {}
}

0 comments on commit 4816d30

Please sign in to comment.