bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

rustc: Fill out remaining parts of C-unwind ABI

Browse source 
This commit intends to fill out some of the remaining pieces of the
C-unwind ABI. This has a number of other changes with it though to move
this design space forward a bit. Notably contained within here is:

* On `panic=unwind`, the `extern "C"` ABI is now considered as "may
  unwind". This fixes a longstanding soundness issue where if you
  `panic!()` in an `extern "C"` function defined in Rust that's actually
  UB because the LLVM representation for the function has the `nounwind`
  attribute, but then you unwind.

* Whether or not a function unwinds now mainly considers the ABI of the
  function instead of first checking the panic strategy. This fixes a
  miscompile of `extern "C-unwind"` with `panic=abort` because that ABI
  can still unwind.

* The aborting stub for non-unwinding ABIs with `panic=unwind` has been
  reimplemented. Previously this was done as a small tweak during MIR
  generation, but this has been moved to a separate and dedicated MIR
  pass. This new pass will, for appropriate functions and function
  calls, insert a `cleanup` landing pad for any function call that may
  unwind within a function that is itself not allowed to unwind. Note
  that this subtly changes some behavior from before where previously on
  an unwind which was caught-to-abort it would run active destructors in
  the function, and now it simply immediately aborts the process.

* The `#[unwind]` attribute has been removed and all users in tests and
  such are now using `C-unwind` and `#![feature(c_unwind)]`.

I think this is largely the last piece of the RFC to implement.
Unfortunately I believe this is still not stabilizable as-is because
activating the feature gate changes the behavior of the existing `extern
"C"` ABI in a way that has no replacement. My thinking for how to enable
this is that we add support for the `C-unwind` ABI on stable Rust first,
and then after it hits stable we change the behavior of the `C` ABI.
That way anyone straddling stable/beta/nightly can switch to `C-unwind`
safely.
This commit is contained in:
Alex Crichton 2021-06-08 11:23:58 -07:00
parent 2939249f29
commit 1c07096a45
46 changed files with 431 additions and 478 deletions
Download patch file Download diff file Expand all files Collapse all files

61
compiler/rustc_mir_build/src/build/mod.rs
View file

@ -2,7 +2,6 @@ use crate::build;
use crate::build::expr::as_place::PlaceBuilder;
use crate::build::scope::DropKind;
use crate::thir::pattern::pat_from_hir;
use rustc_attr::{self as attr, UnwindAttr};
use rustc_errors::ErrorReported;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
@ -19,7 +18,6 @@ use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeckResults};
use rustc_span::symbol::{kw, sym};
use rustc_span::Span;
use rustc_target::spec::abi::Abi;
use rustc_target::spec::PanicStrategy;
use super::lints;
@ -581,60 +579,6 @@ macro_rules! unpack {
}};
}
fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bool {
// Validate `#[unwind]` syntax regardless of platform-specific panic strategy.
let attrs = &tcx.get_attrs(fn_def_id.to_def_id());
let unwind_attr = attr::find_unwind_attr(&tcx.sess, attrs);
// We never unwind, so it's not relevant to stop an unwind.
if tcx.sess.panic_strategy() != PanicStrategy::Unwind {
return false;
}
match unwind_attr {
// If an `#[unwind]` attribute was found, we should adhere to it.
Some(UnwindAttr::Allowed) => false,
Some(UnwindAttr::Aborts) => true,
// If no attribute was found and the panic strategy is `unwind`, then we should examine
// the function's ABI string to determine whether it should abort upon panic.
None if tcx.features().c_unwind => {
use Abi::*;
match abi {
// In the case of ABI's that have an `-unwind` equivalent, check whether the ABI
// permits unwinding. If so, we should not abort. Otherwise, we should.
C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => {
!unwind
}
// Rust and `rust-call` functions are allowed to unwind, and should not abort.
Rust | RustCall => false,
// Other ABI's should abort.
Cdecl
| Fastcall
| Vectorcall
| Aapcs
| Win64
| SysV64
| PtxKernel
| Msp430Interrupt
| X86Interrupt
| AmdGpuKernel
| EfiApi
| AvrInterrupt
| AvrNonBlockingInterrupt
| CCmseNonSecureCall
| Wasm
| RustIntrinsic
| PlatformIntrinsic
| Unadjusted => true,
}
}
// If the `c_unwind` feature gate is not active, follow the behavior that was in place
// prior to #76570. This is a special case: some functions have a C ABI but are meant to
// unwind anyway. Don't stop them.
None => false, // FIXME(#58794); should be `!(abi == Abi::Rust || abi == Abi::RustCall)`
}
}
///////////////////////////////////////////////////////////////////////////
/// the main entry point for building MIR for a function
@ -704,8 +648,7 @@ where
}));
let source_info = builder.source_info(fn_end);
builder.cfg.terminate(return_block, source_info, TerminatorKind::Return);
let should_abort = should_abort_on_panic(tcx, fn_def.did, abi);
builder.build_drop_trees(should_abort);
builder.build_drop_trees();
return_block.unit()
}));
@ -752,7 +695,7 @@ fn construct_const<'a, 'tcx>(
let source_info = builder.source_info(span);
builder.cfg.terminate(block, source_info, TerminatorKind::Return);
builder.build_drop_trees(false);
builder.build_drop_trees();
builder.finish()
}

