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.

src/Cargo.lock

@@ -1919,6 +1919,7 @@ dependencies = [
  "log_settings 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc 0.0.0",
  "rustc_apfloat 0.0.0",
+ "rustc_back 0.0.0",
  "rustc_const_eval 0.0.0",
  "rustc_const_math 0.0.0",
  "rustc_data_structures 0.0.0",

src/librustc/ich/impls_mir.rs

@@ -150,6 +150,7 @@ for mir::TerminatorKind<'gcx> {
                 targets.hash_stable(hcx, hasher);
             }
             mir::TerminatorKind::Resume |
+            mir::TerminatorKind::Abort |
             mir::TerminatorKind::Return |
             mir::TerminatorKind::GeneratorDrop |
             mir::TerminatorKind::Unreachable => {}

src/librustc/mir/mod.rs

@@ -637,6 +637,10 @@ pub enum TerminatorKind<'tcx> {
     /// continue. Emitted by build::scope::diverge_cleanup.
     Resume,
 
+    /// Indicates that the landing pad is finished and that the process
+    /// should abort. Used to prevent unwinding for foreign items.
+    Abort,
+
     /// Indicates a normal return. The return place should have
     /// been filled in by now. This should occur at most once.
     Return,
@@ -759,7 +763,7 @@ impl<'tcx> TerminatorKind<'tcx> {
         match *self {
             Goto { target: ref b } => slice::from_ref(b).into_cow(),
             SwitchInt { targets: ref b, .. } => b[..].into_cow(),
-            Resume | GeneratorDrop => (&[]).into_cow(),
+            Resume | Abort | GeneratorDrop => (&[]).into_cow(),
             Return => (&[]).into_cow(),
             Unreachable => (&[]).into_cow(),
             Call { destination: Some((_, t)), cleanup: Some(c), .. } => vec![t, c].into_cow(),
@@ -794,7 +798,7 @@ impl<'tcx> TerminatorKind<'tcx> {
         match *self {
             Goto { target: ref mut b } => vec![b],
             SwitchInt { targets: ref mut b, .. } => b.iter_mut().collect(),
-            Resume | GeneratorDrop => Vec::new(),
+            Resume | Abort | GeneratorDrop => Vec::new(),
             Return => Vec::new(),
             Unreachable => Vec::new(),
             Call { destination: Some((_, ref mut t)), cleanup: Some(ref mut c), .. } => vec![t, c],
@@ -823,6 +827,7 @@ impl<'tcx> TerminatorKind<'tcx> {
         match *self {
             TerminatorKind::Goto { .. } |
             TerminatorKind::Resume |
+            TerminatorKind::Abort |
             TerminatorKind::Return |
             TerminatorKind::Unreachable |
             TerminatorKind::GeneratorDrop |
@@ -918,6 +923,7 @@ impl<'tcx> TerminatorKind<'tcx> {
             Return => write!(fmt, "return"),
             GeneratorDrop => write!(fmt, "generator_drop"),
             Resume => write!(fmt, "resume"),
+            Abort => write!(fmt, "abort"),
             Yield { ref value, .. } => write!(fmt, "_1 = suspend({:?})", value),
             Unreachable => write!(fmt, "unreachable"),
             Drop { ref location, .. } => write!(fmt, "drop({:?})", location),
@@ -970,7 +976,7 @@ impl<'tcx> TerminatorKind<'tcx> {
     pub fn fmt_successor_labels(&self) -> Vec<Cow<'static, str>> {
         use self::TerminatorKind::*;
         match *self {
-            Return | Resume | Unreachable | GeneratorDrop => vec![],
+            Return | Resume | Abort | Unreachable | GeneratorDrop => vec![],
             Goto { .. } => vec!["".into()],
             SwitchInt { ref values, .. } => {
                 values.iter()
@@ -2102,6 +2108,7 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
             },
             GeneratorDrop => GeneratorDrop,
             Resume => Resume,
+            Abort => Abort,
             Return => Return,
             Unreachable => Unreachable,
             FalseEdges { real_target, ref imaginary_targets } =>
@@ -2143,6 +2150,7 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
             },
             Goto { .. } |
             Resume |
+            Abort |
             Return |
             GeneratorDrop |
             Unreachable |

src/librustc/mir/visit.rs

@@ -432,6 +432,7 @@ macro_rules! make_mir_visitor {
                     }
 
                     TerminatorKind::Resume |
+                    TerminatorKind::Abort |
                     TerminatorKind::Return |
                     TerminatorKind::GeneratorDrop |
                     TerminatorKind::Unreachable => {

src/librustc_mir/Cargo.toml

@@ -14,6 +14,7 @@ graphviz = { path = "../libgraphviz" }
 log = "0.3"
 log_settings = "0.1.1"
 rustc = { path = "../librustc" }
+rustc_back = { path = "../librustc_back" }
 rustc_const_eval = { path = "../librustc_const_eval" }
 rustc_const_math = { path = "../librustc_const_math" }
 rustc_data_structures = { path = "../librustc_data_structures" }

src/librustc_mir/borrow_check/mod.rs

@@ -552,6 +552,7 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
                 });
             }
             TerminatorKind::Goto { target: _ }
+            | TerminatorKind::Abort
             | TerminatorKind::Unreachable
             | TerminatorKind::FalseEdges { .. } => {
                 // no data used, thus irrelevant to borrowck

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

@@ -780,6 +780,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
         match term.kind {
             TerminatorKind::Goto { .. }
             | TerminatorKind::Resume
+            | TerminatorKind::Abort
             | TerminatorKind::Return
             | TerminatorKind::GeneratorDrop
             | TerminatorKind::Unreachable
@@ -1082,6 +1083,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
             TerminatorKind::Resume => if !is_cleanup {
                 span_mirbug!(self, block_data, "resume on non-cleanup block!")
             },
+            TerminatorKind::Abort => if !is_cleanup {
+                span_mirbug!(self, block_data, "abort on non-cleanup block!")
+            },
             TerminatorKind::Return => if is_cleanup {
                 span_mirbug!(self, block_data, "return on cleanup block")
             },

src/librustc_mir/build/mod.rs

@@ -20,6 +20,7 @@ use rustc::mir::visit::{MutVisitor, TyContext};
 use rustc::ty::{self, Ty, TyCtxt};
 use rustc::ty::subst::Substs;
 use rustc::util::nodemap::NodeMap;
+use rustc_back::PanicStrategy;
 use rustc_const_eval::pattern::{BindingMode, PatternKind};
 use rustc_data_structures::indexed_vec::{IndexVec, Idx};
 use shim;
@@ -353,6 +354,27 @@ macro_rules! unpack {
     };
 }
 
+fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
+                                         fn_id: ast::NodeId,
+                                         abi: Abi)
+                                         -> bool {
+
+    // Not callable from C, so we can safely unwind through these
+    if abi == Abi::Rust || abi == Abi::RustCall { return false; }
+
+    // We never unwind, so it's not relevant to stop an unwind
+    if tcx.sess.panic_strategy() != PanicStrategy::Unwind { return false; }
+
+    // We cannot add landing pads, so don't add one
+    if tcx.sess.no_landing_pads() { return false; }
+
+    // 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; }
+
+    return true;
+}
+
 ///////////////////////////////////////////////////////////////////////////
 /// the main entry point for building MIR for a function
 
@@ -383,6 +405,10 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
     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) {
+            builder.schedule_abort();
+        }
+
         let arg_scope_s = (arg_scope, source_info);
         unpack!(block = builder.in_scope(arg_scope_s, LintLevel::Inherited, block, |builder| {
             builder.args_and_body(block, &arguments, arg_scope, &body.value)

src/librustc_mir/build/scope.rs

@@ -612,6 +612,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
         }
     }
 
+    // Schedule an abort block - this is used for some ABIs that cannot unwind
+    pub fn schedule_abort(&mut self) -> BasicBlock {
+        self.scopes[0].needs_cleanup = true;
+        let abortblk = self.cfg.start_new_cleanup_block();
+        let source_info = self.scopes[0].source_info(self.fn_span);
+        self.cfg.terminate(abortblk, source_info, TerminatorKind::Abort);
+        self.cached_resume_block = Some(abortblk);
+        abortblk
+    }
+
     // Scheduling drops
     // ================
     /// Indicates that `place` should be dropped on exit from

src/librustc_mir/dataflow/impls/borrows.rs

@@ -496,6 +496,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
                     }
                 }
             }
+            mir::TerminatorKind::Abort |
             mir::TerminatorKind::SwitchInt {..} |
             mir::TerminatorKind::Drop {..} |
             mir::TerminatorKind::DropAndReplace {..} |

src/librustc_mir/dataflow/mod.rs

@@ -771,6 +771,7 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation
         match bb_data.terminator().kind {
             mir::TerminatorKind::Return |
             mir::TerminatorKind::Resume |
+            mir::TerminatorKind::Abort |
             mir::TerminatorKind::GeneratorDrop |
             mir::TerminatorKind::Unreachable => {}
             mir::TerminatorKind::Goto { ref target } |

src/librustc_mir/dataflow/move_paths/builder.rs

@@ -334,6 +334,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
         match term.kind {
             TerminatorKind::Goto { target: _ } |
             TerminatorKind::Resume |
+            TerminatorKind::Abort |
             TerminatorKind::GeneratorDrop |
             TerminatorKind::FalseEdges { .. } |
             TerminatorKind::Unreachable => { }

src/librustc_mir/interpret/terminator/mod.rs

@@ -163,6 +163,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> {
             GeneratorDrop => unimplemented!(),
             DropAndReplace { .. } => unimplemented!(),
             Resume => unimplemented!(),
+            Abort => unimplemented!(),
             FalseEdges { .. } => bug!("should have been eliminated by `simplify_branches` mir pass"),
             Unreachable => return err!(Unreachable),
         }

src/librustc_mir/lib.rs

@@ -49,6 +49,7 @@ extern crate rustc_errors;
 #[macro_use]
 extern crate syntax;
 extern crate syntax_pos;
+extern crate rustc_back;
 extern crate rustc_const_math;
 extern crate rustc_const_eval;
 extern crate core; // for NonZero

src/librustc_mir/monomorphize/collector.rs

@@ -625,6 +625,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
             mir::TerminatorKind::Goto { .. } |
             mir::TerminatorKind::SwitchInt { .. } |
             mir::TerminatorKind::Resume |
+            mir::TerminatorKind::Abort |
             mir::TerminatorKind::Return |
             mir::TerminatorKind::Unreachable |
             mir::TerminatorKind::Assert { .. } => {}

src/librustc_mir/transform/check_unsafety.rs

@@ -73,6 +73,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
             TerminatorKind::DropAndReplace { .. } |
             TerminatorKind::GeneratorDrop |
             TerminatorKind::Resume |
+            TerminatorKind::Abort |
             TerminatorKind::Return |
             TerminatorKind::Unreachable |
             TerminatorKind::FalseEdges { .. } => {

src/librustc_mir/transform/inline.rs

@@ -806,6 +806,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> {
                     *kind = TerminatorKind::Goto { target: tgt }
                 }
             }
+            TerminatorKind::Abort => { }
             TerminatorKind::Unreachable => { }
             TerminatorKind::FalseEdges { ref mut real_target, ref mut imaginary_targets } => {
                 *real_target = self.update_target(*real_target);

src/librustc_mir/transform/qualify_consts.rs

@@ -324,6 +324,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
                 TerminatorKind::SwitchInt {..} |
                 TerminatorKind::DropAndReplace { .. } |
                 TerminatorKind::Resume |
+                TerminatorKind::Abort |
                 TerminatorKind::GeneratorDrop |
                 TerminatorKind::Yield { .. } |
                 TerminatorKind::Unreachable |

src/librustc_mir/transform/remove_noop_landing_pads.rs

@@ -76,6 +76,7 @@ impl RemoveNoopLandingPads {
             TerminatorKind::GeneratorDrop |
             TerminatorKind::Yield { .. } |
             TerminatorKind::Return |
+            TerminatorKind::Abort |
             TerminatorKind::Unreachable |
             TerminatorKind::Call { .. } |
             TerminatorKind::Assert { .. } |

src/librustc_passes/mir_stats.rs

@@ -113,6 +113,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> {
             TerminatorKind::Goto { .. } => "TerminatorKind::Goto",
             TerminatorKind::SwitchInt { .. } => "TerminatorKind::SwitchInt",
             TerminatorKind::Resume => "TerminatorKind::Resume",
+            TerminatorKind::Abort => "TerminatorKind::Abort",
             TerminatorKind::Return => "TerminatorKind::Return",
             TerminatorKind::Unreachable => "TerminatorKind::Unreachable",
             TerminatorKind::Drop { .. } => "TerminatorKind::Drop",

src/librustc_trans/mir/analyze.rs

@@ -236,6 +236,7 @@ pub fn cleanup_kinds<'a, 'tcx>(mir: &mir::Mir<'tcx>) -> IndexVec<mir::BasicBlock
             match data.terminator().kind {
                 TerminatorKind::Goto { .. } |
                 TerminatorKind::Resume |
+                TerminatorKind::Abort |
                 TerminatorKind::Return |
                 TerminatorKind::GeneratorDrop |
                 TerminatorKind::Unreachable |

src/librustc_trans/mir/block.rs

@@ -180,6 +180,13 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
                 }
             }
 
+            mir::TerminatorKind::Abort => {
+                // Call core::intrinsics::abort()
+                let fnname = bcx.ccx.get_intrinsic(&("llvm.trap"));
+                bcx.call(fnname, &[], None);
+                bcx.unreachable();
+            }
+
             mir::TerminatorKind::Goto { target } => {
                 funclet_br(self, bcx, target);
             }

src/test/run-pass/abort-on-c-abi.rs

@@ -0,0 +1,43 @@
+// Copyright 2012-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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Since we mark some ABIs as "nounwind" to LLVM, we must make sure that
+// we never unwind through them.
+
+// ignore-emscripten no processes
+
+use std::{env, panic};
+use std::io::prelude::*;
+use std::io;
+use std::process::{Command, Stdio};
+
+extern "C" fn panic_in_ffi() {
+    panic!("Test");
+}
+
+fn test() {
+    let _ = panic::catch_unwind(|| { panic_in_ffi(); });
+    // The process should have aborted by now.
+    io::stdout().write(b"This should never be printed.\n");
+    let _ = io::stdout().flush();
+}
+
+fn main() {
+    let args: Vec<String> = env::args().collect();
+    if args.len() > 1 && args[1] == "test" {
+        return test();
+    }
+
+    let mut p = Command::new(&args[0])
+                        .stdout(Stdio::piped())
+                        .stdin(Stdio::piped())
+                        .arg("test").spawn().unwrap();
+    assert!(!p.wait().unwrap().success());
+}