Skip to content

Commit

Permalink
Rollup merge of rust-lang#65346 - RalfJung:nounwind-tests, r=nagisa
Browse files Browse the repository at this point in the history
nounwind tests and cleanup

This is a follow-up to @pnkfelix' rust-lang#65020. In particular it adds some tests as @nagisa  asked. It also does a cleanup that the original PR omitted to reduce backporting risks.

I hope I finally managed to write an uncontroversial PR in this area. ;) This should not change any behavior, just test it better.
  • Loading branch information
Centril authored Oct 13, 2019
2 parents d10702b + 09d7be3 commit 4f8214c
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 48 deletions.
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
});

// 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();
}

0 comments on commit 4f8214c

Please sign in to comment.