Skip to content

Commit

Permalink
Rollup merge of rust-lang#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
Dylan-DPC committed Apr 4, 2021
2 parents d5806c7 + d1c591b commit 5cc3d34
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 29 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

#![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!();
}
8 changes: 6 additions & 2 deletions src/test/codegen/force-no-unwind-tables.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
// compile-flags: -C no-prepopulate-passes -C panic=abort -C force-unwind-tables=n
// compile-flags: -C no-prepopulate-passes -C force-unwind-tables=n
// ignore-windows

#![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 no-prepopulate-passes

#![crate_type = "lib"]

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

This file was deleted.

33 changes: 33 additions & 0 deletions src/test/ui/unwind-no-uwtable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// run-pass
// ignore-windows target requires uwtable
// 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 5cc3d34

Please sign in to comment.