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 Option to Force Unwind Tables #69984

Merged
merged 1 commit into from
May 5, 2020
Merged
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
12 changes: 12 additions & 0 deletions src/doc/rustc/src/codegen-options/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
5 changes: 1 addition & 4 deletions src/librustc_codegen_llvm/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use rustc_middle::ty::query::Providers;
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;
Expand Down Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_interface/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ fn test_codegen_options_tracking_hash() {
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")]);
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_session/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
"extra data to put in each output filename"),
force_frame_pointers: Option<bool> = (None, parse_opt_bool, [TRACKED],
"force use of the frame pointers"),
force_unwind_tables: Option<bool> = (None, parse_opt_bool, [TRACKED],
"force use of unwind tables"),
incremental: Option<String> = (None, parse_opt_string, [UNTRACKED],
"enable incremental compilation"),
inline_threshold: Option<usize> = (None, parse_opt_uint, [TRACKED],
Expand Down
44 changes: 44 additions & 0 deletions src/librustc_session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,33 @@ impl Session {
}
}

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=<yes/no>`
// 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 {
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/test/codegen/force-unwind-tables.rs
Original file line number Diff line number Diff line change
@@ -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() {}
10 changes: 10 additions & 0 deletions src/test/compile-fail/unwind-tables-panic-required.rs
Original file line number Diff line number Diff line change
@@ -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() {
}
11 changes: 11 additions & 0 deletions src/test/compile-fail/unwind-tables-target-required.rs
Original file line number Diff line number Diff line change
@@ -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() {
}