Skip to content

Commit

Permalink
Auto merge of rust-lang#135768 - jieyouxu:migrate-symbol-mangling-has…
Browse files Browse the repository at this point in the history
…hed, r=<try>

tests: Port `symbol-mangling-hashed` to rmake.rs

Part of rust-lang#121876.

This PR supersedes rust-lang#128567 and is co-authored with `@lolbinarycat.`

### Summary

This PR ports `tests/run-make/symbol-mangling-hashed` to rmake.rs. Notable differences when compared to the Makefile version includes:

- It's no longer limited to linux + x86_64 only. In particular, this now is exercised on darwin and windows (esp. msvc) too.
- The test uses `object` crate to be more precise in the filtering, and avoids relying on parsing the human-readable `nm` output for *some* `nm` in the given environment (which isn't really a thing on msvc anyway, and `llvm-nm` doesn't handle msvc dylibs AFAICT).
- Dump the symbols satisfying various criteria on test failure to make it hopefully less of a pain to debug if it ever fails in CI.

### Review advice

- Best reviewed commit-by-commit.
- I'm not *super* sure about the msvc logic, would benefit from a MSVC (PE/COFF) expert taking a look.

---

try-job: x86_64-msvc
try-job: i686-msvc-1
try-job: i686-mingw
try-job: x86_64-mingw-1
try-job: x86_64-apple-1
try-job: aarch64-apple
try-job: test-various
  • Loading branch information
bors committed Jan 20, 2025
2 parents b5741a3 + 897a145 commit 06b5854
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 57 deletions.
12 changes: 12 additions & 0 deletions src/tools/run-make-support/src/external_deps/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,18 @@ impl Rustc {
self
}

/// Specify option of `-C symbol-mangling-version`.
pub fn symbol_mangling_version(&mut self, option: &str) -> &mut Self {
self.cmd.arg(format!("-Csymbol-mangling-version={option}"));
self
}

/// Specify `-C prefer-dynamic`.
pub fn prefer_dynamic(&mut self) -> &mut Self {
self.cmd.arg(format!("-Cprefer-dynamic"));
self
}

