Skip to content

Commit

Permalink
make #[unwind] attribute specify expectations more clearly
Browse files Browse the repository at this point in the history
You can now choose between the following:

- `#[unwind(allowed)]`
- `#[unwind(aborts)]`

Per #48251, the default is `#[unwind(allowed)]`, though
I think we should change this eventually.
  • Loading branch information
nikomatsakis committed Feb 21, 2018
1 parent 27a046e commit a47fd3d
Show file tree
Hide file tree
Showing 13 changed files with 113 additions and 19 deletions.
3 changes: 2 additions & 1 deletion src/libcore/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ pub fn panic_fmt(fmt: fmt::Arguments, file_line_col: &(&'static str, u32, u32))
#[allow(improper_ctypes)]
extern {
#[lang = "panic_fmt"]
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: u32, col: u32) -> !;
}
let (file, line, col) = *file_line_col;
Expand Down
3 changes: 2 additions & 1 deletion src/libpanic_unwind/gcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ unsafe fn find_eh_action(context: *mut uw::_Unwind_Context)
// See docs in the `unwind` module.
#[cfg(all(target_os="windows", target_arch = "x86", target_env="gnu"))]
#[lang = "eh_unwind_resume"]
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
unsafe extern "C" fn rust_eh_unwind_resume(panic_ctx: *mut u8) -> ! {
uw::_Unwind_Resume(panic_ctx as *mut uw::_Unwind_Exception);
}
Expand Down
3 changes: 2 additions & 1 deletion src/libpanic_unwind/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ pub unsafe extern "C" fn __rust_maybe_catch_panic(f: fn(*mut u8),
// Entry point for raising an exception, just delegates to the platform-specific
// implementation.
#[no_mangle]
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub unsafe extern "C" fn __rust_start_panic(data: usize, vtable: usize) -> u32 {
imp::panic(mem::transmute(raw::TraitObject {
data: data as *mut (),
Expand Down
3 changes: 2 additions & 1 deletion src/libpanic_unwind/seh64_gnu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ unsafe extern "C" fn rust_eh_personality(exceptionRecord: *mut c::EXCEPTION_RECO
}

#[lang = "eh_unwind_resume"]
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
unsafe extern "C" fn rust_eh_unwind_resume(panic_ctx: c::LPVOID) -> ! {
let params = [panic_ctx as c::ULONG_PTR];
c::RaiseException(RUST_PANIC,
Expand Down
9 changes: 6 additions & 3 deletions src/libpanic_unwind/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,21 @@ pub enum EXCEPTION_DISPOSITION {
pub use self::EXCEPTION_DISPOSITION::*;

extern "system" {
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub fn RaiseException(dwExceptionCode: DWORD,
dwExceptionFlags: DWORD,
nNumberOfArguments: DWORD,
lpArguments: *const ULONG_PTR);
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub fn RtlUnwindEx(TargetFrame: LPVOID,
TargetIp: LPVOID,
ExceptionRecord: *const EXCEPTION_RECORD,
ReturnValue: LPVOID,
OriginalContext: *const CONTEXT,
HistoryTable: *const UNWIND_HISTORY_TABLE);
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub fn _CxxThrowException(pExceptionObject: *mut c_void, pThrowInfo: *mut u8);
}
19 changes: 14 additions & 5 deletions src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use std::mem;
use std::u32;
use syntax::abi::Abi;
use syntax::ast;
use syntax::attr::{self, UnwindAttr};
use syntax::symbol::keywords;
use syntax_pos::Span;
use transform::MirSource;
Expand Down Expand Up @@ -355,10 +356,9 @@ macro_rules! unpack {
}

fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
fn_id: ast::NodeId,
fn_def_id: DefId,
abi: Abi)
-> bool {

// Not callable from C, so we can safely unwind through these
if abi == Abi::Rust || abi == Abi::RustCall { return false; }

Expand All @@ -370,9 +370,17 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,

// This is a special case: some functions have a C abi but are meant to
// unwind anyway. Don't stop them.
if tcx.has_attr(tcx.hir.local_def_id(fn_id), "unwind") { return false; }
let attrs = &tcx.get_attrs(fn_def_id);
match attr::find_unwind_attr(Some(tcx.sess.diagnostic()), attrs) {
None => {
// FIXME(rust-lang/rust#48251) -- Had to disable
// abort-on-panic for backwards compatibility reasons.
false
}

return true;
Some(UnwindAttr::Allowed) => false,
Some(UnwindAttr::Aborts) => true,
}
}

///////////////////////////////////////////////////////////////////////////
Expand All @@ -399,13 +407,14 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
safety,
return_ty);

let fn_def_id = tcx.hir.local_def_id(fn_id);
let call_site_scope = region::Scope::CallSite(body.value.hir_id.local_id);
let arg_scope = region::Scope::Arguments(body.value.hir_id.local_id);
let mut block = START_BLOCK;
let source_info = builder.source_info(span);
let call_site_s = (call_site_scope, source_info);
unpack!(block = builder.in_scope(call_site_s, LintLevel::Inherited, block, |builder| {
if should_abort_on_panic(tcx, fn_id, abi) {
if should_abort_on_panic(tcx, fn_def_id, abi) {
builder.schedule_abort();
}

Expand Down
6 changes: 4 additions & 2 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ extern {
data: *mut u8,
data_ptr: *mut usize,
vtable_ptr: *mut usize) -> u32;
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
fn __rust_start_panic(data: usize, vtable: usize) -> u32;
}

Expand Down Expand Up @@ -315,7 +316,8 @@ pub fn panicking() -> bool {
/// Entry point of panic from the libcore crate.
#[cfg(not(test))]
#[lang = "panic_fmt"]
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub extern fn rust_begin_panic(msg: fmt::Arguments,
file: &'static str,
line: u32,
Expand Down
45 changes: 45 additions & 0 deletions src/libsyntax/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,51 @@ pub fn find_inline_attr(diagnostic: Option<&Handler>, attrs: &[Attribute]) -> In
})
}

#[derive(Copy, Clone, PartialEq)]
pub enum UnwindAttr {
Allowed,
Aborts,
}

/// Determine what `#[unwind]` attribute is present in `attrs`, if any.
pub fn find_unwind_attr(diagnostic: Option<&Handler>, attrs: &[Attribute]) -> Option<UnwindAttr> {
let syntax_error = |attr: &Attribute| {
mark_used(attr);
diagnostic.map(|d| {
span_err!(d, attr.span, E0633, "malformed `#[unwind]` attribute");
});
None
};

attrs.iter().fold(None, |ia, attr| {
if attr.path != "unwind" {
return ia;
}
let meta = match attr.meta() {
Some(meta) => meta.node,
None => return ia,
};
match meta {
MetaItemKind::Word => {
syntax_error(attr)
}
MetaItemKind::List(ref items) => {
mark_used(attr);
if items.len() != 1 {
syntax_error(attr)
} else if list_contains_name(&items[..], "allowed") {
Some(UnwindAttr::Allowed)
} else if list_contains_name(&items[..], "aborts") {
Some(UnwindAttr::Aborts)
} else {
syntax_error(attr)
}
}
_ => ia,
}
})
}

/// True if `#[inline]` or `#[inline(always)]` is present in `attrs`.
pub fn requests_inline(attrs: &[Attribute]) -> bool {
match find_inline_attr(None, attrs) {
Expand Down
27 changes: 27 additions & 0 deletions src/libsyntax/diagnostic_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,33 @@ fn main() {
```
"##,

E0633: r##"
The `unwind` attribute was malformed.
Erroneous code example:
```ignore (compile_fail not working here; see Issue #43707)
#[unwind()] // error: expected one argument
pub extern fn something() {}
fn main() {}
```
The `#[unwind]` attribute should be used as follows:
- `#[unwind(aborts)]` -- specifies that if a non-Rust ABI function
should abort the process if it attempts to unwind. This is the safer
and preferred option.
- `#[unwind(allowed)]` -- specifies that a non-Rust ABI function
should be allowed to unwind. This can easily result in Undefined
Behavior (UB), so be careful.
NB. The default behavior here is "allowed", but this is unspecified
and likely to change in the future.
"##,

}

register_diagnostics! {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ declare_features! (
// allow `extern "platform-intrinsic" { ... }`
(active, platform_intrinsics, "1.4.0", Some(27731)),

// allow `#[unwind]`
// allow `#[unwind(..)]`
// rust runtime internal
(active, unwind_attributes, "1.4.0", None),

Expand Down
9 changes: 6 additions & 3 deletions src/libunwind/libunwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ pub enum _Unwind_Context {}
pub type _Unwind_Exception_Cleanup_Fn = extern "C" fn(unwind_code: _Unwind_Reason_Code,
exception: *mut _Unwind_Exception);
extern "C" {
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub fn _Unwind_Resume(exception: *mut _Unwind_Exception) -> !;
pub fn _Unwind_DeleteException(exception: *mut _Unwind_Exception);
pub fn _Unwind_GetLanguageSpecificData(ctx: *mut _Unwind_Context) -> *mut c_void;
Expand Down Expand Up @@ -220,7 +221,8 @@ if #[cfg(all(any(target_os = "ios", not(target_arch = "arm"))))] {
if #[cfg(not(all(target_os = "ios", target_arch = "arm")))] {
// Not 32-bit iOS
extern "C" {
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub fn _Unwind_RaiseException(exception: *mut _Unwind_Exception) -> _Unwind_Reason_Code;
pub fn _Unwind_Backtrace(trace: _Unwind_Trace_Fn,
trace_argument: *mut c_void)
Expand All @@ -229,7 +231,8 @@ if #[cfg(not(all(target_os = "ios", target_arch = "arm")))] {
} else {
// 32-bit iOS uses SjLj and does not provide _Unwind_Backtrace()
extern "C" {
#[unwind]
#[cfg_attr(stage0, unwind)]
#[cfg_attr(not(stage0), unwind(allowed))]
pub fn _Unwind_SjLj_RaiseException(e: *mut _Unwind_Exception) -> _Unwind_Reason_Code;
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/codegen/extern-functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ extern {
fn extern_fn();
// CHECK-NOT: Function Attrs: nounwind
// CHECK: declare void @unwinding_extern_fn
#[unwind]
#[unwind(allowed)]
fn unwinding_extern_fn();
}

Expand Down
1 change: 1 addition & 0 deletions src/test/run-pass/abort-on-c-abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::io::prelude::*;
use std::io;
use std::process::{Command, Stdio};

#[unwind(aborts)]
extern "C" fn panic_in_ffi() {
panic!("Test");
}
Expand Down

0 comments on commit a47fd3d

Please sign in to comment.