Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nounwind tests and cleanup #65346

Merged
merged 5 commits into from
Oct 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 3 additions & 23 deletions src/librustc_codegen_llvm/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,23 +270,12 @@ 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 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
Expand All @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #65020 nobody knew why this branch should be needed. And all the tests behave as expected without it. So I think we should remove it.

});

// Always annotate functions with the target-cpu they are compiled for.
Expand Down
19 changes: 0 additions & 19 deletions src/test/codegen/extern-functions.rs

This file was deleted.

6 changes: 0 additions & 6 deletions src/test/codegen/nounwind-extern.rs

This file was deleted.

19 changes: 19 additions & 0 deletions src/test/codegen/unwind-extern-exports.rs
Original file line number Diff line number Diff line change
@@ -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"
// (`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() {}
41 changes: 41 additions & 0 deletions src/test/codegen/unwind-extern-imports.rs
Original file line number Diff line number Diff line change
@@ -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();
}