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

rustc: Fill out remaining parts of C-unwind ABI #86155

Merged
merged 6 commits into from Aug 4, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 0 additions & 44 deletions compiler/rustc_attr/src/builtin.rs
Expand Up @@ -87,50 +87,6 @@ pub enum OptimizeAttr {
Size,
}

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

/// Determine what `#[unwind]` attribute is present in `attrs`, if any.
pub fn find_unwind_attr(sess: &Session, attrs: &[Attribute]) -> Option<UnwindAttr> {
attrs.iter().fold(None, |ia, attr| {
if sess.check_name(attr, sym::unwind) {
if let Some(meta) = attr.meta() {
if let MetaItemKind::List(items) = meta.kind {
if items.len() == 1 {
if items[0].has_name(sym::allowed) {
return Some(UnwindAttr::Allowed);
} else if items[0].has_name(sym::aborts) {
return Some(UnwindAttr::Aborts);
}
}

struct_span_err!(
sess.diagnostic(),
attr.span,
E0633,
"malformed `unwind` attribute input"
)
.span_label(attr.span, "invalid argument")
.span_suggestions(
attr.span,
"the allowed arguments are `allowed` and `aborts`",
(vec!["allowed", "aborts"])
.into_iter()
.map(|s| format!("#[unwind({})]", s)),
Applicability::MachineApplicable,
)
.emit();
}
}
}

ia
})
}

/// Represents the following attributes:
///
/// - `#[stable]`
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_error_codes/src/error_codes/E0633.md
@@ -1,8 +1,10 @@
#### Note: this error code is no longer emitted by the compiler.

The `unwind` attribute was malformed.
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved

Erroneous code example:

```compile_fail,E0633
```compile_fail
#![feature(unwind_attributes)]

