diff options
| author | bors <bors@rust-lang.org> | 2024-08-02 02:28:59 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-08-02 02:28:59 +0000 |
| commit | 05e692ae025fd4650c601e1e7ab51bdc5e19c35b (patch) | |
| tree | ffcd670bf96eef8e933f18d5566d4f28eafdfd00 | |
| parent | 425ae69588182ae140bc3392bab53391f72d91a9 (diff) | |
| parent | ebd6718218642b8b63584214e35f7d69d24ae845 (diff) | |
| download | rust-05e692ae025fd4650c601e1e7ab51bdc5e19c35b.tar.gz rust-05e692ae025fd4650c601e1e7ab51bdc5e19c35b.zip | |
Auto merge of #128147 - lolbinarycat:fmt-write-bloat-rmake, r=jieyouxu
migrate fmt-write-bloat to rmake try-job: aarch64-apple try-job: x86_64-gnu-llvm-18 try-job: dist-x86_64-linux
| -rw-r--r-- | src/tools/run-make-support/src/env.rs | 7 | ||||
| -rw-r--r-- | src/tools/run-make-support/src/lib.rs | 1 | ||||
| -rw-r--r-- | src/tools/run-make-support/src/symbols.rs | 44 | ||||
| -rw-r--r-- | src/tools/tidy/src/allowed_run_make_makefiles.txt | 1 | ||||
| -rw-r--r-- | tests/run-make/fmt-write-bloat/Makefile | 25 | ||||
| -rw-r--r-- | tests/run-make/fmt-write-bloat/rmake.rs | 37 |
6 files changed, 89 insertions, 26 deletions
diff --git a/src/tools/run-make-support/src/env.rs b/src/tools/run-make-support/src/env.rs index f52524e7d54..e6460fb93d3 100644 --- a/src/tools/run-make-support/src/env.rs +++ b/src/tools/run-make-support/src/env.rs @@ -17,3 +17,10 @@ pub fn env_var_os(name: &str) -> OsString { None => panic!("failed to retrieve environment variable {name:?}"), } } + +/// Check if `NO_DEBUG_ASSERTIONS` is set (usually this may be set in CI jobs). +#[track_caller] +#[must_use] +pub fn no_debug_assertions() -> bool { + std::env::var_os("NO_DEBUG_ASSERTIONS").is_some() +} diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index f28f2a120a4..9ccc37b25d7 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -21,6 +21,7 @@ pub mod run; pub mod scoped_run; pub mod string; pub mod targets; +pub mod symbols; // Internally we call our fs-related support module as `fs`, but re-export its content as `rfs` // to tests to avoid colliding with commonly used `use std::fs;`. diff --git a/src/tools/run-make-support/src/symbols.rs b/src/tools/run-make-support/src/symbols.rs new file mode 100644 index 00000000000..fd0c866bcc9 --- /dev/null +++ b/src/tools/run-make-support/src/symbols.rs @@ -0,0 +1,44 @@ +use std::path::Path; + +use object::{self, Object, ObjectSymbol, SymbolIterator}; + +/// Iterate through the symbols in an object file. +/// +/// Uses a callback because `SymbolIterator` does not own its data. +/// +/// Panics if `path` is not a valid object file readable by the current user. +pub fn with_symbol_iter<P, F, R>(path: P, func: F) -> R +where + P: AsRef<Path>, + F: FnOnce(&mut SymbolIterator<'_, '_>) -> R, +{ + let raw_bytes = crate::fs::read(path); + let f = object::File::parse(raw_bytes.as_slice()).expect("unable to parse file"); + let mut iter = f.symbols(); + func(&mut iter) +} + +/// Check an object file's symbols for substrings. +/// +/// Returns `true` if any of the symbols found in the object file at +/// `path` contain a substring listed in `substrings`. +/// +/// Panics if `path` is not a valid object file readable by the current user. +pub fn any_symbol_contains(path: impl AsRef<Path>, substrings: &[&str]) -> bool { + with_symbol_iter(path, |syms| { + for sym in syms { + for substring in substrings { + if sym + .name_bytes() + .unwrap() + .windows(substring.len()) + .any(|x| x == substring.as_bytes()) + { + eprintln!("{:?} contains {}", sym, substring); + return true; + } + } + } + false + }) +} diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index b2b02d3788c..d52ffe6e9f5 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -10,7 +10,6 @@ run-make/dep-info-spaces/Makefile run-make/dep-info/Makefile run-make/emit-to-stdout/Makefile run-make/extern-fn-reachable/Makefile -run-make/fmt-write-bloat/Makefile run-make/foreign-double-unwind/Makefile run-make/foreign-exceptions/Makefile run-make/incr-add-rust-src-component/Makefile diff --git a/tests/run-make/fmt-write-bloat/Makefile b/tests/run-make/fmt-write-bloat/Makefile deleted file mode 100644 index 70e04b9af51..00000000000 --- a/tests/run-make/fmt-write-bloat/Makefile +++ /dev/null @@ -1,25 +0,0 @@ -include ../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 Debug - -# Allow for debug_assert!() in debug builds of std. -ifdef NO_DEBUG_ASSERTIONS -PANIC_SYMS += panicking panic_fmt pad_integral Display Debug -endif - -all: main.rs - $(RUSTC) $< -O - $(NM) $(call RUN_BINFILE,main) | $(CGREP) -v $(PANIC_SYMS) - -endif diff --git a/tests/run-make/fmt-write-bloat/rmake.rs b/tests/run-make/fmt-write-bloat/rmake.rs new file mode 100644 index 00000000000..4ae226ec0e2 --- /dev/null +++ b/tests/run-make/fmt-write-bloat/rmake.rs @@ -0,0 +1,37 @@ +//! 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. + +// Reason: This test is `ignore-windows` because the `no_std` test (using `#[link(name = "c")])` +// doesn't link on windows. +//@ ignore-windows +//@ ignore-cross-compile + +use run_make_support::env::no_debug_assertions; +use run_make_support::rustc; +use run_make_support::symbols::any_symbol_contains; + +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 no_debug_assertions() { + // 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!(!any_symbol_contains("main", &panic_syms)); +} |
