From 95def5b7abb49c7db6949905baf9f91206160ae9 Mon Sep 17 00:00:00 2001 From: James Miller Date: Thu, 15 Jan 2015 19:41:45 +1300 Subject: [PATCH 1/7] Mark extern functions as nounwind Functions that use an ABI other than the Rust one are marked as nounwind. Rust functions that use non-Rust ABI are also marked nounwind. To allow for this, the unwinding is caught and we abort the process. Added a `#[can_unwind]` attribute that allows functions using a non-Rust ABI to unwind without aborting. This is required for the unwinding code in libstd to function. --- src/libcore/panicking.rs | 2 +- src/librustc/lint/builtin.rs | 1 + src/librustc_trans/trans/base.rs | 37 +++++++++++- src/librustc_trans/trans/builder.rs | 7 +++ src/librustc_trans/trans/cleanup.rs | 33 +---------- src/librustc_trans/trans/foreign.rs | 72 +++++++++++++++++++++--- src/librustc_trans/trans/monomorphize.rs | 2 +- src/libstd/rt/libunwind.rs | 3 + src/libstd/rt/unwind.rs | 4 +- 9 files changed, 118 insertions(+), 43 deletions(-) diff --git a/src/libcore/panicking.rs b/src/libcore/panicking.rs index 61b4284e1dd9c..10b022b2e7cc5 100644 --- a/src/libcore/panicking.rs +++ b/src/libcore/panicking.rs @@ -51,7 +51,7 @@ fn panic_bounds_check(file_line: &(&'static str, uint), pub fn panic_fmt(fmt: fmt::Arguments, file_line: &(&'static str, uint)) -> ! { #[allow(improper_ctypes)] extern { - #[lang = "panic_fmt"] + #[lang = "panic_fmt"] #[can_unwind] fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: uint) -> !; } let (file, line) = *file_line; diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 8a27cfc510f4a..d7348525ea555 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -653,6 +653,7 @@ impl LintPass for UnusedAttributes { "no_debug", "omit_gdb_pretty_printer_section", "unsafe_no_drop_flag", + "can_unwind", // used in resolve "prelude_import", diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index eed61ae59a259..7441f86d4f113 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -56,7 +56,7 @@ use trans::cleanup; use trans::closure; use trans::common::{Block, C_bool, C_bytes_in_context, C_i32, C_integral}; use trans::common::{C_null, C_struct_in_context, C_u64, C_u8, C_undef}; -use trans::common::{CrateContext, ExternMap, FunctionContext}; +use trans::common::{CrateContext, ExprId, ExternMap, FunctionContext}; use trans::common::{NodeInfo, Result}; use trans::common::{node_id_type, return_type_is_void}; use trans::common::{tydesc_info, type_is_immediate}; @@ -1011,6 +1011,37 @@ pub fn invoke<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, } } +pub fn get_eh_personality<'a, 'tcx>(ccx: &CrateContext<'a,'tcx>) -> ValueRef { + // The exception handling personality function. + // + // If our compilation unit has the `eh_personality` lang item somewhere + // within it, then we just need to translate that. Otherwise, we're + // building an rlib which will depend on some upstream implementation of + // this function, so we just codegen a generic reference to it. We don't + // specify any of the types for the function, we just make it a symbol + // that LLVM can later use. + match ccx.tcx().lang_items.eh_personality() { + Some(def_id) => { + callee::trans_fn_ref(ccx, def_id, ExprId(0), &Substs::trans_empty()).val + } + None => { + let mut personality = ccx.eh_personality().borrow_mut(); + match *personality { + Some(llpersonality) => llpersonality, + None => { + let fty = Type::variadic_func(&[], &Type::i32(ccx)); + let f = decl_cdecl_fn(ccx, + "rust_eh_personality", + fty, + ccx.tcx().types.i32); + *personality = Some(f); + f + } + } + } + } +} + pub fn need_invoke(bcx: Block) -> bool { if bcx.sess().no_landing_pads() { return false; @@ -2870,7 +2901,9 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef { let abi = ccx.tcx().map.get_foreign_abi(id); let ty = ty::node_id_to_type(ccx.tcx(), ni.id); let name = foreign::link_name(&*ni); - foreign::register_foreign_item_fn(ccx, abi, ty, &name.get()[]) + let llfn = foreign::register_foreign_item_fn(ccx, abi, ty, &name.get()[]); + foreign::add_abi_attributes(ccx, llfn, abi, &ni.attrs[]); + llfn } ast::ForeignItemStatic(..) => { foreign::register_static(ccx, &*ni) diff --git a/src/librustc_trans/trans/builder.rs b/src/librustc_trans/trans/builder.rs index 187b3e2cf2188..720d3a46383f0 100644 --- a/src/librustc_trans/trans/builder.rs +++ b/src/librustc_trans/trans/builder.rs @@ -952,6 +952,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + pub fn add_clause(&self, landing_pad: ValueRef, clause: ValueRef) { + self.count_insn("addclause"); + unsafe { + llvm::LLVMAddClause(landing_pad, clause); + } + } + pub fn resume(&self, exn: ValueRef) -> ValueRef { self.count_insn("resume"); unsafe { diff --git a/src/librustc_trans/trans/cleanup.rs b/src/librustc_trans/trans/cleanup.rs index 5658889aaf368..f94d7721bc5de 100644 --- a/src/librustc_trans/trans/cleanup.rs +++ b/src/librustc_trans/trans/cleanup.rs @@ -19,9 +19,8 @@ pub use self::Heap::*; use llvm::{BasicBlockRef, ValueRef}; use trans::base; use trans::build; -use trans::callee; use trans::common; -use trans::common::{Block, FunctionContext, ExprId, NodeInfo}; +use trans::common::{Block, FunctionContext, NodeInfo}; use trans::debuginfo; use trans::glue; use middle::region; @@ -715,35 +714,7 @@ impl<'blk, 'tcx> CleanupHelperMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx &[Type::i8p(self.ccx), Type::i32(self.ccx)], false); - // The exception handling personality function. - // - // If our compilation unit has the `eh_personality` lang item somewhere - // within it, then we just need to translate that. Otherwise, we're - // building an rlib which will depend on some upstream implementation of - // this function, so we just codegen a generic reference to it. We don't - // specify any of the types for the function, we just make it a symbol - // that LLVM can later use. - let llpersonality = match pad_bcx.tcx().lang_items.eh_personality() { - Some(def_id) => { - callee::trans_fn_ref(pad_bcx.ccx(), def_id, ExprId(0), - pad_bcx.fcx.param_substs).val - } - None => { - let mut personality = self.ccx.eh_personality().borrow_mut(); - match *personality { - Some(llpersonality) => llpersonality, - None => { - let fty = Type::variadic_func(&[], &Type::i32(self.ccx)); - let f = base::decl_cdecl_fn(self.ccx, - "rust_eh_personality", - fty, - self.ccx.tcx().types.i32); - *personality = Some(f); - f - } - } - } - }; + let llpersonality = base::get_eh_personality(pad_bcx.ccx()); // The only landing pad clause will be 'cleanup' let llretval = build::LandingPad(pad_bcx, llretty, llpersonality, 1u); diff --git a/src/librustc_trans/trans/foreign.rs b/src/librustc_trans/trans/foreign.rs index c989d2311be36..3efd703d47098 100644 --- a/src/librustc_trans/trans/foreign.rs +++ b/src/librustc_trans/trans/foreign.rs @@ -460,6 +460,7 @@ pub fn trans_foreign_mod(ccx: &CrateContext, foreign_mod: &ast::ForeignMod) { match foreign_mod.abi { Rust | RustIntrinsic => {} abi => { + let ty = ty::node_id_to_type(ccx.tcx(), foreign_item.id); match ty.sty { ty::ty_bare_fn(_, bft) => gate_simd_ffi(ccx.tcx(), &**decl, bft), @@ -467,8 +468,11 @@ pub fn trans_foreign_mod(ccx: &CrateContext, foreign_mod: &ast::ForeignMod) { "foreign fn's sty isn't a bare_fn_ty?") } - register_foreign_item_fn(ccx, abi, ty, - &lname.get()[]); + let llfn = register_foreign_item_fn(ccx, abi, ty, + &lname.get()[]); + + add_abi_attributes(ccx, llfn, abi, &foreign_item.attrs[]); + // Unlike for other items, we shouldn't call // `base::update_linkage` here. Foreign items have // special linkage requirements, which are handled @@ -568,7 +572,7 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, let llrustfn = build_rust_fn(ccx, decl, body, param_substs, attrs, id, hash); // Build up the foreign wrapper (`foo` above). - return build_wrap_fn(ccx, llrustfn, llwrapfn, &tys, mty); + return build_wrap_fn(ccx, llrustfn, llwrapfn, &tys, mty, attrs); } fn build_rust_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, @@ -618,7 +622,7 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, llrustfn: ValueRef, llwrapfn: ValueRef, tys: &ForeignTypes<'tcx>, - t: Ty<'tcx>) { + t: Ty<'tcx>, attrs: &[ast::Attribute]) { let _icx = push_ctxt( "foreign::trans_rust_fn_with_foreign_abi::build_wrap_fn"); let tcx = ccx.tcx(); @@ -635,11 +639,19 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // // S foo(T i) { // S r; - // foo0(&r, NULL, i); + // try { + // foo0(&r, NULL, i); + // } catch (..) { + // abort(); + // } // return r; // } + // + // Because we mark extern "C" functions as nounwind, we need to + // enforce this for Rust functions that have a non-rust ABI. Hence + // the try-catch we emulate below. - let ptr = "the block\0".as_ptr(); + let ptr = "entry\0".as_ptr(); let the_block = llvm::LLVMAppendBasicBlockInContext(ccx.llcx(), llwrapfn, ptr as *const _); @@ -788,7 +800,41 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, debug!("calling llrustfn = {}, t = {}", ccx.tn().val_to_string(llrustfn), t.repr(ccx.tcx())); let attributes = base::get_fn_llvm_attributes(ccx, t); - let llrust_ret_val = builder.call(llrustfn, llrust_args.as_slice(), Some(attributes)); + let llrust_ret_val = if attr::contains_name(attrs, "can_unwind") { + debug!("fn can unwind - using call"); + builder.call(llrustfn, llrust_args.as_slice(), Some(attributes)) + } else { + debug!("fn can't unwind - using invoke"); + // Create the landing pad and the return blocks + let ptr = "catch\0".as_ptr(); + let catch_block = llvm::LLVMAppendBasicBlockInContext(ccx.llcx(), llwrapfn, + ptr as *const _); + + let ptr = "return\0".as_ptr(); + let return_block = llvm::LLVMAppendBasicBlockInContext(ccx.llcx(), llwrapfn, + ptr as *const _); + + + let llrust_ret_val = builder.invoke(llrustfn, llrust_args.as_slice(), + return_block, catch_block, + Some(attributes)); + + // Populate the landing pad + builder.position_at_end(catch_block); + let pad_ty = Type::struct_(ccx, &[Type::i8p(ccx), Type::i32(ccx)], false); + let llpersonality = base::get_eh_personality(ccx); + + let pad_ret_val = builder.landing_pad(pad_ty, llpersonality, 1us); + builder.add_clause(pad_ret_val, C_null(Type::i8p(ccx))); + let trap = ccx.get_intrinsic(&("llvm.trap")); + builder.call(trap, &[], None); + builder.unreachable(); + + builder.position_at_end(return_block); + + llrust_ret_val + + }; // Get the return value where the foreign fn expects it. let llforeign_ret_ty = match tys.fn_ty.ret_ty.cast { @@ -1004,3 +1050,15 @@ fn add_argument_attributes(tys: &ForeignTypes, i += 1; } } + +pub fn add_abi_attributes(ccx: &CrateContext, llfn: ValueRef, abi: Abi, attrs: &[ast::Attribute]) { + + match ccx.sess().target.target.adjust_abi(abi) { + Rust | RustCall | RustIntrinsic => {} + _ => { + if !attr::contains_name(attrs, "can_unwind") { + llvm::SetFunctionAttribute(llfn, llvm::NoUnwindAttribute); + } + } + } +} diff --git a/src/librustc_trans/trans/monomorphize.rs b/src/librustc_trans/trans/monomorphize.rs index f52e7c0ec94c0..03cc1c0f9504c 100644 --- a/src/librustc_trans/trans/monomorphize.rs +++ b/src/librustc_trans/trans/monomorphize.rs @@ -181,7 +181,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if needs_body { if abi != abi::Rust { foreign::trans_rust_fn_with_foreign_abi( - ccx, &**decl, &**body, &[], d, psubsts, fn_id.node, + ccx, &**decl, &**body, &i.attrs[], d, psubsts, fn_id.node, Some(&hash[])); } else { trans_fn(ccx, &**decl, &**body, d, psubsts, fn_id.node, &[]); diff --git a/src/libstd/rt/libunwind.rs b/src/libstd/rt/libunwind.rs index dd9923307d6f0..9f83ea5100531 100644 --- a/src/libstd/rt/libunwind.rs +++ b/src/libstd/rt/libunwind.rs @@ -113,13 +113,16 @@ extern "C" { // iOS on armv7 uses SjLj exceptions and requires to link // against corresponding routine (..._SjLj_...) #[cfg(not(all(target_os = "ios", target_arch = "arm")))] + #[can_unwind] pub fn _Unwind_RaiseException(exception: *mut _Unwind_Exception) -> _Unwind_Reason_Code; #[cfg(all(target_os = "ios", target_arch = "arm"))] + #[can_unwind] fn _Unwind_SjLj_RaiseException(e: *mut _Unwind_Exception) -> _Unwind_Reason_Code; + #[can_unwind] pub fn _Unwind_DeleteException(exception: *mut _Unwind_Exception); } diff --git a/src/libstd/rt/unwind.rs b/src/libstd/rt/unwind.rs index 73b8f104c2369..e402d360e14c3 100644 --- a/src/libstd/rt/unwind.rs +++ b/src/libstd/rt/unwind.rs @@ -133,6 +133,7 @@ pub unsafe fn try(f: F) -> Result<(), Box> { Err(cause.unwrap()) }; + #[can_unwind] extern fn try_fn(opt_closure: *mut c_void) { let opt_closure = opt_closure as *mut Option; unsafe { (*opt_closure).take().unwrap()(); } @@ -251,6 +252,7 @@ pub mod eabi { } } + #[can_unwind] #[no_mangle] // referenced from rust_try.ll pub extern "C" fn rust_eh_personality_catch( _version: c_int, @@ -479,7 +481,7 @@ pub mod eabi { #[cfg(not(test))] /// Entry point of panic from the libcore crate. -#[lang = "panic_fmt"] +#[lang = "panic_fmt"] #[can_unwind] pub extern fn rust_begin_unwind(msg: fmt::Arguments, file: &'static str, line: uint) -> ! { begin_unwind_fmt(msg, &(file, line)) From 446ea8909ea04022f0df218d63ba61cd98fc1c78 Mon Sep 17 00:00:00 2001 From: James Miller Date: Thu, 15 Jan 2015 20:43:04 +1300 Subject: [PATCH 2/7] Add a nounwind attribute Adds a nounwind attribute that allows functions to be marked as not unwinding. Most useful for cross-crate usage when the nounwind LLVM attribute cannot be inferred. Also fixes marking extern fns as nounwind when the function decl comes from another crate. --- src/liballoc/lib.rs | 1 + src/librustc/lint/builtin.rs | 1 + src/librustc_trans/trans/base.rs | 10 +++++++--- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 811e32e747dfd..da74905f73789 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -97,6 +97,7 @@ pub mod rc; /// Common out-of-memory routine #[cold] #[inline(never)] +#[nounwind] pub fn oom() -> ! { // FIXME(#14674): This really needs to do something other than just abort // here, but any printing done must be *guaranteed* to not diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index d7348525ea555..d53e85fcce3fb 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -654,6 +654,7 @@ impl LintPass for UnusedAttributes { "omit_gdb_pretty_printer_section", "unsafe_no_drop_flag", "can_unwind", + "nounwind", // used in resolve "prelude_import", diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 7441f86d4f113..dfb07a0f2cb6b 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -470,6 +470,7 @@ pub fn set_llvm_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: Val llvm::FunctionIndex as c_uint, llvm::ColdAttribute as uint64_t) }, + "nounwind" => llvm::SetFunctionAttribute(llfn, llvm::NoUnwindAttribute), _ => used = false, } if used { @@ -941,9 +942,12 @@ pub fn trans_external_path<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, RustIntrinsic => { ccx.sess().bug("unexpected intrinsic in trans_external_path") } - _ => { - foreign::register_foreign_item_fn(ccx, fn_ty.abi, t, - &name[]) + abi => { + let attrs = csearch::get_item_attrs(&ccx.sess().cstore, did); + let llfn = foreign::register_foreign_item_fn(ccx, fn_ty.abi, t, + &name[]); + foreign::add_abi_attributes(ccx, llfn, abi, &attrs[]); + llfn } } } From 099b4990abcd17ad6060268ffa5b254a9f92e306 Mon Sep 17 00:00:00 2001 From: James Miller Date: Thu, 15 Jan 2015 21:51:38 +1300 Subject: [PATCH 3/7] Put unwinding attributes behind a feature gate --- src/liballoc/lib.rs | 1 + src/libstd/lib.rs | 4 ++-- src/libsyntax/feature_gate.rs | 7 +++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index da74905f73789..38249103d56b2 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -70,6 +70,7 @@ #![feature(lang_items, unsafe_destructor)] #![feature(box_syntax)] #![feature(optin_builtin_traits)] +#![feature(unwinding_attributes)] #![allow(unknown_features)] #![feature(int_uint)] #[macro_use] diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index ddb8129630f75..e21ce66c800b2 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -104,6 +104,7 @@ html_root_url = "http://doc.rust-lang.org/nightly/", html_playground_url = "http://play.rust-lang.org/")] +#![allow(unstable)] #![allow(unknown_features)] #![feature(linkage, thread_local, asm)] #![feature(lang_items, unsafe_destructor)] @@ -112,8 +113,7 @@ #![feature(old_impl_check)] #![feature(optin_builtin_traits)] #![feature(int_uint)] -#![feature(int_uint)] -#![allow(unstable)] +#![feature(unwinding_attributes)] // Don't link to std. We are std. #![no_std] diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 13b7944998ad8..06d6dd8cd97d4 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -73,6 +73,7 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[ ("box_syntax", Active), ("on_unimplemented", Active), ("simd_ffi", Active), + ("unwinding_attributes", Active), ("if_let", Accepted), ("while_let", Accepted), @@ -259,6 +260,12 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { self.gate_feature("on_unimplemented", i.span, "the `#[rustc_on_unimplemented]` attribute \ is an experimental feature") + } else if attr.name() == "can_unwind" { + self.gate_feature("unwinding_attributes", i.span, + "the `#[can_unwind]` attribute is an experimental feature") + } else if attr.name() == "nounwind" { + self.gate_feature("unwinding_attributes", i.span, + "the `#[nounwind]` attribute is an experimental feature") } } match i.node { From 2892ca40594efabba6af7898132d75dad7ff7166 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 16 Jan 2015 13:19:30 +1300 Subject: [PATCH 4/7] Allow for unwinding out of foreign abi functions We were not allowing for the possibility that we might unwind out of a foreign abi function. Now that we allow for some of these functions to unwind, generate the proper code so clean-ups are performed. This also allowed me to write a test for the functionality, which I have now done. --- src/librustc_trans/trans/build.rs | 19 ++++++++ src/librustc_trans/trans/builder.rs | 34 +++++++++++++++ src/librustc_trans/trans/foreign.rs | 27 +++++++++--- src/test/run-pass/ffi-unwinding.rs | 67 +++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 src/test/run-pass/ffi-unwinding.rs diff --git a/src/librustc_trans/trans/build.rs b/src/librustc_trans/trans/build.rs index 1f77f625c9db3..37d6a82da671a 100644 --- a/src/librustc_trans/trans/build.rs +++ b/src/librustc_trans/trans/build.rs @@ -126,6 +126,25 @@ pub fn Invoke(cx: Block, B(cx).invoke(fn_, args, then, catch, attributes) } +pub fn InvokeWithConv(cx: Block, + fn_: ValueRef, + args: &[ValueRef], + then: BasicBlockRef, + catch: BasicBlockRef, + conv: CallConv, + attributes: Option) + -> ValueRef { + if cx.unreachable.get() { + return C_null(Type::i8(cx.ccx())); + } + check_not_terminated(cx); + terminate(cx, "InvokeWithConv"); + debug!("InvokeWithConv({} with arguments ({}))", + cx.val_to_string(fn_), + args.iter().map(|a| cx.val_to_string(*a)).collect::>().connect(", ")); + B(cx).invoke_with_conv(fn_, args, then, catch, conv, attributes) +} + pub fn Unreachable(cx: Block) { if cx.unreachable.get() { return diff --git a/src/librustc_trans/trans/builder.rs b/src/librustc_trans/trans/builder.rs index 720d3a46383f0..1a96345dfd814 100644 --- a/src/librustc_trans/trans/builder.rs +++ b/src/librustc_trans/trans/builder.rs @@ -184,6 +184,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + pub fn invoke_with_conv(&self, + llfn: ValueRef, + args: &[ValueRef], + then: BasicBlockRef, + catch: BasicBlockRef, + conv: CallConv, + attributes: Option) + -> ValueRef { + self.count_insn("invokewithconv"); + + debug!("Invoke {} with args ({})", + self.ccx.tn().val_to_string(llfn), + args.iter() + .map(|&v| self.ccx.tn().val_to_string(v)) + .collect::>() + .connect(", ")); + + unsafe { + let v = llvm::LLVMBuildInvoke(self.llbuilder, + llfn, + args.as_ptr(), + args.len() as c_uint, + then, + catch, + noname()); + match attributes { + Some(a) => a.apply_callsite(v), + None => {} + } + llvm::SetInstructionCallConv(v, conv); + v + } + } + pub fn unreachable(&self) { self.count_insn("unreachable"); unsafe { diff --git a/src/librustc_trans/trans/foreign.rs b/src/librustc_trans/trans/foreign.rs index 3efd703d47098..8ee7a931af992 100644 --- a/src/librustc_trans/trans/foreign.rs +++ b/src/librustc_trans/trans/foreign.rs @@ -17,6 +17,7 @@ use trans::base::{llvm_linkage_by_name, push_ctxt}; use trans::base; use trans::build::*; use trans::cabi; +use trans::cleanup::CleanupMethods; use trans::common::*; use trans::machine; use trans::monomorphize; @@ -366,11 +367,27 @@ pub fn trans_native_call<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, arg_idx += 1; } - let llforeign_retval = CallWithConv(bcx, - llfn, - &llargs_foreign[], - cc, - Some(attrs)); + // Foreign functions *can* unwind, albeit rarely, so use invoke instead of call. + // We shouldn't really be emitting an invoke all the time, LLVM can optimise them + // out, but we should be able avoid it most of the time + let (llforeign_retval, bcx) = if base::need_invoke(bcx) { + let normal_bcx = bcx.fcx.new_temp_block("normal-return"); + let landing_pad = bcx.fcx.get_landing_pad(); + + let ret = InvokeWithConv(bcx, llfn, &llargs_foreign[], + normal_bcx.llbb, + landing_pad, + cc, + Some(attrs)); + (ret, normal_bcx) + } else { + let ret = CallWithConv(bcx, + llfn, + &llargs_foreign[], + cc, + Some(attrs)); + (ret, bcx) + }; // If the function we just called does not use an outpointer, // store the result into the rust outpointer. Cast the outpointer diff --git a/src/test/run-pass/ffi-unwinding.rs b/src/test/run-pass/ffi-unwinding.rs new file mode 100644 index 0000000000000..cd56fc389a492 --- /dev/null +++ b/src/test/run-pass/ffi-unwinding.rs @@ -0,0 +1,67 @@ +// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(unwinding_attributes)] + +use std::io::Command; +use std::os; +use std::str; + +struct Guard { name: &'static str } + +impl Drop for Guard { + fn drop(&mut self) { + println!("Dropped Guard {}", self.name); + } +} + +fn main() { + let args = os::args(); + + if args.len() > 1 { + if &args[1][] == "abort" { + let _g = Guard { name: "abort" }; + + i_abort(); + + return; + } else if &args[1][] == "unwind" { + let _g = Guard { name: "unwind" }; + + i_unwind(); + + return; + } + } + + let p = Command::new(&args[0][]) + .arg("abort") + .spawn().unwrap().wait_with_output().unwrap(); + + assert!(!p.status.success()); + let mut lines = str::from_utf8(&p.output[]).unwrap().lines(); + + assert!(lines.next().is_none()); + + let p = Command::new(&args[0][]) + .arg("unwind") + .spawn().unwrap().wait_with_output().unwrap(); + + assert!(!p.status.success()); + let mut lines = str::from_utf8(&p.output[]).unwrap().lines(); + + assert_eq!(&lines.next().unwrap()[], "Dropped Guard unwind"); +} + +#[inline(never)] +extern "C" fn i_abort() { panic!() } + +#[inline(never)] #[can_unwind] +extern "C" fn i_unwind() { panic!() } From 63bc48bd92a6701a284e63400da4d9a6d638d052 Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 16 Jan 2015 13:50:50 +1300 Subject: [PATCH 5/7] Rename nounwind -> unsafe_no_unwind --- src/liballoc/lib.rs | 2 +- src/librustc_trans/trans/base.rs | 7 +------ src/libsyntax/feature_gate.rs | 4 ++-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 38249103d56b2..175ed7c8f8235 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -98,7 +98,7 @@ pub mod rc; /// Common out-of-memory routine #[cold] #[inline(never)] -#[nounwind] +#[unsafe_no_nounwind] pub fn oom() -> ! { // FIXME(#14674): This really needs to do something other than just abort // here, but any printing done must be *guaranteed* to not diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index dfb07a0f2cb6b..c62ca20eff377 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -431,11 +431,6 @@ pub fn set_no_inline(f: ValueRef) { llvm::SetFunctionAttribute(f, llvm::NoInlineAttribute) } -#[allow(dead_code)] // useful -pub fn set_no_unwind(f: ValueRef) { - llvm::SetFunctionAttribute(f, llvm::NoUnwindAttribute) -} - // Tell LLVM to emit the information necessary to unwind the stack for the // function f. pub fn set_uwtable(f: ValueRef) { @@ -470,7 +465,7 @@ pub fn set_llvm_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: Val llvm::FunctionIndex as c_uint, llvm::ColdAttribute as uint64_t) }, - "nounwind" => llvm::SetFunctionAttribute(llfn, llvm::NoUnwindAttribute), + "unsafe_no_unwind" => llvm::SetFunctionAttribute(llfn, llvm::NoUnwindAttribute), _ => used = false, } if used { diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 06d6dd8cd97d4..bad8746a44855 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -263,9 +263,9 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { } else if attr.name() == "can_unwind" { self.gate_feature("unwinding_attributes", i.span, "the `#[can_unwind]` attribute is an experimental feature") - } else if attr.name() == "nounwind" { + } else if attr.name() == "unsafe_no_unwind" { self.gate_feature("unwinding_attributes", i.span, - "the `#[nounwind]` attribute is an experimental feature") + "the `#[unsafe_no_unwind]` attribute is an experimental feature") } } match i.node { From e3ab8cbcaac83f4f4a1168776fa29018dc06876d Mon Sep 17 00:00:00 2001 From: James Miller Date: Fri, 16 Jan 2015 19:34:56 +1300 Subject: [PATCH 6/7] Add test for the feature gate --- src/liballoc/lib.rs | 2 +- src/librustc/lint/builtin.rs | 2 +- src/test/compile-fail/feature-gate-unwinding.rs | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 src/test/compile-fail/feature-gate-unwinding.rs diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 175ed7c8f8235..2e157ac8743f4 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -98,7 +98,7 @@ pub mod rc; /// Common out-of-memory routine #[cold] #[inline(never)] -#[unsafe_no_nounwind] +#[unsafe_no_unwind] pub fn oom() -> ! { // FIXME(#14674): This really needs to do something other than just abort // here, but any printing done must be *guaranteed* to not diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index d53e85fcce3fb..3ce82a167213d 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -654,7 +654,7 @@ impl LintPass for UnusedAttributes { "omit_gdb_pretty_printer_section", "unsafe_no_drop_flag", "can_unwind", - "nounwind", + "unsafe_no_unwind", // used in resolve "prelude_import", diff --git a/src/test/compile-fail/feature-gate-unwinding.rs b/src/test/compile-fail/feature-gate-unwinding.rs new file mode 100644 index 0000000000000..e277c03f203c8 --- /dev/null +++ b/src/test/compile-fail/feature-gate-unwinding.rs @@ -0,0 +1,17 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] + +#[can_unwind] +extern fn foo() { } //~ ERROR the `#[can_unwind]` attribute is an experimental feature + +#[unsafe_no_unwind] +fn main() {} //~ ERROR the `#[unsafe_no_unwind]` attribute is an experimental feature From 8575acf1be534c224937df5ea8397c7906ff1ec3 Mon Sep 17 00:00:00 2001 From: James Miller Date: Wed, 21 Jan 2015 12:47:46 +1300 Subject: [PATCH 7/7] Disable aborting at FFI boundary in foreign-abi rust functions --- src/librustc_trans/trans/base.rs | 3 +- src/librustc_trans/trans/foreign.rs | 42 +++--------------- src/test/run-pass/ffi-unwinding.rs | 67 ----------------------------- 3 files changed, 7 insertions(+), 105 deletions(-) delete mode 100644 src/test/run-pass/ffi-unwinding.rs diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index c62ca20eff377..35a2f98cdde60 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -2901,7 +2901,8 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef { let ty = ty::node_id_to_type(ccx.tcx(), ni.id); let name = foreign::link_name(&*ni); let llfn = foreign::register_foreign_item_fn(ccx, abi, ty, &name.get()[]); - foreign::add_abi_attributes(ccx, llfn, abi, &ni.attrs[]); + // FIXME(aatch) Re-enable pending RFC + //foreign::add_abi_attributes(ccx, llfn, abi, &ni.attrs[]); llfn } ast::ForeignItemStatic(..) => { diff --git a/src/librustc_trans/trans/foreign.rs b/src/librustc_trans/trans/foreign.rs index 8ee7a931af992..e9053a3537acb 100644 --- a/src/librustc_trans/trans/foreign.rs +++ b/src/librustc_trans/trans/foreign.rs @@ -1,4 +1,4 @@ -// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT +// Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -639,7 +639,7 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, llrustfn: ValueRef, llwrapfn: ValueRef, tys: &ForeignTypes<'tcx>, - t: Ty<'tcx>, attrs: &[ast::Attribute]) { + t: Ty<'tcx>, _attrs: &[ast::Attribute]) { let _icx = push_ctxt( "foreign::trans_rust_fn_with_foreign_abi::build_wrap_fn"); let tcx = ccx.tcx(); @@ -816,42 +816,10 @@ pub fn trans_rust_fn_with_foreign_abi<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Perform the call itself debug!("calling llrustfn = {}, t = {}", ccx.tn().val_to_string(llrustfn), t.repr(ccx.tcx())); + // FIXME(aatch) Wrap the call in a try-catch and handle unwinding + // pending RFC let attributes = base::get_fn_llvm_attributes(ccx, t); - let llrust_ret_val = if attr::contains_name(attrs, "can_unwind") { - debug!("fn can unwind - using call"); - builder.call(llrustfn, llrust_args.as_slice(), Some(attributes)) - } else { - debug!("fn can't unwind - using invoke"); - // Create the landing pad and the return blocks - let ptr = "catch\0".as_ptr(); - let catch_block = llvm::LLVMAppendBasicBlockInContext(ccx.llcx(), llwrapfn, - ptr as *const _); - - let ptr = "return\0".as_ptr(); - let return_block = llvm::LLVMAppendBasicBlockInContext(ccx.llcx(), llwrapfn, - ptr as *const _); - - - let llrust_ret_val = builder.invoke(llrustfn, llrust_args.as_slice(), - return_block, catch_block, - Some(attributes)); - - // Populate the landing pad - builder.position_at_end(catch_block); - let pad_ty = Type::struct_(ccx, &[Type::i8p(ccx), Type::i32(ccx)], false); - let llpersonality = base::get_eh_personality(ccx); - - let pad_ret_val = builder.landing_pad(pad_ty, llpersonality, 1us); - builder.add_clause(pad_ret_val, C_null(Type::i8p(ccx))); - let trap = ccx.get_intrinsic(&("llvm.trap")); - builder.call(trap, &[], None); - builder.unreachable(); - - builder.position_at_end(return_block); - - llrust_ret_val - - }; + let llrust_ret_val = builder.call(llrustfn, llrust_args.as_slice(), Some(attributes)); // Get the return value where the foreign fn expects it. let llforeign_ret_ty = match tys.fn_ty.ret_ty.cast { diff --git a/src/test/run-pass/ffi-unwinding.rs b/src/test/run-pass/ffi-unwinding.rs deleted file mode 100644 index cd56fc389a492..0000000000000 --- a/src/test/run-pass/ffi-unwinding.rs +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -#![feature(unwinding_attributes)] - -use std::io::Command; -use std::os; -use std::str; - -struct Guard { name: &'static str } - -impl Drop for Guard { - fn drop(&mut self) { - println!("Dropped Guard {}", self.name); - } -} - -fn main() { - let args = os::args(); - - if args.len() > 1 { - if &args[1][] == "abort" { - let _g = Guard { name: "abort" }; - - i_abort(); - - return; - } else if &args[1][] == "unwind" { - let _g = Guard { name: "unwind" }; - - i_unwind(); - - return; - } - } - - let p = Command::new(&args[0][]) - .arg("abort") - .spawn().unwrap().wait_with_output().unwrap(); - - assert!(!p.status.success()); - let mut lines = str::from_utf8(&p.output[]).unwrap().lines(); - - assert!(lines.next().is_none()); - - let p = Command::new(&args[0][]) - .arg("unwind") - .spawn().unwrap().wait_with_output().unwrap(); - - assert!(!p.status.success()); - let mut lines = str::from_utf8(&p.output[]).unwrap().lines(); - - assert_eq!(&lines.next().unwrap()[], "Dropped Guard unwind"); -} - -#[inline(never)] -extern "C" fn i_abort() { panic!() } - -#[inline(never)] #[can_unwind] -extern "C" fn i_unwind() { panic!() }