Skip to content

Commit 6ef2033

Browse files
committed
Fix FFI-unwind unsoundness with mixed panic mode
1 parent 09d52bc commit 6ef2033

File tree

11 files changed

+212
-14
lines changed

11 files changed

+212
-14
lines changed

compiler/rustc_interface/src/passes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,7 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
944944
if !tcx.sess.opts.debugging_opts.thir_unsafeck {
945945
rustc_mir_transform::check_unsafety::check_unsafety(tcx, def_id);
946946
}
947+
tcx.ensure().has_ffi_unwind_calls(def_id);
947948

948949
if tcx.hir().body_const_context(def_id).is_some() {
949950
tcx.ensure()

compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3230,6 +3230,7 @@ declare_lint_pass! {
32303230
UNEXPECTED_CFGS,
32313231
DEPRECATED_WHERE_CLAUSE_LOCATION,
32323232
TEST_UNSTABLE_LINT,
3233+
FFI_UNWIND_CALLS,
32333234
]
32343235
}
32353236

@@ -3895,3 +3896,42 @@ declare_lint! {
38953896
"this unstable lint is only for testing",
38963897
@feature_gate = sym::test_unstable_lint;
38973898
}
3899+
3900+
declare_lint! {
3901+
/// The `ffi_unwind_calls` lint detects calls to foreign functions or function pointers with
3902+
/// `C-unwind` or other FFI-unwind ABIs.
3903+
///
3904+
/// ### Example
3905+
///
3906+
/// ```rust,ignore (need FFI)
3907+
/// #![feature(ffi_unwind_calls)]
3908+
/// #![feature(c_unwind)]
3909+
///
3910+
/// # mod impl {
3911+
/// # #[no_mangle]
3912+
/// # pub fn "C-unwind" fn foo() {}
3913+
/// # }
3914+
///
3915+
/// extern "C-unwind" {
3916+
/// fn foo();
3917+
/// }
3918+
///
3919+
/// fn bar() {
3920+
/// unsafe { foo(); }
3921+
/// let ptr: unsafe extern "C-unwind" fn() = foo;
3922+
/// unsafe { ptr(); }
3923+
/// }
3924+
/// ```
3925+
///
3926+
/// {{produces}}
3927+
///
3928+
/// ### Explanation
3929+
///
3930+
/// For crates containing such calls, if they are compiled with `-C panic=unwind` then the
3931+
/// produced library cannot be linked with crates compiled with `-C panic=abort`. For crates
3932+
/// that desire this ability it is therefore necessary to avoid such calls.
3933+
pub FFI_UNWIND_CALLS,
3934+
Allow,
3935+
"call to foreign functions or function pointers with FFI-unwind ABI",
3936+
@feature_gate = sym::c_unwind;
3937+
}