#[unwind()] // error: expected one argument
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_feature/src/active.rs
Expand Up @@ -311,11 +311,6 @@ declare_features! (
/// Allows `extern "platform-intrinsic" { ... }`.
(active, platform_intrinsics, "1.4.0", Some(27731), None),

/// Allows `#[unwind(..)]`.
///
/// Permits specifying whether a function should permit unwinding or abort on unwind.
(active, unwind_attributes, "1.4.0", Some(58760), None),

/// Allows attributes on expressions and non-item statements.
(active, stmt_expr_attributes, "1.6.0", Some(15701), None),

Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_feature/src/builtin_attrs.rs
Expand Up @@ -419,10 +419,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),
gated!(panic_runtime, AssumedUsed, template!(Word), experimental!(panic_runtime)),
gated!(needs_panic_runtime, AssumedUsed, template!(Word), experimental!(needs_panic_runtime)),
gated!(
unwind, AssumedUsed, template!(List: "allowed|aborts"), unwind_attributes,
experimental!(unwind),
),
gated!(
compiler_builtins, AssumedUsed, template!(Word),
"the `#[compiler_builtins]` attribute is used to identify the `compiler_builtins` crate \
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/removed.rs
Expand Up @@ -156,6 +156,11 @@ declare_features! (
(removed, min_type_alias_impl_trait, "1.56.0", Some(63063), None,
Some("removed in favor of full type_alias_impl_trait")),

/// Allows `#[unwind(..)]`.
///
/// Permits specifying whether a function should permit unwinding or abort on unwind.
(removed, unwind_attributes, "1.56.0", Some(58760), None, Some("use the C-unwind ABI instead")),

// -------------------------------------------------------------------------
// feature-group-end: removed features
// -------------------------------------------------------------------------
Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Expand Up @@ -52,13 +52,9 @@ bitflags! {
/// `#[rustc_allocator]`: a hint to LLVM that the pointer returned from this
/// function is never null.
const ALLOCATOR = 1 << 1;
/// `#[unwind]`: an indicator that this function may unwind despite what
/// its ABI signature may otherwise imply.
const UNWIND = 1 << 2;
/// `#[rust_allocator_nounwind]`, an indicator that an imported FFI
/// function will never unwind. Probably obsolete by recent changes with
/// #[unwind], but hasn't been removed/migrated yet
const RUSTC_ALLOCATOR_NOUNWIND = 1 << 3;
/// An indicator that function will never unwind. Will become obsolete
/// once C-unwind is fully stabilized.
const NEVER_UNWIND = 1 << 3;
/// `#[naked]`: an indicator to LLVM that no function prologue/epilogue
/// should be generated.
const NAKED = 1 << 4;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/terminator.rs
Expand Up @@ -237,7 +237,7 @@ pub enum TerminatorKind<'tcx> {
/// consider it in borrowck. We don't want to accept programs which
/// pass borrowck only when `panic=abort` or some assertions are disabled
/// due to release vs. debug mode builds. This needs to be an `Option` because
/// of the `remove_noop_landing_pads` and `no_landing_pads` passes.
/// of the `remove_noop_landing_pads` and `abort_unwinding_calls` passes.
unwind: Option<BasicBlock>,
},

Expand Down
181 changes: 115 additions & 66 deletions compiler/rustc_middle/src/ty/layout.rs
Expand Up @@ -2601,65 +2601,124 @@ where
fn adjust_for_abi(&mut self, cx: &C, abi: SpecAbi);
}

/// Calculates whether a function's ABI can unwind or not.
///
/// This takes two primary parameters:
///
/// * `codegen_fn_attr_flags` - these are flags calculated as part of the
/// codegen attrs for a defined function. For function pointers this set of
/// flags is the empty set. This is only applicable for Rust-defined
/// functions, and generally isn't needed except for small optimizations where
/// we try to say a function which otherwise might look like it could unwind
/// doesn't actually unwind (such as for intrinsics and such).
///
/// * `abi` - this is the ABI that the function is defined with. This is the
/// primary factor for determining whether a function can unwind or not.
///
/// Note that in this case unwinding is not necessarily panicking in Rust. Rust
/// panics are implemented with unwinds on most platform (when
/// `-Cpanic=unwind`), but this also accounts for `-Cpanic=abort` build modes.
/// Notably unwinding is disallowed for more non-Rust ABIs unless it's
/// specifically in the name (e.g. `"C-unwind"`). Unwinding within each ABI is
/// defined for each ABI individually, but it always corresponds to some form of
/// stack-based unwinding (the exact mechanism of which varies
/// platform-by-platform).
///
/// Rust functions are classfied whether or not they can unwind based on the
/// active "panic strategy". In other words Rust functions are considered to
/// unwind in `-Cpanic=unwind` mode and cannot unwind in `-Cpanic=abort` mode.
/// Note that Rust supports intermingling panic=abort and panic=unwind code, but
/// only if the final panic mode is panic=abort. In this scenario any code
/// previously compiled assuming that a function can unwind is still correct, it
/// just never happens to actually unwind at runtime.
///
/// This function's answer to whether or not a function can unwind is quite
/// impactful throughout the compiler. This affects things like:
///
/// * Calling a function which can't unwind means codegen simply ignores any
/// associated unwinding cleanup.
/// * Calling a function which can unwind from a function which can't unwind
/// causes the `abort_unwinding_calls` MIR pass to insert a landing pad that
/// aborts the process.
/// * This affects whether functions have the LLVM `nounwind` attribute, which
/// affects various optimizations and codegen.
///
/// FIXME: this is actually buggy with respect to Rust functions. Rust functions
/// compiled with `-Cpanic=unwind` and referenced from another crate compiled
/// with `-Cpanic=abort` will look like they can't unwind when in fact they
/// might (from a foreign exception or similar).
pub fn fn_can_unwind(
Copy link
Contributor

Choose a reason for hiding this comment

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

This function could use a doc comment! For example, it wasn't obvious to me that -Cpanic=... affected the result here.

panic_strategy: PanicStrategy,
tcx: TyCtxt<'tcx>,
codegen_fn_attr_flags: CodegenFnAttrFlags,
call_conv: Conv,
abi: SpecAbi,
) -> bool {
if panic_strategy != PanicStrategy::Unwind {
// In panic=abort mode we assume nothing can unwind anywhere, so
// optimize based on this!
false
} else if codegen_fn_attr_flags.contains(CodegenFnAttrFlags::UNWIND) {
// If a specific #[unwind] attribute is present, use that.
true
} else if codegen_fn_attr_flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) {
// Special attribute for allocator functions, which can't unwind.
false
} else {
if call_conv == Conv::Rust {
// Any Rust method (or `extern "Rust" fn` or `extern
// "rust-call" fn`) is explicitly allowed to unwind
// (unless it has no-unwind attribute, handled above).
true
} else {
// Anything else is either:
//
// 1. A foreign item using a non-Rust ABI (like `extern "C" { fn foo(); }`), or
//
// 2. A Rust item using a non-Rust ABI (like `extern "C" fn foo() { ... }`).
//
// In both of these cases, we should refer to the ABI to determine whether or not we
// should unwind. See Rust RFC 2945 for more information on this behavior, here:
// https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
use SpecAbi::*;
match abi {
C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => {
unwind
}
Cdecl
| Fastcall
| Vectorcall
| Aapcs
| Win64
| SysV64
| PtxKernel
| Msp430Interrupt
| X86Interrupt
| AmdGpuKernel
| EfiApi
| AvrInterrupt
| AvrNonBlockingInterrupt
| CCmseNonSecureCall
| Wasm
| RustIntrinsic
| PlatformIntrinsic
| Unadjusted => false,
// In the `if` above, we checked for functions with the Rust calling convention.
Rust | RustCall => unreachable!(),
}
// Special attribute for functions which can't unwind.
if codegen_fn_attr_flags.contains(CodegenFnAttrFlags::NEVER_UNWIND) {
return false;
}

// Otherwise if this isn't special then unwinding is generally determined by
// the ABI of the itself. ABIs like `C` have variants which also
// specifically allow unwinding (`C-unwind`), but not all platform-specific
// ABIs have such an option. Otherwise the only other thing here is Rust
// itself, and those ABIs are determined by the panic strategy configured
// for this compilation.
//
// Unfortunately at this time there's also another caveat. Rust [RFC
// 2945][rfc] has been accepted and is in the process of being implemented
// and stabilized. In this interim state we need to deal with historical
// rustc behavior as well as plan for future rustc behavior.
//
// Historically functions declared with `extern "C"` were marked at the
// codegen layer as `nounwind`. This happened regardless of `panic=unwind`
// or not. This is UB for functions in `panic=unwind` mode that then
// actually panic and unwind. Note that this behavior is true for both
// externally declared functions as well as Rust-defined function.
//
// To fix this UB rustc would like to change in the future to catch unwinds
// from function calls that may unwind within a Rust-defined `extern "C"`
// function and forcibly abort the process, thereby respecting the
// `nounwind` attribut emitted for `extern "C"`. This behavior change isn't
// ready to roll out, so determining whether or not the `C` family of ABIs
// unwinds is conditional not only on their definition but also whether the
// `#![feature(c_unwind)]` feature gate is active.
//
// Note that this means that unlike historical compilers rustc now, by
// default, unconditionally thinks that the `C` ABI may unwind. This will
// prevent some optimization opportunities, however, so we try to scope this
// change and only assume that `C` unwinds with `panic=unwind` (as opposed
// to `panic=abort`).
//
// Eventually the check against `c_unwind` here will ideally get removed and
// this'll be a little cleaner as it'll be a straightforward check of the
// ABI.
//
// [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
use SpecAbi::*;
match abi {
C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => {
unwind
|| (!tcx.features().c_unwind && tcx.sess.panic_strategy() == PanicStrategy::Unwind)
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
}
Cdecl
| Fastcall
| Vectorcall
| Aapcs
| Win64
| SysV64
| PtxKernel
| Msp430Interrupt
| X86Interrupt
| AmdGpuKernel
| EfiApi
| AvrInterrupt
| AvrNonBlockingInterrupt
| CCmseNonSecureCall
| Wasm
| RustIntrinsic
| PlatformIntrinsic
| Unadjusted => false,
Rust | RustCall => tcx.sess.panic_strategy() == PanicStrategy::Unwind,
}
}

Expand Down Expand Up @@ -2695,11 +2754,6 @@ pub fn conv_from_spec_abi(tcx: TyCtxt<'_>, abi: SpecAbi) -> Conv {
}
}

pub fn fn_ptr_codegen_fn_attr_flags() -> CodegenFnAttrFlags {
// Assume that fn pointers may always unwind
CodegenFnAttrFlags::UNWIND
}

impl<'tcx, C> FnAbiExt<'tcx, C> for call::FnAbi<'tcx, Ty<'tcx>>
where
C: LayoutOf<Ty = Ty<'tcx>, TyAndLayout = TyAndLayout<'tcx>>
Expand All @@ -2709,7 +2763,7 @@ where
+ HasParamEnv<'tcx>,
{
fn of_fn_ptr(cx: &C, sig: ty::PolyFnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
call::FnAbi::new_internal(cx, sig, extra_args, None, fn_ptr_codegen_fn_attr_flags(), false)
call::FnAbi::new_internal(cx, sig, extra_args, None, CodegenFnAttrFlags::empty(), false)
}

fn of_instance(cx: &C, instance: ty::Instance<'tcx>, extra_args: &[Ty<'tcx>]) -> Self {
Expand Down Expand Up @@ -2901,12 +2955,7 @@ where
c_variadic: sig.c_variadic,
fixed_count: inputs.len(),
conv,
can_unwind: fn_can_unwind(
cx.tcx().sess.panic_strategy(),
codegen_fn_attr_flags,
conv,
sig.abi,
),
can_unwind: fn_can_unwind(cx.tcx(), codegen_fn_attr_flags, sig.abi),
};
fn_abi.adjust_for_abi(cx, sig.abi);
debug!("FnAbi::new_internal = {:?}", fn_abi);
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_mir/src/interpret/terminator.rs
Expand Up @@ -18,12 +18,7 @@ use super::{

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
fn fn_can_unwind(&self, attrs: CodegenFnAttrFlags, abi: Abi) -> bool {
layout::fn_can_unwind(
self.tcx.sess.panic_strategy(),
attrs,
layout::conv_from_spec_abi(*self.tcx, abi),
abi,
)
layout::fn_can_unwind(*self.tcx, attrs, abi)
}

pub(super) fn eval_terminator(
Expand Down Expand Up @@ -77,7 +72,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(
fn_val,
caller_abi,
self.fn_can_unwind(layout::fn_ptr_codegen_fn_attr_flags(), caller_abi),
self.fn_can_unwind(CodegenFnAttrFlags::empty(), caller_abi),
)
}
ty::FnDef(def_id, substs) => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir/src/shim.rs
Expand Up @@ -16,7 +16,7 @@ use std::fmt;
use std::iter;

use crate::transform::{
add_call_guards, add_moves_for_packed_drops, no_landing_pads, remove_noop_landing_pads,
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, remove_noop_landing_pads,
run_passes, simplify,
};
use crate::util::elaborate_drops::{self, DropElaborator, DropFlagMode, DropStyle};
Expand Down Expand Up @@ -81,10 +81,10 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
MirPhase::Const,
&[&[
&add_moves_for_packed_drops::AddMovesForPackedDrops,
&no_landing_pads::NoLandingPads,
&remove_noop_landing_pads::RemoveNoopLandingPads,
&simplify::SimplifyCfg::new("make_shim"),
&add_call_guards::CriticalCallEdges,
&abort_unwinding_calls::AbortUnwindingCalls,
]],
);

Expand Down