From 635c76b5bfa8799746b197787332460f52407ddb Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 15 Nov 2019 17:06:52 -0500 Subject: [PATCH] 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`). --- src/bootstrap/bin/rustc.rs | 10 +++++- src/libpanic_abort/lib.rs | 1 + src/librustc_codegen_llvm/lib.rs | 4 +++ src/librustc_codegen_llvm/llvm/ffi.rs | 1 + src/librustc_codegen_llvm/llvm_util.rs | 12 ++++--- src/librustc_codegen_utils/codegen_backend.rs | 1 + src/librustc_feature/builtin_attrs.rs | 1 + src/librustc_interface/passes.rs | 23 ++++++++++---- src/librustc_interface/queries.rs | 1 + src/librustc_session/session.rs | 10 ++++++ src/libsyntax_pos/symbol.rs | 1 + src/rustllvm/RustWrapper.cpp | 31 +++++++++++++++++++ 12 files changed, 84 insertions(+), 12 deletions(-) diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index 475f2e904639c..8ab767af33238 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -94,6 +94,14 @@ fn main() { // other crate intentionally as this is the only crate for now that we // ship with panic=abort. // + // This hack is being replaced with a new "rustc_panic_abort_runtime" + // attribute, which moves the special-casing out of `bootstrap` and + // into the compiler itself. Until this change makes its way into + // the bootstrap compiler, we still need this hack when building + // stage0. Once the boostrap compiler is updated, we can remove + // this check entirely. + // cfg(bootstrap): (Added to make this show up when searching for "cfg(bootstrap)") + // // 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. @@ -101,7 +109,7 @@ fn main() { // `compiler_builtins` are unconditionally compiled with panic=abort to // workaround undefined references to `rust_eh_unwind_resume` generated // otherwise, see issue https://github.com/rust-lang/rust/issues/43095. - if crate_name == Some("panic_abort") || + if (crate_name == Some("panic_abort") && stage == "0") || crate_name == Some("compiler_builtins") && stage != "0" { cmd.arg("-C").arg("panic=abort"); } diff --git a/src/libpanic_abort/lib.rs b/src/libpanic_abort/lib.rs index 5509f47bc8858..2d0da0e02b240 100644 --- a/src/libpanic_abort/lib.rs +++ b/src/libpanic_abort/lib.rs @@ -8,6 +8,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/", issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/")] #![panic_runtime] +#![cfg_attr(not(bootstrap), rustc_panic_abort_runtime)] #![allow(unused_features)] diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index 1e1d74cfa9a40..84dd559db3f1c 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -202,6 +202,10 @@ impl CodegenBackend for LlvmCodegenBackend { llvm_util::init(sess); // Make sure llvm is inited } + fn after_expansion(&self, sess: &Session) { + llvm_util::late_init(sess); + } + fn print(&self, req: PrintRequest, sess: &Session) { match req { PrintRequest::RelocationModels => { diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index b8a1003b11866..387178fd32ef3 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1871,4 +1871,5 @@ extern "C" { bytecode: *const c_char, bytecode_len: usize) -> bool; pub fn LLVMRustLinkerFree(linker: &'a mut Linker<'a>); + pub fn LLVMRustEnableEmscriptenCXXExceptions(); } diff --git a/src/librustc_codegen_llvm/llvm_util.rs b/src/librustc_codegen_llvm/llvm_util.rs index 3145b0df63b8a..d080ae753eae2 100644 --- a/src/librustc_codegen_llvm/llvm_util.rs +++ b/src/librustc_codegen_llvm/llvm_util.rs @@ -45,6 +45,13 @@ fn require_inited() { } } +pub(crate) fn late_init(sess: &Session) { + if sess.target.target.target_os == "emscripten" && + sess.panic_strategy() == PanicStrategy::Unwind { + unsafe { llvm::LLVMRustEnableEmscriptenCXXExceptions(); } + } +} + unsafe fn configure_llvm(sess: &Session) { let n_args = sess.opts.cg.llvm_args.len(); let mut llvm_c_strs = Vec::with_capacity(n_args + 1); @@ -95,11 +102,6 @@ unsafe fn configure_llvm(sess: &Session) { } } - if sess.target.target.target_os == "emscripten" && - sess.panic_strategy() == PanicStrategy::Unwind { - add("-enable-emscripten-cxx-exceptions", false); - } - // HACK(eddyb) LLVM inserts `llvm.assume` calls to preserve align attributes // during inlining. Unfortunately these may block other optimizations. add("-preserve-alignment-assumptions-during-inlining=false", false); diff --git a/src/librustc_codegen_utils/codegen_backend.rs b/src/librustc_codegen_utils/codegen_backend.rs index 0e2c3731eae6d..5db1afc830908 100644 --- a/src/librustc_codegen_utils/codegen_backend.rs +++ b/src/librustc_codegen_utils/codegen_backend.rs @@ -21,6 +21,7 @@ pub use rustc_data_structures::sync::MetadataRef; pub trait CodegenBackend { fn init(&self, _sess: &Session) {} + fn after_expansion(&self, _sess: &Session) {} fn print(&self, _req: PrintRequest, _sess: &Session) {} fn target_features(&self, _sess: &Session) -> Vec { vec![] } fn print_passes(&self) {} diff --git a/src/librustc_feature/builtin_attrs.rs b/src/librustc_feature/builtin_attrs.rs index 0a4fb8a224eec..2883d37ed8aa2 100644 --- a/src/librustc_feature/builtin_attrs.rs +++ b/src/librustc_feature/builtin_attrs.rs @@ -379,6 +379,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_attr!(rustc_allocator, Whitelisted, template!(Word), IMPL_DETAIL), rustc_attr!(rustc_allocator_nounwind, Whitelisted, template!(Word), IMPL_DETAIL), gated!(alloc_error_handler, Normal, template!(Word), experimental!(alloc_error_handler)), + rustc_attr!(rustc_panic_abort_runtime, Whitelisted, template!(Word), IMPL_DETAIL), gated!( default_lib_allocator, Whitelisted, template!(Word), allocator_internals, experimental!(default_lib_allocator), diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 2a4bc41f85072..911d73ce2b6cb 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -2,8 +2,8 @@ use crate::interface::{Compiler, Result}; use crate::util; use crate::proc_macro_decls; -use log::{info, warn, log_enabled}; use rustc::arena::Arena; +use log::{debug, info, warn, log_enabled}; use rustc::dep_graph::DepGraph; use rustc::hir; use rustc::hir::lowering::lower_crate; @@ -39,7 +39,7 @@ use syntax::early_buffered_lints::BufferedEarlyLint; use syntax_expand::base::ExtCtxt; use syntax::mut_visit::MutVisitor; use syntax::util::node_count::NodeCounter; -use syntax::symbol::Symbol; +use syntax::symbol::{sym, Symbol}; use syntax_pos::FileName; use syntax_ext; @@ -112,6 +112,7 @@ declare_box_region_type!( /// /// Returns `None` if we're aborting after handling -W help. pub fn configure_and_expand( + codegen_backend: Lrc>, sess: Lrc, lint_store: Lrc, metadata_loader: Box, @@ -123,15 +124,16 @@ pub fn configure_and_expand( // item, much like we do for macro expansion. In other words, the hash reflects not just // its contents but the results of name resolution on those contents. Hopefully we'll push // this back at some point. - let crate_name = crate_name.to_string(); + let crate_name_inner = crate_name.to_string(); + let sess_inner = sess.clone(); let (result, resolver) = BoxedResolver::new(static move || { - let sess = &*sess; let resolver_arenas = Resolver::arenas(); let res = configure_and_expand_inner( - sess, + &**codegen_backend, + &sess_inner, &lint_store, krate, - &crate_name, + &crate_name_inner, &resolver_arenas, &*metadata_loader, ); @@ -227,6 +229,7 @@ pub fn register_plugins<'a>( } fn configure_and_expand_inner<'a>( + codegen_backend: &dyn CodegenBackend, sess: &'a Session, lint_store: &'a lint::LintStore, mut krate: ast::Crate, @@ -346,6 +349,14 @@ fn configure_and_expand_inner<'a>( krate }); + let force_panic_abort = krate.attrs.iter().any(|attr| { + attr.check_name(sym::rustc_panic_abort_runtime) + }); + debug!("Setting force_panic_abort for crate `{}`: {}", crate_name, force_panic_abort); + sess.force_panic_abort.set(force_panic_abort); + + codegen_backend.after_expansion(sess); + time(sess, "maybe building test harness", || { syntax_ext::test_harness::inject( &sess.parse_sess, diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index e429b4d101a90..00e370cae491e 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -186,6 +186,7 @@ impl<'tcx> Queries<'tcx> { let crate_name = self.crate_name()?.peek().clone(); let (krate, lint_store) = self.register_plugins()?.take(); passes::configure_and_expand( + self.codegen_backend().clone(), self.session().clone(), lint_store.clone(), self.codegen_backend().metadata_loader(), diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 150d207e5bfe9..91edac9d05cff 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -125,6 +125,11 @@ pub struct Session { /// false positives about a job server in our environment. pub jobserver: Client, + /// Whether or not to force the panic strategy for this + /// crate to be PanicStrategy::Abort. Only used + /// for 'libpanic_abort' + pub force_panic_abort: Once, + /// Cap lint level specified by a driver specifically. pub driver_lint_caps: FxHashMap, @@ -543,6 +548,10 @@ 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 { + if *self.force_panic_abort.get() { + return PanicStrategy::Abort + } + self.opts .cg .panic @@ -1164,6 +1173,7 @@ fn build_session_( print_fuel_crate, print_fuel, jobserver: jobserver::client(), + force_panic_abort: Once::new(), driver_lint_caps, trait_methods_not_found: Lock::new(Default::default()), confused_type_with_std_module: Lock::new(Default::default()), diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index d3e80fc4fddd6..3b4d2d0caf891 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -630,6 +630,7 @@ symbols! { rustc_object_lifetime_default, rustc_on_unimplemented, rustc_outlives, + rustc_panic_abort_runtime, rustc_paren_sugar, rustc_partition_codegened, rustc_partition_reused, diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 720928e48e382..13387e555c762 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -1537,3 +1537,34 @@ extern "C" LLVMValueRef LLVMRustBuildMaxNum(LLVMBuilderRef B, LLVMValueRef LHS, LLVMValueRef RHS) { return wrap(unwrap(B)->CreateMaxNum(unwrap(LHS),unwrap(RHS))); } + +extern "C" void +LLVMRustEnableEmscriptenCXXExceptions() { + // This is a little sketchy. Ideally, we would just pass + // '-enable-emscripten-cxx-exceptions' along with all + // of our other LLVM arguments when we initialize LLVM. + // Unfortunately, whether or not we pass this flag depends + // on the crate panic strategy. Determining the crate + // panic strategy requires us to have paresed crate arributes + // (so that we can have special handling for `libpanic_abort`). + // Parsing crate attributes actually requires us to have initialized + // LLVM, so that we can handle cfg-gating involving LLVM target + // features. + // + // We break this circular dependency by manually enabling + // "enable-emscripten-cxx-exceptions" after we've initialized + // LLVM. The methods involved are not well-documented - however, + // the flag we are modifiying ("enable-emscripten-cxx-exceptions") + // is only used in one place, and only when a PassManger is created. + // Thus, enabling it later than normal should have no visible effects. + // + // If this logic ever becomes incorrect (e.g. due to an LLVM upgrade), + // it should cause panic-related tests to start failing under emscripten, + // since they require this flag for proper unwinding support. + StringMap &Map = cl::getRegisteredOptions(); + Map["enable-emscripten-cxx-exceptions"]->addOccurrence( + 0, + StringRef("enable-emscripten-cxx-exceptions"), + StringRef("") + ); +}