From 76066d7949257a4b7f73b2b67d6d3a9c5c1a8919 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Sun, 2 Nov 2025 15:59:21 +0800 Subject: [PATCH] test: remove `tests/run-make/fmt-write-bloat/` This test suffers from multiple issues that make it very, very difficult to fix, and even if fixed, it would still be too fragile. For some background context, this test tries to check that the optimization introduced in [PR-78122] is not regressed. The optimization is for eliding `usize` formatting machinery and padding code from the final binary. Previously, writing any `fmt::Arguments` would cause the `usize` formatting and padding machinery to be included in the final binary since indexing used in `fmt::write` generates code using `panic_bounds_check` (that prints the index and length). Those bounds check are never hit, since `fmt::Arguments` never contain any out-of-bounds indicies. The `Makefile` version of `fmt-write-bloat` was ported to the present `rmake.rs` test infra in [PR-128147]. However, this PR just tries to maintain the original test logic. The original test, it turns out, already have multiple limitations: - It only runs on non-Windows, since the `no_std` test of the original version tries to link against a `libc`. [PR-128807] worked around this by using a substitute name. We re-enabled this test in [PR-142841], but it turns out the assertions are too weak, it will even vacuously pass for no symbols at all. - In [PR-143669], we tried to make this test more robust by comparing the set of expected versus unexpected panic-related symbols, subject to if std was built with debug assertions. However, in working on [PR-143669], we've come to realize that this test is fundamentally very fragile: - The set of panic symbols depend on whether the standard library was built with or without debug assertions. - Different platforms often have different sets of panic machinery modules, functions and paths, and thus different sets of panic symbols. For instance, x86_64 msvc and i686 msvc have different panic codepaths. - This comes back to the way the test is trying to gauge the absence of panic symbols -- it tries to look for symbol substring matches for "known" panic symbols. This is fundamentally fragile, because the test is trying to peek into the symbols of the resultant binary post-linking, based on fuzzy matches (the symbols are mangled as well). Based on this assessment, we determined that we should remove this test. This is not intended to exclude the possibility of reintroducing a more robust version of this test. For instance, we could consider some kind of more controllable post-link "end product" integration codegen test suite. [PR-78122]: https://github.com/rust-lang/rust/pull/78122 [PR-128147]: https://github.com/rust-lang/rust/pull/128147 [PR-128807]: https://github.com/rust-lang/rust/pull/128807 [PR-142841]: https://github.com/rust-lang/rust/pull/142841 [PR-143669]: https://github.com/rust-lang/rust/pull/143669 --- tests/run-make/fmt-write-bloat/main.rs | 32 ---------------------- tests/run-make/fmt-write-bloat/rmake.rs | 35 ------------------------- 2 files changed, 67 deletions(-) delete mode 100644 tests/run-make/fmt-write-bloat/main.rs delete mode 100644 tests/run-make/fmt-write-bloat/rmake.rs diff --git a/tests/run-make/fmt-write-bloat/main.rs b/tests/run-make/fmt-write-bloat/main.rs deleted file mode 100644 index b50461c0a0279..0000000000000 --- a/tests/run-make/fmt-write-bloat/main.rs +++ /dev/null @@ -1,32 +0,0 @@ -#![feature(lang_items)] -#![no_main] -#![no_std] - -use core::fmt; -use core::fmt::Write; - -#[cfg_attr(not(windows), link(name = "c"))] -extern "C" {} - -struct Dummy; - -impl fmt::Write for Dummy { - #[inline(never)] - fn write_str(&mut self, _: &str) -> fmt::Result { - Ok(()) - } -} - -#[no_mangle] -extern "C" fn main(_argc: core::ffi::c_int, _argv: *const *const u8) -> core::ffi::c_int { - let _ = writeln!(Dummy, "Hello World"); - 0 -} - -#[lang = "eh_personality"] -fn eh_personality() {} - -#[panic_handler] -fn panic(_: &core::panic::PanicInfo) -> ! { - loop {} -} diff --git a/tests/run-make/fmt-write-bloat/rmake.rs b/tests/run-make/fmt-write-bloat/rmake.rs deleted file mode 100644 index b7f18b384cb00..0000000000000 --- a/tests/run-make/fmt-write-bloat/rmake.rs +++ /dev/null @@ -1,35 +0,0 @@ -//! Before #78122, 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. The test is a `run-make` test, because it needs to check the result after linking. A -//! codegen or assembly test doesn't check the parts that will be pulled in from `core` by the -//! linker. -//! -//! In this test, we try to check that the `usize` formatting and padding code are not present in -//! the final binary by checking that panic symbols such as `panic_bounds_check` are **not** -//! present. -//! -//! Some CI jobs try to run faster by disabling debug assertions (through setting -//! `NO_DEBUG_ASSERTIONS=1`). If debug assertions are disabled, then we can check for the absence of -//! additional `usize` formatting and padding related symbols. - -//@ ignore-cross-compile - -use run_make_support::artifact_names::bin_name; -use run_make_support::env::std_debug_assertions_enabled; -use run_make_support::rustc; -use run_make_support::symbols::object_contains_any_symbol_substring; - -fn main() { - rustc().input("main.rs").opt().run(); - // panic machinery identifiers, these should not appear in the final binary - let mut panic_syms = vec!["panic_bounds_check", "Debug"]; - if std_debug_assertions_enabled() { - // if debug assertions are allowed, we need to allow these, - // otherwise, add them to the list of symbols to deny. - panic_syms.extend_from_slice(&["panicking", "panic_fmt", "pad_integral", "Display"]); - } - assert!(!object_contains_any_symbol_substring(bin_name("main"), &panic_syms)); -}