Skip to content

Commit 635c76b

Browse files
committed
Add new rustc_panic_abort_runtime attribute for libpanic_abort
Supersedes #66311 This replaces the hack in `bootstrap`, allowing the special handling for `libpanic_abort` to be encoded into the crate source itself, rather than existing as special knowledge in the build system. This will allow Miri (and any other users of Xargo) to correctly build `libpanic_abort` without relying on `bootstrap` or custom wrapepr hacks. The trickeist part of this PR is how we handle LLVM. The `emscripten` target family requires the "-enable-emscripten-cxx-exceptions" flag to be passed to LLVM when the panic strategy is set to "unwind". Unfortunately, the location of this emscripten-specific check ends up creating a circular dependency between LLVM and attribute resoltion. When we check the panic strategy, we need to have already parsed crate attributes, so that we determine if `rustc_panic_abort_runtime` was set. However, attribute parsing requires LLVM to be initialized, so that we can proerly handle cfg-gating of target-specific features. However, the whole point of checking the panic strategy is to determinne which flags to use during LLVM initialization! To break this circular dependency, we explicitly set the "-enable-emscripten-cxx-exceptions" in LLVM's argument-parsing framework (using public but poorly-documented APIs). While this approach is unfortunate, it only affects emscripten, and only modifies a command-line flag which is never used until much later (when we create a `PassManager`).
1 parent 0a440b1 commit 635c76b

File tree

12 files changed

+84
-12
lines changed

12 files changed

+84
-12
lines changed

