Skip to content

Commit 51b47dc

Browse files
committed
Auto merge of #46833 - diwic:7c-abort-ffi, r=arielb1
Prevent unwinding past FFI boundaries Second attempt to write a patch to solve this. r? @nikomatsakis ~~So, my biggest issue with this patch is the way the patch determines *what* functions should have an abort landing pad (in `construct_fn`). I would ideally have this code match [src/librustc_trans/callee.rs::get_fn](https://github.com/rust-lang/rust/blob/master/src/librustc_trans/callee.rs#L107-L115) but couldn't find an id that returns true for `is_foreign_item`. Also tried `tcx.has_attr("unwind")` with no luck.~~ FIXED Other issues: * llvm.trap is an SIGILL on amd64. Ideally we could use panic-abort's version of aborting which is nicer but we don't want to depend on that library... * ~~Mir inlining is a stub currently.~~ FIXED (no-op) Also, when reviewing please take into account that I'm new to the code and only partially know what I'm doing... and that I've mostly made made matches on `TerminatorKind::Abort` match either `TerminatorKind::Resume` or `TerminatorKind::Unreachable` based on what looked best.
2 parents 8d4da4f + 4910ed2 commit 51b47dc

File tree

23 files changed

+118
-3
lines changed

23 files changed

+118
-3
lines changed

src/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/librustc/ich/impls_mir.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ for mir::TerminatorKind<'gcx> {
150150
targets.hash_stable(hcx, hasher);
151151
}
152152
mir::TerminatorKind::Resume |
153+
mir::TerminatorKind::Abort |
153154
mir::TerminatorKind::Return |
154155
mir::TerminatorKind::GeneratorDrop |
155156
mir::TerminatorKind::Unreachable => {}

src/librustc/mir/mod.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,10 @@ pub enum TerminatorKind<'tcx> {
637637
/// continue. Emitted by build::scope::diverge_cleanup.
638638
Resume,
639639

