Skip to content

Commit 1395e24

Browse files
committed
Auto merge of #135768 - jieyouxu:migrate-symbol-mangling-hashed, r=<try>
tests: Port `symbol-mangling-hashed` to rmake.rs Part of #121876. This PR supersedes #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-1 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
2 parents 122fb29 + ebdcf01 commit 1395e24

File tree

13 files changed

+178
-81
lines changed

13 files changed

+178
-81
lines changed

Diff for: src/tools/run-make-support/src/external_deps/rustc.rs

+12
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,18 @@ impl Rustc {
215215
self
216216
}
217217

218+
/// Specify option of `-C symbol-mangling-version`.
219+
pub fn symbol_mangling_version(&mut self, option: &str) -> &mut Self {
220+
self.cmd.arg(format!("-Csymbol-mangling-version={option}"));
221+
self
222+
}
223+
224+
/// Specify `-C prefer-dynamic`.
225+
pub fn prefer_dynamic(&mut self) -> &mut Self {
226+
self.cmd.arg(format!("-Cprefer-dynamic"));
227+
self
228+
}
229+
218230
/// Specify error format to use
219231
pub fn error_format(&mut self, format: &str) -> &mut Self {
220232
self.cmd.arg(format!("--error-format={format}"));

Diff for: src/tools/run-make-support/src/lib.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ pub use wasmparser;
4747
// tidy-alphabetical-end
4848

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

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

8183
/// Helpers for checking target information.
82-
pub use targets::{is_aix, is_darwin, is_msvc, is_windows, llvm_components_contain, target, uname, apple_os};
84+
pub use targets::{
85+
apple_os, is_aix, is_darwin, is_msvc, is_windows, is_windows_gnu, llvm_components_contain,
86+
target, uname,
87+
};
8388

8489
/// Helpers for building names of output artifacts that are potentially target-specific.
8590
pub use artifact_names::{
@@ -104,4 +109,3 @@ pub use assertion_helpers::{
104109
pub use string::{
105110
count_regex_matches_in_files_with_extension, invalid_utf8_contains, invalid_utf8_not_contains,
106111
};
107-
use crate::external_deps::cargo;

Diff for: src/tools/run-make-support/src/symbols.rs

+25-9
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,44 @@ use std::path::Path;
22

33
use object::{self, Object, ObjectSymbol, SymbolIterator};
44

5-
/// Iterate through the symbols in an object file.
6-
///
7-
/// Uses a callback because `SymbolIterator` does not own its data.
5+
/// Given an [`object::File`], find the exported dynamic symbol names via
6+
/// [`object::Object::exports`]. This does not distinguish between which section the symbols appear
7+
/// in.
8+
#[track_caller]
9+
pub fn exported_dynamic_symbol_names<'file>(file: &'file object::File<'file>) -> Vec<&'file str> {
10+
file.exports()
11+
.unwrap()
12+
.into_iter()
13+
.filter_map(|sym| std::str::from_utf8(sym.name()).ok())
14+
.collect()
15+
}
16+
17+
/// Iterate through the symbols in an object file. See [`object::Object::symbols`].
818
///
9-
/// Panics if `path` is not a valid object file readable by the current user.
19+
/// Panics if `path` is not a valid object file readable by the current user or if `path` cannot be
20+
/// parsed as a recognized object file.
21+
#[track_caller]
1022
pub fn with_symbol_iter<P, F, R>(path: P, func: F) -> R
1123
where
1224
P: AsRef<Path>,
1325
F: FnOnce(&mut SymbolIterator<'_, '_>) -> R,
1426
{
15-
let raw_bytes = crate::fs::read(path);
16-
let f = object::File::parse(raw_bytes.as_slice()).expect("unable to parse file");
27+
let path = path.as_ref();
28+
let blob = crate::fs::read(path);
29+
let f = object::File::parse(&*blob)
30+
.unwrap_or_else(|e| panic!("failed to parse `{}`: {e}", path.display()));
1731
let mut iter = f.symbols();
1832
func(&mut iter)
1933
}
2034

2135
/// Check an object file's symbols for substrings.
2236
///
23-
/// Returns `true` if any of the symbols found in the object file at
24-
/// `path` contain a substring listed in `substrings`.
37+
/// Returns `true` if any of the symbols found in the object file at `path` contain a substring
38+
/// listed in `substrings`.
2539
///
26-
/// Panics if `path` is not a valid object file readable by the current user.
40+
/// Panics if `path` is not a valid object file readable by the current user or if `path` cannot be
41+
/// parsed as a recognized object file.
42+
#[track_caller]
2743
pub fn any_symbol_contains(path: impl AsRef<Path>, substrings: &[&str]) -> bool {
2844
with_symbol_iter(path, |syms| {
2945
for sym in syms {

Diff for: src/tools/run-make-support/src/targets.rs

+6
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ pub fn is_msvc() -> bool {
2222
target().contains("msvc")
2323
}
2424

25+
/// Check if target is windows-gnu.
26+
#[must_use]
27+
pub fn is_windows_gnu() -> bool {
28+
target().ends_with("windows-gnu")
29+
}
30+
2531
/// Check if target uses macOS.
2632
#[must_use]
2733
pub fn is_darwin() -> bool {

Diff for: src/tools/tidy/src/allowed_run_make_makefiles.txt

-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
11
run-make/split-debuginfo/Makefile
2-
run-make/symbol-mangling-hashed/Makefile
32
run-make/translation/Makefile

Diff for: tests/run-make/symbol-mangling-hashed/Makefile

-48
This file was deleted.

Diff for: tests/run-make/symbol-mangling-hashed/b_bin.rs

-9
This file was deleted.

Diff for: tests/run-make/symbol-mangling-hashed/b_dylib.rs

-9
This file was deleted.

Diff for: tests/run-make/symbol-mangling-hashed/default_bin.rs

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
extern crate default_dylib;
2+
extern crate hashed_dylib;
3+
extern crate hashed_rlib;
4+
5+
fn main() {
6+
hashed_rlib::hrhello();
7+
hashed_dylib::hdhello();
8+
default_dylib::ddhello();
9+
}
+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#![crate_type = "dylib"]
2+
3+
extern crate hashed_dylib;
4+
extern crate hashed_rlib;
5+
6+
pub fn ddhello() {
7+
hashed_rlib::hrhello();
8+
hashed_dylib::hdhello();
9+
}
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
#![crate_type = "dylib"]
2-
pub fn hello() {
2+
pub fn hdhello() {
33
println!("hello dylib");
44
}
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![crate_type = "rlib"]
22

3-
pub fn hello() {
3+
pub fn hrhello() {
44
println!("hello rlib");
55
}

Diff for: tests/run-make/symbol-mangling-hashed/rmake.rs

+108
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// ignore-tidy-linelength
2+
//! Basic smoke test for the unstable option `-C symbol_mangling_version=hashed` which aims to
3+
//! replace full symbol mangling names based on hash digests to shorten symbol name lengths in
4+
//! dylibs for space savings.
5+
//!
6+
//! # References
7+
//!
8+
//! - MCP #705: Provide option to shorten symbol names by replacing them with a digest:
9+
//! <https://github.com/rust-lang/compiler-team/issues/705>.
10+
//! - Implementation PR: <https://github.com/rust-lang/rust/pull/118636>.
11+
//! - PE format: <https://learn.microsoft.com/en-us/windows/win32/debug/pe-format>.
12+
13+
//@ ignore-cross-compile
14+
15+
#![deny(warnings)]
16+
17+
use run_make_support::symbols::exported_dynamic_symbol_names;
18+
use run_make_support::{bin_name, cwd, dynamic_lib_name, is_darwin, object, rfs, run, rustc};
19+
20+
macro_rules! adjust_symbol_prefix {
21+
($name:literal) => {
22+
if is_darwin() { concat!("_", $name) } else { $name }
23+
};
24+
}
25+
26+
fn main() {
27+
rustc()
28+
.input("hashed_dylib.rs")
29+
.prefer_dynamic()
30+
.arg("-Zunstable-options")
31+
.symbol_mangling_version("hashed")
32+
.metadata("foo")
33+
.run();
34+
35+
rustc()
36+
.input("hashed_rlib.rs")
37+
.prefer_dynamic()
38+
.arg("-Zunstable-options")
39+
.symbol_mangling_version("hashed")
40+
.metadata("bar")
41+
.run();
42+
43+
rustc().input("default_dylib.rs").library_search_path(cwd()).prefer_dynamic().run();
44+
rustc().input("default_bin.rs").library_search_path(cwd()).prefer_dynamic().run();
45+
46+
{
47+
// Check hashed symbol name
48+
49+
let dylib_filename = dynamic_lib_name("hashed_dylib");
50+
println!("checking dylib `{dylib_filename}`");
51+
52+
let dylib_blob = rfs::read(&dylib_filename);
53+
let dylib_file = object::File::parse(&*dylib_blob)
54+
.unwrap_or_else(|e| panic!("failed to parse `{dylib_filename}`: {e}"));
55+
56+
let dynamic_symbols = exported_dynamic_symbol_names(&dylib_file);
57+
58+
if dynamic_symbols.iter().filter(|sym| sym.contains("hdhello")).count() != 0 {
59+
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
60+
panic!("expected no occurrence of `hdhello`");
61+
}
62+
63+
let expected_prefix = adjust_symbol_prefix!("_RNxC12hashed_dylib");
64+
if dynamic_symbols.iter().filter(|sym| sym.starts_with(expected_prefix)).count() != 2 {
65+
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
66+
panic!("expected two dynamic symbols starting with `{expected_prefix}`");
67+
}
68+
}
69+
70+
{
71+
let dylib_filename = dynamic_lib_name("default_dylib");
72+
println!("checking so `{dylib_filename}`");
73+
74+
let dylib_blob = rfs::read(&dylib_filename);
75+
let dylib_file = object::File::parse(&*dylib_blob)
76+
.unwrap_or_else(|e| panic!("failed to parse `{dylib_filename}`: {e}"));
77+
78+
let dynamic_symbols = exported_dynamic_symbol_names(&dylib_file);
79+
80+
if dynamic_symbols
81+
.iter()
82+
.filter(|sym| sym.contains("default_dylib") && sym.contains("ddhello"))
83+
.count()
84+
!= 1
85+
{
86+
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
87+
panic!("expected one occurrence of mangled `ddhello`");
88+
}
89+
90+
let expected_rlib_prefix = adjust_symbol_prefix!("_RNxC11hashed_rlib");
91+
if dynamic_symbols.iter().filter(|sym| sym.starts_with(expected_rlib_prefix)).count() != 2 {
92+
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
93+
panic!("expected two exported symbols starting with `{expected_rlib_prefix}`");
94+
}
95+
96+
let expected_dylib_prefix = adjust_symbol_prefix!("_RNxC12hashed_dylib");
97+
if dynamic_symbols.iter().any(|sym| sym.starts_with("_RNxC12hashed_dylib")) {
98+
eprintln!("exported dynamic symbols: {:#?}", dynamic_symbols);
99+
panic!("did not expect any symbols starting with `{expected_dylib_prefix}`");
100+
}
101+
}
102+
103+
// Check that the final binary can be run.
104+
{
105+
let bin_filename = bin_name("default_bin");
106+
run(&bin_filename);
107+
}
108+
}

0 commit comments

Comments
 (0)