Skip to content

Commit

Permalink
Add internal RUSTC_INTERNAL_FORCE_PANIC_ABORT env var for `libpanic…
Browse files Browse the repository at this point in the history
…_abort`

Currently, we require `libpanic_abort` to always be build with the
`abort` panic strategy, regardless of what panic strategy would
normally be passed to `rustc` (e.g. `unwind` when building `libstd`).

This ensures that when downstream crates build against `libpanic_abort`,
it is properly detected as the panic runtime for the `abort` strategy.

Previously, this was done by special-casing `libpanic_abort` in
bootstrap. This meant that building `libpanic_abort` without using
`x.py` (e.g by using `xargo`) would result in the wrong panic strategy
being applied, leading to an error when trying to build against it:

```
error: the crate `panic_abort` does not have the panic strategy `abort`
```

This is a problem for tools like Miri, which require a custom-build
libstd, and cannot use `x.py`.

To fix this, we add a special environment variable
`RUSTC_INTERNAL_FORCE_PANIC_ABORT`. This is set in the `build.rs`
for `libpanic_abort`, and checked by the compiler when determining the
panic strategy of the current crate. While this is still a hack, it's a
much better one - the special case is now represented in the
`libpanic_abort` crate itself, rather than in `bootstrap`.

Ideally, this would be an internal attribute (e.g.
`#[rustc_panic_abort]`) that we apply to `libpanic_abort`.

Unfortunately, `emscripten` targets require that we be able to determine
the panic strategy very early on, before we've even started parsing the
crate:

https://github.com/rust-lang/rust/blob/3fc30d884ae0c988d98452a06737705cfe34806a/src/librustc_codegen_llvm/llvm_util.rs#L77

To avoid invasive changes to emscripten and/or the codewgen backend
infrastructure, I chose to add a new environment variable.

Note that the hack in bootstrap needs to remain until this changes makes
its way into the bootstrap compiler, to ensure that we can still build a
correct `libpanic_abort` with a bootstrap compiler that doesn't know
about this special environment variable.
  • Loading branch information
Aaron1011 committed Nov 11, 2019
1 parent 56237d7 commit 5a826c7
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/bootstrap/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ fn main() {
// other crate intentionally as this is the only crate for now that we
// ship with panic=abort.
//
// FIXME: Remove the special case for "panic_abort"
// once https://github.com/rust-lang/rust/pull/60026
// rides the release train into the bootstrap compiler.
// `cfg(bootstrap)`: (added to make this easier to grep for)
//
// This... is a bit of a hack how we detect this. Ideally this
// information should be encoded in the crate I guess? Would likely
// require an RFC amendment to RFC 1513, however.
Expand Down
7 changes: 7 additions & 0 deletions src/libpanic_abort/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
// Hack to force this crate to be compiled with the `abort`
// panic strategy, regardless of what strategy Cargo
// passes to the compiler.
// See `rustc::session::Session::panic_strategy` for more details
println!("cargo:rustc-env=RUSTC_INTERNAL_FORCE_PANIC_ABORT=1");
}
12 changes: 12 additions & 0 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,18 @@ impl Session {
/// Returns the panic strategy for this compile session. If the user explicitly selected one
/// using '-C panic', use that, otherwise use the panic strategy defined by the target.
pub fn panic_strategy(&self) -> PanicStrategy {
// This is a hack to support `libpanic_abort`. We require `libpanic_abort`
// to always be built with `PanicStrategy::Abort`, regardless of what panic
// strategy is suppplied to the compiler. Ideally, this would be an intenral
// attribute in `libpanic_abort` - howevver, emscripten needs to access
// the panic strategy before we even start parsing the crate:
// https://github.com/rust-lang/rust/blob/3fc30d8/src/librustc_codegen_llvm/llvm_util.rs#L77
//
// As a result, we need to use a special environment variable, which is set from
// the `build.rs` of `libpanic_abort`
if env::var("RUSTC_INTERNAL_FORCE_PANIC_ABORT").is_ok() {
return PanicStrategy::Abort;
}
self.opts
.cg
.panic
Expand Down

0 comments on commit 5a826c7

Please sign in to comment.