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

fix nounwind attribute logic #63909

Closed
wants to merge 4 commits into from
Closed
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
23 changes: 3 additions & 20 deletions src/librustc_codegen_llvm/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ use rustc::hir::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc::hir::def_id::{DefId, LOCAL_CRATE};
use rustc::session::Session;
use rustc::session::config::{Sanitizer, OptLevel};
use rustc::ty::{self, TyCtxt, PolyFnSig};
use rustc::ty::{TyCtxt, PolyFnSig};
use rustc::ty::layout::HasTyCtxt;
use rustc::ty::query::Providers;
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_data_structures::fx::FxHashMap;
use rustc_target::spec::PanicStrategy;
use rustc_codegen_ssa::traits::*;

use crate::abi::Abi;
use crate::attributes;
use crate::llvm::{self, Attribute};
use crate::llvm::AttributePlace::Function;
Expand Down Expand Up @@ -201,7 +200,7 @@ pub fn from_fn_attrs(
cx: &CodegenCx<'ll, 'tcx>,
llfn: &'ll Value,
id: Option<DefId>,
sig: PolyFnSig<'tcx>,
_sig: PolyFnSig<'tcx>,
) {
let codegen_fn_attrs = id.map(|id| cx.tcx.codegen_fn_attrs(id))
.unwrap_or_else(|| CodegenFnAttrs::new());
Expand Down Expand Up @@ -268,27 +267,11 @@ 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
false
} else if let Some(id) = id {
let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
if cx.tcx.is_foreign_item(id) {
// Foreign items like `extern "C" { fn foo(); }` are assumed not to
// unwind
false
} else if sig.abi != Abi::Rust && sig.abi != Abi::RustCall {
// Any items defined in Rust that *don't* have the `extern` ABI are
// defined to not unwind. We insert shims to abort if an unwind
// happens to enforce this.
false
} else {
// Anything else defined in Rust is assumed that it can possibly
// unwind
true
}
} else {
// assume this can possibly unwind, avoiding the application of a
// `nounwind` attribute below.
Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2528,6 +2528,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
} else if attr.check_name(sym::rustc_allocator) {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR;
} else if attr.check_name(sym::unwind) {
// FIXME: We currently assume it can unwind even with `#[unwind(aborts)]`.
codegen_fn_attrs.flags |= CodegenFnAttrFlags::UNWIND;
} else if attr.check_name(sym::ffi_returns_twice) {
if tcx.is_foreign_item(id) {
Expand Down
28 changes: 25 additions & 3 deletions src/test/codegen/extern-functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,38 @@
#![feature(unwind_attributes)]

extern {
// CHECK: Function Attrs: nounwind
// CHECK-NEXT: declare void @extern_fn
// CHECK-NOT: nounwind
// CHECK: declare void @extern_fn
fn extern_fn();
// CHECK-NOT: Function Attrs: nounwind
// CHECK-NOT: 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 don't 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 don't 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();
}
6 changes: 0 additions & 6 deletions src/test/codegen/nounwind-extern.rs

This file was deleted.

15 changes: 15 additions & 0 deletions src/test/codegen/unwind-extern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// compile-flags: -C opt-level=0

#![crate_type = "lib"]
#![feature(unwind_attributes)]

// make sure these all do *not* get the attribute
// CHECK-NOT: nounwind

pub extern fn foo() {} // right now we don't abort-on-panic, so we also shouldn't have `nounwind`
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
#[unwind(allowed)]
pub extern fn foo_allowed() {}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

pub extern "Rust" fn bar() {}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
#[unwind(allowed)]
pub extern "Rust" fn bar_allowed() {}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved