Skip to content

Commit

Permalink
Replace std feature with probing
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
purplesyringa committed Jan 24, 2025
1 parent a847645 commit 08cfcc9
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 84 deletions.
64 changes: 35 additions & 29 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion Cross.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[build.env]
passthrough = ["LITHIUM_BACKEND"]
passthrough = ["LITHIUM_BACKEND", "LITHIUM_THREAD_LOCAL"]

[target.mips-unknown-linux-gnu]
build-std = ["std"]
Expand Down
56 changes: 53 additions & 3 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}\"");
Expand All @@ -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\"");
}
}
1 change: 0 additions & 1 deletion src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/backend/emscripten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
2 changes: 1 addition & 1 deletion src/backend/itanium.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
1 change: 0 additions & 1 deletion src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 08cfcc9

Please sign in to comment.