From 6ef203388483688b0a4598efbc63ee7295bff975 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 18 May 2022 03:51:52 +0100 Subject: [PATCH 1/8] Fix FFI-unwind unsoundness with mixed panic mode --- compiler/rustc_interface/src/passes.rs | 1 + compiler/rustc_lint_defs/src/builtin.rs | 40 +++++ compiler/rustc_metadata/src/creader.rs | 2 +- .../rustc_metadata/src/dependency_format.rs | 11 +- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 11 +- .../src/ffi_unwind_calls.rs | 148 ++++++++++++++++++ compiler/rustc_mir_transform/src/lib.rs | 5 + library/std/src/lib.rs | 2 + 11 files changed, 212 insertions(+), 14 deletions(-) create mode 100644 compiler/rustc_mir_transform/src/ffi_unwind_calls.rs diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 3c867e308c40e..ef9c7859dbb7a 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 a067534b18938..48d89a785c117 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 947d563ae3cd1..3f6d1f050056d 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 245b2076ebca9..349ff08124cf6 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 03ac82b467b8e..658c51bf62043 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 { self.root.panic_strategy } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 8867e008e42fa..cf685a7bd6217 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 04f0847f5cccc..60510e535b244 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, 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 5b48f164016f7..5d0d11a1b784a 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 { + desc { "compute the required panic strategy for the current crate" } + } + query panic_strategy(_: CrateNum) -> Option { 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 0000000000000..e6f8ef5759a11 --- /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 { + 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 0e57c3c697999..e4591c3f23e71 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 b1c68ec43bc99..fc2ff3635133c 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))] From 77fd0cc566b4eca108b3580312f1e298fc9af8df Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 19 May 2022 17:38:54 +0100 Subject: [PATCH 2/8] Handle panic runtime specially --- compiler/rustc_metadata/src/dependency_format.rs | 13 +++++++++---- .../rustc_mir_transform/src/ffi_unwind_calls.rs | 6 +++++- .../ui/panic-runtime/transitive-link-a-bunch.stderr | 6 ++---- src/test/ui/panic-runtime/want-unwind-got-abort.rs | 2 +- .../ui/panic-runtime/want-unwind-got-abort.stderr | 4 +--- .../ui/panic-runtime/want-unwind-got-abort2.stderr | 6 ++---- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 349ff08124cf6..e22c903bf336e 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -366,14 +366,19 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { prev_name, cur_name )); } - panic_runtime = Some((cnum, tcx.panic_strategy(cnum).unwrap())); + panic_runtime = Some(( + cnum, + tcx.panic_strategy(cnum).unwrap_or_else(|| { + bug!("cannot determine panic strategy of a panic runtime"); + }), + )); } } // If we found a panic runtime, then we know by this point that it's the // only one, but we perform validation here that all the panic strategy // compilation modes for the whole DAG are valid. - if let Some((cnum, found_strategy)) = panic_runtime { + if let Some((runtime_cnum, found_strategy)) = panic_runtime { let desired_strategy = sess.panic_strategy(); // First up, validate that our selected panic runtime is indeed exactly @@ -383,7 +388,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { "the linked panic runtime `{}` is \ not compiled with this crate's \ panic strategy `{}`", - tcx.crate_name(cnum), + tcx.crate_name(runtime_cnum), desired_strategy.desc() )); } @@ -397,7 +402,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { continue; } let cnum = CrateNum::new(i + 1); - if tcx.is_compiler_builtins(cnum) { + if cnum == runtime_cnum || tcx.is_compiler_builtins(cnum) { continue; } diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index e6f8ef5759a11..78809b105359d 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -1,5 +1,5 @@ use rustc_hir::def::DefKind; -use rustc_hir::def_id::LocalDefId; +use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::query::Providers; @@ -129,6 +129,10 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { } fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option { + if tcx.is_panic_runtime(LOCAL_CRATE) { + return Some(tcx.sess.panic_strategy()); + } + if tcx.sess.panic_strategy() == PanicStrategy::Abort { return Some(PanicStrategy::Abort); } diff --git a/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr b/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr index 4af754c81f97c..7f4a8ed290ecf 100644 --- a/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr +++ b/src/test/ui/panic-runtime/transitive-link-a-bunch.stderr @@ -2,9 +2,7 @@ error: cannot link together two panic runtimes: panic_runtime_unwind and panic_r error: the linked panic runtime `panic_runtime_abort` is not compiled with this crate's panic strategy `unwind` -error: the crate `wants_panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` +error: the crate `wants_panic_runtime_abort` requires panic strategy `abort` which is incompatible with this crate's strategy of `unwind` -error: the crate `panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors diff --git a/src/test/ui/panic-runtime/want-unwind-got-abort.rs b/src/test/ui/panic-runtime/want-unwind-got-abort.rs index c48caaf079077..23bfea6af15c1 100644 --- a/src/test/ui/panic-runtime/want-unwind-got-abort.rs +++ b/src/test/ui/panic-runtime/want-unwind-got-abort.rs @@ -1,6 +1,6 @@ // build-fail // needs-unwind -// error-pattern:is incompatible with this crate's strategy of `unwind` +// error-pattern:is not compiled with this crate's panic strategy `unwind` // aux-build:panic-runtime-abort.rs // aux-build:panic-runtime-lang-items.rs // ignore-wasm32-bare compiled with panic=abort by default diff --git a/src/test/ui/panic-runtime/want-unwind-got-abort.stderr b/src/test/ui/panic-runtime/want-unwind-got-abort.stderr index d4fd2cca81fdc..d306ce6c5ea28 100644 --- a/src/test/ui/panic-runtime/want-unwind-got-abort.stderr +++ b/src/test/ui/panic-runtime/want-unwind-got-abort.stderr @@ -1,6 +1,4 @@ error: the linked panic runtime `panic_runtime_abort` is not compiled with this crate's panic strategy `unwind` -error: the crate `panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr b/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr index 364a27a24eb70..014437b7f1b66 100644 --- a/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr +++ b/src/test/ui/panic-runtime/want-unwind-got-abort2.stderr @@ -1,8 +1,6 @@ error: the linked panic runtime `panic_runtime_abort` is not compiled with this crate's panic strategy `unwind` -error: the crate `wants_panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` +error: the crate `wants_panic_runtime_abort` requires panic strategy `abort` which is incompatible with this crate's strategy of `unwind` -error: the crate `panic_runtime_abort` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind` - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors From bc5afd9e347c79d7e0c910eb4f452410579ca4b6 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 19 May 2022 23:37:26 +0100 Subject: [PATCH 3/8] Ensure ffi_unwind_calls lint is gated behind c_unwind --- .../ui/unwind-abis/feature-gate-c-unwind.rs | 4 +++ .../unwind-abis/feature-gate-c-unwind.stderr | 25 +++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.rs b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs index f02a368d4e097..ba72f74f20ce6 100644 --- a/src/test/ui/unwind-abis/feature-gate-c-unwind.rs +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs @@ -1,6 +1,10 @@ // Test that the "C-unwind" ABI is feature-gated, and cannot be used when the // `c_unwind` feature gate is not used. +#![allow(ffi_unwind_calls)] +//~^ WARNING unknown lint: `ffi_unwind_calls` +//~| WARNING unknown lint: `ffi_unwind_calls` + extern "C-unwind" fn f() {} //~^ ERROR C-unwind ABI is experimental and subject to change [E0658] diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr index f4c785a235f67..a67f46cd2e3b6 100644 --- a/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr @@ -1,5 +1,16 @@ +warning: unknown lint: `ffi_unwind_calls` + --> $DIR/feature-gate-c-unwind.rs:4:1 + | +LL | #![allow(ffi_unwind_calls)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unknown_lints)]` on by default + = note: the `ffi_unwind_calls` lint is unstable + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + error[E0658]: C-unwind ABI is experimental and subject to change - --> $DIR/feature-gate-c-unwind.rs:4:8 + --> $DIR/feature-gate-c-unwind.rs:8:8 | LL | extern "C-unwind" fn f() {} | ^^^^^^^^^^ @@ -7,6 +18,16 @@ LL | extern "C-unwind" fn f() {} = note: see issue #74990 for more information = help: add `#![feature(c_unwind)]` to the crate attributes to enable -error: aborting due to previous error +warning: unknown lint: `ffi_unwind_calls` + --> $DIR/feature-gate-c-unwind.rs:4:1 + | +LL | #![allow(ffi_unwind_calls)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `ffi_unwind_calls` lint is unstable + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + +error: aborting due to previous error; 2 warnings emitted For more information about this error, try `rustc --explain E0658`. From 1750a2f723a796b9c98b223df6013f3b1b0254df Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Fri, 20 May 2022 21:52:00 +0100 Subject: [PATCH 4/8] Add tests for mixed panic mode restriction and lints --- .../ui/panic-runtime/auxiliary/needs-abort.rs | 5 ++++ .../panic-runtime/auxiliary/needs-unwind.rs | 13 ++++++++++ .../ui/panic-runtime/need-abort-got-unwind.rs | 9 +++++++ .../need-abort-got-unwind.stderr | 4 +++ .../ui/panic-runtime/need-unwind-got-abort.rs | 8 ++++++ .../need-unwind-got-abort.stderr | 6 +++++ .../ui/unwind-abis/ffi-unwind-calls-lint.rs | 25 +++++++++++++++++++ .../unwind-abis/ffi-unwind-calls-lint.stderr | 20 +++++++++++++++ 8 files changed, 90 insertions(+) create mode 100644 src/test/ui/panic-runtime/auxiliary/needs-abort.rs create mode 100644 src/test/ui/panic-runtime/auxiliary/needs-unwind.rs create mode 100644 src/test/ui/panic-runtime/need-abort-got-unwind.rs create mode 100644 src/test/ui/panic-runtime/need-abort-got-unwind.stderr create mode 100644 src/test/ui/panic-runtime/need-unwind-got-abort.rs create mode 100644 src/test/ui/panic-runtime/need-unwind-got-abort.stderr create mode 100644 src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs create mode 100644 src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr diff --git a/src/test/ui/panic-runtime/auxiliary/needs-abort.rs b/src/test/ui/panic-runtime/auxiliary/needs-abort.rs new file mode 100644 index 0000000000000..8fad49b5e9d32 --- /dev/null +++ b/src/test/ui/panic-runtime/auxiliary/needs-abort.rs @@ -0,0 +1,5 @@ +// compile-flags:-C panic=abort +// no-prefer-dynamic + +#![crate_type = "rlib"] +#![no_std] diff --git a/src/test/ui/panic-runtime/auxiliary/needs-unwind.rs b/src/test/ui/panic-runtime/auxiliary/needs-unwind.rs new file mode 100644 index 0000000000000..d555b531986b0 --- /dev/null +++ b/src/test/ui/panic-runtime/auxiliary/needs-unwind.rs @@ -0,0 +1,13 @@ +// compile-flags:-C panic=unwind +// no-prefer-dynamic + +#![crate_type = "rlib"] +#![no_std] +#![feature(c_unwind)] + +extern "C-unwind" fn foo() {} + +fn bar() { + let ptr: extern "C-unwind" fn() = foo; + ptr(); +} diff --git a/src/test/ui/panic-runtime/need-abort-got-unwind.rs b/src/test/ui/panic-runtime/need-abort-got-unwind.rs new file mode 100644 index 0000000000000..c72fb96e357f0 --- /dev/null +++ b/src/test/ui/panic-runtime/need-abort-got-unwind.rs @@ -0,0 +1,9 @@ +// build-fail +// needs-unwind +// error-pattern:is incompatible with this crate's strategy of `unwind` +// aux-build:needs-abort.rs +// ignore-wasm32-bare compiled with panic=abort by default + +extern crate needs_abort; + +fn main() {} diff --git a/src/test/ui/panic-runtime/need-abort-got-unwind.stderr b/src/test/ui/panic-runtime/need-abort-got-unwind.stderr new file mode 100644 index 0000000000000..d29c7875fd0ff --- /dev/null +++ b/src/test/ui/panic-runtime/need-abort-got-unwind.stderr @@ -0,0 +1,4 @@ +error: the crate `needs_abort` requires panic strategy `abort` which is incompatible with this crate's strategy of `unwind` + +error: aborting due to previous error + diff --git a/src/test/ui/panic-runtime/need-unwind-got-abort.rs b/src/test/ui/panic-runtime/need-unwind-got-abort.rs new file mode 100644 index 0000000000000..3bcc0aa39ac96 --- /dev/null +++ b/src/test/ui/panic-runtime/need-unwind-got-abort.rs @@ -0,0 +1,8 @@ +// build-fail +// error-pattern:is incompatible with this crate's strategy of `abort` +// aux-build:needs-unwind.rs +// compile-flags:-C panic=abort + +extern crate needs_unwind; + +fn main() {} diff --git a/src/test/ui/panic-runtime/need-unwind-got-abort.stderr b/src/test/ui/panic-runtime/need-unwind-got-abort.stderr new file mode 100644 index 0000000000000..a53b7ffe57485 --- /dev/null +++ b/src/test/ui/panic-runtime/need-unwind-got-abort.stderr @@ -0,0 +1,6 @@ +error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort` + +error: the crate `needs_unwind` requires panic strategy `unwind` which is incompatible with this crate's strategy of `abort` + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs new file mode 100644 index 0000000000000..67c20e9eac54e --- /dev/null +++ b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.rs @@ -0,0 +1,25 @@ +// build-pass + +#![feature(c_unwind)] +#![warn(ffi_unwind_calls)] + +mod foo { + #[no_mangle] + pub extern "C-unwind" fn foo() {} +} + +extern "C-unwind" { + fn foo(); +} + +fn main() { + // Call to Rust function is fine. + foo::foo(); + // Call to foreign function should warn. + unsafe { foo(); } + //~^ WARNING call to foreign function with FFI-unwind ABI + let ptr: extern "C-unwind" fn() = foo::foo; + // Call to function pointer should also warn. + ptr(); + //~^ WARNING call to function pointer with FFI-unwind ABI +} diff --git a/src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr new file mode 100644 index 0000000000000..cf8a7782e35ee --- /dev/null +++ b/src/test/ui/unwind-abis/ffi-unwind-calls-lint.stderr @@ -0,0 +1,20 @@ +warning: call to foreign function with FFI-unwind ABI + --> $DIR/ffi-unwind-calls-lint.rs:19:14 + | +LL | unsafe { foo(); } + | ^^^^^ call to foreign function with FFI-unwind ABI + | +note: the lint level is defined here + --> $DIR/ffi-unwind-calls-lint.rs:4:9 + | +LL | #![warn(ffi_unwind_calls)] + | ^^^^^^^^^^^^^^^^ + +warning: call to function pointer with FFI-unwind ABI + --> $DIR/ffi-unwind-calls-lint.rs:23:5 + | +LL | ptr(); + | ^^^^^ call to function pointer with FFI-unwind ABI + +warning: 2 warnings emitted + From 9e6c044ee6860d8b97324c75cf3dfd6f47e2488e Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 23 May 2022 21:30:38 +0100 Subject: [PATCH 5/8] Use is_fn_like instead of matching on DefKind --- compiler/rustc_mir_transform/src/abort_unwinding_calls.rs | 7 +------ compiler/rustc_mir_transform/src/ffi_unwind_calls.rs | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs index 11980382ffdb2..2c8389e532dd7 100644 --- a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs +++ b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs @@ -1,6 +1,5 @@ use crate::MirPass; use rustc_ast::InlineAsmOptions; -use rustc_hir::def::DefKind; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::{self, TyCtxt}; @@ -31,11 +30,7 @@ impl<'tcx> MirPass<'tcx> for AbortUnwindingCalls { // We don't simplify the MIR of constants at this time because that // namely results in a cyclic query when we call `tcx.type_of` below. - let is_function = match kind { - DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true, - _ => tcx.is_closure(def_id), - }; - if !is_function { + if !kind.is_fn_like() { return; } diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index 78809b105359d..b8ef68f608cc3 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -1,4 +1,3 @@ -use rustc_hir::def::DefKind; use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; use rustc_middle::mir::*; use rustc_middle::ty::layout; @@ -44,11 +43,7 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { // 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 { + if !kind.is_fn_like() { return false; } From 14d155a3dc42b35856e45fd9f4212ac0c432cd10 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 8 Jun 2022 21:32:17 +0100 Subject: [PATCH 6/8] Rename `panic_strategy` query to `required_panic_strategy` --- compiler/rustc_metadata/src/creader.rs | 2 +- compiler/rustc_metadata/src/dependency_format.rs | 4 ++-- compiler/rustc_metadata/src/rmeta/decoder.rs | 4 ++-- compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 5 +---- compiler/rustc_mir_transform/src/ffi_unwind_calls.rs | 6 ++++-- 8 files changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 3f6d1f050056d..907324f0e4fc4 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() != Some(desired_strategy) { + if data.required_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 e22c903bf336e..770d164894a73 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -368,7 +368,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { } panic_runtime = Some(( cnum, - tcx.panic_strategy(cnum).unwrap_or_else(|| { + tcx.required_panic_strategy(cnum).unwrap_or_else(|| { bug!("cannot determine panic strategy of a panic runtime"); }), )); @@ -406,7 +406,7 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { continue; } - if let Some(found_strategy) = tcx.panic_strategy(cnum) && desired_strategy != found_strategy { + if let Some(found_strategy) = tcx.required_panic_strategy(cnum) && desired_strategy != found_strategy { sess.err(&format!( "the crate `{}` requires \ panic strategy `{}` which is \ diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 658c51bf62043..b8efca19fb28b 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1759,8 +1759,8 @@ impl CrateMetadata { self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind)) } - pub(crate) fn panic_strategy(&self) -> Option { - self.root.panic_strategy + pub(crate) fn required_panic_strategy(&self) -> Option { + self.root.required_panic_strategy } pub(crate) fn needs_panic_runtime(&self) -> bool { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index e3581a7607fb8..1237ac4ec4780 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -246,7 +246,7 @@ provide! { <'tcx> tcx, def_id, other, cdata, has_global_allocator => { cdata.root.has_global_allocator } has_panic_handler => { cdata.root.has_panic_handler } is_profiler_runtime => { cdata.root.profiler_runtime } - panic_strategy => { cdata.root.panic_strategy } + required_panic_strategy => { cdata.root.required_panic_strategy } panic_in_drop_strategy => { cdata.root.panic_in_drop_strategy } extern_crate => { let r = *cdata.extern_crate.lock(); diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index cf685a7bd6217..12cc5a72aa736 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.required_panic_strategy(()), + required_panic_strategy: tcx.required_panic_strategy(LOCAL_CRATE), 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 60510e535b244..900d9b9842631 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: Option, + required_panic_strategy: Option, 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 5d0d11a1b784a..bcf7497b832b4 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1369,10 +1369,7 @@ rustc_queries! { 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 { - desc { "compute the required panic strategy for the current crate" } - } - query panic_strategy(_: CrateNum) -> Option { + query required_panic_strategy(_: CrateNum) -> Option { fatal_cycle desc { "query a crate's required panic strategy" } separate_provide_extern diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index b8ef68f608cc3..d09d2a0b26346 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -1,4 +1,4 @@ -use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, LocalDefId, LOCAL_CRATE}; use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::query::Providers; @@ -123,7 +123,9 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { tainted } -fn required_panic_strategy(tcx: TyCtxt<'_>, (): ()) -> Option { +fn required_panic_strategy(tcx: TyCtxt<'_>, cnum: CrateNum) -> Option { + assert_eq!(cnum, LOCAL_CRATE); + if tcx.is_panic_runtime(LOCAL_CRATE) { return Some(tcx.sess.panic_strategy()); } From ce774e377881671b968d193440d99884e84039b6 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 8 Jun 2022 22:42:05 +0100 Subject: [PATCH 7/8] Add a explanation about required panic strategy computation --- .../src/ffi_unwind_calls.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index d09d2a0b26346..7728fdaffb0dc 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -136,6 +136,27 @@ fn required_panic_strategy(tcx: TyCtxt<'_>, cnum: CrateNum) -> Option Date: Tue, 28 Jun 2022 17:18:01 +0100 Subject: [PATCH 8/8] Fix test for non-prefer-dynamic target --- src/test/ui/panic-runtime/need-unwind-got-abort.rs | 1 + src/test/ui/panic-runtime/need-unwind-got-abort.stderr | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/ui/panic-runtime/need-unwind-got-abort.rs b/src/test/ui/panic-runtime/need-unwind-got-abort.rs index 3bcc0aa39ac96..6752ecf90d2f7 100644 --- a/src/test/ui/panic-runtime/need-unwind-got-abort.rs +++ b/src/test/ui/panic-runtime/need-unwind-got-abort.rs @@ -2,6 +2,7 @@ // error-pattern:is incompatible with this crate's strategy of `abort` // aux-build:needs-unwind.rs // compile-flags:-C panic=abort +// no-prefer-dynamic extern crate needs_unwind; diff --git a/src/test/ui/panic-runtime/need-unwind-got-abort.stderr b/src/test/ui/panic-runtime/need-unwind-got-abort.stderr index a53b7ffe57485..4c71df3ebc147 100644 --- a/src/test/ui/panic-runtime/need-unwind-got-abort.stderr +++ b/src/test/ui/panic-runtime/need-unwind-got-abort.stderr @@ -1,6 +1,4 @@ -error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort` - error: the crate `needs_unwind` requires panic strategy `unwind` which is incompatible with this crate's strategy of `abort` -error: aborting due to 2 previous errors +error: aborting due to previous error