Skip to content

Commit 12fc1e0

Browse files
authored
Rollup merge of #69984 - lenary:lenary/force-uwtables, r=hanna-kruppe
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 production 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 came up in discussion on #69890, and turned out to be a fairly simple addition. r? @hanna-kruppe
2 parents de27cd7 + cda9946 commit 12fc1e0

File tree

9 files changed

+89
-5
lines changed

9 files changed

+89
-5
lines changed

src/doc/rustc/src/codegen-options/index.md

+12
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ values:
9898
The default behaviour, if frame pointers are not force-enabled, depends on the
9999
target.
100100

101+
## force-unwind-tables
102+
103+
This flag forces the generation of unwind tables. It takes one of the following
104+
values:
105+
106+
* `y`, `yes`, `on`, or no value: Unwind tables are forced to be generated.
107+
* `n`, `no`, or `off`: Unwind tables are not forced to be generated. If unwind
108+
tables are required by the target or `-C panic=unwind`, an error will be
109+
emitted.
110+
111+
The default if not specified depends on the target.
112+
101113
## incremental
102114

103115
This flag allows you to enable incremental compilation, which allows `rustc`

src/librustc_codegen_llvm/allocator.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut ModuleLlvm, kind: Alloc
5454
if tcx.sess.target.target.options.default_hidden_visibility {
5555
llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden);
5656
}
57-
if tcx.sess.target.target.options.requires_uwtable {
57+
if tcx.sess.must_emit_unwind_tables() {
5858
attributes::emit_uwtable(llfn, true);
5959
}
6060

src/librustc_codegen_llvm/attributes.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use rustc_middle::ty::query::Providers;
1313
use rustc_middle::ty::{self, TyCtxt};
1414
use rustc_session::config::{OptLevel, Sanitizer};
1515
use rustc_session::Session;
16-
use rustc_target::spec::PanicStrategy;
1716

1817
use crate::attributes;
1918
use crate::llvm::AttributePlace::Function;
@@ -271,9 +270,7 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value, instance: ty::
271270
//
272271
// You can also find more info on why Windows is whitelisted here in:
273272
// https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
274-
if cx.sess().panic_strategy() == PanicStrategy::Unwind
275-
|| cx.sess().target.target.options.requires_uwtable
276-
{
273+
if cx.sess().must_emit_unwind_tables() {
277274
attributes::emit_uwtable(llfn, true);
278275
}
279276

src/librustc_interface/tests.rs

+1
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ fn test_codegen_options_tracking_hash() {
415415
tracked!(debuginfo, 0xdeadbeef);
416416
tracked!(embed_bitcode, false);
417417
tracked!(force_frame_pointers, Some(false));
418+
tracked!(force_unwind_tables, Some(true));
418419
tracked!(inline_threshold, Some(0xf007ba11));
419420
tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto);
420421
tracked!(llvm_args, vec![String::from("1"), String::from("2")]);

src/librustc_session/options.rs

+2
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
668668
"extra data to put in each output filename"),
669669
force_frame_pointers: Option<bool> = (None, parse_opt_bool, [TRACKED],
670670
"force use of the frame pointers"),
671+
force_unwind_tables: Option<bool> = (None, parse_opt_bool, [TRACKED],
672+
"force use of unwind tables"),
671673
incremental: Option<String> = (None, parse_opt_string, [UNTRACKED],
672674
"enable incremental compilation"),
673675
inline_threshold: Option<usize> = (None, parse_opt_uint, [TRACKED],

src/librustc_session/session.rs

+44
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,33 @@ impl Session {
646646
}
647647
}
648648

649+
pub fn must_emit_unwind_tables(&self) -> bool {
650+
// This is used to control the emission of the `uwtable` attribute on
651+
// LLVM functions.
652+
//
653+
// At the very least, unwind tables are needed when compiling with
654+
// `-C panic=unwind`.
655+
//
656+
// On some targets (including windows), however, exceptions include
657+
// other events such as illegal instructions, segfaults, etc. This means
658+
// that on Windows we end up still needing unwind tables even if the `-C
659+
// panic=abort` flag is passed.
660+
//
661+
// You can also find more info on why Windows needs unwind tables in:
662+
// https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
663+
//
664+
// If a target requires unwind tables, then they must be emitted.
665+
// Otherwise, we can defer to the `-C force-unwind-tables=<yes/no>`
666+
// value, if it is provided, or disable them, if not.
667+
if self.panic_strategy() == PanicStrategy::Unwind {
668+
true
669+
} else if self.target.target.options.requires_uwtable {
670+
true
671+
} else {
672+
self.opts.cg.force_unwind_tables.unwrap_or(false)
673+
}
674+
}
675+
649676
/// Returns the symbol name for the registrar function,
650677
/// given the crate `Svh` and the function `DefIndex`.
651678
pub fn generate_plugin_registrar_symbol(&self, disambiguator: CrateDisambiguator) -> String {
@@ -1224,6 +1251,23 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
12241251
}
12251252
}
12261253

1254+
// Unwind tables cannot be disabled if the target requires them.
1255+
if let Some(include_uwtables) = sess.opts.cg.force_unwind_tables {
1256+
if sess.panic_strategy() == PanicStrategy::Unwind && !include_uwtables {
1257+
sess.err(
1258+
"panic=unwind requires unwind tables, they cannot be disabled \
1259+
with `-C force-unwind-tables=no`.",
1260+
);
1261+
}
1262+
1263+
if sess.target.target.options.requires_uwtable && !include_uwtables {
1264+
sess.err(
1265+
"target requires unwind tables, they cannot be disabled with \
1266+
`-C force-unwind-tables=no`.",
1267+
);
1268+
}
1269+
}
1270+
12271271
// PGO does not work reliably with panic=unwind on Windows. Let's make it
12281272
// an error to combine the two for now. It always runs into an assertions
12291273
// if LLVM is built with assertions, but without assertions it sometimes
+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// min-llvm-version 8.0
2+
// compile-flags: -C no-prepopulate-passes -C force-unwind-tables=y
3+
4+
#![crate_type="lib"]
5+
6+
// CHECK: attributes #{{.*}} uwtable
7+
pub fn foo() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Tests that the compiler errors if the user tries to turn off unwind tables
2+
// when they are required.
3+
//
4+
// compile-flags: -C panic=unwind -C force-unwind-tables=no
5+
// ignore-tidy-linelength
6+
//
7+
// error-pattern: panic=unwind requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`.
8+
9+
pub fn main() {
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Tests that the compiler errors if the user tries to turn off unwind tables
2+
// when they are required.
3+
//
4+
// only-x86_64-windows-msvc
5+
// compile-flags: -C force-unwind-tables=no
6+
// ignore-tidy-linelength
7+
//
8+
// error-pattern: target requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`.
9+
10+
pub fn main() {
11+
}

0 commit comments

Comments
 (0)