Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new rustc_panic_abort_runtime attribute for libpanic_abort #66489

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/bootstrap/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,22 @@ 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.
//
// `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");
}
Expand Down
1 change: 1 addition & 0 deletions src/libpanic_abort/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this in turn break cargo's -Zbuild-std which builds this crate with the unwind strategy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building this crate with the unwind strategy is the wrong behavior - it should always be built with the abort strategy. So, this will correctly change how -Zbuild-std builds this crate - but it's a bugfix, not breakage.


#![allow(unused_features)]

Expand Down
4 changes: 4 additions & 0 deletions src/librustc_codegen_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_llvm/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
12 changes: 7 additions & 5 deletions src/librustc_codegen_llvm/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_utils/codegen_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Symbol> { vec![] }
fn print_passes(&self) {}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_feature/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
23 changes: 17 additions & 6 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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<Box<dyn CodegenBackend>>,
sess: Lrc<Session>,
lint_store: Lrc<lint::LintStore>,
metadata_loader: Box<MetadataLoaderDyn>,
Expand All @@ -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,
);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_interface/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>,

/// Cap lint level specified by a driver specifically.
pub driver_lint_caps: FxHashMap<lint::LintId, lint::Level>,

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()),
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax_pos/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
31 changes: 31 additions & 0 deletions src/rustllvm/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 parsed crate attributes
// (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<cl::Option*> &Map = cl::getRegisteredOptions();
Map["enable-emscripten-cxx-exceptions"]->addOccurrence(
0,
StringRef("enable-emscripten-cxx-exceptions"),
StringRef("")
);
}