src/bootstrap/bin/rustc.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,22 @@ fn main() {
9494
// other crate intentionally as this is the only crate for now that we
9595
// ship with panic=abort.
9696
//
97+
// This hack is being replaced with a new "rustc_panic_abort_runtime"
98+
// attribute, which moves the special-casing out of `bootstrap` and
99+
// into the compiler itself. Until this change makes its way into
100+
// the bootstrap compiler, we still need this hack when building
101+
// stage0. Once the boostrap compiler is updated, we can remove
102+
// this check entirely.
103+
// cfg(bootstrap): (Added to make this show up when searching for "cfg(bootstrap)")
104+
//
97105
// This... is a bit of a hack how we detect this. Ideally this
98106
// information should be encoded in the crate I guess? Would likely
99107
// require an RFC amendment to RFC 1513, however.
100108
//
101109
// `compiler_builtins` are unconditionally compiled with panic=abort to
102110
// workaround undefined references to `rust_eh_unwind_resume` generated
103111
// otherwise, see issue https://github.com/rust-lang/rust/issues/43095.
104-
if crate_name == Some("panic_abort") ||
112+
if (crate_name == Some("panic_abort") && stage == "0") ||
105113
crate_name == Some("compiler_builtins") && stage != "0" {
106114
cmd.arg("-C").arg("panic=abort");
107115
}

src/libpanic_abort/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/",
99
issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/")]
1010
#![panic_runtime]
11+
#![cfg_attr(not(bootstrap), rustc_panic_abort_runtime)]
1112

1213
#![allow(unused_features)]
1314

src/librustc_codegen_llvm/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,10 @@ impl CodegenBackend for LlvmCodegenBackend {
202202
llvm_util::init(sess); // Make sure llvm is inited
203203
}
204204

205+
fn after_expansion(&self, sess: &Session) {
206+
llvm_util::late_init(sess);
207+
}
208+
205209
fn print(&self, req: PrintRequest, sess: &Session) {
206210
match req {
207211
PrintRequest::RelocationModels => {

src/librustc_codegen_llvm/llvm/ffi.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1871,4 +1871,5 @@ extern "C" {
18711871
bytecode: *const c_char,
18721872
bytecode_len: usize) -> bool;
18731873
pub fn LLVMRustLinkerFree(linker: &'a mut Linker<'a>);
1874+
pub fn LLVMRustEnableEmscriptenCXXExceptions();
18741875
}

src/librustc_codegen_llvm/llvm_util.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ fn require_inited() {
4545
}
4646
}
4747

48+
pub(crate) fn late_init(sess: &Session) {
49+
if sess.target.target.target_os == "emscripten" &&
50+
sess.panic_strategy() == PanicStrategy::Unwind {
51+
unsafe { llvm::LLVMRustEnableEmscriptenCXXExceptions(); }
52+
}
53+
}
54+
4855
unsafe fn configure_llvm(sess: &Session) {
4956
let n_args = sess.opts.cg.llvm_args.len();
5057
let mut llvm_c_strs = Vec::with_capacity(n_args + 1);
@@ -95,11 +102,6 @@ unsafe fn configure_llvm(sess: &Session) {
95102
}
96103
}
97104

98-
if sess.target.target.target_os == "emscripten" &&
99-
sess.panic_strategy() == PanicStrategy::Unwind {
100-
add("-enable-emscripten-cxx-exceptions", false);
101-
}
102-
103105
// HACK(eddyb) LLVM inserts `llvm.assume` calls to preserve align attributes
104106
// during inlining. Unfortunately these may block other optimizations.
105107
add("-preserve-alignment-assumptions-during-inlining=false", false);

src/librustc_codegen_utils/codegen_backend.rs

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub use rustc_data_structures::sync::MetadataRef;
2121

2222
pub trait CodegenBackend {
2323
fn init(&self, _sess: &Session) {}
24+
fn after_expansion(&self, _sess: &Session) {}
2425
fn print(&self, _req: PrintRequest, _sess: &Session) {}
2526
fn target_features(&self, _sess: &Session) -> Vec<Symbol> { vec![] }
2627
fn print_passes(&self) {}

src/librustc_feature/builtin_attrs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
379379
rustc_attr!(rustc_allocator, Whitelisted, template!(Word), IMPL_DETAIL),
380380
rustc_attr!(rustc_allocator_nounwind, Whitelisted, template!(Word), IMPL_DETAIL),
381381
gated!(alloc_error_handler, Normal, template!(Word), experimental!(alloc_error_handler)),
382+
rustc_attr!(rustc_panic_abort_runtime, Whitelisted, template!(Word), IMPL_DETAIL),
382383
gated!(
383384
default_lib_allocator, Whitelisted, template!(Word), allocator_internals,
384385
experimental!(default_lib_allocator),

src/librustc_interface/passes.rs

+17-6
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use crate::interface::{Compiler, Result};
22
use crate::util;
33
use crate::proc_macro_decls;
44

5-
use log::{info, warn, log_enabled};
65
use rustc::arena::Arena;
6+
use log::{debug, info, warn, log_enabled};
77
use rustc::dep_graph::DepGraph;
88
use rustc::hir;
99
use rustc::hir::lowering::lower_crate;
@@ -39,7 +39,7 @@ use syntax::early_buffered_lints::BufferedEarlyLint;
3939
use syntax_expand::base::ExtCtxt;
4040
use syntax::mut_visit::MutVisitor;
4141
use syntax::util::node_count::NodeCounter;
42-
use syntax::symbol::Symbol;
42+
use syntax::symbol::{sym, Symbol};
4343
use syntax_pos::FileName;
4444
use syntax_ext;
4545

@@ -112,6 +112,7 @@ declare_box_region_type!(
112112
///
113113
/// Returns `None` if we're aborting after handling -W help.
114114
pub fn configure_and_expand(
115+
codegen_backend: Lrc<Box<dyn CodegenBackend>>,
115116
sess: Lrc<Session>,
116117
lint_store: Lrc<lint::LintStore>,
117118
metadata_loader: Box<MetadataLoaderDyn>,
@@ -123,15 +124,16 @@ pub fn configure_and_expand(
123124
// item, much like we do for macro expansion. In other words, the hash reflects not just
124125
// its contents but the results of name resolution on those contents. Hopefully we'll push
125126
// this back at some point.
126-
let crate_name = crate_name.to_string();
127+
let crate_name_inner = crate_name.to_string();
128+
let sess_inner = sess.clone();
127129
let (result, resolver) = BoxedResolver::new(static move || {
128-
let sess = &*sess;
129130
let resolver_arenas = Resolver::arenas();
130131
let res = configure_and_expand_inner(
131-
sess,
132+
&**codegen_backend,
133+
&sess_inner,
132134
&lint_store,
133135
krate,
134-
&crate_name,
136+
&crate_name_inner,
135137
&resolver_arenas,
136138
&*metadata_loader,
137139
);
@@ -227,6 +229,7 @@ pub fn register_plugins<'a>(
227229
}
228230

229231
fn configure_and_expand_inner<'a>(
232+
codegen_backend: &dyn CodegenBackend,
230233
sess: &'a Session,
231234
lint_store: &'a lint::LintStore,
232235
mut krate: ast::Crate,
@@ -346,6 +349,14 @@ fn configure_and_expand_inner<'a>(
346349
krate
347350
});
348351

352+
let force_panic_abort = krate.attrs.iter().any(|attr| {
353+
attr.check_name(sym::rustc_panic_abort_runtime)
354+
});
355+
debug!("Setting force_panic_abort for crate `{}`: {}", crate_name, force_panic_abort);
356+
sess.force_panic_abort.set(force_panic_abort);
357+
358+
codegen_backend.after_expansion(sess);
359+
349360
time(sess, "maybe building test harness", || {
350361
syntax_ext::test_harness::inject(
351362
&sess.parse_sess,

src/librustc_interface/queries.rs

+1
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ impl<'tcx> Queries<'tcx> {
186186
let crate_name = self.crate_name()?.peek().clone();
187187
let (krate, lint_store) = self.register_plugins()?.take();
188188
passes::configure_and_expand(
189+
self.codegen_backend().clone(),
189190
self.session().clone(),
190191
lint_store.clone(),
191192
self.codegen_backend().metadata_loader(),

src/librustc_session/session.rs

+10
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,11 @@ pub struct Session {
125125
/// false positives about a job server in our environment.
126126
pub jobserver: Client,
127127

128+
/// Whether or not to force the panic strategy for this
129+
/// crate to be PanicStrategy::Abort. Only used
130+
/// for 'libpanic_abort'
131+
pub force_panic_abort: Once<bool>,
132+
128133
/// Cap lint level specified by a driver specifically.
129134
pub driver_lint_caps: FxHashMap<lint::LintId, lint::Level>,
130135

@@ -543,6 +548,10 @@ impl Session {
543548
/// Returns the panic strategy for this compile session. If the user explicitly selected one
544549
/// using '-C panic', use that, otherwise use the panic strategy defined by the target.
545550
pub fn panic_strategy(&self) -> PanicStrategy {
551+
if *self.force_panic_abort.get() {
552+
return PanicStrategy::Abort
553+
}
554+
546555
self.opts
547556
.cg
548557
.panic
@@ -1164,6 +1173,7 @@ fn build_session_(
11641173
print_fuel_crate,
11651174
print_fuel,
11661175
jobserver: jobserver::client(),
1176+
force_panic_abort: Once::new(),
11671177
driver_lint_caps,
11681178
trait_methods_not_found: Lock::new(Default::default()),
11691179
confused_type_with_std_module: Lock::new(Default::default()),

src/libsyntax_pos/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@ symbols! {
630630
rustc_object_lifetime_default,
631631
rustc_on_unimplemented,
632632
rustc_outlives,
633+
rustc_panic_abort_runtime,
633634
rustc_paren_sugar,
634635
rustc_partition_codegened,
635636
rustc_partition_reused,

src/rustllvm/RustWrapper.cpp

+31
Original file line numberDiff line numberDiff line change
@@ -1537,3 +1537,34 @@ extern "C" LLVMValueRef
15371537
LLVMRustBuildMaxNum(LLVMBuilderRef B, LLVMValueRef LHS, LLVMValueRef RHS) {
15381538
return wrap(unwrap(B)->CreateMaxNum(unwrap(LHS),unwrap(RHS)));
15391539
}
1540+
1541+
extern "C" void
1542+
LLVMRustEnableEmscriptenCXXExceptions() {
1543+
// This is a little sketchy. Ideally, we would just pass
1544+
// '-enable-emscripten-cxx-exceptions' along with all
1545+
// of our other LLVM arguments when we initialize LLVM.
1546+
// Unfortunately, whether or not we pass this flag depends
1547+
// on the crate panic strategy. Determining the crate
1548+
// panic strategy requires us to have paresed crate arributes
1549+
// (so that we can have special handling for `libpanic_abort`).
1550+
// Parsing crate attributes actually requires us to have initialized
1551+
// LLVM, so that we can handle cfg-gating involving LLVM target
1552+
// features.
1553+
//
1554+
// We break this circular dependency by manually enabling
1555+
// "enable-emscripten-cxx-exceptions" after we've initialized
1556+
// LLVM. The methods involved are not well-documented - however,
1557+
// the flag we are modifiying ("enable-emscripten-cxx-exceptions")
1558+
// is only used in one place, and only when a PassManger is created.
1559+
// Thus, enabling it later than normal should have no visible effects.
1560+
//
1561+
// If this logic ever becomes incorrect (e.g. due to an LLVM upgrade),
1562+
// it should cause panic-related tests to start failing under emscripten,
1563+
// since they require this flag for proper unwinding support.
1564+
StringMap<cl::Option*> &Map = cl::getRegisteredOptions();
1565+
Map["enable-emscripten-cxx-exceptions"]->addOccurrence(
1566+
0,
1567+
StringRef("enable-emscripten-cxx-exceptions"),
1568+
StringRef("")
1569+
);
1570+
}

0 commit comments

Comments
 (0)