From fb663e6349c4274401355fd222fcefb9b7d5c926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Fri, 7 Feb 2025 23:18:17 +0800 Subject: [PATCH 1/2] run-make-support: add `build_native_static_lib_cxx_optimized` helper These should eventually be cleaned up, but let's add a helper for the immediate need of porting the `cpp_smoke_test` test. --- .../src/external_deps/c_build.rs | 39 +++++++++++++++++-- src/tools/run-make-support/src/lib.rs | 3 +- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/tools/run-make-support/src/external_deps/c_build.rs b/src/tools/run-make-support/src/external_deps/c_build.rs index 9dd30713f958b..621b5592230c6 100644 --- a/src/tools/run-make-support/src/external_deps/c_build.rs +++ b/src/tools/run-make-support/src/external_deps/c_build.rs @@ -6,7 +6,8 @@ use crate::external_deps::llvm::llvm_ar; use crate::path_helpers::path; use crate::targets::{is_darwin, is_msvc, is_windows}; -// FIXME(Oneirical): These native build functions should take a Path-based generic. +// FIXME(jieyouxu): figure out a way to improve the usability of these helpers... They are really +// messy. /// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. /// Built from a C file. @@ -75,8 +76,8 @@ pub fn build_native_dynamic_lib(lib_name: &str) -> PathBuf { path(lib_path) } -/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. -/// Built from a C++ file. +/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. Built +/// from a C++ file. Unoptimized. #[track_caller] pub fn build_native_static_lib_cxx(lib_name: &str) -> PathBuf { let obj_file = if is_msvc() { format!("{lib_name}") } else { format!("{lib_name}.o") }; @@ -95,3 +96,35 @@ pub fn build_native_static_lib_cxx(lib_name: &str) -> PathBuf { llvm_ar().obj_to_ar().output_input(&lib_path, &obj_file).run(); path(lib_path) } + +/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. Built +/// from a C++ file. Optimized with the provided `opt_level` flag. Note that the `opt_level_flag` +/// can differ between different C++ compilers! Consult relevant docs for `g++`/`clang++`/MSVC. +#[track_caller] +pub fn build_native_static_lib_cxx_optimized(lib_name: &str, opt_level_flag: &str) -> PathBuf { + let obj_file = if is_msvc() { format!("{lib_name}") } else { format!("{lib_name}.o") }; + let src = format!("{lib_name}.cpp"); + let lib_path = static_lib_name(lib_name); + + // NOTE: generate debuginfo + if is_msvc() { + cxx() + .arg(opt_level_flag) + .arg("-Zi") + .arg("-debug") + .arg("-EHs") + .arg("-c") + .out_exe(&obj_file) + .input(src) + .run(); + } else { + cxx().arg(opt_level_flag).arg("-g").arg("-c").out_exe(&obj_file).input(src).run(); + }; + let obj_file = if is_msvc() { + PathBuf::from(format!("{lib_name}.obj")) + } else { + PathBuf::from(format!("{lib_name}.o")) + }; + llvm_ar().obj_to_ar().output_input(&lib_path, &obj_file).run(); + path(lib_path) +} diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 7e63ab3159ace..b7563380a1b90 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -37,6 +37,7 @@ pub mod rfs { // Re-exports of third-party library crates. // tidy-alphabetical-start pub use bstr; +pub use cc as crate_cc; // NOTE: aliased to avoid being confused with `c_cxx_compiler::cc`. pub use gimli; pub use libc; pub use object; @@ -55,7 +56,7 @@ pub use external_deps::{ pub use c_cxx_compiler::{Cc, Gcc, cc, cxx, extra_c_flags, extra_cxx_flags, gcc}; pub use c_build::{ build_native_dynamic_lib, build_native_static_lib, build_native_static_lib_cxx, - build_native_static_lib_optimized, + build_native_static_lib_optimized, build_native_static_lib_cxx_optimized, }; pub use cargo::cargo; pub use clang::{clang, Clang}; From 7bb88c24cd4c6d3395d8aae5ac4372f7d90f043d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Fri, 7 Feb 2025 23:18:58 +0800 Subject: [PATCH 2/2] tests: port `backtrace`'s `cpp_smoke_test` as a `run-make` test This test exercises the `backtrace` submodule included under the `library/` tree, which should be the `backtrace` that is used and thus transitively shipped with the standard library. The `cpp_smoke_test` was disabled in the `backtrace` repo many years ago. This is seemingly because the test failed years ago due to C++ symbol demangling troubles in `cpp_demangle` (which are now fixed; see ). Note that this test is disabled on Windows MSVC because I couldn't get it to work locally: - At MSVC `-O1`, the test fails because there are no backtrace symbols. - At MSVC `-O0`, the test fails because both the C++ templated trampolined symbol has a different signature (w/ a `__cdecl`) and also `cpp_trampoline` is missing. Co-authored-by: Pyrode --- .../run-make/cpp-smoke-test/cpp_smoke_test.rs | 72 +++++++++++++++++++ tests/run-make/cpp-smoke-test/rmake.rs | 65 +++++++++++++++++ tests/run-make/cpp-smoke-test/trampoline.cpp | 14 ++++ 3 files changed, 151 insertions(+) create mode 100644 tests/run-make/cpp-smoke-test/cpp_smoke_test.rs create mode 100644 tests/run-make/cpp-smoke-test/rmake.rs create mode 100644 tests/run-make/cpp-smoke-test/trampoline.cpp diff --git a/tests/run-make/cpp-smoke-test/cpp_smoke_test.rs b/tests/run-make/cpp-smoke-test/cpp_smoke_test.rs new file mode 100644 index 0000000000000..64eb68e5a0bd1 --- /dev/null +++ b/tests/run-make/cpp-smoke-test/cpp_smoke_test.rs @@ -0,0 +1,72 @@ +extern crate backtrace; + +use std::sync::atomic::{AtomicBool, Ordering}; + +unsafe extern "C" { + unsafe fn cpp_trampoline(func: extern "C" fn()); +} + +fn main() { + static RAN_ASSERTS: AtomicBool = AtomicBool::new(false); + + extern "C" fn assert_cpp_frames() { + let mut physical_frames = Vec::new(); + backtrace::trace(|cx| { + physical_frames.push(cx.ip()); + + // We only want to capture: + // + // 1. this closure's frame + // 2. `assert_cpp_frames`, + // 3. `space::templated_trampoline`, and + // 4. `cpp_trampoline`. + // + // Those are logical frames, which might be inlined into fewer physical frames, so we + // may end up with extra logical frames after resolving these. + physical_frames.len() < 4 + }); + + let names: Vec<_> = physical_frames + .into_iter() + .flat_map(|ip| { + let mut logical_frame_names = vec![]; + + backtrace::resolve(ip, |sym| { + let sym_name = sym.name().expect("Should have a symbol name"); + let demangled = sym_name.to_string(); + logical_frame_names.push(demangled); + }); + + assert!( + !logical_frame_names.is_empty(), + "Should have resolved at least one symbol for the physical frame" + ); + + logical_frame_names + }) + // Skip the backtrace::trace closure and assert_cpp_frames, and then + // take the two C++ frame names. + .skip_while(|name| !name.contains("trampoline")) + .take(2) + .collect(); + + println!("actual names = {names:#?}"); + + let expected = + ["void space::templated_trampoline(void (*)())", "cpp_trampoline"]; + println!("expected names = {expected:#?}"); + + assert_eq!(names.len(), expected.len()); + for (actual, expected) in names.iter().zip(expected.iter()) { + assert_eq!(actual, expected); + } + + RAN_ASSERTS.store(true, Ordering::SeqCst); + } + + assert!(!RAN_ASSERTS.load(Ordering::SeqCst)); + unsafe { + cpp_trampoline(assert_cpp_frames); + } + assert!(RAN_ASSERTS.load(Ordering::SeqCst)); +} diff --git a/tests/run-make/cpp-smoke-test/rmake.rs b/tests/run-make/cpp-smoke-test/rmake.rs new file mode 100644 index 0000000000000..27ccf5a6c865f --- /dev/null +++ b/tests/run-make/cpp-smoke-test/rmake.rs @@ -0,0 +1,65 @@ +//! `backtrace`'s `cpp_smoke_test` ported to rust-lang/rust. +//! +//! A basic smoke test that exercises `backtrace` to see if it can resolve basic C++ templated + +//! trampolined symbol names. + +//@ ignore-cross-compile (binary needs to run) +//@ only-nightly + +//@ ignore-windows-msvc (test fails due to backtrace symbol mismatches) +// FIXME: on MSVC, at `-O1`, there are no symbols available. At `-O0`, the test fails looking like: +// ``` +// actual names = [ +// "space::templated_trampoline", +// ] +// expected names = [ +// "void space::templated_trampoline(void (*)())", +// "cpp_trampoline", +// ] +// ``` + +use run_make_support::rustc::sysroot; +use run_make_support::{ + build_native_static_lib_cxx_optimized, cargo, crate_cc, cwd, path, rfs, run, rustc, + source_root, target, +}; + +fn main() { + let target_dir = path("target"); + let src_root = source_root(); + let backtrace_submodule = src_root.join("library").join("backtrace"); + let backtrace_toml = backtrace_submodule.join("Cargo.toml"); + + // Build the `backtrace` package (the `library/backtrace` submodule to make sure we exercise the + // same `backtrace` as shipped with std). + cargo() + // NOTE: needed to skip trying to link in `windows.0.52.0.lib` which is pre-built but not + // available in *this* scenario. + .env("RUSTFLAGS", "--cfg=windows_raw_dylib") + .arg("build") + .args(&["--manifest-path", &backtrace_toml.to_str().unwrap()]) + .args(&["--target", &target()]) + .arg("--features=cpp_demangle") + .env("CARGO_TARGET_DIR", &target_dir) + // Visual Studio 2022 requires that the LIB env var be set so it can + // find the Windows SDK. + .env("LIB", std::env::var("LIB").unwrap_or_default()) + .run(); + + let rlibs_path = target_dir.join(target()).join("debug").join("deps"); + + // FIXME: this test is *really* fragile. Even on `x86_64-unknown-linux-gnu`, this fails if a + // different opt-level is passed. On `-O2` this test fails due to no symbols found. On `-O0` + // this test fails because it's missing one of the expected symbols. + build_native_static_lib_cxx_optimized("trampoline", "-O1"); + + rustc() + .input("cpp_smoke_test.rs") + .library_search_path(&rlibs_path) + .library_search_path(cwd()) + .debuginfo("2") + .arg("-ltrampoline") + .run(); + + run("cpp_smoke_test"); +} diff --git a/tests/run-make/cpp-smoke-test/trampoline.cpp b/tests/run-make/cpp-smoke-test/trampoline.cpp new file mode 100644 index 0000000000000..61e09604c82b6 --- /dev/null +++ b/tests/run-make/cpp-smoke-test/trampoline.cpp @@ -0,0 +1,14 @@ +#include + +namespace space { + template + void templated_trampoline(FuncT func) { + func(); + } +} + +typedef void (*FuncPtr)(); + +extern "C" void cpp_trampoline(FuncPtr func) { + space::templated_trampoline(func); +}