Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate fmt-write-bloat to rmake #128147

Merged
merged 2 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/tools/run-make-support/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
1 change: 1 addition & 0 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;`.
Expand Down
43 changes: 43 additions & 0 deletions src/tools/run-make-support/src/symbols.rs
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use object::{self, Object, ObjectSymbol, SymbolIterator};
use std::path::Path;

/// 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 {
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
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
})
}
1 change: 0 additions & 1 deletion src/tools/tidy/src/allowed_run_make_makefiles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions tests/run-make/fmt-write-bloat/Makefile

This file was deleted.

35 changes: 35 additions & 0 deletions tests/run-make/fmt-write-bloat/rmake.rs
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//! 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
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
//@ ignore-cross-compile

use run_make_support::{env::no_debug_assertions, rustc, 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));
}
Loading