640+
/// Indicates that the landing pad is finished and that the process
641+
/// should abort. Used to prevent unwinding for foreign items.
642+
Abort,
643+
640644
/// Indicates a normal return. The return place should have
641645
/// been filled in by now. This should occur at most once.
642646
Return,
@@ -759,7 +763,7 @@ impl<'tcx> TerminatorKind<'tcx> {
759763
match *self {
760764
Goto { target: ref b } => slice::from_ref(b).into_cow(),
761765
SwitchInt { targets: ref b, .. } => b[..].into_cow(),
762-
Resume | GeneratorDrop => (&[]).into_cow(),
766+
Resume | Abort | GeneratorDrop => (&[]).into_cow(),
763767
Return => (&[]).into_cow(),
764768
Unreachable => (&[]).into_cow(),
765769
Call { destination: Some((_, t)), cleanup: Some(c), .. } => vec![t, c].into_cow(),
@@ -794,7 +798,7 @@ impl<'tcx> TerminatorKind<'tcx> {
794798
match *self {
795799
Goto { target: ref mut b } => vec![b],
796800
SwitchInt { targets: ref mut b, .. } => b.iter_mut().collect(),
797-
Resume | GeneratorDrop => Vec::new(),
801+
Resume | Abort | GeneratorDrop => Vec::new(),
798802
Return => Vec::new(),
799803
Unreachable => Vec::new(),
800804
Call { destination: Some((_, ref mut t)), cleanup: Some(ref mut c), .. } => vec![t, c],
@@ -823,6 +827,7 @@ impl<'tcx> TerminatorKind<'tcx> {
823827
match *self {
824828
TerminatorKind::Goto { .. } |
825829
TerminatorKind::Resume |
830+
TerminatorKind::Abort |
826831
TerminatorKind::Return |
827832
TerminatorKind::Unreachable |
828833
TerminatorKind::GeneratorDrop |
@@ -918,6 +923,7 @@ impl<'tcx> TerminatorKind<'tcx> {
918923
Return => write!(fmt, "return"),
919924
GeneratorDrop => write!(fmt, "generator_drop"),
920925
Resume => write!(fmt, "resume"),
926+
Abort => write!(fmt, "abort"),
921927
Yield { ref value, .. } => write!(fmt, "_1 = suspend({:?})", value),
922928
Unreachable => write!(fmt, "unreachable"),
923929
Drop { ref location, .. } => write!(fmt, "drop({:?})", location),
@@ -970,7 +976,7 @@ impl<'tcx> TerminatorKind<'tcx> {
970976
pub fn fmt_successor_labels(&self) -> Vec<Cow<'static, str>> {
971977
use self::TerminatorKind::*;
972978
match *self {
973-
Return | Resume | Unreachable | GeneratorDrop => vec![],
979+
Return | Resume | Abort | Unreachable | GeneratorDrop => vec![],
974980
Goto { .. } => vec!["".into()],
975981
SwitchInt { ref values, .. } => {
976982
values.iter()
@@ -2102,6 +2108,7 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
21022108
},
21032109
GeneratorDrop => GeneratorDrop,
21042110
Resume => Resume,
2111+
Abort => Abort,
21052112
Return => Return,
21062113
Unreachable => Unreachable,
21072114
FalseEdges { real_target, ref imaginary_targets } =>
@@ -2143,6 +2150,7 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
21432150
},
21442151
Goto { .. } |
21452152
Resume |
2153+
Abort |
21462154
Return |
21472155
GeneratorDrop |
21482156
Unreachable |

src/librustc/mir/visit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@ macro_rules! make_mir_visitor {
432432
}
433433

434434
TerminatorKind::Resume |
435+
TerminatorKind::Abort |
435436
TerminatorKind::Return |
436437
TerminatorKind::GeneratorDrop |
437438
TerminatorKind::Unreachable => {

src/librustc_mir/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ graphviz = { path = "../libgraphviz" }
1414
log = "0.3"
1515
log_settings = "0.1.1"
1616
rustc = { path = "../librustc" }
17+
rustc_back = { path = "../librustc_back" }
1718
rustc_const_eval = { path = "../librustc_const_eval" }
1819
rustc_const_math = { path = "../librustc_const_math" }
1920
rustc_data_structures = { path = "../librustc_data_structures" }

src/librustc_mir/borrow_check/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
552552
});
553553
}
554554
TerminatorKind::Goto { target: _ }
555+
| TerminatorKind::Abort
555556
| TerminatorKind::Unreachable
556557
| TerminatorKind::FalseEdges { .. } => {
557558
// no data used, thus irrelevant to borrowck

src/librustc_mir/borrow_check/nll/type_check/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
780780
match term.kind {
781781
TerminatorKind::Goto { .. }
782782
| TerminatorKind::Resume
783+
| TerminatorKind::Abort
783784
| TerminatorKind::Return
784785
| TerminatorKind::GeneratorDrop
785786
| TerminatorKind::Unreachable
@@ -1082,6 +1083,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
10821083
TerminatorKind::Resume => if !is_cleanup {
10831084
span_mirbug!(self, block_data, "resume on non-cleanup block!")
10841085
},
1086+
TerminatorKind::Abort => if !is_cleanup {
1087+
span_mirbug!(self, block_data, "abort on non-cleanup block!")
1088+
},
10851089
TerminatorKind::Return => if is_cleanup {
10861090
span_mirbug!(self, block_data, "return on cleanup block")
10871091
},

src/librustc_mir/build/mod.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use rustc::mir::visit::{MutVisitor, TyContext};
2020
use rustc::ty::{self, Ty, TyCtxt};
2121
use rustc::ty::subst::Substs;
2222
use rustc::util::nodemap::NodeMap;
23+
use rustc_back::PanicStrategy;
2324
use rustc_const_eval::pattern::{BindingMode, PatternKind};
2425
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
2526
use shim;
@@ -353,6 +354,27 @@ macro_rules! unpack {
353354
};
354355
}
355356

357+
fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
358+
fn_id: ast::NodeId,
359+
abi: Abi)
360+
-> bool {
361+
362+
// Not callable from C, so we can safely unwind through these
363+
if abi == Abi::Rust || abi == Abi::RustCall { return false; }
364+
365+
// We never unwind, so it's not relevant to stop an unwind
366+
if tcx.sess.panic_strategy() != PanicStrategy::Unwind { return false; }
367+
368+
// We cannot add landing pads, so don't add one
369+
if tcx.sess.no_landing_pads() { return false; }
370+
371+
// This is a special case: some functions have a C abi but are meant to
372+
// unwind anyway. Don't stop them.
373+
if tcx.has_attr(tcx.hir.local_def_id(fn_id), "unwind") { return false; }
374+
375+
return true;
376+
}
377+
356378
///////////////////////////////////////////////////////////////////////////
357379
/// the main entry point for building MIR for a function
358380
@@ -383,6 +405,10 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
383405
let source_info = builder.source_info(span);
384406
let call_site_s = (call_site_scope, source_info);
385407
unpack!(block = builder.in_scope(call_site_s, LintLevel::Inherited, block, |builder| {
408+
if should_abort_on_panic(tcx, fn_id, abi) {
409+
builder.schedule_abort();
410+
}
411+
386412
let arg_scope_s = (arg_scope, source_info);
387413
unpack!(block = builder.in_scope(arg_scope_s, LintLevel::Inherited, block, |builder| {
388414
builder.args_and_body(block, &arguments, arg_scope, &body.value)

src/librustc_mir/build/scope.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
612612
}
613613
}
614614

615+
// Schedule an abort block - this is used for some ABIs that cannot unwind
616+
pub fn schedule_abort(&mut self) -> BasicBlock {
617+
self.scopes[0].needs_cleanup = true;
618+
let abortblk = self.cfg.start_new_cleanup_block();
619+
let source_info = self.scopes[0].source_info(self.fn_span);
620+
self.cfg.terminate(abortblk, source_info, TerminatorKind::Abort);
621+
self.cached_resume_block = Some(abortblk);
622+
abortblk
623+
}
624+
615625
// Scheduling drops
616626
// ================
617627
/// Indicates that `place` should be dropped on exit from

src/librustc_mir/dataflow/impls/borrows.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
496496
}
497497
}
498498
}
499+
mir::TerminatorKind::Abort |
499500
mir::TerminatorKind::SwitchInt {..} |
500501
mir::TerminatorKind::Drop {..} |
501502
mir::TerminatorKind::DropAndReplace {..} |

0 commit comments

Comments
 (0)