/// Specify error format to use
pub fn error_format(&mut self, format: &str) -> &mut Self {
self.cmd.arg(format!("--error-format={format}"));
Expand Down
10 changes: 7 additions & 3 deletions src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ pub use wasmparser;
// tidy-alphabetical-end

// Re-exports of external dependencies.
pub use external_deps::{c_build, c_cxx_compiler, clang, htmldocck, llvm, python, rustc, rustdoc};
pub use external_deps::{
cargo, c_build, c_cxx_compiler, clang, htmldocck, llvm, python, rustc, rustdoc
};

// These rely on external dependencies.
pub use c_cxx_compiler::{Cc, Gcc, cc, cxx, extra_c_flags, extra_cxx_flags, gcc};
Expand Down Expand Up @@ -79,7 +81,10 @@ pub use env::{env_var, env_var_os, set_current_dir};
pub use run::{cmd, run, run_fail, run_with_args};

/// Helpers for checking target information.
pub use targets::{is_aix, is_darwin, is_msvc, is_windows, llvm_components_contain, target, uname, apple_os};
pub use targets::{
apple_os, is_aix, is_darwin, is_msvc, is_windows, is_windows_gnu, llvm_components_contain,
target, uname,
};

/// Helpers for building names of output artifacts that are potentially target-specific.
pub use artifact_names::{
Expand All @@ -104,4 +109,3 @@ pub use assertion_helpers::{
pub use string::{
count_regex_matches_in_files_with_extension, invalid_utf8_contains, invalid_utf8_not_contains,
};
use crate::external_deps::cargo;
25 changes: 20 additions & 5 deletions src/tools/run-make-support/src/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,32 @@ 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.
/// Given an [`object::File`], find the exported dynamic symbol names via
/// [`object::Object::exports`]. This does **not** impose any filters on the specific dynamic
/// symbols, e.g. if they are global or local, if they are defined or not, and in which section the
/// dynamic symbols reside in.
#[track_caller]
pub fn exported_dynamic_symbol_names<'file>(file: &'file object::File<'file>) -> Vec<&'file str> {
file.exports()
.unwrap()
.into_iter()
.filter_map(|sym| std::str::from_utf8(sym.name()).ok())
.collect()
}

/// Iterate through the symbols in an object file. See [`object::Object::symbols`].
///
/// Panics if `path` is not a valid object file readable by the current user.
#[track_caller]
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 path = path.as_ref();
let blob = crate::fs::read(path);
let f = object::File::parse(&*blob)
.unwrap_or_else(|e| panic!("failed to parse `{}`: {e}", path.display()));
let mut iter = f.symbols();
func(&mut iter)
}
Expand All @@ -24,6 +38,7 @@ where
/// `path` contain a substring listed in `substrings`.
///
/// Panics if `path` is not a valid object file readable by the current user.
#[track_caller]
pub fn any_symbol_contains(path: impl AsRef<Path>, substrings: &[&str]) -> bool {
with_symbol_iter(path, |syms| {
for sym in syms {
Expand Down
6 changes: 6 additions & 0 deletions src/tools/run-make-support/src/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ pub fn is_msvc() -> bool {
target().contains("msvc")
}

/// Check if target is windows-gnu.
#[must_use]
pub fn is_windows_gnu() -> bool {
target().ends_with("windows-gnu")
}

/// Check if target uses macOS.
#[must_use]
pub fn is_darwin() -> bool {
Expand Down
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
@@ -1,5 +1,4 @@
run-make/cat-and-grep-sanity-check/Makefile
run-make/jobserver-error/Makefile
run-make/split-debuginfo/Makefile
run-make/symbol-mangling-hashed/Makefile
run-make/translation/Makefile
48 changes: 0 additions & 48 deletions tests/run-make/symbol-mangling-hashed/Makefile

This file was deleted.

116 changes: 116 additions & 0 deletions tests/run-make/symbol-mangling-hashed/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// ignore-tidy-linelength
//! Basic smoke test for the unstable option `-C symbol_mangling_version=hashed` which aims to
//! replace full symbol mangling names based on hash digests to shorten symbol name lengths in
//! dylibs for space savings.
//!
//! # References
//!
//! - MCP #705: Provide option to shorten symbol names by replacing them with a digest:
//! <https://github.com/rust-lang/compiler-team/issues/705>.
//! - Implementation PR: <https://github.com/rust-lang/rust/pull/118636>.
//! - PE format: <https://learn.microsoft.com/en-us/windows/win32/debug/pe-format>.
//@ ignore-cross-compile

#![deny(warnings)]

use run_make_support::symbols::exported_dynamic_symbol_names;
use run_make_support::{bin_name, cwd, dynamic_lib_name, object, rfs, rustc};

fn main() {
rustc()
.input("a_dylib.rs")
.prefer_dynamic()
.arg("-Zunstable-options")
.symbol_mangling_version("hashed")
.metadata("foo")
.run();

rustc()
.input("a_rlib.rs")
.prefer_dynamic()
.arg("-Zunstable-options")
.symbol_mangling_version("hashed")
.metadata("bar")
.run();

rustc().input("b_dylib.rs").library_search_path(cwd()).prefer_dynamic().run();
rustc().input("b_bin.rs").library_search_path(cwd()).prefer_dynamic().run();

// Check hashed symbol name

{
let dylib_filename = dynamic_lib_name("a_dylib");
println!("checking dylib `{dylib_filename}`");

let dylib_blob = rfs::read(&dylib_filename);
let dylib_file = object::File::parse(&*dylib_blob)
.unwrap_or_else(|e| panic!("failed to parse `{dylib_filename}`: {e}"));

let dynamic_symbols = exported_dynamic_symbol_names(&dylib_file);

if dynamic_symbols.iter().filter(|sym| sym.contains("hello")).count() != 0 {
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
panic!("expected no occurrence of `hello`");
}

if dynamic_symbols.iter().filter(|sym| sym.starts_with("_RNxC7a_dylib")).count() != 2 {
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
panic!("expected two dynamic symbols starting with `_RNxC7a_dylib`");
}
}

{
let so_filename = dynamic_lib_name("b_dylib");
println!("checking so `{so_filename}`");

let so_blob = rfs::read(&so_filename);
let so_file = object::File::parse(&*so_blob)
.unwrap_or_else(|e| panic!("failed to parse `{so_filename}`: {e}"));

let dynamic_symbols = exported_dynamic_symbol_names(&so_file);

if dynamic_symbols
.iter()
.filter(|sym| sym.contains("b_dylib") && sym.contains("hello"))
.count()
!= 1
{
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
panic!("expected one occurrence of mangled `hello`");
}

if dynamic_symbols.iter().filter(|sym| sym.starts_with("_RNxC6a_rlib")).count() != 2 {
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
panic!("expected two exported symbols starting with `_RNxC6a_rlib`");
}

if dynamic_symbols.iter().any(|sym| sym.starts_with("_RNxC7a_dylib")) {
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
panic!("did not expect any symbols starting with `_RNxC7a_dylib`");
}
}

{
let bin_filename = bin_name("b_bin");
println!("checking bin `{bin_filename}`");

let bin_blob = rfs::read(&bin_filename);
let bin_file = object::File::parse(&*bin_blob)
.unwrap_or_else(|e| panic!("failed to parse `{bin_filename}`: {e}"));

let dynamic_symbols = exported_dynamic_symbol_names(&bin_file);

if dynamic_symbols.iter().any(|sym| {
sym.starts_with("_RNxC6a_rlib")
|| sym.starts_with("_RNxC7a_dylib")
|| (sym.contains("b_dylib") && sym.contains("hello"))
}) {
eprintln!("dynamic symbols: {:#?}", dynamic_symbols);
panic!(
"did not expect any symbols to (1) start with `_RNxC6a_rlib` or (2) start with \
`_RNxC7a_dylib` or (3) to be of the form `*b_dylib*hello*`"
);
}
}
}

0 comments on commit 06b5854

Please sign in to comment.