19
compiler/rustc_mir_build/src/build/scope.rs
View file

@ -1249,21 +1249,20 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> {
}
/// Build the unwind and generator drop trees.
crate fn build_drop_trees(&mut self, should_abort: bool) {
crate fn build_drop_trees(&mut self) {
if self.generator_kind.is_some() {
self.build_generator_drop_trees(should_abort);
self.build_generator_drop_trees();
} else {
Self::build_unwind_tree(
&mut self.cfg,
&mut self.scopes.unwind_drops,
self.fn_span,
should_abort,
&mut None,
);
}
}
fn build_generator_drop_trees(&mut self, should_abort: bool) {
fn build_generator_drop_trees(&mut self) {
// Build the drop tree for dropping the generator while it's suspended.
let drops = &mut self.scopes.generator_drops;
let cfg = &mut self.cfg;
@ -1281,7 +1280,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> {
// Build the drop tree for unwinding in the normal control flow paths.
let resume_block = &mut None;
let unwind_drops = &mut self.scopes.unwind_drops;
Self::build_unwind_tree(cfg, unwind_drops, fn_span, should_abort, resume_block);
Self::build_unwind_tree(cfg, unwind_drops, fn_span, resume_block);
// Build the drop tree for unwinding when dropping a suspended
// generator.
@ -1296,26 +1295,20 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> {
drops.entry_points.push((drop_data.1, blocks[drop_idx].unwrap()));
}
}
Self::build_unwind_tree(cfg, drops, fn_span, should_abort, resume_block);
Self::build_unwind_tree(cfg, drops, fn_span, resume_block);
}
fn build_unwind_tree(
cfg: &mut CFG<'tcx>,
drops: &mut DropTree,
fn_span: Span,
should_abort: bool,
resume_block: &mut Option<BasicBlock>,
) {
let mut blocks = IndexVec::from_elem(None, &drops.drops);
blocks[ROOT_NODE] = *resume_block;
drops.build_mir::<Unwind>(cfg, &mut blocks);
if let (None, Some(resume)) = (*resume_block, blocks[ROOT_NODE]) {
// `TerminatorKind::Abort` is used for `#[unwind(aborts)]`
// functions.
let terminator =
if should_abort { TerminatorKind::Abort } else { TerminatorKind::Resume };
cfg.terminate(resume, SourceInfo::outermost(fn_span), terminator);
cfg.terminate(resume, SourceInfo::outermost(fn_span), TerminatorKind::Resume);
*resume_block = blocks[ROOT_NODE];
}