From d80f127a75017dcdc91f4535b26a668976e2cfc7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 23:58:42 +0200 Subject: [PATCH 1/3] 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. --- library/core/src/fmt/mod.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 0963c6d6cd7..04edf4611ec 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -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; } } @@ -1096,25 +1098,36 @@ 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]; + + // 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 { +unsafe fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option { 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) => { + // SAFETY: cnt and args come from the same Arguments, + // which guarantees this index is always within bounds. + unsafe { args.get_unchecked(i).as_usize() } + } } } From ea24395617745dd0483a7c4114da04b114a7d3d8 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 20 Oct 2020 20:20:06 +0200 Subject: [PATCH 2/3] Add debug_asserts for the unsafe indexing in fmt::write. --- library/core/src/fmt/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 04edf4611ec..cc84bf14a33 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -1110,7 +1110,7 @@ unsafe fn run(fmt: &mut Formatter<'_>, arg: &rt::v1::Argument, args: &[ArgumentV } // Extract the correct argument - + 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) }; @@ -1124,6 +1124,7 @@ unsafe fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option Some(n), rt::v1::Count::Implied => None, 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() } From 1da5780303b5df0505f20d7653778454225bb3cc Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 20 Oct 2020 20:20:31 +0200 Subject: [PATCH 3/3] Add test to check for fmt::write bloat. It checks that fmt::write by itself doesn't pull in any panicking or or display code. --- src/test/run-make/fmt-write-bloat/Makefile | 25 +++++++++++++++++ src/test/run-make/fmt-write-bloat/main.rs | 32 ++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 src/test/run-make/fmt-write-bloat/Makefile create mode 100644 src/test/run-make/fmt-write-bloat/main.rs 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 {} +}