From cda994633ee109639b9c4c12c20e2aacb6a879cd Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Mon, 4 May 2020 12:08:35 +0100 Subject: [PATCH] Add Option to Force Unwind Tables When panic != unwind, `nounwind` is added to all functions for a target. This can cause issues when a panic happens with RUST_BACKTRACE=1, as there needs to be a way to reconstruct the backtrace. There are three possible sources of this information: forcing frame pointers (for which an option exists already), debug info (for which an option exists), or unwind tables. Especially for embedded devices, forcing frame pointers can have code size overheads (RISC-V sees ~10% overheads, ARM sees ~2-3% overheads). In code, it can be the case that debug info is not kept, so it is useful to provide this third option, unwind tables, that users can use to reconstruct the call stack. Reconstructing this stack is harder than with frame pointers, but it is still possible. This commit adds a compiler option which allows a user to force the addition of unwind tables. Unwind tables cannot be disabled on targets that require them for correctness, or when using `-C panic=unwind`. --- src/doc/rustc/src/codegen-options/index.md | 12 +++++ src/librustc_codegen_llvm/allocator.rs | 2 +- src/librustc_codegen_llvm/attributes.rs | 5 +-- src/librustc_interface/tests.rs | 1 + src/librustc_session/options.rs | 2 + src/librustc_session/session.rs | 44 +++++++++++++++++++ src/test/codegen/force-unwind-tables.rs | 7 +++ .../unwind-tables-panic-required.rs | 10 +++++ .../unwind-tables-target-required.rs | 11 +++++ 9 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 src/test/codegen/force-unwind-tables.rs create mode 100644 src/test/compile-fail/unwind-tables-panic-required.rs create mode 100644 src/test/compile-fail/unwind-tables-target-required.rs diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 08b5ab10817..4ffa6207b97 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -98,6 +98,18 @@ values: The default behaviour, if frame pointers are not force-enabled, depends on the target. +## force-unwind-tables + +This flag forces the generation of unwind tables. It takes one of the following +values: + +* `y`, `yes`, `on`, or no value: Unwind tables are forced to be generated. +* `n`, `no`, or `off`: Unwind tables are not forced to be generated. If unwind + tables are required by the target or `-C panic=unwind`, an error will be + emitted. + +The default if not specified depends on the target. + ## incremental This flag allows you to enable incremental compilation, which allows `rustc` diff --git a/src/librustc_codegen_llvm/allocator.rs b/src/librustc_codegen_llvm/allocator.rs index a78546571e2..bc1d9e1818c 100644 --- a/src/librustc_codegen_llvm/allocator.rs +++ b/src/librustc_codegen_llvm/allocator.rs @@ -54,7 +54,7 @@ pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut ModuleLlvm, kind: Alloc if tcx.sess.target.target.options.default_hidden_visibility { llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); } - if tcx.sess.target.target.options.requires_uwtable { + if tcx.sess.must_emit_unwind_tables() { attributes::emit_uwtable(llfn, true); } diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index fc357ebb05d..64412843f6d 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -13,7 +13,6 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_session::config::{OptLevel, Sanitizer}; use rustc_session::Session; -use rustc_target::spec::PanicStrategy; use crate::attributes; use crate::llvm::AttributePlace::Function; @@ -271,9 +270,7 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value, instance: ty:: // // You can also find more info on why Windows is whitelisted here in: // https://bugzilla.mozilla.org/show_bug.cgi?id=1302078 - if cx.sess().panic_strategy() == PanicStrategy::Unwind - || cx.sess().target.target.options.requires_uwtable - { + if cx.sess().must_emit_unwind_tables() { attributes::emit_uwtable(llfn, true); } diff --git a/src/librustc_interface/tests.rs b/src/librustc_interface/tests.rs index 0a200426e38..f600b1dbf54 100644 --- a/src/librustc_interface/tests.rs +++ b/src/librustc_interface/tests.rs @@ -415,6 +415,7 @@ macro_rules! tracked { tracked!(debuginfo, 0xdeadbeef); tracked!(embed_bitcode, false); tracked!(force_frame_pointers, Some(false)); + tracked!(force_unwind_tables, Some(true)); tracked!(inline_threshold, Some(0xf007ba11)); tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto); tracked!(llvm_args, vec![String::from("1"), String::from("2")]); diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index b03fc00d93d..984d47956ca 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -668,6 +668,8 @@ fn parse_src_file_hash(slot: &mut Option, v: Option<&st "extra data to put in each output filename"), force_frame_pointers: Option = (None, parse_opt_bool, [TRACKED], "force use of the frame pointers"), + force_unwind_tables: Option = (None, parse_opt_bool, [TRACKED], + "force use of unwind tables"), incremental: Option = (None, parse_opt_string, [UNTRACKED], "enable incremental compilation"), inline_threshold: Option = (None, parse_opt_uint, [TRACKED], diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 69e1b46de4d..871893d4565 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -601,6 +601,33 @@ pub fn must_not_eliminate_frame_pointers(&self) -> bool { } } + pub fn must_emit_unwind_tables(&self) -> bool { + // This is used to control the emission of the `uwtable` attribute on + // LLVM functions. + // + // At the very least, unwind tables are needed when compiling with + // `-C panic=unwind`. + // + // On some targets (including windows), however, exceptions include + // other events such as illegal instructions, segfaults, etc. This means + // that on Windows we end up still needing unwind tables even if the `-C + // panic=abort` flag is passed. + // + // You can also find more info on why Windows needs unwind tables in: + // https://bugzilla.mozilla.org/show_bug.cgi?id=1302078 + // + // If a target requires unwind tables, then they must be emitted. + // Otherwise, we can defer to the `-C force-unwind-tables=` + // value, if it is provided, or disable them, if not. + if self.panic_strategy() == PanicStrategy::Unwind { + true + } else if self.target.target.options.requires_uwtable { + true + } else { + self.opts.cg.force_unwind_tables.unwrap_or(false) + } + } + /// Returns the symbol name for the registrar function, /// given the crate `Svh` and the function `DefIndex`. pub fn generate_plugin_registrar_symbol(&self, disambiguator: CrateDisambiguator) -> String { @@ -1178,6 +1205,23 @@ fn validate_commandline_args_with_session_available(sess: &Session) { } } + // Unwind tables cannot be disabled if the target requires them. + if let Some(include_uwtables) = sess.opts.cg.force_unwind_tables { + if sess.panic_strategy() == PanicStrategy::Unwind && !include_uwtables { + sess.err( + "panic=unwind requires unwind tables, they cannot be disabled \ + with `-C force-unwind-tables=no`.", + ); + } + + if sess.target.target.options.requires_uwtable && !include_uwtables { + sess.err( + "target requires unwind tables, they cannot be disabled with \ + `-C force-unwind-tables=no`.", + ); + } + } + // PGO does not work reliably with panic=unwind on Windows. Let's make it // an error to combine the two for now. It always runs into an assertions // if LLVM is built with assertions, but without assertions it sometimes diff --git a/src/test/codegen/force-unwind-tables.rs b/src/test/codegen/force-unwind-tables.rs new file mode 100644 index 00000000000..fbaf38d69df --- /dev/null +++ b/src/test/codegen/force-unwind-tables.rs @@ -0,0 +1,7 @@ +// min-llvm-version 8.0 +// compile-flags: -C no-prepopulate-passes -C force-unwind-tables=y + +#![crate_type="lib"] + +// CHECK: attributes #{{.*}} uwtable +pub fn foo() {} diff --git a/src/test/compile-fail/unwind-tables-panic-required.rs b/src/test/compile-fail/unwind-tables-panic-required.rs new file mode 100644 index 00000000000..314d9e778d5 --- /dev/null +++ b/src/test/compile-fail/unwind-tables-panic-required.rs @@ -0,0 +1,10 @@ +// Tests that the compiler errors if the user tries to turn off unwind tables +// when they are required. +// +// compile-flags: -C panic=unwind -C force-unwind-tables=no +// ignore-tidy-linelength +// +// error-pattern: panic=unwind requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`. + +pub fn main() { +} diff --git a/src/test/compile-fail/unwind-tables-target-required.rs b/src/test/compile-fail/unwind-tables-target-required.rs new file mode 100644 index 00000000000..14c17893764 --- /dev/null +++ b/src/test/compile-fail/unwind-tables-target-required.rs @@ -0,0 +1,11 @@ +// Tests that the compiler errors if the user tries to turn off unwind tables +// when they are required. +// +// only-x86_64-windows-msvc +// compile-flags: -C force-unwind-tables=no +// ignore-tidy-linelength +// +// error-pattern: target requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`. + +pub fn main() { +}