about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGary Guo <gary@garyguo.net>2022-05-18 03:51:52 +0100
committerGary Guo <gary@garyguo.net>2022-06-08 21:32:41 +0100
commit6ef203388483688b0a4598efbc63ee7295bff975 (patch)
tree987623cb6b1214023462a9cb6e69f1dbb6471c08
parent09d52bc5d4260bac8b9a2ea8ac7a07c5c72906f1 (diff)
downloadrust-6ef203388483688b0a4598efbc63ee7295bff975.tar.gz
rust-6ef203388483688b0a4598efbc63ee7295bff975.zip
Fix FFI-unwind unsoundness with mixed panic mode
-rw-r--r--compiler/rustc_interface/src/passes.rs1
-rw-r--r--compiler/rustc_lint_defs/src/builtin.rs40
-rw-r--r--compiler/rustc_metadata/src/creader.rs2
-rw-r--r--compiler/rustc_metadata/src/dependency_format.rs11
-rw-r--r--compiler/rustc_metadata/src/rmeta/decoder.rs2
-rw-r--r--compiler/rustc_metadata/src/rmeta/encoder.rs2
-rw-r--r--compiler/rustc_metadata/src/rmeta/mod.rs2
-rw-r--r--compiler/rustc_middle/src/query/mod.rs11
-rw-r--r--compiler/rustc_mir_transform/src/ffi_unwind_calls.rs148
-rw-r--r--compiler/rustc_mir_transform/src/lib.rs5
-rw-r--r--library/std/src/lib.rs2
11 files changed, 212 insertions, 14 deletions
diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs
index 3c867e308c4..ef9c7859dbb 100644
--- a/compiler/rustc_interface/src/passes.rs
+++ b/compiler/rustc_interface/src/passes.rs
@@ -944,6 +944,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
             if !tcx.sess.opts.debugging_opts.thir_unsafeck {
                 rustc_mir_transform::check_unsafety::check_unsafety(tcx, def_id);
             }
+            tcx.ensure().has_ffi_unwind_calls(def_id);
 
             if tcx.hir().body_const_context(def_id).is_some() {
                 tcx.ensure()
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
index a067534b189..48d89a785c1 100644
--- a/compiler/rustc_lint_defs/src/builtin.rs
+++ b/compiler/rustc_lint_defs/src/builtin.rs
@@ -3230,6 +3230,7 @@ declare_lint_pass! {
         UNEXPECTED_CFGS,
         DEPRECATED_WHERE_CLAUSE_LOCATION,
         TEST_UNSTABLE_LINT,
+        FFI_UNWIND_CALLS,
     ]
 }
 
@@ -3895,3 +3896,42 @@ declare_lint! {
     "this unstable lint is only for testing",
     @feature_gate = sym::test_unstable_lint;
 }
