From d9bee1c735c2a1c423a7a7492abb7e1e74ce4d7b Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 27 Aug 2020 11:49:18 -0400 Subject: [PATCH 1/8] 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. ### 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 | 33 +++++++++ .../rustc_codegen_cranelift/src/abi/mod.rs | 8 +-- compiler/rustc_feature/src/active.rs | 4 ++ 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 | 69 ++++++++++++++----- 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 ++++ .../c-unwind/feature-gate-c-unwind-enabled.rs | 12 ++++ src/test/ui/c-unwind/feature-gate-c-unwind.rs | 9 +++ .../ui/c-unwind/feature-gate-c-unwind.stderr | 12 ++++ .../c-unwind/feature-gate-stdcall-unwind.rs | 9 +++ .../feature-gate-stdcall-unwind.stderr | 12 ++++ .../ui/c-unwind/feature-gate-system-unwind.rs | 9 +++ .../feature-gate-system-unwind.stderr | 12 ++++ .../c-unwind/feature-gate-thiscall-unwind.rs | 9 +++ .../feature-gate-thiscall-unwind.stderr | 12 ++++ src/test/ui/codemap_tests/unicode.stderr | 2 +- src/test/ui/parser/issue-8537.stderr | 2 +- src/test/ui/symbol-names/impl1.rs | 6 +- 29 files changed, 270 insertions(+), 60 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/c-unwind.md create mode 100644 src/test/ui/c-unwind/feature-gate-c-unwind-enabled.rs create mode 100644 src/test/ui/c-unwind/feature-gate-c-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-c-unwind.stderr create mode 100644 src/test/ui/c-unwind/feature-gate-stdcall-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-stdcall-unwind.stderr create mode 100644 src/test/ui/c-unwind/feature-gate-system-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-system-unwind.stderr create mode 100644 src/test/ui/c-unwind/feature-gate-thiscall-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-thiscall-unwind.stderr diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 69257ce1c19e9..994553415b085 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -319,10 +319,10 @@ impl<'hir> LoweringContext<'_, 'hir> { ItemKind::Mod(ref m) => hir::ItemKind::Mod(self.lower_mod(m)), 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))), @@ -1315,8 +1315,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 7bd805f91c857..7b2d5726ae344 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -156,6 +156,39 @@ impl<'a> PostExpansionVisitor<'a> { "efiapi ABI is experimental and subject to change" ); } + // FFI Unwinding ABI's + "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 76e1987459f87..ef60009ad3e99 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -101,7 +101,7 @@ fn clif_sig_from_fn_sig<'tcx>( requires_caller_location: bool, ) -> Signature { let abi = match sig.abi { - Abi::System => Abi::C, + Abi::System { unwind: false } => Abi::C { unwind: false }, abi => abi, }; let (call_conv, inputs, output): (CallConv, Vec>, Ty<'tcx>) = match abi { @@ -110,7 +110,7 @@ fn clif_sig_from_fn_sig<'tcx>( sig.inputs().to_vec(), sig.output(), ), - Abi::C | Abi::Unadjusted => ( + Abi::C { unwind: false } | Abi::Unadjusted => ( CallConv::triple_default(triple), sig.inputs().to_vec(), sig.output(), @@ -126,7 +126,7 @@ fn clif_sig_from_fn_sig<'tcx>( inputs.extend(extra_args.types()); (CallConv::triple_default(triple), inputs, sig.output()) } - Abi::System => unreachable!(), + Abi::System { .. } => unreachable!(), Abi::RustIntrinsic => ( CallConv::triple_default(triple), sig.inputs().to_vec(), @@ -664,7 +664,7 @@ 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 { + if !matches!(fn_sig.abi, Abi::C { .. }) { fx.tcx.sess.span_fatal( span, &format!("Variadic call for non-C abi {:?}", fn_sig.abi), diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index cd3c8fded633f..c80c4507a6992 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -631,6 +631,10 @@ declare_features! ( /// Allows using `pointer` and `reference` in intra-doc links (active, intra_doc_pointers, "1.51.0", Some(80896), None), + + /// Allows `extern "C-unwind" fn` to enable unwinding across ABI boundaries. + (active, c_unwind, "1.51.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 ef467ed651454..bc32f469d15ce 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2615,14 +2615,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 575667f9a9525..1fdbcfa068c3a 100644 --- a/compiler/rustc_mir/src/interpret/terminator.rs +++ b/compiler/rustc_mir/src/interpret/terminator.rs @@ -244,9 +244,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 7b90e5b611cd1..5100eaef38f6f 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -315,6 +315,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 7b6e6ad0696a1..510a99939c4f8 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -441,7 +441,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 1e45739ca22b4..387813f5d636a 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -8,24 +8,16 @@ 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, @@ -38,7 +30,7 @@ pub enum Abi { AvrNonBlockingInterrupt, // Multiplatform / generic ABIs - System, + System { unwind: bool }, RustIntrinsic, RustCall, PlatformIntrinsic, @@ -60,13 +52,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 }, @@ -82,7 +77,8 @@ const AbiDatas: &[AbiData] = &[ 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 }, @@ -101,7 +97,40 @@ 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, + // Cross-platform ABIs + System { unwind: false } => 20, + System { unwind: true } => 21, + RustIntrinsic => 22, + RustCall => 23, + PlatformIntrinsic => 24, + Unadjusted => 25, + } } #[inline] @@ -120,6 +149,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 90d35efaa25bd..27d96c0c282a1 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1208,24 +1208,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 d589989511db1..ba86e5601edfa 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2569,7 +2569,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 dde4a62ffbf3d..e4dfec71c4f5b 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -117,14 +117,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/c-unwind/feature-gate-c-unwind-enabled.rs b/src/test/ui/c-unwind/feature-gate-c-unwind-enabled.rs new file mode 100644 index 0000000000000..6ff5dbda2d560 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-c-unwind.rs b/src/test/ui/c-unwind/feature-gate-c-unwind.rs new file mode 100644 index 0000000000000..f02a368d4e097 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-c-unwind.stderr b/src/test/ui/c-unwind/feature-gate-c-unwind.stderr new file mode 100644 index 0000000000000..f4c785a235f67 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-stdcall-unwind.rs b/src/test/ui/c-unwind/feature-gate-stdcall-unwind.rs new file mode 100644 index 0000000000000..86c29d4146e91 --- /dev/null +++ b/src/test/ui/c-unwind/feature-gate-stdcall-unwind.rs @@ -0,0 +1,9 @@ +// 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/c-unwind/feature-gate-stdcall-unwind.stderr b/src/test/ui/c-unwind/feature-gate-stdcall-unwind.stderr new file mode 100644 index 0000000000000..8478811a1ad42 --- /dev/null +++ b/src/test/ui/c-unwind/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:4: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/c-unwind/feature-gate-system-unwind.rs b/src/test/ui/c-unwind/feature-gate-system-unwind.rs new file mode 100644 index 0000000000000..26c2de4e81767 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-system-unwind.stderr b/src/test/ui/c-unwind/feature-gate-system-unwind.stderr new file mode 100644 index 0000000000000..87877336475b4 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-thiscall-unwind.rs b/src/test/ui/c-unwind/feature-gate-thiscall-unwind.rs new file mode 100644 index 0000000000000..0d5331ca6ebec --- /dev/null +++ b/src/test/ui/c-unwind/feature-gate-thiscall-unwind.rs @@ -0,0 +1,9 @@ +// 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/c-unwind/feature-gate-thiscall-unwind.stderr b/src/test/ui/c-unwind/feature-gate-thiscall-unwind.stderr new file mode 100644 index 0000000000000..d6673ebe38421 --- /dev/null +++ b/src/test/ui/c-unwind/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:4: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`. diff --git a/src/test/ui/codemap_tests/unicode.stderr b/src/test/ui/codemap_tests/unicode.stderr index 8b85adb58453f..ef2dd1cfbb132 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, 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, 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 e33adb239d78d..c04e3242289b8 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, 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, 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 24bdf6d669e88..7dd74bd229d3f 100644 --- a/src/test/ui/symbol-names/impl1.rs +++ b/src/test/ui/symbol-names/impl1.rs @@ -4,7 +4,7 @@ //[legacy]compile-flags: -Z symbol-mangling-version=legacy //[v0]compile-flags: -Z symbol-mangling-version=v0 //[legacy]normalize-stderr-32bit: "hee444285569b39c2" -> "SYMBOL_HASH" -//[legacy]normalize-stderr-64bit: "h310ea0259fc3d32d" -> "SYMBOL_HASH" +//[legacy]normalize-stderr-64bit: "hd949d7797008991f" -> "SYMBOL_HASH" #![feature(auto_traits, rustc_attrs)] #![allow(dead_code)] @@ -75,3 +75,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. From af78f302da058395a6061f7fa0eb655714a01a23 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 10 Sep 2020 13:38:39 -0400 Subject: [PATCH 2/8] 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. --- 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 | 29 ++++++++++++++++++ .../codegen/unwind-abis/system-unwind-abi.rs | 29 ++++++++++++++++++ .../unwind-abis/thiscall-unwind-abi.rs | 30 +++++++++++++++++++ 7 files changed, 170 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 bc32f469d15ce..b3f62b4365ec7 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2523,6 +2523,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 @@ -2547,17 +2548,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, + } } } } @@ -2783,7 +2783,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 996615995259d..75016b1a5ff10 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -546,7 +546,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); @@ -556,12 +556,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..f67ad2f292f41 --- /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..5a47e0ebb8cea --- /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..ec7ebf8621986 --- /dev/null +++ b/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs @@ -0,0 +1,29 @@ +// compile-flags: -C opt-level=0 + +// 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..a7c8caad5082e --- /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..ce3b4adb2a251 --- /dev/null +++ b/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs @@ -0,0 +1,30 @@ +// compile-flags: -C opt-level=0 + +// 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 b4884c61a9cc8e2f8bfe5cea14f9ad0adcd8fc13 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 23 Oct 2020 18:49:34 -0400 Subject: [PATCH 3/8] pr review: abi debug assertion, exhaustive matches ### 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. Co-authored-by: Niko Matsakis --- compiler/rustc_middle/src/ty/layout.rs | 19 ++++++++++++++++++- compiler/rustc_mir_build/src/build/mod.rs | 17 ++++++++++++++++- compiler/rustc_target/src/spec/abi.rs | 15 +++++++++++++-- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index b3f62b4365ec7..e1da37d4639e8 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2556,7 +2556,24 @@ 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 + | 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 75016b1a5ff10..12db7e3bda46d 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -573,7 +573,22 @@ 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 + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => true, } } } diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index 387813f5d636a..f31ca2f10d3c3 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -100,7 +100,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, @@ -130,7 +130,18 @@ impl Abi { RustCall => 23, PlatformIntrinsic => 24, Unadjusted => 25, - } + }; + 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] From 35c90a2ca8f5daa7c312934ed70f0c45071c1c2b Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 10 Dec 2020 18:56:09 -0500 Subject: [PATCH 4/8] add `rust_eh_personality` to `#[no_std]` alloc tests ### Changes 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'." --- .../ui/allocator/no_std-alloc-error-handler-custom.rs | 9 ++++++++- .../ui/allocator/no_std-alloc-error-handler-default.rs | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) 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]); From bfd67f093ca1aa0ff19e6bb78d0055d7db235701 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 11 Dec 2020 20:54:47 -0500 Subject: [PATCH 5/8] add integration tests, unwind across FFI boundary ### Changes This commit introduces some new fixtures to the `run-make-fulldeps` test suite. * c-unwind-abi-hello-world: A simple "happy path" fixture. This involves calling from Rust, into C, and back into a Rust `C-unwind` function. * c-unwind-abi-catch-panic: Exercise unwinding a panic. This reuses much of the same code, but instead catches a panic 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. --- .../c-unwind-abi-catch-lib-panic/Makefile | 18 +++++++++ .../c-unwind-abi-catch-lib-panic/add.c | 12 ++++++ .../c-unwind-abi-catch-lib-panic/main.rs | 30 ++++++++++++++ .../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 | 39 +++++++++++++++++++ .../c-unwind-abi-hello-world/Makefile | 5 +++ .../c-unwind-abi-hello-world/add.c | 12 ++++++ .../c-unwind-abi-hello-world/main.rs | 29 ++++++++++++++ 10 files changed, 174 insertions(+) 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 create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-hello-world/Makefile create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-hello-world/add.c create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-hello-world/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..7156d02ec8a5a --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile @@ -0,0 +1,18 @@ +-include ../tools.mk + +all: + # Compile `panic.rs` and `add.c` into object files. + # + # Note that we invoke `rustc` directly, so we may emit an object rather + # than an archive. We'll do that later. + $(BARE_RUSTC) $(RUSTFLAGS) \ + --out-dir $(TMPDIR) \ + --emit=obj panic.rs + $(call COMPILE_OBJ,$(TMPDIR)/add.o,add.c) + + # Now, create an archive using these two objects. + $(AR) crus $(TMPDIR)/libadd.a $(TMPDIR)/add.o $(TMPDIR)/panic.o + + # Compile `main.rs`, which will link into our library, and run it. + $(RUSTC) main.rs + $(call RUN,main) 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..8b7f817d45435 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs @@ -0,0 +1,30 @@ +//! 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 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 panic_u32 = *panic_obj.downcast_ref::().unwrap(); + assert_eq!(panic_u32, 11); +} + +#[link(name = "add")] +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..c7146e8618414 --- /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..f479e90ac69e1 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs @@ -0,0 +1,39 @@ +//! 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 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 panic_u32 = *panic_obj.downcast_ref::().unwrap(); + assert_eq!(panic_u32, 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/run-make-fulldeps/c-unwind-abi-hello-world/Makefile b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/Makefile new file mode 100644 index 0000000000000..9553b7aeeb983 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/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-hello-world/add.c b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/add.c new file mode 100644 index 0000000000000..444359451f6ec --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/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-hello-world/main.rs b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/main.rs new file mode 100644 index 0000000000000..786af22ddc5e3 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/main.rs @@ -0,0 +1,29 @@ +//! A test for calling `C-unwind` functions across foreign function boundaries. +//! +//! This test does *not* trigger a panic. The exercises the "happy path" when calling a foreign +//! function that calls *back* into Rust. +#![feature(c_unwind)] + +fn main() { + let (a, b) = (9, 1); + let c = unsafe { add_small_numbers(a, b) }; + assert_eq!(c, 10); +} + +#[link(name = "add", kind = "static")] +extern { + /// 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! + } +} From 6013541361b472d54a84da3b881e79558c5eb1af Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Mon, 14 Dec 2020 15:05:47 -0500 Subject: [PATCH 6/8] update 32-bit hash in `impl1` test --- src/test/ui/symbol-names/impl1.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/ui/symbol-names/impl1.rs b/src/test/ui/symbol-names/impl1.rs index 7dd74bd229d3f..a352d255c782d 100644 --- a/src/test/ui/symbol-names/impl1.rs +++ b/src/test/ui/symbol-names/impl1.rs @@ -3,7 +3,7 @@ // revisions: legacy v0 //[legacy]compile-flags: -Z symbol-mangling-version=legacy //[v0]compile-flags: -Z symbol-mangling-version=v0 -//[legacy]normalize-stderr-32bit: "hee444285569b39c2" -> "SYMBOL_HASH" +//[legacy]normalize-stderr-32bit: "h13cfe136e83a9196" -> "SYMBOL_HASH" //[legacy]normalize-stderr-64bit: "hd949d7797008991f" -> "SYMBOL_HASH" #![feature(auto_traits, rustc_attrs)] @@ -75,7 +75,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 15fb65e572656da034f6c44862575494dc6be908 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Tue, 5 Jan 2021 11:44:33 -0500 Subject: [PATCH 7/8] pr review: list all `-unwind` ABI's in unstable book ### Changes This is a small commit, updating 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 --- src/doc/unstable-book/src/language-features/c-unwind.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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. From 5d75cde97a6221b8824377e07695b0194e47b08d Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Wed, 27 Jan 2021 10:46:59 -0500 Subject: [PATCH 8/8] bump tidy's `src/test/ui` file limit This commit addresses a tidy error that currently occurs on this branch: ``` tidy error: following path contains more than 1458 entries, you should move the test to some relevant subdirectory (current: 1459): /checkout/src/test/ui ``` Co-authored-by: bjorn3 --- src/tools/tidy/src/ui_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index e687901f212b6..21d05226fb42c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -7,7 +7,7 @@ use std::path::Path; const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. -const ROOT_ENTRY_LIMIT: usize = 1458; +const ROOT_ENTRY_LIMIT: usize = 1459; const ISSUES_ENTRY_LIMIT: usize = 2669; fn check_entries(path: &Path, bad: &mut bool) {