From 9a0b9c69600946c8c665705df2cec9ed7ce63d71 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Oct 2019 19:46:03 +0200 Subject: [PATCH 1/5] remove old branch of unwind logic --- src/librustc_codegen_llvm/attributes.rs | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 2260747602114..210d6cc16967e 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -275,18 +275,7 @@ pub fn from_fn_attrs( } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) { // Special attribute for allocator functions, which can't unwind false - } else if let Some(_) = id { - // rust-lang/rust#64655, rust-lang/rust#63909: to minimize - // risk associated with changing cases where nounwind - // attribute is attached, this code is deliberately mimicking - // old control flow based on whether `id` is `Some` or `None`. - // - // However, in the long term we should either: - // - fold this into final else (i.e. stop inspecting `id`) - // - or, adopt Rust PR #63909. - // - // see also Rust RFC 2753. - + } else { let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig); if sig.abi == Abi::Rust || sig.abi == Abi::RustCall { // Any Rust method (or `extern "Rust" fn` or `extern @@ -312,15 +301,6 @@ pub fn from_fn_attrs( // In either case, we mark item as explicitly nounwind. false } - } else { - // assume this can possibly unwind, avoiding the application of a - // `nounwind` attribute below. - // - // (But: See comments in previous branch. Specifically, it is - // unclear whether there is real value in the assumption this - // can unwind. The conservatism here may just be papering over - // a real problem by making some UB a bit harder to hit.) - true }); // Always annotate functions with the target-cpu they are compiled for. From a1a8f33abbf5ee00e6918246f4525c6e4458225f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Oct 2019 19:48:58 +0200 Subject: [PATCH 2/5] update test for nounwind on FFI imports --- src/test/codegen/extern-functions.rs | 19 ----------- src/test/codegen/unwind-extern-imports.rs | 41 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 19 deletions(-) delete mode 100644 src/test/codegen/extern-functions.rs create mode 100644 src/test/codegen/unwind-extern-imports.rs diff --git a/src/test/codegen/extern-functions.rs b/src/test/codegen/extern-functions.rs deleted file mode 100644 index a935d88652267..0000000000000 --- a/src/test/codegen/extern-functions.rs +++ /dev/null @@ -1,19 +0,0 @@ -// compile-flags: -C no-prepopulate-passes - -#![crate_type = "lib"] -#![feature(unwind_attributes)] - -extern { -// CHECK: Function Attrs: nounwind -// CHECK-NEXT: declare void @extern_fn - fn extern_fn(); -// CHECK-NOT: Function Attrs: nounwind -// CHECK: declare void @unwinding_extern_fn - #[unwind(allowed)] - fn unwinding_extern_fn(); -} - -pub unsafe fn force_declare() { - extern_fn(); - unwinding_extern_fn(); -} diff --git a/src/test/codegen/unwind-extern-imports.rs b/src/test/codegen/unwind-extern-imports.rs new file mode 100644 index 0000000000000..1c79162536f32 --- /dev/null +++ b/src/test/codegen/unwind-extern-imports.rs @@ -0,0 +1,41 @@ +// compile-flags: -C no-prepopulate-passes + +#![crate_type = "lib"] +#![feature(unwind_attributes)] + +extern { +// CHECK: Function Attrs: nounwind +// CHECK-NEXT: declare void @extern_fn + fn extern_fn(); +// CHECK-NOT: Function Attrs: nounwind +// CHECK: declare void @unwinding_extern_fn + #[unwind(allowed)] + fn unwinding_extern_fn(); +// CHECK-NOT: nounwind +// CHECK: declare void @aborting_extern_fn + #[unwind(aborts)] + fn aborting_extern_fn(); // FIXME: we want to have the attribute here +} + +extern "Rust" { +// CHECK-NOT: nounwind +// CHECK: declare void @rust_extern_fn + fn rust_extern_fn(); +// CHECK-NOT: nounwind +// CHECK: declare void @rust_unwinding_extern_fn + #[unwind(allowed)] + fn rust_unwinding_extern_fn(); +// CHECK-NOT: nounwind +// CHECK: declare void @rust_aborting_extern_fn + #[unwind(aborts)] + fn rust_aborting_extern_fn(); // FIXME: we want to have the attribute here +} + +pub unsafe fn force_declare() { + extern_fn(); + unwinding_extern_fn(); + aborting_extern_fn(); + rust_extern_fn(); + rust_unwinding_extern_fn(); + rust_aborting_extern_fn(); +} From 63af27f9ea81a71eb4ea9fb076228993f923cebb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Oct 2019 20:40:03 +0200 Subject: [PATCH 3/5] also (properly) test nounwind on function definitions --- src/test/codegen/nounwind-extern.rs | 6 ------ src/test/codegen/unwind-extern-exports.rs | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) delete mode 100644 src/test/codegen/nounwind-extern.rs create mode 100644 src/test/codegen/unwind-extern-exports.rs diff --git a/src/test/codegen/nounwind-extern.rs b/src/test/codegen/nounwind-extern.rs deleted file mode 100644 index 54d6a8d2794ba..0000000000000 --- a/src/test/codegen/nounwind-extern.rs +++ /dev/null @@ -1,6 +0,0 @@ -// compile-flags: -O - -#![crate_type = "lib"] - -// CHECK: Function Attrs: norecurse nounwind -pub extern fn foo() {} diff --git a/src/test/codegen/unwind-extern-exports.rs b/src/test/codegen/unwind-extern-exports.rs new file mode 100644 index 0000000000000..e7aad9ac72333 --- /dev/null +++ b/src/test/codegen/unwind-extern-exports.rs @@ -0,0 +1,19 @@ +// compile-flags: -C opt-level=0 + +#![crate_type = "lib"] +#![feature(unwind_attributes)] + +// Make sure these all do *not* get the attribute. +// We disable optimizations to prevent LLVM from infering the attribute. +// CHECK-NOT: nounwind + +// "C" ABI +// pub extern fn foo() {} // FIXME right now we don't abort-on-panic but add `nounwind` nevertheless +#[unwind(allowed)] +pub extern fn foo_allowed() {} + +// "Rust" ABI (`extrn "Rust"` could be removed as all `fn` get it implicitly; we leave it +// in for clarity.) +pub extern "Rust" fn bar() {} +#[unwind(allowed)] +pub extern "Rust" fn bar_allowed() {} From 79c623f1462646da2c480b6b1c662aa9cf8afee7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Oct 2019 20:40:10 +0200 Subject: [PATCH 4/5] some typography --- src/librustc_codegen_llvm/attributes.rs | 4 ++-- src/test/codegen/unwind-extern-exports.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 210d6cc16967e..6a36a4a50cbf3 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -270,10 +270,10 @@ pub fn from_fn_attrs( // optimize based on this! false } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) { - // If a specific #[unwind] attribute is present, use that + // If a specific #[unwind] attribute is present, use that. true } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) { - // Special attribute for allocator functions, which can't unwind + // Special attribute for allocator functions, which can't unwind. false } else { let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig); diff --git a/src/test/codegen/unwind-extern-exports.rs b/src/test/codegen/unwind-extern-exports.rs index e7aad9ac72333..ddb3a4f6b4dd8 100644 --- a/src/test/codegen/unwind-extern-exports.rs +++ b/src/test/codegen/unwind-extern-exports.rs @@ -12,8 +12,8 @@ #[unwind(allowed)] pub extern fn foo_allowed() {} -// "Rust" ABI (`extrn "Rust"` could be removed as all `fn` get it implicitly; we leave it -// in for clarity.) +// "Rust" +// (`extern "Rust"` could be removed as all `fn` get it implicitly; we leave it in for clarity.) pub extern "Rust" fn bar() {} #[unwind(allowed)] pub extern "Rust" fn bar_allowed() {} From 09d7be39fadf00e0b179bfaef86679fc489e796f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Oct 2019 23:08:57 +0200 Subject: [PATCH 5/5] make tests more robust --- src/test/codegen/unwind-extern-imports.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/codegen/unwind-extern-imports.rs b/src/test/codegen/unwind-extern-imports.rs index 1c79162536f32..485e8bbcd4289 100644 --- a/src/test/codegen/unwind-extern-imports.rs +++ b/src/test/codegen/unwind-extern-imports.rs @@ -4,10 +4,10 @@ #![feature(unwind_attributes)] extern { -// CHECK: Function Attrs: nounwind +// CHECK: Function Attrs:{{.*}}nounwind // CHECK-NEXT: declare void @extern_fn fn extern_fn(); -// CHECK-NOT: Function Attrs: nounwind +// CHECK-NOT: Function Attrs:{{.*}}nounwind // CHECK: declare void @unwinding_extern_fn #[unwind(allowed)] fn unwinding_extern_fn();