+
+declare_lint! {
+    /// The `ffi_unwind_calls` lint detects calls to foreign functions or function pointers with
+    /// `C-unwind` or other FFI-unwind ABIs.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (need FFI)
+    /// #![feature(ffi_unwind_calls)]
+    /// #![feature(c_unwind)]
+    ///
+    /// # mod impl {
+    /// #     #[no_mangle]
+    /// #     pub fn "C-unwind" fn foo() {}
+    /// # }
+    ///
+    /// extern "C-unwind" {
+    ///     fn foo();
+    /// }
+    ///
+    /// fn bar() {
+    ///     unsafe { foo(); }
+    ///     let ptr: unsafe extern "C-unwind" fn() = foo;
+    ///     unsafe { ptr(); }
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// For crates containing such calls, if they are compiled with `-C panic=unwind` then the
+    /// produced library cannot be linked with crates compiled with `-C panic=abort`. For crates
+    /// that desire this ability it is therefore necessary to avoid such calls.
+    pub FFI_UNWIND_CALLS,
+    Allow,
+    "call to foreign functions or function pointers with FFI-unwind ABI",
+    @feature_gate = sym::c_unwind;
+}
diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs
index 947d563ae3c..3f6d1f05005 100644
--- a/compiler/rustc_metadata/src/creader.rs
+++ b/compiler/rustc_metadata/src/creader.rs
@@ -744,7 +744,7 @@ impl<'a> CrateLoader<'a> {
         if !data.is_panic_runtime() {
             self.sess.err(&format!("the crate `{}` is not a panic runtime", name));
         }
-        if data.panic_strategy() != desired_strategy {
+        if data.panic_strategy() != Some(desired_strategy) {
             self.sess.err(&format!(
                 "the crate `{}` does not have the panic \
                                     strategy `{}`",
diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs
index 245b2076ebc..349ff08124c 100644
--- a/compiler/rustc_metadata/src/dependency_format.rs
+++ b/compiler/rustc_metadata/src/dependency_format.rs
@@ -60,7 +60,6 @@ use rustc_middle::ty::TyCtxt;
 use rustc_session::config::CrateType;
 use rustc_session::cstore::CrateDepKind;
 use rustc_session::cstore::LinkagePreference::{self, RequireDynamic, RequireStatic};
-use rustc_target::spec::PanicStrategy;
 
 pub(crate) fn calculate(tcx: TyCtxt<'_>) -> Dependencies {
     tcx.sess
@@ -367,7 +366,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) {
                     prev_name, cur_name
                 ));
             }
-            panic_runtime = Some((cnum, tcx.panic_strategy(cnum)));
+            panic_runtime = Some((cnum, tcx.panic_strategy(cnum).unwrap()));
         }
     }
 
@@ -397,18 +396,14 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) {
             if let Linkage::NotLinked = *linkage {
                 continue;
             }
-            if desired_strategy == PanicStrategy::Abort {
-                continue;
-            }
             let cnum = CrateNum::new(i + 1);
             if tcx.is_compiler_builtins(cnum) {
                 continue;
             }
 
-            let found_strategy = tcx.panic_strategy(cnum);
-            if desired_strategy != found_strategy {
+            if let Some(found_strategy) = tcx.panic_strategy(cnum) && desired_strategy != found_strategy {
                 sess.err(&format!(
-                    "the crate `{}` is compiled with the \
+                    "the crate `{}` requires \
                                panic strategy `{}` which is \
                                incompatible with this crate's \
                                strategy of `{}`",
diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs
index 03ac82b467b..658c51bf620 100644
--- a/compiler/rustc_metadata/src/rmeta/decoder.rs
+++ b/compiler/rustc_metadata/src/rmeta/decoder.rs
@@ -1759,7 +1759,7 @@ impl CrateMetadata {
         self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind))
     }
 
-    pub(crate) fn panic_strategy(&self) -> PanicStrategy {
+    pub(crate) fn panic_strategy(&self) -> Option<PanicStrategy> {
         self.root.panic_strategy
     }
 
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
index 8867e008e42..cf685a7bd62 100644
--- a/compiler/rustc_metadata/src/rmeta/encoder.rs
+++ b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -665,7 +665,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             triple: tcx.sess.opts.target_triple.clone(),
             hash: tcx.crate_hash(LOCAL_CRATE),
             stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(),
-            panic_strategy: tcx.sess.panic_strategy(),
+            panic_strategy: tcx.required_panic_strategy(()),
             panic_in_drop_strategy: tcx.sess.opts.debugging_opts.panic_in_drop,
             edition: tcx.sess.edition(),
             has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE),
diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs
index 04f0847f5cc..60510e535b2 100644
--- a/compiler/rustc_metadata/src/rmeta/mod.rs
+++ b/compiler/rustc_metadata/src/rmeta/mod.rs
@@ -217,7 +217,7 @@ pub(crate) struct CrateRoot {
     extra_filename: String,
     hash: Svh,
     stable_crate_id: StableCrateId,
-    panic_strategy: PanicStrategy,
+    panic_strategy: Option<PanicStrategy>,
     panic_in_drop_strategy: PanicStrategy,
     edition: Edition,
     has_global_allocator: bool,
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
index 5b48f164016..5d0d11a1b78 100644
--- a/compiler/rustc_middle/src/query/mod.rs
+++ b/compiler/rustc_middle/src/query/mod.rs
@@ -1365,9 +1365,16 @@ rustc_queries! {
         desc { "query a crate is `#![profiler_runtime]`" }
         separate_provide_extern
     }
-    query panic_strategy(_: CrateNum) -> PanicStrategy {
+    query has_ffi_unwind_calls(key: LocalDefId) -> bool {
+        desc { |tcx| "check if `{}` contains FFI-unwind calls", tcx.def_path_str(key.to_def_id()) }
+        cache_on_disk_if { true }
+    }
+    query required_panic_strategy(_: ()) -> Option<PanicStrategy> {
+        desc { "compute the required panic strategy for the current crate" }
+    }
+    query panic_strategy(_: CrateNum) -> Option<PanicStrategy> {
         fatal_cycle
-        desc { "query a crate's configured panic strategy" }
+        desc { "query a crate's required panic strategy" }
         separate_provide_extern
     }
     query panic_in_drop_strategy(_: CrateNum) -> PanicStrategy {
diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
new file mode 100644
index 00000000000..e6f8ef5759a
--- /dev/null
+++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
@@ -0,0 +1,148 @@
+use rustc_hir::def::DefKind;
+use rustc_hir::def_id::LocalDefId;
+use rustc_middle::mir::*;
+use rustc_middle::ty::layout;
+use rustc_middle::ty::query::Providers;
+use rustc_middle::ty::{self, TyCtxt};
+use rustc_session::lint::builtin::FFI_UNWIND_CALLS;
+use rustc_target::spec::abi::Abi;
+use rustc_target::spec::PanicStrategy;
+
+fn abi_can_unwind(abi: Abi) -> bool {
+    use Abi::*;
+    match abi {
+        C { unwind }
+        | System { unwind }
+        | Cdecl { unwind }
+        | Stdcall { unwind }
+        | Fastcall { unwind }
+        | Vectorcall { unwind }
+        | Thiscall { unwind }
+        | Aapcs { unwind }
+        | Win64 { unwind }
+        | SysV64 { unwind } => unwind,
+        PtxKernel
+        | Msp430Interrupt
+        | X86Interrupt
+        | AmdGpuKernel
+        | EfiApi
+        | AvrInterrupt
+        | AvrNonBlockingInterrupt
+        | CCmseNonSecureCall
+        | Wasm
+        | RustIntrinsic
+        | PlatformIntrinsic
+        | Unadjusted => false,
+        Rust | RustCall | RustCold => true,
+    }
+}
+
+// Check if the body of this def_id can possibly leak a foreign unwind into Rust code.
+fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool {
+    debug!("has_ffi_unwind_calls({local_def_id:?})");
+
+    // Only perform check on functions because constants cannot call FFI functions.
+    let def_id = local_def_id.to_def_id();
+    let kind = tcx.def_kind(def_id);
+    let is_function = match kind {
+        DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true,
+        _ => tcx.is_closure(def_id),
+    };
+    if !is_function {
+        return false;
+    }
+
+    let body = &*tcx.mir_built(ty::WithOptConstParam::unknown(local_def_id)).borrow();
+
+    let body_ty = tcx.type_of(def_id);
+    let body_abi = match body_ty.kind() {
+        ty::FnDef(..) => body_ty.fn_sig(tcx).abi(),
+        ty::Closure(..) => Abi::RustCall,
+        ty::Generator(..) => Abi::Rust,
+        _ => span_bug!(body.span, "unexpected body ty: {:?}", body_ty),
+    };
+    let body_can_unwind = layout::fn_can_unwind(tcx, Some(def_id), body_abi);
+
+    // Foreign unwinds cannot leak past functions that themselves cannot unwind.
+    if !body_can_unwind {
+        return false;
+    }
+
+    let mut tainted = false;
+
+    for block in body.basic_blocks() {
+        if block.is_cleanup {
+            continue;
+        }
+        let Some(terminator) = &block.terminator else { continue };
+        let TerminatorKind::Call { func, .. } = &terminator.kind else { continue };
+
+        let ty = func.ty(body, tcx);
+        let sig = ty.fn_sig(tcx);
+
+        // Rust calls cannot themselves create foreign unwinds.
+        if let Abi::Rust | Abi::RustCall | Abi::RustCold = sig.abi() {
+            continue;
+        };
+
+        let fn_def_id = match ty.kind() {
+            ty::FnPtr(_) => None,
+            &ty::FnDef(def_id, _) => {
+                // Rust calls cannot themselves create foreign unwinds.
+                if !tcx.is_foreign_item(def_id) {
+                    continue;
+                }
+                Some(def_id)
+            }
+            _ => bug!("invalid callee of type {:?}", ty),
+        };
+
+        if layout::fn_can_unwind(tcx, fn_def_id, sig.abi()) && abi_can_unwind(sig.abi()) {
+            // We have detected a call that can possibly leak foreign unwind.
+            //
+            // Because the function body itself can unwind, we are not aborting this function call
+            // upon unwind, so this call can possibly leak foreign unwind into Rust code if the
+            // panic runtime linked is panic-abort.
+
+            let lint_root = body.source_scopes[terminator.source_info.scope]
+                .local_data
+                .as_ref()
+                .assert_crate_local()
+                .lint_root;
+            let span = terminator.source_info.span;
+
+            tcx.struct_span_lint_hir(FFI_UNWIND_CALLS, lint_root, span, |lint| {
+                let msg = match fn_def_id {
+                    Some(_) => "call to foreign function with FFI-unwind ABI",
+                    None => "call to function pointer with FFI-unwind ABI",
+                };
+                let mut db = lint.build(msg);
+                db.span_label(span, msg);
+                db.emit();
+            });
+
+            tainted = true;
+        }
+    }
+
+    tainted
+}
+
+fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option<PanicStrategy> {
+    if tcx.sess.panic_strategy() == PanicStrategy::Abort {
+        return Some(PanicStrategy::Abort);
+    }
+
+    for def_id in tcx.hir().body_owners() {
+        if tcx.has_ffi_unwind_calls(def_id) {
+            return Some(PanicStrategy::Unwind);
+        }
+    }
+
+    // This crate can be linked with either runtime.
+    None
+}
+
+pub(crate) fn provide(providers: &mut Providers) {
+    *providers = Providers { has_ffi_unwind_calls, required_panic_strategy, ..*providers };
+}
diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs
index 0e57c3c6979..e4591c3f23e 100644
--- a/compiler/rustc_mir_transform/src/lib.rs
+++ b/compiler/rustc_mir_transform/src/lib.rs
@@ -57,6 +57,7 @@ mod dest_prop;
 pub mod dump_mir;
 mod early_otherwise_branch;
 mod elaborate_drops;
+mod ffi_unwind_calls;
 mod function_item_references;
 mod generator;
 mod inline;
@@ -96,6 +97,7 @@ pub fn provide(providers: &mut Providers) {
     check_unsafety::provide(providers);
     check_packed_ref::provide(providers);
     coverage::query::provide(providers);
+    ffi_unwind_calls::provide(providers);
     shim::provide(providers);
     *providers = Providers {
         mir_keys,
@@ -221,6 +223,9 @@ fn mir_const<'tcx>(
         }
     }
 
+    // has_ffi_unwind_calls query uses the raw mir, so make sure it is run.
+    tcx.ensure().has_ffi_unwind_calls(def.did);
+
     let mut body = tcx.mir_built(def).steal();
 
     rustc_middle::mir::dump_mir(tcx, None, "mir_map", &0, &body, |_, _| Ok(()));
diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs
index b1c68ec43bc..fc2ff363513 100644
--- a/library/std/src/lib.rs
+++ b/library/std/src/lib.rs
@@ -210,6 +210,8 @@
 #![allow(unused_lifetimes)]
 // Tell the compiler to link to either panic_abort or panic_unwind
 #![needs_panic_runtime]
+// Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind`
+#![cfg_attr(not(bootstrap), deny(ffi_unwind_calls))]
 // std may use features in a platform-specific way
 #![allow(unused_features)]
 #![cfg_attr(test, feature(internal_output_capture, print_internals, update_panic_count))]