From 08cfcc980370cf31b09a240c07685b7b554a55be Mon Sep 17 00:00:00 2001 From: Alisa Sireneva Date: Fri, 24 Jan 2025 11:28:10 +0300 Subject: [PATCH] Replace `std` feature with probing The current state of codegen changing depending on whether the `std` feature is enabled is messy. The main problem is that features are controlled by immediate dependents, which are usually libraries in our case, but they cannot be expected to know whether `std` is safe to enable or disable. Unlike typical features, Lithium's `std` doesn't gate any functionality: the API exposed to users stays the same. However: - Under SEH, `std` controls whether panics within `intercept` surface or abort the program. A library often has no idea whether a certain function can panic, and as such does not have enough information to know whether `std` needs to be enabled. As such, libraries often need to enable `std`. - `std` gates printing errors to stderr on abort. This is not something libraries should control, it's the responsibility of the binary crate. - `std` also controls the TLS mechanism. Certain targets don't have a working `#[thread_local]` implementation, so `std` becomes a required feature on those targets. Platform-independent crates cannot be expected to use platform-dependent dependency feature sets. - `std` is effectively required on stable and optional on nightly. The Rust version is a responsibility of either the binary crate or the package maintainers, but certainly not of library crates. All of this makes the `std` feature broken, so instead, we gate previously-`std`-only capabilities behind probing in `build.rs`, which is guaranteed to produce valid code in the majority of cases: - As SEH can handle Rust panics whenever `panic_unwind` is linked in, even without `std`, and we only support `-C panic=unwind`, we forcibly enable sane panic handling under SEH. - We detect presence of `std::io` and always print errors in that case. This might, unfortunately, bring in I/O machinery to `#![no_main]` binaries, but should be cheap otherwise. - We choose between `#[thread_local]` and `std::thread_local!` depending on availability. `std::thread_local!` is the last choice if we can't avoid using it, so this shouldn't lead to regressions. This also fixes TLS on the GNU/Windows target, which ships a broken `#[thread_local]` that we detect with `cfg(target_thread_local)`. - We use `extern crate std;` only when it's available, and raise a compile-time error if we need it but it's unavailable. --- .github/workflows/ci.yml | 64 +++++++++++++++++++++------------------ Cargo.lock | 1 + Cargo.toml | 9 ++++-- Cross.toml | 2 +- build.rs | 56 ++++++++++++++++++++++++++++++++-- src/api.rs | 1 - src/backend/emscripten.rs | 2 +- src/backend/itanium.rs | 2 +- src/backend/mod.rs | 1 - src/backend/seh.rs | 46 +++++++++------------------- src/lib.rs | 17 ++++++----- src/stacked_exceptions.rs | 12 +++++--- 12 files changed, 129 insertions(+), 84 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6136a96..e8a3996 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,14 +34,14 @@ jobs: run: LITHIUM_BACKEND=panic cargo test --target $target - name: Test with Itanium backend (debug) run: LITHIUM_BACKEND=itanium cargo test --target $target - - name: Test without std (debug) - run: cargo test --target $target --no-default-features + - name: Test with std thread locals (debug) + run: LITHIUM_THREAD_LOCAL=std cargo test --target $target - name: Test with panic backend (release) run: LITHIUM_BACKEND=panic cargo test --target $target --release - name: Test with Itanium backend (release) run: LITHIUM_BACKEND=itanium cargo test --target $target --release - - name: Test without std (release) - run: cargo test --target $target --release --no-default-features + - name: Test with std thread locals (release) + run: LITHIUM_THREAD_LOCAL=std cargo test --target $target --release linux-cross: timeout-minutes: 5 @@ -83,14 +83,14 @@ jobs: run: LITHIUM_BACKEND=panic cross test --target $target - name: Test with Itanium backend (debug) run: LITHIUM_BACKEND=itanium cross test --target $target - - name: Test without std (debug) - run: cross test --target $target --no-default-features + - name: Test with std thread locals (debug) + run: LITHIUM_THREAD_LOCAL=std cross test --target $target - name: Test with panic backend (release) run: LITHIUM_BACKEND=panic cross test --target $target --release - name: Test with Itanium backend (release) run: LITHIUM_BACKEND=itanium cross test --target $target --release - - name: Test without std (release) - run: cross test --target $target --release --no-default-features + - name: Test with std thread locals (release) + run: LITHIUM_THREAD_LOCAL=std cross test --target $target --release emscripten: timeout-minutes: 5 @@ -109,10 +109,14 @@ jobs: run: LITHIUM_BACKEND=panic cross test --target $target - name: Test with Emscripten backend (debug) run: LITHIUM_BACKEND=emscripten cross test --target $target + - name: Test with std thread locals (debug) + run: LITHIUM_THREAD_LOCAL=std cross test --target $target - name: Test with panic backend (release) run: LITHIUM_BACKEND=panic cross test --target $target --release - name: Test with Emscripten backend (release) run: LITHIUM_BACKEND=emscripten cross test --target $target --release + - name: Test with std thread locals (release) + run: LITHIUM_THREAD_LOCAL=std cross test --target $target --release wasi: timeout-minutes: 3 @@ -137,15 +141,15 @@ jobs: run: LITHIUM_BACKEND=panic ci/cargo-wasi test --target $target - name: Test with Itanium backend (debug) run: LITHIUM_BACKEND=itanium ci/cargo-wasi test --target $target - - name: Test without std (debug) - run: ci/cargo-wasi test --target $target --no-default-features + - name: Test with std thread locals (debug) + run: LITHIUM_THREAD_LOCAL=std ci/cargo-wasi test --target $target - name: Test with panic backend (release) run: LITHIUM_BACKEND=panic ci/cargo-wasi test --target $target --release # XXX: Upstream bug at https://github.com/rust-lang/rust/issues/132416 # - name: Test with Itanium backend (release) # run: LITHIUM_BACKEND=itanium ci/cargo-wasi test --target $target --release - # - name: Test without std (release) - # run: ci/cargo-wasi test --target $target --release --no-default-features + # - name: Test with std thread locals (release) + # run: LITHIUM_THREAD_LOCAL=std ci/cargo-wasi test --target $target --release darwin: timeout-minutes: 3 @@ -166,14 +170,14 @@ jobs: run: LITHIUM_BACKEND=panic cargo test - name: Test with Itanium backend (debug) run: LITHIUM_BACKEND=itanium cargo test - - name: Test without std (debug) - run: cargo test + - name: Test with std thread locals (debug) + run: LITHIUM_THREAD_LOCAL=std cargo test - name: Test with panic backend (release) run: LITHIUM_BACKEND=panic cargo test --release - name: Test with Itanium backend (release) run: LITHIUM_BACKEND=itanium cargo test --release - - name: Test without std (release) - run: cargo test --release + - name: Test with std thread locals (release) + run: LITHIUM_THREAD_LOCAL=std cargo test --release windows: timeout-minutes: 5 @@ -205,9 +209,11 @@ jobs: env: LITHIUM_BACKEND: itanium run: cargo test - - name: Test without std (debug) + - name: Test with std thread locals (debug) if: matrix.abi == 'msvc' - run: cargo test --no-default-features + run: cargo test + env: + LITHIUM_THREAD_LOCAL: std - name: Test with panic backend (release) env: LITHIUM_BACKEND: panic @@ -222,9 +228,11 @@ jobs: env: LITHIUM_BACKEND: itanium run: cargo test --release - - name: Test without std (release) + - name: Test with std thread locals (release) if: matrix.abi == 'msvc' - run: cargo test --release --no-default-features + run: cargo test --release + env: + LITHIUM_THREAD_LOCAL: std miri-linux: timeout-minutes: 5 @@ -252,8 +260,8 @@ jobs: run: LITHIUM_BACKEND=panic cargo miri test --target $target - name: Test with Itanium backend run: LITHIUM_BACKEND=itanium cargo miri test --target $target - - name: Test without std - run: cargo miri test --target $target --no-default-features + - name: Test with std thread locals + run: LITHIUM_THREAD_LOCAL=std cargo miri test --target $target valgrind: timeout-minutes: 5 @@ -272,14 +280,14 @@ jobs: run: LITHIUM_BACKEND=panic cargo valgrind test - name: Test with Itanium backend (debug) run: LITHIUM_BACKEND=itanium cargo valgrind test - - name: Test without std (debug) - run: cargo valgrind test --no-default-features + - name: Test with std thread locals (debug) + run: LITHIUM_THREAD_LOCAL=std cargo valgrind test - name: Test with panic backend (release) run: LITHIUM_BACKEND=panic cargo valgrind test --release - name: Test with Itanium backend (release) run: LITHIUM_BACKEND=itanium cargo valgrind test --release - - name: Test without std (release) - run: cargo valgrind test --release --no-default-features + - name: Test with std thread locals (release) + run: LITHIUM_THREAD_LOCAL=std cargo valgrind test --release test-stable: timeout-minutes: 3 @@ -305,6 +313,7 @@ jobs: thread_local: [std, attribute] env: LITHIUM_BACKEND: ${{ matrix.backend }} + LITHIUM_THREAD_LOCAL: ${{ matrix.thread_local }} steps: - name: Checkout uses: actions/checkout@v4 @@ -316,6 +325,3 @@ jobs: run: cargo fmt -- --check - name: Clippy run: cargo clippy -- -D warnings - - name: Clippy without std - if: matrix.backend != 'panic' - run: cargo clippy --no-default-features -- -D warnings diff --git a/Cargo.lock b/Cargo.lock index e004b49..3eaeb7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -198,6 +198,7 @@ name = "lithium" version = "1.0.2" dependencies = [ "anyhow", + "autocfg", "criterion", "replace_with", "rustc_version", diff --git a/Cargo.toml b/Cargo.toml index 7817090..74e1821 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,11 +19,10 @@ criterion = { version = "0.5", default-features = false, features = ["cargo_benc replace_with = "0.1.7" [build-dependencies] +autocfg = "1.4.0" rustc_version = "0.4.1" [features] -default = ["std"] -std = [] sound-under-stacked-borrows = [] [package.metadata."docs.rs"] @@ -35,7 +34,11 @@ harness = false [lints.rust.unexpected_cfgs] level = "warn" -check-cfg = ["cfg(nightly)", "cfg(backend, values(\"itanium\", \"seh\", \"emscripten\", \"panic\", \"unimplemented\"))"] +check-cfg = [ + "cfg(abort, values(\"std\", \"core\"))", + "cfg(backend, values(\"itanium\", \"seh\", \"emscripten\", \"panic\", \"unimplemented\"))", + "cfg(thread_local, values(\"std\", \"attribute\", \"unimplemented\"))", +] [profile.dev] panic = "unwind" diff --git a/Cross.toml b/Cross.toml index c84aaac..ae29287 100644 --- a/Cross.toml +++ b/Cross.toml @@ -1,5 +1,5 @@ [build.env] -passthrough = ["LITHIUM_BACKEND"] +passthrough = ["LITHIUM_BACKEND", "LITHIUM_THREAD_LOCAL"] [target.mips-unknown-linux-gnu] build-std = ["std"] diff --git a/build.rs b/build.rs index f79b681..496eb4d 100644 --- a/build.rs +++ b/build.rs @@ -17,8 +17,31 @@ fn main() { println!("cargo::rustc-cfg=feature=\"sound-under-stacked-borrows\""); } + let ac = autocfg::new(); let is_nightly = version_meta().unwrap().channel == Channel::Nightly; + println!("cargo::rerun-if-env-changed=LITHIUM_THREAD_LOCAL"); + if let Ok(thread_local) = std::env::var("LITHIUM_THREAD_LOCAL") { + println!("cargo::rustc-cfg=thread_local=\"{thread_local}\""); + } else if is_nightly && has_cfg("target_thread_local") { + println!("cargo::rustc-cfg=thread_local=\"attribute\""); + } else if ac + .probe_raw( + r" + #![no_std] + extern crate std; + std::thread_local! { + static FOO: () = (); + } + ", + ) + .is_ok() + { + println!("cargo::rustc-cfg=thread_local=\"std\""); + } else { + println!("cargo::rustc-cfg=thread_local=\"unimplemented\""); + } + println!("cargo::rerun-if-env-changed=LITHIUM_BACKEND"); if let Ok(backend) = std::env::var("LITHIUM_BACKEND") { println!("cargo::rustc-cfg=backend=\"{backend}\""); @@ -32,10 +55,37 @@ fn main() { println!("cargo::rustc-cfg=backend=\"itanium\""); } else if is_nightly && (has_cfg("windows") && cfg("target_env") == "msvc") && !is_miri { println!("cargo::rustc-cfg=backend=\"seh\""); - } else { - #[cfg(feature = "std")] + } else if ac + .probe_raw( + r" + #![no_std] + extern crate std; + use std::panic::{catch_unwind, resume_unwind}; + ", + ) + .is_ok() + { println!("cargo::rustc-cfg=backend=\"panic\""); - #[cfg(not(feature = "std"))] + } else { println!("cargo::rustc-cfg=backend=\"unimplemented\""); } + + if ac + .probe_raw( + r#" + #![no_std] + extern crate std; + use std::io::Write; + fn main() { + let _ = std::io::stderr().write_all(b"hello"); + std::process::abort(); + } + "#, + ) + .is_ok() + { + println!("cargo::rustc-cfg=abort=\"std\""); + } else { + println!("cargo::rustc-cfg=abort=\"core\""); + } } diff --git a/src/api.rs b/src/api.rs index de76047..6fb6eb2 100644 --- a/src/api.rs +++ b/src/api.rs @@ -200,7 +200,6 @@ mod test { assert_eq!(result.unwrap_err(), "Hello, world!"); } - #[cfg(feature = "std")] #[test] fn catch_panic() { struct Dropper<'a>(&'a mut bool); diff --git a/src/backend/emscripten.rs b/src/backend/emscripten.rs index e4f20a3..5b12d29 100644 --- a/src/backend/emscripten.rs +++ b/src/backend/emscripten.rs @@ -142,5 +142,5 @@ extern "C-unwind" { /// /// `ex` must point at a valid exception object. unsafe extern "C" fn cleanup(_ex: *mut ()) -> *mut () { - abort("A Lithium exception was caught by a non-Lithium catch mechanism. This is undefined behavior. The process will now terminate."); + abort("A Lithium exception was caught by a non-Lithium catch mechanism. This is undefined behavior. The process will now terminate.\n"); } diff --git a/src/backend/itanium.rs b/src/backend/itanium.rs index 4bea0bd..f3a5172 100644 --- a/src/backend/itanium.rs +++ b/src/backend/itanium.rs @@ -156,7 +156,7 @@ const fn get_unwinder_private_word_count() -> usize { /// /// `ex` must point at a valid exception object. unsafe extern "C" fn cleanup(_code: i32, _ex: *mut Header) { - abort("A Lithium exception was caught by a non-Lithium catch mechanism. This is undefined behavior. The process will now terminate."); + abort("A Lithium exception was caught by a non-Lithium catch mechanism. This is undefined behavior. The process will now terminate.\n"); } #[cfg(not(target_arch = "wasm32"))] diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 25edff3..73255eb 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -206,7 +206,6 @@ mod test { assert_eq!(caught_ex, "Hello, world!"); } - #[cfg(feature = "std")] #[test] fn intercept_panic() { let result = std::panic::catch_unwind(|| unsafe { diff --git a/src/backend/seh.rs b/src/backend/seh.rs index a9a45cd..c66f007 100644 --- a/src/backend/seh.rs +++ b/src/backend/seh.rs @@ -6,16 +6,13 @@ use super::{ super::{abort, intrinsic::intercept}, RethrowHandle, ThrowByValue, }; +use alloc::boxed::Box; +use core::any::Any; use core::marker::{FnPtr, PhantomData}; use core::mem::ManuallyDrop; +use core::panic::PanicPayload; use core::sync::atomic::{AtomicU32, Ordering}; -#[cfg(feature = "std")] -use { - alloc::boxed::Box, - core::{any::Any, panic::PanicPayload}, -}; - pub(crate) struct ActiveBackend; /// SEH-based unwinding. @@ -62,8 +59,6 @@ unsafe impl ThrowByValue for ActiveBackend { unsafe fn intercept R, R, E>(func: Func) -> Result { enum CaughtUnwind { LithiumException(E), - - #[cfg(feature = "std")] RustPanic(Box), } @@ -72,7 +67,7 @@ unsafe impl ThrowByValue for ActiveBackend { if ex.is_null() { // This is a foreign exception. abort( - "Lithium caught a foreign exception. This is unsupported. The process will now terminate.", + "Lithium caught a foreign exception. This is unsupported. The process will now terminate.\n", ); } @@ -82,22 +77,13 @@ unsafe impl ThrowByValue for ActiveBackend { // thrown by us or by the Rust runtime; both have the `header.canary` field as the first // field in their structures. if unsafe { (*ex_lithium).header.canary } != (&raw const THROW_INFO).cast() { - // This is a Rust exception - #[cfg(feature = "std")] - { - // We can't rethrow it immediately from this nounwind callback, so let's catch - // it first. - // SAFETY: `ex` is the callback value of `core::intrinsics::catch_unwind`. - let payload = unsafe { __rust_panic_cleanup(ex) }; - // SAFETY: `__rust_panic_cleanup` returns a Box. - let payload = unsafe { Box::from_raw(payload) }; - return CaughtUnwind::RustPanic(payload); - } - #[cfg(not(feature = "std"))] - { - // In no-std mode, we can't handle this. - core::intrinsics::abort(); - } + // This is a Rust exception. We can't rethrow it immediately from this nounwind + // callback, so let's catch it first. + // SAFETY: `ex` is the callback value of `core::intrinsics::catch_unwind`. + let payload = unsafe { __rust_panic_cleanup(ex) }; + // SAFETY: `__rust_panic_cleanup` returns a Box. + let payload = unsafe { Box::from_raw(payload) }; + return CaughtUnwind::RustPanic(payload); } // We catch the exception by reference, so the C++ runtime will drop it. Tell our @@ -115,10 +101,7 @@ unsafe impl ThrowByValue for ActiveBackend { match intercept(func, catch) { Ok(value) => Ok(value), - Err(CaughtUnwind::LithiumException(cause)) => Err((cause, SehRethrowHandle)), - - #[cfg(feature = "std")] Err(CaughtUnwind::RustPanic(payload)) => throw_std_panic(payload), } } @@ -271,7 +254,7 @@ static THROW_INFO: ThrowInfo = ThrowInfo { }; fn abort_on_caught_by_cxx() -> ! { - abort("A Lithium exception was caught by a non-Lithium catch mechanism. This is undefined behavior. The process will now terminate."); + abort("A Lithium exception was caught by a non-Lithium catch mechanism. This is undefined behavior. The process will now terminate.\n"); } thiscall! { @@ -373,18 +356,17 @@ extern "system-unwind" { ) -> !; } -#[cfg(feature = "std")] +// This is provided by the `panic_unwind` built-in crate, so it's always available if +// panic = "unwind" holds extern "Rust" { fn __rust_start_panic(payload: &mut dyn PanicPayload) -> u32; } -#[cfg(feature = "std")] extern "C" { #[expect(improper_ctypes, reason = "Copied from std")] fn __rust_panic_cleanup(payload: *mut u8) -> *mut (dyn Any + Send + 'static); } -#[cfg(feature = "std")] fn throw_std_panic(payload: Box) -> ! { // We can't use resume_unwind here, as it increments the panic count, and we didn't decrement it // upon catching the panic. Call `__rust_start_panic` directly instead. diff --git a/src/lib.rs b/src/lib.rs index d7febff..a8b058f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,10 +81,7 @@ //! callbacks can only interact with exceptions in an isolated manner. #![no_std] -#![cfg_attr( - all(not(feature = "std"), not(backend = "unimplemented")), - feature(thread_local) -)] +#![cfg_attr(all(thread_local = "attribute"), feature(thread_local))] #![cfg_attr( any(backend = "itanium", backend = "seh", backend = "emscripten"), expect( @@ -164,7 +161,7 @@ #[cfg(panic = "abort")] compile_error!("Using Lithium with panic = \"abort\" is unsupported"); -#[cfg(any(feature = "std", test))] +#[cfg(any(abort = "std", backend = "panic", thread_local = "std", test))] extern crate std; extern crate alloc; @@ -189,12 +186,16 @@ pub use api::{catch, intercept, throw, InFlightException}; #[cold] #[inline(never)] fn abort(message: &str) -> ! { - #[cfg(feature = "std")] + #[cfg(abort = "std")] { - std::eprintln!("{message}"); + use std::io::Write; + let _ = std::io::stderr().write_all(message.as_bytes()); std::process::abort(); } - #[cfg(not(feature = "std"))] + + // This is a nightly-only method, but all three backends this is enabled under require nightly + // anyway, so this is no big deal. + #[cfg(not(abort = "std"))] { let _ = message; core::intrinsics::abort(); diff --git a/src/stacked_exceptions.rs b/src/stacked_exceptions.rs index 3cbff8a..21d78f9 100644 --- a/src/stacked_exceptions.rs +++ b/src/stacked_exceptions.rs @@ -133,13 +133,13 @@ impl Exception { } } -#[cfg(feature = "std")] +#[cfg(thread_local = "std")] std::thread_local! { /// Thread-local exception stack. static STACK: Stack
= const { Stack::new() }; } -#[cfg(not(feature = "std"))] +#[cfg(thread_local = "attribute")] #[thread_local] static STACK: Stack
= const { Stack::new() }; @@ -153,14 +153,18 @@ static STACK: Stack
= const { Stack::new() }; // to inline. #[inline] unsafe fn get_stack() -> &'static Stack
{ - #[cfg(feature = "std")] + #[cfg(thread_local = "std")] // SAFETY: We require the caller to not use the reference anywhere near the end of the thread, // so as long as `with` succeeds, there is no problem. return STACK.with(|r| unsafe { core::mem::transmute(r) }); - #[cfg(not(feature = "std"))] + + #[cfg(thread_local = "attribute")] // SAFETY: We require the caller to not use the reference anywhere near the end of the thread, // so if `&STACK` is sound in the first place, there is no problem. return unsafe { core::mem::transmute::<&Stack
, &'static Stack
>(&STACK) }; + + #[cfg(thread_local = "unimplemented")] + compile_error!("Unable to compile Lithium on a platform does not support thread locals") } const fn get_alloc_size() -> usize {