about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-04-11 19:50:19 +0000
committerbors <bors@rust-lang.org>2021-04-11 19:50:19 +0000
commita8661245649f3d1c0dc5b23270bdac0bbd2d8f64 (patch)
tree2745fde13d5cd97d43de9a28e521956fb8cb050e
parent7953910464e073eb3876d1544a3fd5b5ba0ca49b (diff)
parent2fd4dd20d717b3e8af4bdff2873b348920426425 (diff)
downloadrust-a8661245649f3d1c0dc5b23270bdac0bbd2d8f64.tar.gz
rust-a8661245649f3d1c0dc5b23270bdac0bbd2d8f64.zip
Auto merge of #83482 - hyd-dev:uwtable, r=nagisa
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 :thinking::
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`.
-rw-r--r--compiler/rustc_session/src/session.rs25
-rw-r--r--src/test/assembly/panic-no-unwind-no-uwtable.rs8
-rw-r--r--src/test/assembly/panic-unwind-no-uwtable.rs12
-rw-r--r--src/test/codegen/force-no-unwind-tables.rs6
-rw-r--r--src/test/codegen/panic-unwind-default-uwtable.rs6
-rw-r--r--src/test/ui/panic-runtime/unwind-tables-panic-required.rs10
-rw-r--r--src/test/ui/unwind-no-uwtable.rs34
7 files changed, 74 insertions, 27 deletions
diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs
index 3488efacd11..cc2583be944 100644
--- a/compiler/rustc_session/src/session.rs
+++ b/compiler/rustc_session/src/session.rs
@@ -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
@@ -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,
@@ -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 \
diff --git a/src/test/assembly/panic-no-unwind-no-uwtable.rs b/src/test/assembly/panic-no-unwind-no-uwtable.rs
new file mode 100644
index 00000000000..499d4e69867
--- /dev/null
+++ b/src/test/assembly/panic-no-unwind-no-uwtable.rs
@@ -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() {}
diff --git a/src/test/assembly/panic-unwind-no-uwtable.rs b/src/test/assembly/panic-unwind-no-uwtable.rs
new file mode 100644
index 00000000000..8eed72b2fca
--- /dev/null
+++ b/src/test/assembly/panic-unwind-no-uwtable.rs
@@ -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!();
+}
diff --git a/src/test/codegen/force-no-unwind-tables.rs b/src/test/codegen/force-no-unwind-tables.rs
index dc77e6cb709..3ee23f05eb2 100644
--- a/src/test/codegen/force-no-unwind-tables.rs
+++ b/src/test/codegen/force-no-unwind-tables.rs
@@ -3,5 +3,9 @@
 
 #![crate_type="lib"]
 
+// CHECK-LABEL: define{{.*}}void @foo
 // CHECK-NOT: attributes #{{.*}} uwtable
-pub fn foo() {}
+#[no_mangle]
+fn foo() {
+    panic!();
+}
diff --git a/src/test/codegen/panic-unwind-default-uwtable.rs b/src/test/codegen/panic-unwind-default-uwtable.rs
new file mode 100644
index 00000000000..4c85008cf35
--- /dev/null
+++ b/src/test/codegen/panic-unwind-default-uwtable.rs
@@ -0,0 +1,6 @@
+// compile-flags: -C panic=unwind -C no-prepopulate-passes
+
+#![crate_type = "lib"]
+
+// CHECK: attributes #{{.*}} uwtable
+pub fn foo() {}
diff --git a/src/test/ui/panic-runtime/unwind-tables-panic-required.rs b/src/test/ui/panic-runtime/unwind-tables-panic-required.rs
deleted file mode 100644
index 79e91879051..00000000000
--- a/src/test/ui/panic-runtime/unwind-tables-panic-required.rs
+++ /dev/null
@@ -1,10 +0,0 @@
-// Tests that the compiler errors if the user tries to turn off unwind tables
-// when they are required.
-//
-// dont-check-compiler-stderr
-// compile-flags: -C panic=unwind -C force-unwind-tables=no
-//
-// 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/ui/unwind-no-uwtable.rs b/src/test/ui/unwind-no-uwtable.rs
new file mode 100644
index 00000000000..f249d3f4574
--- /dev/null
+++ b/src/test/ui/unwind-no-uwtable.rs
@@ -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);
+}