From 3ed644a4ff3821cae8a6a7991b9979db0d00475f Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 19 Mar 2025 02:52:35 +0000 Subject: [PATCH 1/4] Don't call `rust_begin_unwind` directly `builtins-test-intrinsics` has long included a call to an unmangled `rust_begin_unwind` (the name `rustc` gives the `#[panic_handler]`), since [1], which I believe was intended to ensure panic machinery links correctly. However, since [2], `rust_begin_unwind` is mangled and unavailable for calling directly, which explains the recent CI failures. Instead of calling the function directly, we can just panic; do so here. Additionally, put this call behind `black_box(false)` rather than unconditional, which means we can run the binary and ensure there are no runtime issues. [1]: https://github.com/rust-lang/compiler-builtins/pull/360 [2]: https://github.com/rust-lang/rust/pull/127173 --- builtins-test-intrinsics/build.rs | 5 +++++ builtins-test-intrinsics/src/main.rs | 28 ++++++++++++++++++---------- ci/run.sh | 26 ++++++++++++++++---------- 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/builtins-test-intrinsics/build.rs b/builtins-test-intrinsics/build.rs index a38c6c1f..1981943d 100644 --- a/builtins-test-intrinsics/build.rs +++ b/builtins-test-intrinsics/build.rs @@ -8,4 +8,9 @@ fn main() { let target = builtins_configure::Target::from_env(); builtins_configure::configure_f16_f128(&target); builtins_configure::configure_aliases(&target); + + if target.os == "windows" { + // Needed for using the `mainCRTStartup` entrypoint + println!("cargo::rustc-link-arg=/subsystem:console"); + } } diff --git a/builtins-test-intrinsics/src/main.rs b/builtins-test-intrinsics/src/main.rs index e90cfb33..264ac384 100644 --- a/builtins-test-intrinsics/src/main.rs +++ b/builtins-test-intrinsics/src/main.rs @@ -13,6 +13,9 @@ #![no_std] #![no_main] +// Ensure this repo's version of `compiler_builtins` gets used, rather than what gets injected from +// the sysroot. +extern crate compiler_builtins; extern crate panic_handler; #[cfg(all(not(thumb), not(windows), not(target_arch = "wasm32")))] @@ -626,12 +629,9 @@ fn run() { something_with_a_dtor(&|| assert_eq!(bb(1), 1)); - extern "C" { - fn rust_begin_unwind(x: usize); - } - - unsafe { - rust_begin_unwind(0); + // Ensure panic machinery gets linked, but still allow this to run to completion. + if bb(false) { + panic!(); } } @@ -648,15 +648,23 @@ fn something_with_a_dtor(f: &dyn Fn()) { } #[no_mangle] -#[cfg(not(thumb))] -fn main(_argc: core::ffi::c_int, _argv: *const *const u8) -> core::ffi::c_int { +#[cfg(not(any(thumb, windows)))] +extern "C" fn main(_argc: core::ffi::c_int, _argv: *const *const u8) -> core::ffi::c_int { + run(); + 0 +} + +#[no_mangle] +#[cfg(windows)] +#[allow(non_snake_case)] +extern "C" fn mainCRTStartup() -> core::ffi::c_int { run(); 0 } #[no_mangle] #[cfg(thumb)] -pub fn _start() -> ! { +extern "C" fn _start() -> ! { run(); loop {} } @@ -681,7 +689,7 @@ pub fn _Unwind_Resume() {} #[cfg(not(any(windows, target_os = "cygwin")))] #[lang = "eh_personality"] #[no_mangle] -pub extern "C" fn eh_personality() {} +pub extern "system" fn eh_personality() {} #[cfg(any(all(windows, target_env = "gnu"), target_os = "cygwin"))] mod mingw_unwinding { diff --git a/ci/run.sh b/ci/run.sh index 3625dde7..0882b77e 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -120,22 +120,28 @@ done rm -f "${rlib_paths[@]}" -build_intrinsics_test() { - cargo build --target "$target" -v --package builtins-test-intrinsics "$@" +run_intrinsics_test() { + # FIXME(windows): We should be able to run this test on Windows too, but it + # seems to run into a lot more missing symbols. + if [[ "${NO_STD:-}" = "1" || "$target" == *"windows" ]]; then + cmd=build + else + cmd=run + fi + + cargo "$cmd" --target "$target" -v --package builtins-test-intrinsics "$@" } # Verify that we haven't dropped any intrinsics/symbols -build_intrinsics_test -build_intrinsics_test --release -build_intrinsics_test --features c -build_intrinsics_test --features c --release +run_intrinsics_test +run_intrinsics_test --release +run_intrinsics_test --features c +run_intrinsics_test --features c --release # Verify that there are no undefined symbols to `panic` within our # implementations -CARGO_PROFILE_DEV_LTO=true \ - cargo build --target "$target" --package builtins-test-intrinsics -CARGO_PROFILE_RELEASE_LTO=true \ - cargo build --target "$target" --package builtins-test-intrinsics --release +CARGO_PROFILE_DEV_LTO=true run_intrinsics_test +CARGO_PROFILE_RELEASE_LTO=true run_intrinsics_test --release # Ensure no references to any symbols from core update_rlib_paths From 815a0f0ce6e749b473620205c2b6a51c95b5b175 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 19 Mar 2025 03:51:47 +0000 Subject: [PATCH 2/4] update --- builtins-test-intrinsics/Cargo.toml | 4 ++++ builtins-test-intrinsics/src/main.rs | 3 +++ 2 files changed, 7 insertions(+) diff --git a/builtins-test-intrinsics/Cargo.toml b/builtins-test-intrinsics/Cargo.toml index 9b2e5bb7..676488af 100644 --- a/builtins-test-intrinsics/Cargo.toml +++ b/builtins-test-intrinsics/Cargo.toml @@ -8,5 +8,9 @@ publish = false compiler_builtins = { path = "../", features = ["compiler-builtins"]} panic-handler = { path = '../crates/panic-handler' } +[target.'cfg(target_family = "windows")'.dependencies] +# enable mem on Windows, required for panic machinery +compiler_builtins = { path = "../", features = ["compiler-builtins", "mem"]} + [features] c = ["compiler_builtins/c"] diff --git a/builtins-test-intrinsics/src/main.rs b/builtins-test-intrinsics/src/main.rs index 264ac384..4f0a42bb 100644 --- a/builtins-test-intrinsics/src/main.rs +++ b/builtins-test-intrinsics/src/main.rs @@ -630,6 +630,9 @@ fn run() { something_with_a_dtor(&|| assert_eq!(bb(1), 1)); // Ensure panic machinery gets linked, but still allow this to run to completion. + // FIXME(windows): we should have this on Windows too but it requires a lot more + // missing symbols. + #[cfg(not(windows))] if bb(false) { panic!(); } From d1145fdda2412591ccf53a9ddf25b60f2dafa5af Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 19 Mar 2025 03:54:21 +0000 Subject: [PATCH 3/4] update --- builtins-test-intrinsics/src/main.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/builtins-test-intrinsics/src/main.rs b/builtins-test-intrinsics/src/main.rs index 4f0a42bb..9eab5590 100644 --- a/builtins-test-intrinsics/src/main.rs +++ b/builtins-test-intrinsics/src/main.rs @@ -684,16 +684,22 @@ pub fn __aeabi_unwind_cpp_pr0() {} #[no_mangle] pub fn __aeabi_unwind_cpp_pr1() {} -#[cfg(not(any(windows, target_os = "cygwin")))] -#[allow(non_snake_case)] #[no_mangle] +#[allow(non_snake_case)] +#[cfg(not(any(windows, target_os = "cygwin")))] pub fn _Unwind_Resume() {} -#[cfg(not(any(windows, target_os = "cygwin")))] -#[lang = "eh_personality"] #[no_mangle] +#[lang = "eh_personality"] +#[cfg(not(any(windows, target_os = "cygwin")))] pub extern "system" fn eh_personality() {} +#[cfg(windows)] +#[unsafe(no_mangle)] +extern "system" fn __CxxFrameHandler3() -> ! { + unimplemented!() +} + #[cfg(any(all(windows, target_env = "gnu"), target_os = "cygwin"))] mod mingw_unwinding { #[no_mangle] From c02c22d1c850c82a230b184084f13acfe73a2c8a Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 19 Mar 2025 04:01:34 +0000 Subject: [PATCH 4/4] comment update --- builtins-test-intrinsics/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtins-test-intrinsics/Cargo.toml b/builtins-test-intrinsics/Cargo.toml index 676488af..b1e9cda7 100644 --- a/builtins-test-intrinsics/Cargo.toml +++ b/builtins-test-intrinsics/Cargo.toml @@ -9,7 +9,7 @@ compiler_builtins = { path = "../", features = ["compiler-builtins"]} panic-handler = { path = '../crates/panic-handler' } [target.'cfg(target_family = "windows")'.dependencies] -# enable mem on Windows, required for panic machinery +# `#![no_std]` on Windows needs `string.h` routines. compiler_builtins = { path = "../", features = ["compiler-builtins", "mem"]} [features]