From df45c579de97777617b6f12c77a5e6224f54c897 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 27 Aug 2020 11:49:18 -0400 Subject: [PATCH 1/4] rustc_target: add "unwind" payloads to `Abi` ### Overview This commit begins the implementation work for RFC 2945. For more information, see the rendered RFC [1] and tracking issue [2]. A boolean `unwind` payload is added to the `C`, `System`, `Stdcall`, and `Thiscall` variants, marking whether unwinding across FFI boundaries is acceptable. The cases where each of these variants' `unwind` member is true correspond with the `C-unwind`, `system-unwind`, `stdcall-unwind`, and `thiscall-unwind` ABI strings introduced in RFC 2945 [3]. ### Feature Gate and Unstable Book This commit adds a `c_unwind` feature gate for the new ABI strings. Tests for this feature gate are included in `src/test/ui/c-unwind/`, which ensure that this feature gate works correctly for each of the new ABIs. A new language features entry in the unstable book is added as well. ### Further Work To Be Done This commit does not proceed to implement the new unwinding ABIs, and is intentionally scoped specifically to *defining* the ABIs and their feature flag. ### One Note on Test Churn This will lead to some test churn, in re-blessing hash tests, as the deleted comment in `src/librustc_target/spec/abi.rs` mentioned, because we can no longer guarantee the ordering of the `Abi` variants. While this is a downside, this decision was made bearing in mind that RFC 2945 states the following, in the "Other `unwind` Strings" section [3]: > More unwind variants of existing ABI strings may be introduced, > with the same semantics, without an additional RFC. Adding a new variant for each of these cases, rather than specifying a payload for a given ABI, would quickly become untenable, and make working with the `Abi` enum prone to mistakes. This approach encodes the unwinding information *into* a given ABI, to account for the future possibility of other `-unwind` ABI strings. ### Ignore Directives `ignore-*` directives are used in two of our `*-unwind` ABI test cases. Specifically, the `stdcall-unwind` and `thiscall-unwind` test cases ignore architectures that do not support `stdcall` and `thiscall`, respectively. These directives are cribbed from `src/test/ui/c-variadic/variadic-ffi-1.rs` for `stdcall`, and `src/test/ui/extern/extern-thiscall.rs` for `thiscall`. This would otherwise fail on some targets, see: https://github.com/rust-lang-ci/rust/commit/fcf697f90206e9c87b39d494f94ab35d976bfc60 ### Footnotes [1]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md [2]: https://github.com/rust-lang/rust/issues/74990 [3]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md#other-unwind-abi-strings --- compiler/rustc_ast_lowering/src/item.rs | 8 +-- compiler/rustc_ast_passes/src/feature_gate.rs | 32 +++++++++ .../rustc_codegen_cranelift/src/abi/mod.rs | 7 +- compiler/rustc_feature/src/active.rs | 3 + compiler/rustc_middle/src/ty/layout.rs | 8 +-- .../rustc_mir/src/interpret/terminator.rs | 6 +- compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_symbol_mangling/src/v0.rs | 2 +- compiler/rustc_target/src/spec/abi.rs | 65 +++++++++++++++---- compiler/rustc_target/src/spec/arm_base.rs | 11 +++- .../src/spec/mipsel_unknown_none.rs | 6 +- compiler/rustc_target/src/spec/mod.rs | 19 ++++-- .../src/spec/nvptx64_nvidia_cuda.rs | 6 +- compiler/rustc_target/src/spec/riscv_base.rs | 6 +- compiler/rustc_typeck/src/collect.rs | 2 +- compiler/rustc_typeck/src/lib.rs | 21 +++--- .../src/language-features/c-unwind.md | 14 ++++ src/test/ui/codemap_tests/unicode.stderr | 2 +- src/test/ui/parser/issue-8537.stderr | 2 +- src/test/ui/symbol-names/impl1.rs | 4 ++ .../feature-gate-c-unwind-enabled.rs | 12 ++++ .../ui/unwind-abis/feature-gate-c-unwind.rs | 9 +++ .../unwind-abis/feature-gate-c-unwind.stderr | 12 ++++ .../feature-gate-stdcall-unwind.rs | 13 ++++ .../feature-gate-stdcall-unwind.stderr | 12 ++++ .../unwind-abis/feature-gate-system-unwind.rs | 9 +++ .../feature-gate-system-unwind.stderr | 12 ++++ .../feature-gate-thiscall-unwind.rs | 13 ++++ .../feature-gate-thiscall-unwind.stderr | 12 ++++ 29 files changed, 277 insertions(+), 52 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/c-unwind.md create mode 100644 src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-c-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-c-unwind.stderr create mode 100644 src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr create mode 100644 src/test/ui/unwind-abis/feature-gate-system-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-system-unwind.stderr create mode 100644 src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 8b740b7774089..2267f09ec1f11 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -322,10 +322,10 @@ impl<'hir> LoweringContext<'_, 'hir> { }, ItemKind::ForeignMod(ref fm) => { if fm.abi.is_none() { - self.maybe_lint_missing_abi(span, id, abi::Abi::C); + self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false }); } hir::ItemKind::ForeignMod { - abi: fm.abi.map_or(abi::Abi::C, |abi| self.lower_abi(abi)), + abi: fm.abi.map_or(abi::Abi::C { unwind: false }, |abi| self.lower_abi(abi)), items: self .arena .alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))), @@ -1322,8 +1322,8 @@ impl<'hir> LoweringContext<'_, 'hir> { match ext { Extern::None => abi::Abi::Rust, Extern::Implicit => { - self.maybe_lint_missing_abi(span, id, abi::Abi::C); - abi::Abi::C + self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false }); + abi::Abi::C { unwind: false } } Extern::Explicit(abi) => self.lower_abi(abi), } diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index 053fa5b7897dc..8a17ac90a023d 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -164,6 +164,38 @@ impl<'a> PostExpansionVisitor<'a> { "C-cmse-nonsecure-call ABI is experimental and subject to change" ); } + "C-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "C-unwind ABI is experimental and subject to change" + ); + } + "stdcall-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "stdcall-unwind ABI is experimental and subject to change" + ); + } + "system-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "system-unwind ABI is experimental and subject to change" + ); + } + "thiscall-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "thiscall-unwind ABI is experimental and subject to change" + ); + } abi => self .sess .parse_sess diff --git a/compiler/rustc_codegen_cranelift/src/abi/mod.rs b/compiler/rustc_codegen_cranelift/src/abi/mod.rs index c79889f8ca1da..b158d73f3a1a8 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -476,8 +476,11 @@ pub(crate) fn codegen_terminator_call<'tcx>( // FIXME find a cleaner way to support varargs if fn_sig.c_variadic { - if fn_sig.abi != Abi::C { - fx.tcx.sess.span_fatal(span, &format!("Variadic call for non-C abi {:?}", fn_sig.abi)); + if !matches!(fn_sig.abi, Abi::C { .. }) { + fx.tcx.sess.span_fatal( + span, + &format!("Variadic call for non-C abi {:?}", fn_sig.abi), + ); } let sig_ref = fx.bcx.func.dfg.call_signature(call_inst).unwrap(); let abi_params = call_args diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 8ee995a59d80d..95ae6a1ac9a26 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -644,6 +644,9 @@ declare_features! ( /// Allows associated types in inherent impls. (active, inherent_associated_types, "1.52.0", Some(8995), None), + /// Allows `extern "C-unwind" fn` to enable unwinding across ABI boundaries. + (active, c_unwind, "1.52.0", Some(74990), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 6d2ab0e5f5a80..2f5380861c495 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2654,14 +2654,14 @@ where RustIntrinsic | PlatformIntrinsic | Rust | RustCall => Conv::Rust, // It's the ABI's job to select this, not ours. - System => bug!("system abi should be selected elsewhere"), + System { .. } => bug!("system abi should be selected elsewhere"), EfiApi => bug!("eficall abi should be selected elsewhere"), - Stdcall => Conv::X86Stdcall, + Stdcall { .. } => Conv::X86Stdcall, Fastcall => Conv::X86Fastcall, Vectorcall => Conv::X86VectorCall, - Thiscall => Conv::X86ThisCall, - C => Conv::C, + Thiscall { .. } => Conv::X86ThisCall, + C { .. } => Conv::C, Unadjusted => Conv::C, Win64 => Conv::X86_64Win64, SysV64 => Conv::X86_64SysV, diff --git a/compiler/rustc_mir/src/interpret/terminator.rs b/compiler/rustc_mir/src/interpret/terminator.rs index 0807949a2d91b..4aa1360d53539 100644 --- a/compiler/rustc_mir/src/interpret/terminator.rs +++ b/compiler/rustc_mir/src/interpret/terminator.rs @@ -248,9 +248,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; if normalize_abi(caller_abi) != normalize_abi(callee_abi) { throw_ub_format!( - "calling a function with ABI {:?} using caller ABI {:?}", - callee_abi, - caller_abi + "calling a function with ABI {} using caller ABI {}", + callee_abi.name(), + caller_abi.name() ) } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 507eb1e1cbe89..90c5fd422cd51 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -330,6 +330,7 @@ symbols! { bridge, bswap, c_str, + c_unwind, c_variadic, call, call_mut, diff --git a/compiler/rustc_symbol_mangling/src/v0.rs b/compiler/rustc_symbol_mangling/src/v0.rs index bbf7ecc39cfd9..12c0a147990fa 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -440,7 +440,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> { } match sig.abi { Abi::Rust => {} - Abi::C => cx.push("KC"), + Abi::C { unwind: false } => cx.push("KC"), abi => { cx.push("K"); let name = abi.name(); diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index 65e8a4e8db2ad..f7f9c30d3b7a6 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -8,24 +8,21 @@ mod tests; #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug)] #[derive(HashStable_Generic, Encodable, Decodable)] pub enum Abi { - // N.B., this ordering MUST match the AbiDatas array below. - // (This is ensured by the test indices_are_correct().) - // Multiplatform / generic ABIs // // These ABIs come first because every time we add a new ABI, we // have to re-bless all the hashing tests. These are used in many // places, so giving them stable values reduces test churn. The // specific values are meaningless. - Rust = 0, - C = 1, + Rust, + C { unwind: bool }, // Single platform ABIs Cdecl, - Stdcall, + Stdcall { unwind: bool }, Fastcall, Vectorcall, - Thiscall, + Thiscall { unwind: bool }, Aapcs, Win64, SysV64, @@ -39,7 +36,7 @@ pub enum Abi { CCmseNonSecureCall, // Multiplatform / generic ABIs - System, + System { unwind: bool }, RustIntrinsic, RustCall, PlatformIntrinsic, @@ -61,13 +58,16 @@ pub struct AbiData { const AbiDatas: &[AbiData] = &[ // Cross-platform ABIs AbiData { abi: Abi::Rust, name: "Rust", generic: true }, - AbiData { abi: Abi::C, name: "C", generic: true }, + AbiData { abi: Abi::C { unwind: false }, name: "C", generic: true }, + AbiData { abi: Abi::C { unwind: true }, name: "C-unwind", generic: true }, // Platform-specific ABIs AbiData { abi: Abi::Cdecl, name: "cdecl", generic: false }, - AbiData { abi: Abi::Stdcall, name: "stdcall", generic: false }, + AbiData { abi: Abi::Stdcall { unwind: false }, name: "stdcall", generic: false }, + AbiData { abi: Abi::Stdcall { unwind: true }, name: "stdcall-unwind", generic: false }, AbiData { abi: Abi::Fastcall, name: "fastcall", generic: false }, AbiData { abi: Abi::Vectorcall, name: "vectorcall", generic: false }, - AbiData { abi: Abi::Thiscall, name: "thiscall", generic: false }, + AbiData { abi: Abi::Thiscall { unwind: false }, name: "thiscall", generic: false }, + AbiData { abi: Abi::Thiscall { unwind: true }, name: "thiscall-unwind", generic: false }, AbiData { abi: Abi::Aapcs, name: "aapcs", generic: false }, AbiData { abi: Abi::Win64, name: "win64", generic: false }, AbiData { abi: Abi::SysV64, name: "sysv64", generic: false }, @@ -84,7 +84,8 @@ const AbiDatas: &[AbiData] = &[ }, AbiData { abi: Abi::CCmseNonSecureCall, name: "C-cmse-nonsecure-call", generic: false }, // Cross-platform ABIs - AbiData { abi: Abi::System, name: "system", generic: true }, + AbiData { abi: Abi::System { unwind: false }, name: "system", generic: true }, + AbiData { abi: Abi::System { unwind: true }, name: "system-unwind", generic: true }, AbiData { abi: Abi::RustIntrinsic, name: "rust-intrinsic", generic: true }, AbiData { abi: Abi::RustCall, name: "rust-call", generic: true }, AbiData { abi: Abi::PlatformIntrinsic, name: "platform-intrinsic", generic: true }, @@ -103,7 +104,41 @@ pub fn all_names() -> Vec<&'static str> { impl Abi { #[inline] pub fn index(self) -> usize { - self as usize + // N.B., this ordering MUST match the AbiDatas array above. + // (This is ensured by the test indices_are_correct().) + use Abi::*; + match self { + // Cross-platform ABIs + Rust => 0, + C { unwind: false } => 1, + C { unwind: true } => 2, + // Platform-specific ABIs + Cdecl => 3, + Stdcall { unwind: false } => 4, + Stdcall { unwind: true } => 5, + Fastcall => 6, + Vectorcall => 7, + Thiscall { unwind: false } => 8, + Thiscall { unwind: true } => 9, + Aapcs => 10, + Win64 => 11, + SysV64 => 12, + PtxKernel => 13, + Msp430Interrupt => 14, + X86Interrupt => 15, + AmdGpuKernel => 16, + EfiApi => 17, + AvrInterrupt => 18, + AvrNonBlockingInterrupt => 19, + CCmseNonSecureCall => 20, + // Cross-platform ABIs + System { unwind: false } => 21, + System { unwind: true } => 22, + RustIntrinsic => 23, + RustCall => 24, + PlatformIntrinsic => 25, + Unadjusted => 26, + } } #[inline] @@ -122,6 +157,8 @@ impl Abi { impl fmt::Display for Abi { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "\"{}\"", self.name()) + match self { + abi => write!(f, "\"{}\"", abi.name()), + } } } diff --git a/compiler/rustc_target/src/spec/arm_base.rs b/compiler/rustc_target/src/spec/arm_base.rs index b74d80dc6bb2b..01f573313c97f 100644 --- a/compiler/rustc_target/src/spec/arm_base.rs +++ b/compiler/rustc_target/src/spec/arm_base.rs @@ -2,5 +2,14 @@ use crate::spec::abi::Abi; // All the calling conventions trigger an assertion(Unsupported calling convention) in llvm on arm pub fn unsupported_abis() -> Vec { - vec![Abi::Stdcall, Abi::Fastcall, Abi::Vectorcall, Abi::Thiscall, Abi::Win64, Abi::SysV64] + vec![ + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, + Abi::Fastcall, + Abi::Vectorcall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, + Abi::Win64, + Abi::SysV64, + ] } diff --git a/compiler/rustc_target/src/spec/mipsel_unknown_none.rs b/compiler/rustc_target/src/spec/mipsel_unknown_none.rs index 0f9d3c3de1543..110c8dd80ea77 100644 --- a/compiler/rustc_target/src/spec/mipsel_unknown_none.rs +++ b/compiler/rustc_target/src/spec/mipsel_unknown_none.rs @@ -23,10 +23,12 @@ pub fn target() -> Target { panic_strategy: PanicStrategy::Abort, relocation_model: RelocModel::Static, unsupported_abis: vec![ - Abi::Stdcall, + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, Abi::Fastcall, Abi::Vectorcall, - Abi::Thiscall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, Abi::Win64, Abi::SysV64, ], diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index eb1916637529b..85e7c74472f16 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1283,24 +1283,31 @@ impl Target { /// Given a function ABI, turn it into the correct ABI for this target. pub fn adjust_abi(&self, abi: Abi) -> Abi { match abi { - Abi::System => { + Abi::System { unwind } => { if self.is_like_windows && self.arch == "x86" { - Abi::Stdcall + Abi::Stdcall { unwind } } else { - Abi::C + Abi::C { unwind } } } // These ABI kinds are ignored on non-x86 Windows targets. // See https://docs.microsoft.com/en-us/cpp/cpp/argument-passing-and-naming-conventions // and the individual pages for __stdcall et al. - Abi::Stdcall | Abi::Fastcall | Abi::Vectorcall | Abi::Thiscall => { - if self.is_like_windows && self.arch != "x86" { Abi::C } else { abi } + Abi::Stdcall { unwind } | Abi::Thiscall { unwind } => { + if self.is_like_windows && self.arch != "x86" { Abi::C { unwind } } else { abi } + } + Abi::Fastcall | Abi::Vectorcall => { + if self.is_like_windows && self.arch != "x86" { + Abi::C { unwind: false } + } else { + abi + } } Abi::EfiApi => { if self.arch == "x86_64" { Abi::Win64 } else { - Abi::C + Abi::C { unwind: false } } } abi => abi, diff --git a/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs b/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs index 3c9c7d578fbd4..15d8e4843f976 100644 --- a/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs +++ b/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs @@ -49,10 +49,12 @@ pub fn target() -> Target { // create the tests for this. unsupported_abis: vec![ Abi::Cdecl, - Abi::Stdcall, + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, Abi::Fastcall, Abi::Vectorcall, - Abi::Thiscall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, Abi::Aapcs, Abi::Win64, Abi::SysV64, diff --git a/compiler/rustc_target/src/spec/riscv_base.rs b/compiler/rustc_target/src/spec/riscv_base.rs index 64cf890037e51..5bcbb2e621bd0 100644 --- a/compiler/rustc_target/src/spec/riscv_base.rs +++ b/compiler/rustc_target/src/spec/riscv_base.rs @@ -5,10 +5,12 @@ use crate::spec::abi::Abi; pub fn unsupported_abis() -> Vec { vec![ Abi::Cdecl, - Abi::Stdcall, + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, Abi::Fastcall, Abi::Vectorcall, - Abi::Thiscall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, Abi::Aapcs, Abi::Win64, Abi::SysV64, diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 3881d55ef9169..a175da3270638 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2666,7 +2666,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } else if tcx.sess.check_name(attr, sym::used) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED; } else if tcx.sess.check_name(attr, sym::cmse_nonsecure_entry) { - if tcx.fn_sig(id).abi() != abi::Abi::C { + if !matches!(tcx.fn_sig(id).abi(), abi::Abi::C { .. }) { struct_span_err!( tcx.sess, attr.span, diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs index 6ddc26efeae35..fb864c5b4a7d7 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -118,14 +118,19 @@ use astconv::AstConv; use bounds::Bounds; fn require_c_abi_if_c_variadic(tcx: TyCtxt<'_>, decl: &hir::FnDecl<'_>, abi: Abi, span: Span) { - if decl.c_variadic && !(abi == Abi::C || abi == Abi::Cdecl) { - let mut err = struct_span_err!( - tcx.sess, - span, - E0045, - "C-variadic function must have C or cdecl calling convention" - ); - err.span_label(span, "C-variadics require C or cdecl calling convention").emit(); + match (decl.c_variadic, abi) { + // The function has the correct calling convention, or isn't a "C-variadic" function. + (false, _) | (true, Abi::C { .. }) | (true, Abi::Cdecl) => {} + // The function is a "C-variadic" function with an incorrect calling convention. + (true, _) => { + let mut err = struct_span_err!( + tcx.sess, + span, + E0045, + "C-variadic function must have C or cdecl calling convention" + ); + err.span_label(span, "C-variadics require C or cdecl calling convention").emit(); + } } } diff --git a/src/doc/unstable-book/src/language-features/c-unwind.md b/src/doc/unstable-book/src/language-features/c-unwind.md new file mode 100644 index 0000000000000..c1705d59acc21 --- /dev/null +++ b/src/doc/unstable-book/src/language-features/c-unwind.md @@ -0,0 +1,14 @@ +# `c_unwind` + +The tracking issue for this feature is: [#74990] + +[#74990]: https://github.com/rust-lang/rust/issues/74990 + +------------------------ + +Introduces a new ABI string, "C-unwind", to enable unwinding from other +languages (such as C++) into Rust frames and from Rust into other languages. + +See [RFC 2945] for more information. + +[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md diff --git a/src/test/ui/codemap_tests/unicode.stderr b/src/test/ui/codemap_tests/unicode.stderr index 61c3f4f1c9847..b7ba4fa46d92b 100644 --- a/src/test/ui/codemap_tests/unicode.stderr +++ b/src/test/ui/codemap_tests/unicode.stderr @@ -4,7 +4,7 @@ error[E0703]: invalid ABI: found `路濫狼á́́` LL | extern "路濫狼á́́" fn foo() {} | ^^^^^^^^^ invalid ABI | - = help: valid ABIs: Rust, C, cdecl, stdcall, fastcall, vectorcall, thiscall, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, rust-intrinsic, rust-call, platform-intrinsic, unadjusted + = help: valid ABIs: Rust, C, C-unwind, cdecl, stdcall, stdcall-unwind, fastcall, vectorcall, thiscall, thiscall-unwind, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, system-unwind, rust-intrinsic, rust-call, platform-intrinsic, unadjusted error: aborting due to previous error diff --git a/src/test/ui/parser/issue-8537.stderr b/src/test/ui/parser/issue-8537.stderr index 3f63c08021098..85e2b77d867b9 100644 --- a/src/test/ui/parser/issue-8537.stderr +++ b/src/test/ui/parser/issue-8537.stderr @@ -4,7 +4,7 @@ error[E0703]: invalid ABI: found `invalid-ab_isize` LL | "invalid-ab_isize" | ^^^^^^^^^^^^^^^^^^ invalid ABI | - = help: valid ABIs: Rust, C, cdecl, stdcall, fastcall, vectorcall, thiscall, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, rust-intrinsic, rust-call, platform-intrinsic, unadjusted + = help: valid ABIs: Rust, C, C-unwind, cdecl, stdcall, stdcall-unwind, fastcall, vectorcall, thiscall, thiscall-unwind, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, system-unwind, rust-intrinsic, rust-call, platform-intrinsic, unadjusted error: aborting due to previous error diff --git a/src/test/ui/symbol-names/impl1.rs b/src/test/ui/symbol-names/impl1.rs index 771695330d8bd..d3f74c4a4b7b0 100644 --- a/src/test/ui/symbol-names/impl1.rs +++ b/src/test/ui/symbol-names/impl1.rs @@ -74,3 +74,7 @@ fn main() { } }; } + +// FIXME(katie): The 32-bit symbol hash probably needs updating as well, but I'm slightly unsure +// about how to do that. This comment is here so that we don't break the test due to error messages +// including incorrect line numbers. diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs b/src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs new file mode 100644 index 0000000000000..6ff5dbda2d560 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs @@ -0,0 +1,12 @@ +// Test that the "C-unwind" ABI is feature-gated, and *can* be used when the +// `c_unwind` feature gate is enabled. + +// check-pass + +#![feature(c_unwind)] + +extern "C-unwind" fn f() {} + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.rs b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs new file mode 100644 index 0000000000000..f02a368d4e097 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs @@ -0,0 +1,9 @@ +// Test that the "C-unwind" ABI is feature-gated, and cannot be used when the +// `c_unwind` feature gate is not used. + +extern "C-unwind" fn f() {} +//~^ ERROR C-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr new file mode 100644 index 0000000000000..f4c785a235f67 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: C-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-c-unwind.rs:4:8 + | +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 + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs new file mode 100644 index 0000000000000..7d4dc8c9343f1 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs @@ -0,0 +1,13 @@ +// ignore-arm stdcall isn't supported +// ignore-aarch64 stdcall isn't supported +// ignore-riscv64 stdcall isn't supported + +// Test that the "stdcall-unwind" ABI is feature-gated, and cannot be used when +// the `c_unwind` feature gate is not used. + +extern "stdcall-unwind" fn f() {} +//~^ ERROR stdcall-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr new file mode 100644 index 0000000000000..e3d569f464f87 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: stdcall-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-stdcall-unwind.rs:8:8 + | +LL | extern "stdcall-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 + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unwind-abis/feature-gate-system-unwind.rs b/src/test/ui/unwind-abis/feature-gate-system-unwind.rs new file mode 100644 index 0000000000000..26c2de4e81767 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-system-unwind.rs @@ -0,0 +1,9 @@ +// Test that the "system-unwind" ABI is feature-gated, and cannot be used when +// the `c_unwind` feature gate is not used. + +extern "system-unwind" fn f() {} +//~^ ERROR system-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-system-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-system-unwind.stderr new file mode 100644 index 0000000000000..87877336475b4 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-system-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: system-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-system-unwind.rs:4:8 + | +LL | extern "system-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 + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs new file mode 100644 index 0000000000000..2f4cefccc1967 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs @@ -0,0 +1,13 @@ +// ignore-arm thiscall isn't supported +// ignore-aarch64 thiscall isn't supported +// ignore-riscv64 thiscall isn't supported + +// Test that the "thiscall-unwind" ABI is feature-gated, and cannot be used when +// the `c_unwind` feature gate is not used. + +extern "thiscall-unwind" fn f() {} +//~^ ERROR thiscall-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr new file mode 100644 index 0000000000000..b103bb8d56585 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: thiscall-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-thiscall-unwind.rs:8:8 + | +LL | extern "thiscall-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 + +For more information about this error, try `rustc --explain E0658`. From 0f33e9f2816358a4d8afd02af35bd23a4d6d0857 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 10 Sep 2020 13:38:39 -0400 Subject: [PATCH 2/4] implement unwinding abi's (RFC 2945) ### Changes This commit implements unwind ABI's, specified in RFC 2945. We adjust the `rustc_middle::ty::layout::fn_can_unwind` function, used to compute whether or not a `FnAbi` object represents a function that should be able to unwind when `panic=unwind` is in use. Changes are also made to `rustc_mir_build::build::should_abort_on_panic` so that the function ABI is used to determind whether it should abort, assuming that the `panic=unwind` strategy is being used, and no explicit unwind attribute was provided. ### Tests Unit tests, checking that the behavior is correct for `C-unwind`, `stdcall-unwind`, `system-unwind`, and `thiscall-unwind`, are included. These alternative `unwind` ABI strings are specified in RFC 2945, in the "_Other `unwind` ABI strings_" section. Additionally, a test case is included to assert that the LLVM IR generated for an external function defined with the `C-unwind` ABI will be appropriately labeled with the `nounwind` LLVM attribute when the `panic=abort` compilation flag is used. ### Ignore Directives This commit uses `ignore-*` directives in two of our `*-unwind` ABI test cases. Specifically, the `stdcall-unwind` and `thiscall-unwind` test cases ignore architectures that do not support `stdcall` and `thiscall`, respectively. These directives are cribbed from `src/test/ui/c-variadic/variadic-ffi-1.rs` for `stdcall`, and `src/test/ui/extern/extern-thiscall.rs` for `thiscall`. --- compiler/rustc_middle/src/ty/layout.rs | 29 +++++++++------- compiler/rustc_mir_build/src/build/mod.rs | 22 ++++++++++--- .../unwind-abis/c-unwind-abi-panic-abort.rs | 18 ++++++++++ src/test/codegen/unwind-abis/c-unwind-abi.rs | 29 ++++++++++++++++ .../codegen/unwind-abis/stdcall-unwind-abi.rs | 32 ++++++++++++++++++ .../codegen/unwind-abis/system-unwind-abi.rs | 29 ++++++++++++++++ .../unwind-abis/thiscall-unwind-abi.rs | 33 +++++++++++++++++++ 7 files changed, 176 insertions(+), 16 deletions(-) create mode 100644 src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs create mode 100644 src/test/codegen/unwind-abis/c-unwind-abi.rs create mode 100644 src/test/codegen/unwind-abis/stdcall-unwind-abi.rs create mode 100644 src/test/codegen/unwind-abis/system-unwind-abi.rs create mode 100644 src/test/codegen/unwind-abis/thiscall-unwind-abi.rs diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 2f5380861c495..ee2dffd8baeb9 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2562,6 +2562,7 @@ fn fn_can_unwind( panic_strategy: PanicStrategy, codegen_fn_attr_flags: CodegenFnAttrFlags, call_conv: Conv, + abi: SpecAbi, ) -> bool { if panic_strategy != PanicStrategy::Unwind { // In panic=abort mode we assume nothing can unwind anywhere, so @@ -2586,17 +2587,16 @@ fn fn_can_unwind( // // 2. A Rust item using a non-Rust ABI (like `extern "C" fn foo() { ... }`). // - // Foreign items (case 1) are assumed to not unwind; it is - // UB otherwise. (At least for now; see also - // rust-lang/rust#63909 and Rust RFC 2753.) - // - // Items defined in Rust with non-Rust ABIs (case 2) are also - // not supposed to unwind. Whether this should be enforced - // (versus stating it is UB) and *how* it would be enforced - // is currently under discussion; see rust-lang/rust#58794. - // - // In either case, we mark item as explicitly nounwind. - false + // In both of these cases, we should refer to the ABI to determine whether or not we + // should unwind. See Rust RFC 2945 for more information on this behavior, here: + // https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md + use SpecAbi::*; + match abi { + C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => { + unwind + } + _ => false, + } } } } @@ -2823,7 +2823,12 @@ where c_variadic: sig.c_variadic, fixed_count: inputs.len(), conv, - can_unwind: fn_can_unwind(cx.tcx().sess.panic_strategy(), codegen_fn_attr_flags, conv), + can_unwind: fn_can_unwind( + cx.tcx().sess.panic_strategy(), + codegen_fn_attr_flags, + conv, + sig.abi, + ), }; fn_abi.adjust_for_abi(cx, sig.abi); debug!("FnAbi::new_internal = {:?}", fn_abi); diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index b928458df8ee4..dd9c859544f61 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -548,7 +548,7 @@ macro_rules! unpack { }}; } -fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, _abi: Abi) -> bool { +fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bool { // Validate `#[unwind]` syntax regardless of platform-specific panic strategy. let attrs = &tcx.get_attrs(fn_def_id.to_def_id()); let unwind_attr = attr::find_unwind_attr(&tcx.sess, attrs); @@ -558,12 +558,26 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, _abi: Abi) -> b return false; } - // This is a special case: some functions have a C abi but are meant to - // unwind anyway. Don't stop them. match unwind_attr { - None => false, // FIXME(#58794); should be `!(abi == Abi::Rust || abi == Abi::RustCall)` + // If an `#[unwind]` attribute was found, we should adhere to it. Some(UnwindAttr::Allowed) => false, Some(UnwindAttr::Aborts) => true, + // If no attribute was found and the panic strategy is `unwind`, then we should examine + // the function's ABI string to determine whether it should abort upon panic. + None => { + use Abi::*; + match abi { + // In the case of ABI's that have an `-unwind` equivalent, check whether the ABI + // permits unwinding. If so, we should not abort. Otherwise, we should. + C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => { + !unwind + } + // Rust and `rust-call` functions are allowed to unwind, and should not abort. + Rust | RustCall => false, + // Other ABI's should abort. + _ => true, + } + } } } diff --git a/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs b/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs new file mode 100644 index 0000000000000..afd65ff6741a6 --- /dev/null +++ b/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs @@ -0,0 +1,18 @@ +// compile-flags: -C panic=abort -C opt-level=0 + +// Test that `nounwind` atributes are applied to `C-unwind` extern functions when the +// code is compiled with `panic=abort`. We disable optimizations above to prevent LLVM from +// inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "C-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make sure that the LLVM attributes for this functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } diff --git a/src/test/codegen/unwind-abis/c-unwind-abi.rs b/src/test/codegen/unwind-abis/c-unwind-abi.rs new file mode 100644 index 0000000000000..f157653675322 --- /dev/null +++ b/src/test/codegen/unwind-abis/c-unwind-abi.rs @@ -0,0 +1,29 @@ +// compile-flags: -C opt-level=0 + +// Test that `nounwind` atributes are correctly applied to exported `C` and `C-unwind` extern +// functions. `C-unwind` functions MUST NOT have this attribute. We disable optimizations above +// to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "C" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "C-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } diff --git a/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs b/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs new file mode 100644 index 0000000000000..ed804ca278d2d --- /dev/null +++ b/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs @@ -0,0 +1,32 @@ +// compile-flags: -C opt-level=0 +// ignore-arm stdcall isn't supported +// ignore-aarch64 stdcall isn't supported +// ignore-riscv64 stdcall isn't supported + +// Test that `nounwind` atributes are correctly applied to exported `stdcall` and `stdcall-unwind` +// extern functions. `stdcall-unwind` functions MUST NOT have this attribute. We disable +// optimizations above to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "stdcall" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "stdcall-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } diff --git a/src/test/codegen/unwind-abis/system-unwind-abi.rs b/src/test/codegen/unwind-abis/system-unwind-abi.rs new file mode 100644 index 0000000000000..c4d51328352c0 --- /dev/null +++ b/src/test/codegen/unwind-abis/system-unwind-abi.rs @@ -0,0 +1,29 @@ +// compile-flags: -C opt-level=0 + +// Test that `nounwind` atributes are correctly applied to exported `system` and `system-unwind` +// extern functions. `system-unwind` functions MUST NOT have this attribute. We disable +// optimizations above to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "system" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "system-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } diff --git a/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs b/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs new file mode 100644 index 0000000000000..aaa63ae55c3ac --- /dev/null +++ b/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs @@ -0,0 +1,33 @@ +// compile-flags: -C opt-level=0 +// ignore-arm thiscall isn't supported +// ignore-aarch64 thiscall isn't supported +// ignore-riscv64 thiscall isn't supported + +// Test that `nounwind` atributes are correctly applied to exported `thiscall` and +// `thiscall-unwind` extern functions. `thiscall-unwind` functions MUST NOT have this attribute. We +// disable optimizations above to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(abi_thiscall)] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "thiscall" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "thiscall-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } From baf227ea0c1e07fc54395a51e4b3881d701180cb Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 11 Dec 2020 20:54:47 -0500 Subject: [PATCH 3/4] add integration tests, unwind across FFI boundary ### Integration Tests This commit introduces some new fixtures to the `run-make-fulldeps` test suite. * c-unwind-abi-catch-panic: Exercise unwinding a panic. This catches a panic across an FFI boundary and downcasts it into an integer. * c-unwind-abi-catch-lib-panic: This is similar to the previous `*catch-panic` test, however in this case the Rust code that panics resides in a separate crate. ### Add `rust_eh_personality` to `#[no_std]` alloc tests This commit addresses some test failures that now occur in the following two tests: * no_std-alloc-error-handler-custom.rs * no_std-alloc-error-handler-default.rs Each test now defines a `rust_eh_personality` extern function, in the same manner as shown in the "Writing an executable without stdlib" section of the `lang_items` documentation here: https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib Without this change, these tests would fail to compile due to a linking error explaining that there was an "undefined reference to `rust_eh_personality'." ### Updated hash * update 32-bit hash in `impl1` test ### Panics This commit uses `panic!` macro invocations that return a string, rather than using an integer as a panic payload. Doing so avoids the following warnings that were observed during rollup for the `*-msvc-1` targets: ``` warning: panic message is not a string literal --> panic.rs:10:16 | 10 | panic!(x); // That is too big! | ^ | = note: `#[warn(non_fmt_panic)]` on by default = note: this is no longer accepted in Rust 2021 help: add a "{}" format string to Display the message | 10 | panic!("{}", x); // That is too big! | ^^^^^ help: or use std::panic::panic_any instead | 10 | std::panic::panic_any(x); // That is too big! | ^^^^^^^^^^^^^^^^^^^^^ warning: 1 warning emitted ``` See: https://github.com/rust-lang-ci/rust/runs/1992118428 As these errors imply, panicking without a format string will be disallowed in Rust 2021, per #78500. --- .../c-unwind-abi-catch-lib-panic/Makefile | 30 +++++++++++++ .../c-unwind-abi-catch-lib-panic/add.c | 12 +++++ .../c-unwind-abi-catch-lib-panic/main.rs | 35 +++++++++++++++ .../c-unwind-abi-catch-lib-panic/panic.rs | 12 +++++ .../c-unwind-abi-catch-panic/Makefile | 5 +++ .../c-unwind-abi-catch-panic/add.c | 12 +++++ .../c-unwind-abi-catch-panic/main.rs | 44 +++++++++++++++++++ .../no_std-alloc-error-handler-custom.rs | 9 +++- .../no_std-alloc-error-handler-default.rs | 9 +++- src/test/ui/symbol-names/impl1.rs | 4 -- 10 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile new file mode 100644 index 0000000000000..a8515c533af57 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile @@ -0,0 +1,30 @@ +-include ../tools.mk + +all: archive + # Compile `main.rs`, which will link into our library, and run it. + $(RUSTC) main.rs + $(call RUN,main) + +ifdef IS_MSVC +archive: add.o panic.o + # Now, create an archive using these two objects. + $(AR) crus $(TMPDIR)/add.lib $(TMPDIR)/add.o $(TMPDIR)/panic.o +else +archive: add.o panic.o + # Now, create an archive using these two objects. + $(AR) crus $(TMPDIR)/libadd.a $(TMPDIR)/add.o $(TMPDIR)/panic.o +endif + +# Compile `panic.rs` into an object file. +# +# Note that we invoke `rustc` directly, so we may emit an object rather +# than an archive. We'll do that later. +panic.o: + $(BARE_RUSTC) $(RUSTFLAGS) \ + --out-dir $(TMPDIR) \ + --emit=obj panic.rs + +# Compile `add.c` into an object file. +add.o: + $(call COMPILE_OBJ,$(TMPDIR)/add.o,add.c) + diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c new file mode 100644 index 0000000000000..444359451f6ec --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c @@ -0,0 +1,12 @@ +#ifdef _WIN32 +__declspec(dllexport) +#endif + +// An external function, defined in Rust. +extern void panic_if_greater_than_10(unsigned x); + +unsigned add_small_numbers(unsigned a, unsigned b) { + unsigned c = a + b; + panic_if_greater_than_10(c); + return c; +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs new file mode 100644 index 0000000000000..78a71219c7811 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs @@ -0,0 +1,35 @@ +//! A test for calling `C-unwind` functions across foreign function boundaries. +//! +//! This test triggers a panic in a Rust library that our foreign function invokes. This shows +//! that we can unwind through the C code in that library, and catch the underlying panic. +#![feature(c_unwind)] + +use std::panic::{catch_unwind, AssertUnwindSafe}; + +fn main() { + // Call `add_small_numbers`, passing arguments that will NOT trigger a panic. + let (a, b) = (9, 1); + let c = unsafe { add_small_numbers(a, b) }; + assert_eq!(c, 10); + + // Call `add_small_numbers`, passing arguments that will trigger a panic, and catch it. + let caught_unwind = catch_unwind(AssertUnwindSafe(|| { + let (a, b) = (10, 1); + let _c = unsafe { add_small_numbers(a, b) }; + unreachable!("should have unwound instead of returned"); + })); + + // Assert that we did indeed panic, then unwrap and downcast the panic into the sum. + assert!(caught_unwind.is_err()); + let panic_obj = caught_unwind.unwrap_err(); + let msg = panic_obj.downcast_ref::().unwrap(); + assert_eq!(msg, "11"); +} + +#[link(name = "add", kind = "static")] +extern "C-unwind" { + /// An external function, defined in C. + /// + /// Returns the sum of two numbers, or panics if the sum is greater than 10. + fn add_small_numbers(a: u32, b: u32) -> u32; +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs new file mode 100644 index 0000000000000..a99a04d5c6f4b --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs @@ -0,0 +1,12 @@ +#![crate_type = "staticlib"] +#![feature(c_unwind)] + +/// This function will panic if `x` is greater than 10. +/// +/// This function is called by `add_small_numbers`. +#[no_mangle] +pub extern "C-unwind" fn panic_if_greater_than_10(x: u32) { + if x > 10 { + panic!("{}", x); // That is too big! + } +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile new file mode 100644 index 0000000000000..9553b7aeeb983 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile @@ -0,0 +1,5 @@ +-include ../tools.mk + +all: $(call NATIVE_STATICLIB,add) + $(RUSTC) main.rs + $(call RUN,main) || exit 1 diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c new file mode 100644 index 0000000000000..444359451f6ec --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c @@ -0,0 +1,12 @@ +#ifdef _WIN32 +__declspec(dllexport) +#endif + +// An external function, defined in Rust. +extern void panic_if_greater_than_10(unsigned x); + +unsigned add_small_numbers(unsigned a, unsigned b) { + unsigned c = a + b; + panic_if_greater_than_10(c); + return c; +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs new file mode 100644 index 0000000000000..15d38d7216058 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs @@ -0,0 +1,44 @@ +//! A test for calling `C-unwind` functions across foreign function boundaries. +//! +//! This test triggers a panic when calling a foreign function that calls *back* into Rust. +#![feature(c_unwind)] + +use std::panic::{catch_unwind, AssertUnwindSafe}; + +fn main() { + // Call `add_small_numbers`, passing arguments that will NOT trigger a panic. + let (a, b) = (9, 1); + let c = unsafe { add_small_numbers(a, b) }; + assert_eq!(c, 10); + + // Call `add_small_numbers`, passing arguments that will trigger a panic, and catch it. + let caught_unwind = catch_unwind(AssertUnwindSafe(|| { + let (a, b) = (10, 1); + let _c = unsafe { add_small_numbers(a, b) }; + unreachable!("should have unwound instead of returned"); + })); + + // Assert that we did indeed panic, then unwrap and downcast the panic into the sum. + assert!(caught_unwind.is_err()); + let panic_obj = caught_unwind.unwrap_err(); + let msg = panic_obj.downcast_ref::().unwrap(); + assert_eq!(msg, "11"); +} + +#[link(name = "add", kind = "static")] +extern "C-unwind" { + /// An external function, defined in C. + /// + /// Returns the sum of two numbers, or panics if the sum is greater than 10. + fn add_small_numbers(a: u32, b: u32) -> u32; +} + +/// This function will panic if `x` is greater than 10. +/// +/// This function is called by `add_small_numbers`. +#[no_mangle] +pub extern "C-unwind" fn panic_if_greater_than_10(x: u32) { + if x > 10 { + panic!("{}", x); // That is too big! + } +} diff --git a/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs b/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs index 4d40c7d0d2237..c9b4abbfd3fd3 100644 --- a/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs +++ b/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs @@ -7,7 +7,7 @@ // compile-flags:-C panic=abort // aux-build:helper.rs -#![feature(start, rustc_private, new_uninit, panic_info_message)] +#![feature(start, rustc_private, new_uninit, panic_info_message, lang_items)] #![feature(alloc_error_handler)] #![no_std] @@ -84,6 +84,13 @@ fn panic(panic_info: &core::panic::PanicInfo) -> ! { } } +// Because we are compiling this code with `-C panic=abort`, this wouldn't normally be needed. +// However, `core` and `alloc` are both compiled with `-C panic=unwind`, which means that functions +// in these libaries will refer to `rust_eh_personality` if LLVM can not *prove* the contents won't +// unwind. So, for this test case we will define the symbol. +#[lang = "eh_personality"] +extern fn rust_eh_personality() {} + #[derive(Debug)] struct Page([[u64; 32]; 16]); diff --git a/src/test/ui/allocator/no_std-alloc-error-handler-default.rs b/src/test/ui/allocator/no_std-alloc-error-handler-default.rs index 4f8c44f1763c8..d6cd4a6af855f 100644 --- a/src/test/ui/allocator/no_std-alloc-error-handler-default.rs +++ b/src/test/ui/allocator/no_std-alloc-error-handler-default.rs @@ -8,7 +8,7 @@ // aux-build:helper.rs // gate-test-default_alloc_error_handler -#![feature(start, rustc_private, new_uninit, panic_info_message)] +#![feature(start, rustc_private, new_uninit, panic_info_message, lang_items)] #![feature(default_alloc_error_handler)] #![no_std] @@ -71,6 +71,13 @@ fn panic(panic_info: &core::panic::PanicInfo) -> ! { } } +// Because we are compiling this code with `-C panic=abort`, this wouldn't normally be needed. +// However, `core` and `alloc` are both compiled with `-C panic=unwind`, which means that functions +// in these libaries will refer to `rust_eh_personality` if LLVM can not *prove* the contents won't +// unwind. So, for this test case we will define the symbol. +#[lang = "eh_personality"] +extern fn rust_eh_personality() {} + #[derive(Debug)] struct Page([[u64; 32]; 16]); diff --git a/src/test/ui/symbol-names/impl1.rs b/src/test/ui/symbol-names/impl1.rs index d3f74c4a4b7b0..771695330d8bd 100644 --- a/src/test/ui/symbol-names/impl1.rs +++ b/src/test/ui/symbol-names/impl1.rs @@ -74,7 +74,3 @@ fn main() { } }; } - -// FIXME(katie): The 32-bit symbol hash probably needs updating as well, but I'm slightly unsure -// about how to do that. This comment is here so that we don't break the test due to error messages -// including incorrect line numbers. From 05bf037fecfad619e140877769379a1d24952bad Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 23 Oct 2020 18:49:34 -0400 Subject: [PATCH 4/4] address pr review comments ### Add debug assertion to check `AbiDatas` ordering This makes a small alteration to `Abi::index`, so that we include a debug assertion to check that the index we are returning corresponds with the same abi in our data array. This will help prevent ordering bugs in the future, which can manifest in rather strange errors. ### Using exhaustive ABI matches This slightly modifies the changes from our previous commits, favoring exhaustive matches in place of `_ => ...` fall-through arms. This should help with maintenance in the future, when additional ABI's are added, or when existing ABI's are modified. ### List all `-unwind` ABI's in unstable book This updates the `c-unwind` page in the unstable book to list _all_ of the other ABI strings that are introduced by this feature gate. Now, all of the ABI's specified by RFC 2945 are shown. Co-authored-by: Amanieu d'Antras Co-authored-by: Niko Matsakis --- compiler/rustc_middle/src/ty/layout.rs | 20 ++++++++++++++++++- compiler/rustc_mir_build/src/build/mod.rs | 18 ++++++++++++++++- compiler/rustc_target/src/spec/abi.rs | 15 ++++++++++++-- .../src/language-features/c-unwind.md | 3 ++- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index ee2dffd8baeb9..814581a6cf171 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2595,7 +2595,25 @@ fn fn_can_unwind( C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => { unwind } - _ => false, + Cdecl + | Fastcall + | Vectorcall + | Aapcs + | Win64 + | SysV64 + | PtxKernel + | Msp430Interrupt + | X86Interrupt + | AmdGpuKernel + | EfiApi + | AvrInterrupt + | AvrNonBlockingInterrupt + | CCmseNonSecureCall + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => false, + // In the `if` above, we checked for functions with the Rust calling convention. + Rust | RustCall => unreachable!(), } } } diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index dd9c859544f61..3ac9c631d031e 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -575,7 +575,23 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bo // Rust and `rust-call` functions are allowed to unwind, and should not abort. Rust | RustCall => false, // Other ABI's should abort. - _ => true, + Cdecl + | Fastcall + | Vectorcall + | Aapcs + | Win64 + | SysV64 + | PtxKernel + | Msp430Interrupt + | X86Interrupt + | AmdGpuKernel + | EfiApi + | AvrInterrupt + | AvrNonBlockingInterrupt + | CCmseNonSecureCall + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => true, } } } diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index f7f9c30d3b7a6..17eb33b8f2eaa 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -107,7 +107,7 @@ impl Abi { // N.B., this ordering MUST match the AbiDatas array above. // (This is ensured by the test indices_are_correct().) use Abi::*; - match self { + let i = match self { // Cross-platform ABIs Rust => 0, C { unwind: false } => 1, @@ -138,7 +138,18 @@ impl Abi { RustCall => 24, PlatformIntrinsic => 25, Unadjusted => 26, - } + }; + debug_assert!( + AbiDatas + .iter() + .enumerate() + .find(|(_, AbiData { abi, .. })| *abi == self) + .map(|(index, _)| index) + .expect("abi variant has associated data") + == i, + "Abi index did not match `AbiDatas` ordering" + ); + i } #[inline] diff --git a/src/doc/unstable-book/src/language-features/c-unwind.md b/src/doc/unstable-book/src/language-features/c-unwind.md index c1705d59acc21..2801d9b5e7778 100644 --- a/src/doc/unstable-book/src/language-features/c-unwind.md +++ b/src/doc/unstable-book/src/language-features/c-unwind.md @@ -6,7 +6,8 @@ The tracking issue for this feature is: [#74990] ------------------------ -Introduces a new ABI string, "C-unwind", to enable unwinding from other +Introduces four new ABI strings: "C-unwind", "stdcall-unwind", +"thiscall-unwind", and "system-unwind". These enable unwinding from other languages (such as C++) into Rust frames and from Rust into other languages. See [RFC 2945] for more information.