Skip to content

Commit

Permalink
Auto merge of #83482 - hyd-dev:uwtable, r=nagisa
Browse files Browse the repository at this point in the history
Allow using `-C force-unwind-tables=no` when `panic=unwind`

It seems LLVM still generates proper unwind tables even there is no `uwtable` attribute, unless I looked at the wrong place 🤔:
https://github.com/llvm/llvm-project/blob/c21016715f0ee4a36affdf7150ac135ca98b0eae/llvm/include/llvm/IR/Function.h#L666

Therefore, I *assume* it's safe to omit `uwtable` even when `panic=unwind`, and this PR removes the restriction that disallows using `-C force-unwind-tables=no` when `panic=unwind`.
  • Loading branch information
bors committed Apr 11, 2021
2 parents 7953910 + 2fd4dd2 commit a866124
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 27 deletions.
25 changes: 9 additions & 16 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,8 +807,11 @@ impl Session {
// 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`.
// Unwind tables are needed when compiling with `-C panic=unwind`, but
// LLVM won't omit unwind tables unless the function is also marked as
// `nounwind`, so users are allowed to disable `uwtable` emission.
// Historically rustc always emits `uwtable` attributes by default, so
// even they can be disabled, they're still emitted by default.
//
// On some targets (including windows), however, exceptions include
// other events such as illegal instructions, segfaults, etc. This means
Expand All @@ -821,13 +824,10 @@ impl Session {
// 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.requires_uwtable {
true
} else {
self.opts.cg.force_unwind_tables.unwrap_or(self.target.default_uwtable)
}
self.target.requires_uwtable
|| self.opts.cg.force_unwind_tables.unwrap_or(
self.panic_strategy() == PanicStrategy::Unwind || self.target.default_uwtable,
)
}

/// Returns the symbol name for the registrar function,
Expand Down Expand Up @@ -1483,13 +1483,6 @@ 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.requires_uwtable && !include_uwtables {
sess.err(
"target requires unwind tables, they cannot be disabled with \
Expand Down
8 changes: 8 additions & 0 deletions src/test/assembly/panic-no-unwind-no-uwtable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// assembly-output: emit-asm
// only-x86_64-unknown-linux-gnu
// compile-flags: -C panic=unwind -C force-unwind-tables=n -O

#![crate_type = "lib"]

// CHECK-NOT: .cfi_startproc
pub fn foo() {}
12 changes: 12 additions & 0 deletions src/test/assembly/panic-unwind-no-uwtable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// assembly-output: emit-asm
// only-x86_64-unknown-linux-gnu
// compile-flags: -C panic=unwind -C force-unwind-tables=n

#![crate_type = "lib"]

// CHECK-LABEL: foo:
// CHECK: .cfi_startproc
#[no_mangle]
fn foo() {
panic!();
}
6 changes: 5 additions & 1 deletion src/test/codegen/force-no-unwind-tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,9 @@

#![crate_type="lib"]

// CHECK-LABEL: define{{.*}}void @foo
// CHECK-NOT: attributes #{{.*}} uwtable
pub fn foo() {}
#[no_mangle]
fn foo() {
panic!();
}
6 changes: 6 additions & 0 deletions src/test/codegen/panic-unwind-default-uwtable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// compile-flags: -C panic=unwind -C no-prepopulate-passes

#![crate_type = "lib"]

// CHECK: attributes #{{.*}} uwtable
pub fn foo() {}
10 changes: 0 additions & 10 deletions src/test/ui/panic-runtime/unwind-tables-panic-required.rs

This file was deleted.

34 changes: 34 additions & 0 deletions src/test/ui/unwind-no-uwtable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// run-pass
// ignore-windows target requires uwtable
// ignore-wasm32-bare no proper panic=unwind support
// compile-flags: -C panic=unwind -C force-unwind-tables=n

use std::panic::{self, AssertUnwindSafe};

struct Increase<'a>(&'a mut u8);

impl Drop for Increase<'_> {
fn drop(&mut self) {
*self.0 += 1;
}
}

#[inline(never)]
fn unwind() {
panic!();
}

#[inline(never)]
fn increase(count: &mut u8) {
let _increase = Increase(count);
unwind();
}

fn main() {
let mut count = 0;
assert!(panic::catch_unwind(AssertUnwindSafe(
#[inline(never)]
|| increase(&mut count)
)).is_err());
assert_eq!(count, 1);
}

0 comments on commit a866124

Please sign in to comment.