From 94da7b157a5ebcde7ee23f4bd2041307e579891f Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 28 Jun 2024 15:04:06 -0400 Subject: [PATCH] rewrite stable-symbol-names to rmake --- .../src/external_deps/llvm.rs | 3 +- src/tools/run-make-support/src/fs.rs | 48 ++++++------- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/reproducible-build-2/rmake.rs | 26 +++---- tests/run-make/stable-symbol-names/Makefile | 41 ----------- tests/run-make/stable-symbol-names/rmake.rs | 68 +++++++++++++++++++ 6 files changed, 105 insertions(+), 82 deletions(-) delete mode 100644 tests/run-make/stable-symbol-names/Makefile create mode 100644 tests/run-make/stable-symbol-names/rmake.rs diff --git a/src/tools/run-make-support/src/external_deps/llvm.rs b/src/tools/run-make-support/src/external_deps/llvm.rs index 259bb615946..bd01275598b 100644 --- a/src/tools/run-make-support/src/external_deps/llvm.rs +++ b/src/tools/run-make-support/src/external_deps/llvm.rs @@ -137,7 +137,8 @@ pub fn program_headers(&mut self) -> &mut Self { self } - /// Pass `--symbols` to display the symbol. + /// Pass `--symbols` to display the symbol table, including both local + /// and global symbols. pub fn symbols(&mut self) -> &mut Self { self.cmd.arg("--symbols"); self diff --git a/src/tools/run-make-support/src/fs.rs b/src/tools/run-make-support/src/fs.rs index b896168b0a5..2c35ba52a62 100644 --- a/src/tools/run-make-support/src/fs.rs +++ b/src/tools/run-make-support/src/fs.rs @@ -9,11 +9,19 @@ pub fn create_symlink, Q: AsRef>(original: P, link: Q) { if link.as_ref().exists() { std::fs::remove_dir(link.as_ref()).unwrap(); } - std::os::windows::fs::symlink_file(original.as_ref(), link.as_ref()).expect(&format!( - "failed to create symlink {:?} for {:?}", - link.as_ref().display(), - original.as_ref().display(), - )); + if original.as_ref().is_file() { + std::os::windows::fs::symlink_file(original.as_ref(), link.as_ref()).expect(&format!( + "failed to create symlink {:?} for {:?}", + link.as_ref().display(), + original.as_ref().display(), + )); + } else { + std::os::windows::fs::symlink_dir(original.as_ref(), link.as_ref()).expect(&format!( + "failed to create symlink {:?} for {:?}", + link.as_ref().display(), + original.as_ref().display(), + )); + } } /// Creates a new symlink to a path on the filesystem, adjusting for Windows or Unix. @@ -41,6 +49,8 @@ fn copy_dir_all_inner(src: impl AsRef, dst: impl AsRef) -> io::Resul let ty = entry.file_type()?; if ty.is_dir() { copy_dir_all_inner(entry.path(), dst.join(entry.file_name()))?; + } else if ty.is_symlink() { + copy_symlink(entry.path(), dst.join(entry.file_name()))?; } else { std::fs::copy(entry.path(), dst.join(entry.file_name()))?; } @@ -59,6 +69,12 @@ fn copy_dir_all_inner(src: impl AsRef, dst: impl AsRef) -> io::Resul } } +fn copy_symlink, Q: AsRef>(from: P, to: Q) -> io::Result<()> { + let target_path = std::fs::read_link(from).unwrap(); + create_symlink(target_path, to); + Ok(()) +} + /// Helper for reading entries in a given directory. pub fn read_dir_entries, F: FnMut(&Path)>(dir: P, mut callback: F) { for entry in read_dir(dir) { @@ -83,28 +99,6 @@ pub fn copy, Q: AsRef>(from: P, to: Q) { )); } -#[track_caller] -/// An extension of [`std::fs::copy`] which can copy a directory recursively. -pub fn copy_dir_all, Q: AsRef>(from: P, to: Q) { - create_dir_all(&to); - for entry in read_dir(from) { - let entry = entry.unwrap(); - let ty = entry.file_type().unwrap(); - if ty.is_dir() { - copy_dir_all(entry.path(), to.as_ref().join(entry.file_name())); - } else if ty.is_symlink() { - copy_symlink(entry.path(), to.as_ref().join(entry.file_name())); - } else { - copy(entry.path(), to.as_ref().join(entry.file_name())); - } - } -} - -fn copy_symlink, Q: AsRef>(from: P, to: Q) { - let target_path = fs::read_link(from).unwrap(); - std::os::unix::fs::symlink(target_path, to).unwrap(); -} - /// A wrapper around [`std::fs::File::create`] which includes the file path in the panic message. #[track_caller] pub fn create_file>(path: P) { diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 13230239796..ea50d05e1da 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -45,7 +45,6 @@ run-make/reproducible-build/Makefile run-make/rlib-format-packed-bundled-libs/Makefile run-make/simd-ffi/Makefile run-make/split-debuginfo/Makefile -run-make/stable-symbol-names/Makefile run-make/staticlib-dylib-linkage/Makefile run-make/symbol-mangling-hashed/Makefile run-make/sysroot-crates-are-unstable/Makefile diff --git a/tests/run-make/reproducible-build-2/rmake.rs b/tests/run-make/reproducible-build-2/rmake.rs index 6286b607605..c500c4238b0 100644 --- a/tests/run-make/reproducible-build-2/rmake.rs +++ b/tests/run-make/reproducible-build-2/rmake.rs @@ -6,17 +6,22 @@ // Outputs should be identical. // See https://github.com/rust-lang/rust/issues/34902 -//FIXME(Oneirical): excluded ignore-musl ignore-windows ignore-cross-compile +//@ ignore-windows +// Reasons: +// 1. The object files are reproducible, but their paths are not, which causes +// the first assertion in the test to fail. +// 2. When the sysroot gets copied, some symlinks must be re-created, +// which is a privileged action on Windows. -use run_make_support::{fs_wrapper, rust_lib_name, rustc}; +use run_make_support::{bin_name, rfs, rust_lib_name, rustc}; fn main() { // test 1: fat lto rustc().input("reproducible-build-aux.rs").run(); - rustc().input("reproducible-build.rs").arg("-Clto=fat").run(); - fs_wrapper::rename("reproducible-build", "reproducible-build-a"); - rustc().input("reproducible-build.rs").arg("-Clto=fat").run(); - assert_eq!(fs_wrapper::read("reproducible-build"), fs_wrapper::read("reproducible-build-a")); + rustc().input("reproducible-build.rs").arg("-Clto=fat").output("reproducible-build").run(); + rfs::rename("reproducible-build", "reproducible-build-a"); + rustc().input("reproducible-build.rs").arg("-Clto=fat").output("reproducible-build").run(); + assert_eq!(rfs::read("reproducible-build"), rfs::read("reproducible-build-a")); // test 2: sysroot let sysroot = rustc().print("sysroot").run().stdout_utf8(); @@ -29,8 +34,8 @@ fn main() { .sysroot(&sysroot) .arg(format!("--remap-path-prefix={sysroot}=/sysroot")) .run(); - fs_wrapper::copy_dir_all(&sysroot, "sysroot"); - fs_wrapper::rename(rust_lib_name("reproducible_build"), rust_lib_name("foo")); + rfs::copy_dir_all(&sysroot, "sysroot"); + rfs::rename(rust_lib_name("reproducible_build"), rust_lib_name("foo")); rustc() .input("reproducible-build.rs") .crate_type("rlib") @@ -38,8 +43,5 @@ fn main() { .arg("--remap-path-prefix=/sysroot=/sysroot") .run(); - assert_eq!( - fs_wrapper::read(rust_lib_name("reproducible_build")), - fs_wrapper::read(rust_lib_name("foo")) - ); + assert_eq!(rfs::read(rust_lib_name("reproducible_build")), rfs::read(rust_lib_name("foo"))); } diff --git a/tests/run-make/stable-symbol-names/Makefile b/tests/run-make/stable-symbol-names/Makefile deleted file mode 100644 index bbfb8e38881..00000000000 --- a/tests/run-make/stable-symbol-names/Makefile +++ /dev/null @@ -1,41 +0,0 @@ -include ../tools.mk - -# The following command will: -# 1. dump the symbols of a library using `nm` -# 2. extract only those lines that we are interested in via `grep` -# 3. from those lines, extract just the symbol name via `sed`, which: -# * always starts with "_ZN" and ends with "E" (`legacy` mangling) -# * always starts with "_R" (`v0` mangling) -# 4. sort those symbol names for deterministic comparison -# 5. write the result into a file - -dump-symbols = nm "$(TMPDIR)/lib$(1).rlib" \ - | grep -E "$(2)" \ - | sed -E "s/.*(_ZN.*E|_R[a-zA-Z0-9_]*).*/\1/" \ - | sort \ - > "$(TMPDIR)/$(1)$(3).nm" - -# This test -# - compiles each of the two crates 2 times and makes sure each time we get -# exactly the same symbol names -# - makes sure that both crates agree on the same symbol names for monomorphic -# functions - -all: - $(RUSTC) stable-symbol-names1.rs - $(call dump-symbols,stable_symbol_names1,generic_|mono_,_v1) - rm $(TMPDIR)/libstable_symbol_names1.rlib - $(RUSTC) stable-symbol-names1.rs - $(call dump-symbols,stable_symbol_names1,generic_|mono_,_v2) - cmp "$(TMPDIR)/stable_symbol_names1_v1.nm" "$(TMPDIR)/stable_symbol_names1_v2.nm" - - $(RUSTC) stable-symbol-names2.rs - $(call dump-symbols,stable_symbol_names2,generic_|mono_,_v1) - rm $(TMPDIR)/libstable_symbol_names2.rlib - $(RUSTC) stable-symbol-names2.rs - $(call dump-symbols,stable_symbol_names2,generic_|mono_,_v2) - cmp "$(TMPDIR)/stable_symbol_names2_v1.nm" "$(TMPDIR)/stable_symbol_names2_v2.nm" - - $(call dump-symbols,stable_symbol_names1,mono_,_cross) - $(call dump-symbols,stable_symbol_names2,mono_,_cross) - cmp "$(TMPDIR)/stable_symbol_names1_cross.nm" "$(TMPDIR)/stable_symbol_names2_cross.nm" diff --git a/tests/run-make/stable-symbol-names/rmake.rs b/tests/run-make/stable-symbol-names/rmake.rs new file mode 100644 index 00000000000..402f411c7f5 --- /dev/null +++ b/tests/run-make/stable-symbol-names/rmake.rs @@ -0,0 +1,68 @@ +// A typo in rustc caused generic symbol names to be non-deterministic - +// that is, it was possible to compile the same file twice with no changes +// and get outputs with different symbol names. +// This test compiles each of the two crates twice, and checks that each output +// contains exactly the same symbol names. +// Additionally, both crates should agree on the same symbol names for monomorphic +// functions. +// See https://github.com/rust-lang/rust/issues/32554 + +use std::collections::HashSet; + +use run_make_support::{llvm_readobj, regex, rfs, rust_lib_name, rustc}; + +static LEGACY_PATTERN: std::sync::OnceLock = std::sync::OnceLock::new(); +static V0_PATTERN: std::sync::OnceLock = std::sync::OnceLock::new(); + +fn main() { + LEGACY_PATTERN.set(regex::Regex::new(r"_ZN.*E").unwrap()).unwrap(); + V0_PATTERN.set(regex::Regex::new(r"_R[a-zA-Z0-9_]*").unwrap()).unwrap(); + // test 1: first file + rustc().input("stable-symbol-names1.rs").run(); + let sym1 = process_symbols("stable_symbol_names1", "generic_|mono_"); + rfs::remove_file(rust_lib_name("stable_symbol_names1")); + rustc().input("stable-symbol-names1.rs").run(); + let sym2 = process_symbols("stable_symbol_names1", "generic_|mono_"); + assert_eq!(sym1, sym2); + + // test 2: second file + rustc().input("stable-symbol-names2.rs").run(); + let sym1 = process_symbols("stable_symbol_names2", "generic_|mono_"); + rfs::remove_file(rust_lib_name("stable_symbol_names2")); + rustc().input("stable-symbol-names2.rs").run(); + let sym2 = process_symbols("stable_symbol_names2", "generic_|mono_"); + assert_eq!(sym1, sym2); + + // test 3: crossed files + let sym1 = process_symbols("stable_symbol_names1", "mono_"); + let sym2 = process_symbols("stable_symbol_names2", "mono_"); + assert_eq!(sym1, sym2); +} + +#[track_caller] +fn process_symbols(path: &str, symbol: &str) -> Vec { + // Dump all symbols. + let out = llvm_readobj().input(rust_lib_name(path)).symbols().run().stdout_utf8(); + // Extract only lines containing `symbol`. + let symbol_regex = regex::Regex::new(symbol).unwrap(); + let out = out.lines().filter(|&line| symbol_regex.find(line).is_some()); + + // HashSet - duplicates should be excluded! + let mut symbols: HashSet = HashSet::new(); + // From those lines, extract just the symbol name via `regex`, which: + // * always starts with "_ZN" and ends with "E" (`legacy` mangling) + // * always starts with "_R" (`v0` mangling) + for line in out { + if let Some(mat) = LEGACY_PATTERN.get().unwrap().find(line) { + symbols.insert(mat.as_str().to_string()); + } + if let Some(mat) = V0_PATTERN.get().unwrap().find(line) { + symbols.insert(mat.as_str().to_string()); + } + } + + let mut symbols: Vec = symbols.into_iter().collect(); + // Sort those symbol names for deterministic comparison. + symbols.sort(); + symbols +}