Skip to content

Commit 896b013

Browse files
committed
Auto merge of #136695 - jieyouxu:cpp_smoke_test, r=<try>
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 #136182. r? `@ChrisDenton` (or `@workingjubilee)` try-job: x86_64-apple-1 try-job: aarch64-apple
2 parents 64e06c0 + 7bb88c2 commit 896b013

File tree

5 files changed

+189
-4
lines changed

5 files changed

+189
-4
lines changed

src/tools/run-make-support/src/external_deps/c_build.rs

+36-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ use crate::external_deps::llvm::llvm_ar;
66
use crate::path_helpers::path;
77
use crate::targets::{is_darwin, is_msvc, is_windows};
88

9-
// FIXME(Oneirical): These native build functions should take a Path-based generic.
9+
// FIXME(jieyouxu): figure out a way to improve the usability of these helpers... They are really
10+
// messy.
1011

1112
/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name.
1213
/// Built from a C file.
@@ -75,8 +76,8 @@ pub fn build_native_dynamic_lib(lib_name: &str) -> PathBuf {
7576
path(lib_path)
7677
}
7778

78-
/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name.
79-
/// Built from a C++ file.
79+
/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. Built
80+
/// from a C++ file. Unoptimized.
8081
#[track_caller]
8182
pub fn build_native_static_lib_cxx(lib_name: &str) -> PathBuf {
8283
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 {
9596
llvm_ar().obj_to_ar().output_input(&lib_path, &obj_file).run();
9697
path(lib_path)
9798
}
99+
100+
/// Builds a static lib (`.lib` on Windows MSVC and `.a` for the rest) with the given name. Built
101+
/// from a C++ file. Optimized with the provided `opt_level` flag. Note that the `opt_level_flag`
102+
/// can differ between different C++ compilers! Consult relevant docs for `g++`/`clang++`/MSVC.
103+
#[track_caller]
104+
pub fn build_native_static_lib_cxx_optimized(lib_name: &str, opt_level_flag: &str) -> PathBuf {
105+
let obj_file = if is_msvc() { format!("{lib_name}") } else { format!("{lib_name}.o") };
106+
let src = format!("{lib_name}.cpp");
107+
let lib_path = static_lib_name(lib_name);
108+
109+
// NOTE: generate debuginfo
110+
if is_msvc() {
111+
cxx()
112+
.arg(opt_level_flag)
113+
.arg("-Zi")
114+
.arg("-debug")
115+
.arg("-EHs")
116+
.arg("-c")
117+
.out_exe(&obj_file)
118+
.input(src)
119+
.run();
120+
} else {
121+
cxx().arg(opt_level_flag).arg("-g").arg("-c").out_exe(&obj_file).input(src).run();
122+
};
123+
let obj_file = if is_msvc() {
124+
PathBuf::from(format!("{lib_name}.obj"))
125+
} else {
126+
PathBuf::from(format!("{lib_name}.o"))
127+
};
128+
llvm_ar().obj_to_ar().output_input(&lib_path, &obj_file).run();
129+
path(lib_path)
130+
}

src/tools/run-make-support/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub mod rfs {
3737
// Re-exports of third-party library crates.
3838
// tidy-alphabetical-start
3939
pub use bstr;
40+
pub use cc as crate_cc; // NOTE: aliased to avoid being confused with `c_cxx_compiler::cc`.
4041
pub use gimli;
4142
pub use libc;
4243
pub use object;
@@ -55,7 +56,7 @@ pub use external_deps::{
5556
pub use c_cxx_compiler::{Cc, Gcc, cc, cxx, extra_c_flags, extra_cxx_flags, gcc};
5657
pub use c_build::{
5758
build_native_dynamic_lib, build_native_static_lib, build_native_static_lib_cxx,
58-
build_native_static_lib_optimized,
59+
build_native_static_lib_optimized, build_native_static_lib_cxx_optimized,
5960
};
6061
pub use cargo::cargo;
6162
pub use clang::{clang, Clang};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
extern crate backtrace;
2+
3+
use std::sync::atomic::{AtomicBool, Ordering};
4+
5+
unsafe extern "C" {
6+
unsafe fn cpp_trampoline(func: extern "C" fn());
7+
}
8+
9+
fn main() {
10+
static RAN_ASSERTS: AtomicBool = AtomicBool::new(false);
11+
12+
extern "C" fn assert_cpp_frames() {
13+
let mut physical_frames = Vec::new();
14+
backtrace::trace(|cx| {
15+
physical_frames.push(cx.ip());
16+
17+
// We only want to capture:
18+
//
19+
// 1. this closure's frame
20+
// 2. `assert_cpp_frames`,
21+
// 3. `space::templated_trampoline`, and
22+
// 4. `cpp_trampoline`.
23+
//
24+
// Those are logical frames, which might be inlined into fewer physical frames, so we
25+
// may end up with extra logical frames after resolving these.
26+
physical_frames.len() < 4
27+
});
28+
29+
let names: Vec<_> = physical_frames
30+
.into_iter()
31+
.flat_map(|ip| {
32+
let mut logical_frame_names = vec![];
33+
34+
backtrace::resolve(ip, |sym| {
35+
let sym_name = sym.name().expect("Should have a symbol name");
36+
let demangled = sym_name.to_string();
37+
logical_frame_names.push(demangled);
38+
});
39+
40+
assert!(
41+
!logical_frame_names.is_empty(),
42+
"Should have resolved at least one symbol for the physical frame"
43+
);
44+
45+
logical_frame_names
46+
})
47+
// Skip the backtrace::trace closure and assert_cpp_frames, and then
48+
// take the two C++ frame names.
49+
.skip_while(|name| !name.contains("trampoline"))
50+
.take(2)
51+
.collect();
52+
53+
println!("actual names = {names:#?}");
54+
55+
let expected =
56+
["void space::templated_trampoline<void (*)()>(void (*)())", "cpp_trampoline"];
57+
println!("expected names = {expected:#?}");
58+
59+
assert_eq!(names.len(), expected.len());
60+
for (actual, expected) in names.iter().zip(expected.iter()) {
61+
assert_eq!(actual, expected);
62+
}
63+
64+
RAN_ASSERTS.store(true, Ordering::SeqCst);
65+
}
66+
67+
assert!(!RAN_ASSERTS.load(Ordering::SeqCst));
68+
unsafe {
69+
cpp_trampoline(assert_cpp_frames);
70+
}
71+
assert!(RAN_ASSERTS.load(Ordering::SeqCst));
72+
}
+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
//! `backtrace`'s `cpp_smoke_test` ported to rust-lang/rust.
2+
//!
3+
//! A basic smoke test that exercises `backtrace` to see if it can resolve basic C++ templated +
4+
//! trampolined symbol names.
5+
6+
//@ ignore-cross-compile (binary needs to run)
7+
//@ only-nightly
8+
9+
//@ ignore-windows-msvc (test fails due to backtrace symbol mismatches)
10+
// FIXME: on MSVC, at `-O1`, there are no symbols available. At `-O0`, the test fails looking like:
11+
// ```
12+
// actual names = [
13+
// "space::templated_trampoline<void (__cdecl*)(void)>",
14+
// ]
15+
// expected names = [
16+
// "void space::templated_trampoline<void (*)()>(void (*)())",
17+
// "cpp_trampoline",
18+
// ]
19+
// ```
20+
21+
use run_make_support::rustc::sysroot;
22+
use run_make_support::{
23+
build_native_static_lib_cxx_optimized, cargo, crate_cc, cwd, path, rfs, run, rustc,
24+
source_root, target,
25+
};
26+
27+
fn main() {
28+
let target_dir = path("target");
29+
let src_root = source_root();
30+
let backtrace_submodule = src_root.join("library").join("backtrace");
31+
let backtrace_toml = backtrace_submodule.join("Cargo.toml");
32+
33+
// Build the `backtrace` package (the `library/backtrace` submodule to make sure we exercise the
34+
// same `backtrace` as shipped with std).
35+
cargo()
36+
// NOTE: needed to skip trying to link in `windows.0.52.0.lib` which is pre-built but not
37+
// available in *this* scenario.
38+
.env("RUSTFLAGS", "--cfg=windows_raw_dylib")
39+
.arg("build")
40+
.args(&["--manifest-path", &backtrace_toml.to_str().unwrap()])
41+
.args(&["--target", &target()])
42+
.arg("--features=cpp_demangle")
43+
.env("CARGO_TARGET_DIR", &target_dir)
44+
// Visual Studio 2022 requires that the LIB env var be set so it can
45+
// find the Windows SDK.
46+
.env("LIB", std::env::var("LIB").unwrap_or_default())
47+
.run();
48+
49+
let rlibs_path = target_dir.join(target()).join("debug").join("deps");
50+
51+
// FIXME: this test is *really* fragile. Even on `x86_64-unknown-linux-gnu`, this fails if a
52+
// different opt-level is passed. On `-O2` this test fails due to no symbols found. On `-O0`
53+
// this test fails because it's missing one of the expected symbols.
54+
build_native_static_lib_cxx_optimized("trampoline", "-O1");
55+
56+
rustc()
57+
.input("cpp_smoke_test.rs")
58+
.library_search_path(&rlibs_path)
59+
.library_search_path(cwd())
60+
.debuginfo("2")
61+
.arg("-ltrampoline")
62+
.run();
63+
64+
run("cpp_smoke_test");
65+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#include <stdio.h>
2+
3+
namespace space {
4+
template <typename FuncT>
5+
void templated_trampoline(FuncT func) {
6+
func();
7+
}
8+
}
9+
10+
typedef void (*FuncPtr)();
11+
12+
extern "C" void cpp_trampoline(FuncPtr func) {
13+
space::templated_trampoline(func);
14+
}

0 commit comments

Comments
 (0)