Skip to content

Commit

Permalink
Auto merge of rust-lang#136695 - jieyouxu:cpp_smoke_test, r=<try>
Browse files Browse the repository at this point in the history
Port `rust-lang/backtrace`'s `cpp_smoke_test` to `rust-lang/rust` test suite

Ports https://github.com/rust-lang/backtrace-rs/tree/e33eaac8caf46d0d3cc57f8d152529e8b7ae1b78/crates/cpp_smoke_test to the main repo.

I can't say I'm 100% convinced by the value of this test living in the main repo for several reasons:

1. This test (or at least the actual `cpp_smoke_test.rs` is **extremely** fragile. It's sensitive to debuginfo levels AND optimizations. On `x86_64-unknown-linux-gnu`, the C++ file **must** be compiled with **exactly** `-O1`. No more, no less, or else the backtrace symbol names do not line up in symbol count or name.
2. I could not get this test to work on Windows MSVC, no matter which combination of debuginfo levels and optimization levels I tried.

Note that I have not tried other targets, e.g. Apple friends, which may fail for yet other reasons.

Possibly supersedes rust-lang#136182.

r? `@ChrisDenton` (or `@workingjubilee)`

try-job: x86_64-apple-1
try-job: aarch64-apple
  • Loading branch information
bors committed Feb 7, 2025
2 parents 64e06c0 + 7bb88c2 commit 896b013
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 4 deletions.
39 changes: 36 additions & 3 deletions src/tools/run-make-support/src/external_deps/c_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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") };
Expand All @@ -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)
}
3 changes: 2 additions & 1 deletion src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down
72 changes: 72 additions & 0 deletions tests/run-make/cpp-smoke-test/cpp_smoke_test.rs
Original file line number Diff line number Diff line change
@@ -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 (*)()>(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));
}
65 changes: 65 additions & 0 deletions tests/run-make/cpp-smoke-test/rmake.rs
Original file line number Diff line number Diff line change
@@ -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<void (__cdecl*)(void)>",
// ]
// expected names = [
// "void space::templated_trampoline<void (*)()>(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");
}
14 changes: 14 additions & 0 deletions tests/run-make/cpp-smoke-test/trampoline.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#include <stdio.h>

namespace space {
template <typename FuncT>
void templated_trampoline(FuncT func) {
func();
}
}

typedef void (*FuncPtr)();

extern "C" void cpp_trampoline(FuncPtr func) {
space::templated_trampoline(func);
}

0 comments on commit 896b013

Please sign in to comment.