compiler/rustc_metadata/src/creader.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ impl<'a> CrateLoader<'a> {
744744
if !data.is_panic_runtime() {
745745
self.sess.err(&format!("the crate `{}` is not a panic runtime", name));
746746
}
747-
if data.panic_strategy() != desired_strategy {
747+
if data.panic_strategy() != Some(desired_strategy) {
748748
self.sess.err(&format!(
749749
"the crate `{}` does not have the panic \
750750
strategy `{}`",

compiler/rustc_metadata/src/dependency_format.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ use rustc_middle::ty::TyCtxt;
6060
use rustc_session::config::CrateType;
6161
use rustc_session::cstore::CrateDepKind;
6262
use rustc_session::cstore::LinkagePreference::{self, RequireDynamic, RequireStatic};
63-
use rustc_target::spec::PanicStrategy;
6463

6564
pub(crate) fn calculate(tcx: TyCtxt<'_>) -> Dependencies {
6665
tcx.sess
@@ -367,7 +366,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) {
367366
prev_name, cur_name
368367
));
369368
}
370-
panic_runtime = Some((cnum, tcx.panic_strategy(cnum)));
369+
panic_runtime = Some((cnum, tcx.panic_strategy(cnum).unwrap()));
371370
}
372371
}
373372

@@ -397,18 +396,14 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) {
397396
if let Linkage::NotLinked = *linkage {
398397
continue;
399398
}
400-
if desired_strategy == PanicStrategy::Abort {
401-
continue;
402-
}
403399
let cnum = CrateNum::new(i + 1);
404400
if tcx.is_compiler_builtins(cnum) {
405401
continue;
406402
}
407403

408-
let found_strategy = tcx.panic_strategy(cnum);
409-
if desired_strategy != found_strategy {
404+
if let Some(found_strategy) = tcx.panic_strategy(cnum) && desired_strategy != found_strategy {
410405
sess.err(&format!(
411-
"the crate `{}` is compiled with the \
406+
"the crate `{}` requires \
412407
panic strategy `{}` which is \
413408
incompatible with this crate's \
414409
strategy of `{}`",

compiler/rustc_metadata/src/rmeta/decoder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1759,7 +1759,7 @@ impl CrateMetadata {
17591759
self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind))
17601760
}
17611761

1762-
pub(crate) fn panic_strategy(&self) -> PanicStrategy {
1762+
pub(crate) fn panic_strategy(&self) -> Option<PanicStrategy> {
17631763
self.root.panic_strategy
17641764
}
17651765

compiler/rustc_metadata/src/rmeta/encoder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
665665
triple: tcx.sess.opts.target_triple.clone(),
666666
hash: tcx.crate_hash(LOCAL_CRATE),
667667
stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(),
668-
panic_strategy: tcx.sess.panic_strategy(),
668+
panic_strategy: tcx.required_panic_strategy(()),
669669
panic_in_drop_strategy: tcx.sess.opts.debugging_opts.panic_in_drop,
670670
edition: tcx.sess.edition(),
671671
has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE),

compiler/rustc_metadata/src/rmeta/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ pub(crate) struct CrateRoot {
217217
extra_filename: String,
218218
hash: Svh,
219219
stable_crate_id: StableCrateId,
220-
panic_strategy: PanicStrategy,
220+
panic_strategy: Option<PanicStrategy>,
221221
panic_in_drop_strategy: PanicStrategy,
222222
edition: Edition,
223223
has_global_allocator: bool,

compiler/rustc_middle/src/query/mod.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,9 +1365,16 @@ rustc_queries! {
13651365
desc { "query a crate is `#![profiler_runtime]`" }
13661366
separate_provide_extern
13671367
}
1368-
query panic_strategy(_: CrateNum) -> PanicStrategy {
1368+
query has_ffi_unwind_calls(key: LocalDefId) -> bool {
1369+
desc { |tcx| "check if `{}` contains FFI-unwind calls", tcx.def_path_str(key.to_def_id()) }
1370+
cache_on_disk_if { true }
1371+
}
1372+
query required_panic_strategy(_: ()) -> Option<PanicStrategy> {
1373+
desc { "compute the required panic strategy for the current crate" }
1374+
}
1375+
query panic_strategy(_: CrateNum) -> Option<PanicStrategy> {
13691376
fatal_cycle
1370-
desc { "query a crate's configured panic strategy" }
1377+
desc { "query a crate's required panic strategy" }
13711378
separate_provide_extern
13721379
}
13731380
query panic_in_drop_strategy(_: CrateNum) -> PanicStrategy {
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
use rustc_hir::def::DefKind;
2+
use rustc_hir::def_id::LocalDefId;
3+
use rustc_middle::mir::*;
4+
use rustc_middle::ty::layout;
5+
use rustc_middle::ty::query::Providers;
6+
use rustc_middle::ty::{self, TyCtxt};
7+
use rustc_session::lint::builtin::FFI_UNWIND_CALLS;
8+
use rustc_target::spec::abi::Abi;
9+
use rustc_target::spec::PanicStrategy;
10+
11+
fn abi_can_unwind(abi: Abi) -> bool {
12+
use Abi::*;
13+
match abi {
14+
C { unwind }
15+
| System { unwind }
16+
| Cdecl { unwind }
17+
| Stdcall { unwind }
18+
| Fastcall { unwind }
19+
| Vectorcall { unwind }
20+
| Thiscall { unwind }
21+
| Aapcs { unwind }
22+
| Win64 { unwind }
23+
| SysV64 { unwind } => unwind,
24+
PtxKernel
25+
| Msp430Interrupt
26+
| X86Interrupt
27+
| AmdGpuKernel
28+
| EfiApi
29+
| AvrInterrupt
30+
| AvrNonBlockingInterrupt
31+
| CCmseNonSecureCall
32+
| Wasm
33+
| RustIntrinsic
34+
| PlatformIntrinsic
35+
| Unadjusted => false,
36+
Rust | RustCall | RustCold => true,
37+
}
38+
}
39+
40+
// Check if the body of this def_id can possibly leak a foreign unwind into Rust code.
41+
fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool {
42+
debug!("has_ffi_unwind_calls({local_def_id:?})");
43+
44+
// Only perform check on functions because constants cannot call FFI functions.
45+
let def_id = local_def_id.to_def_id();
46+
let kind = tcx.def_kind(def_id);
47+
let is_function = match kind {
48+
DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true,
49+
_ => tcx.is_closure(def_id),
50+
};
51+
if !is_function {
52+
return false;
53+
}
54+
55+
let body = &*tcx.mir_built(ty::WithOptConstParam::unknown(local_def_id)).borrow();
56+
57+
let body_ty = tcx.type_of(def_id);
58+
let body_abi = match body_ty.kind() {
59+
ty::FnDef(..) => body_ty.fn_sig(tcx).abi(),
60+
ty::Closure(..) => Abi::RustCall,
61+
ty::Generator(..) => Abi::Rust,
62+
_ => span_bug!(body.span, "unexpected body ty: {:?}", body_ty),
63+
};
64+
let body_can_unwind = layout::fn_can_unwind(tcx, Some(def_id), body_abi);
65+
66+
// Foreign unwinds cannot leak past functions that themselves cannot unwind.
67+
if !body_can_unwind {
68+
return false;
69+
}
70+
71+
let mut tainted = false;
72+
73+
for block in body.basic_blocks() {
74+
if block.is_cleanup {
75+
continue;
76+
}
77+
let Some(terminator) = &block.terminator else { continue };
78+
let TerminatorKind::Call { func, .. } = &terminator.kind else { continue };
79+
80+
let ty = func.ty(body, tcx);
81+
let sig = ty.fn_sig(tcx);
82+
83+
// Rust calls cannot themselves create foreign unwinds.
84+
if let Abi::Rust | Abi::RustCall | Abi::RustCold = sig.abi() {
85+
continue;
86+
};
87+
88+
let fn_def_id = match ty.kind() {
89+
ty::FnPtr(_) => None,
90+
&ty::FnDef(def_id, _) => {
91+
// Rust calls cannot themselves create foreign unwinds.
92+
if !tcx.is_foreign_item(def_id) {
93+
continue;
94+
}
95+
Some(def_id)
96+
}
97+
_ => bug!("invalid callee of type {:?}", ty),
98+
};
99+
100+
if layout::fn_can_unwind(tcx, fn_def_id, sig.abi()) && abi_can_unwind(sig.abi()) {
101+
// We have detected a call that can possibly leak foreign unwind.
102+
//
103+
// Because the function body itself can unwind, we are not aborting this function call
104+
// upon unwind, so this call can possibly leak foreign unwind into Rust code if the
105+
// panic runtime linked is panic-abort.
106+
107+
let lint_root = body.source_scopes[terminator.source_info.scope]
108+
.local_data
109+
.as_ref()
110+
.assert_crate_local()
111+
.lint_root;
112+
let span = terminator.source_info.span;
113+
114+
tcx.struct_span_lint_hir(FFI_UNWIND_CALLS, lint_root, span, |lint| {
115+
let msg = match fn_def_id {
116+
Some(_) => "call to foreign function with FFI-unwind ABI",
117+
None => "call to function pointer with FFI-unwind ABI",
118+
};
119+
let mut db = lint.build(msg);
120+
db.span_label(span, msg);
121+
db.emit();
122+
});
123+
124+
tainted = true;
125+
}
126+
}
127+
128+
tainted
129+
}
130+
131+
fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option<PanicStrategy> {
132+
if tcx.sess.panic_strategy() == PanicStrategy::Abort {
133+
return Some(PanicStrategy::Abort);
134+
}
135+
136+
for def_id in tcx.hir().body_owners() {
137+
if tcx.has_ffi_unwind_calls(def_id) {
138+
return Some(PanicStrategy::Unwind);
139+
}
140+
}
141+
142+
// This crate can be linked with either runtime.
143+
None
144+
}
145+
146+
pub(crate) fn provide(providers: &mut Providers) {
147+
*providers = Providers { has_ffi_unwind_calls, required_panic_strategy, ..*providers };
148+
}

compiler/rustc_mir_transform/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ mod dest_prop;
5757
pub mod dump_mir;
5858
mod early_otherwise_branch;
5959
mod elaborate_drops;
60+
mod ffi_unwind_calls;
6061
mod function_item_references;
6162
mod generator;
6263
mod inline;
@@ -96,6 +97,7 @@ pub fn provide(providers: &mut Providers) {
9697
check_unsafety::provide(providers);
9798
check_packed_ref::provide(providers);
9899
coverage::query::provide(providers);
100+
ffi_unwind_calls::provide(providers);
99101
shim::provide(providers);
100102
*providers = Providers {
101103
mir_keys,
@@ -221,6 +223,9 @@ fn mir_const<'tcx>(
221223
}
222224
}
223225

226+
// has_ffi_unwind_calls query uses the raw mir, so make sure it is run.
227+
tcx.ensure().has_ffi_unwind_calls(def.did);
228+
224229
let mut body = tcx.mir_built(def).steal();
225230

226231
rustc_middle::mir::dump_mir(tcx, None, "mir_map", &0, &body, |_, _| Ok(()));

library/std/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@
210210
#![allow(unused_lifetimes)]
211211
// Tell the compiler to link to either panic_abort or panic_unwind
212212
#![needs_panic_runtime]
213+
// Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind`
214+
#![cfg_attr(not(bootstrap), deny(ffi_unwind_calls))]
213215
// std may use features in a platform-specific way
214216
#![allow(unused_features)]
215217
#![cfg_attr(test, feature(internal_output_capture, print_internals, update_panic_count))]

0 commit comments

Comments
 (0)