Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[beta] Revert c unwind abi PR #84672

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
},
ItemKind::ForeignMod(ref fm) => {
if fm.abi.is_none() {
self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false });
self.maybe_lint_missing_abi(span, id, abi::Abi::C);
}
hir::ItemKind::ForeignMod {
abi: fm.abi.map_or(abi::Abi::C { unwind: false }, |abi| self.lower_abi(abi)),
abi: fm.abi.map_or(abi::Abi::C, |abi| self.lower_abi(abi)),
items: self
.arena
.alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))),
Expand Down Expand Up @@ -1334,8 +1334,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
match ext {
Extern::None => abi::Abi::Rust,
Extern::Implicit => {
self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false });
abi::Abi::C { unwind: false }
self.maybe_lint_missing_abi(span, id, abi::Abi::C);
abi::Abi::C
}
Extern::Explicit(abi) => self.lower_abi(abi),
}
Expand Down
32 changes: 0 additions & 32 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,38 +164,6 @@ 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
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,8 @@ pub(crate) fn codegen_terminator_call<'tcx>(

// FIXME find a cleaner way to support varargs
if fn_sig.c_variadic {
if !matches!(fn_sig.abi, Abi::C { .. }) {
fx.tcx.sess.span_fatal(
span,
&format!("Variadic call for non-C abi {:?}", fn_sig.abi),
);
if 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
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,9 +644,6 @@ 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
// -------------------------------------------------------------------------
Expand Down
55 changes: 16 additions & 39 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2562,7 +2562,6 @@ 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
Expand All @@ -2587,34 +2586,17 @@ fn fn_can_unwind(
//
// 2. A Rust item using a non-Rust ABI (like `extern "C" fn foo() { ... }`).
//
// 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
}
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!(),
}
// 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
}
}
}
Expand Down Expand Up @@ -2672,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,
Expand Down Expand Up @@ -2841,12 +2823,7 @@ 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,
sig.abi,
),
can_unwind: fn_can_unwind(cx.tcx().sess.panic_strategy(), codegen_fn_attr_flags, conv),
};
fn_abi.adjust_for_abi(cx, sig.abi);
debug!("FnAbi::new_internal = {:?}", fn_abi);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.name(),
caller_abi.name()
"calling a function with ABI {:?} using caller ABI {:?}",
callee_abi,
caller_abi
)
}
}
Expand Down
38 changes: 4 additions & 34 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,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);
Expand All @@ -573,42 +573,12 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bo
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 {
// If an `#[unwind]` attribute was found, we should adhere to it.
None => false, // FIXME(#58794); should be `!(abi == Abi::Rust || abi == Abi::RustCall)`
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.
Cdecl
| Fastcall
| Vectorcall
| Aapcs
| Win64
| SysV64
| PtxKernel
| Msp430Interrupt
| X86Interrupt
| AmdGpuKernel
| EfiApi
| AvrInterrupt
| AvrNonBlockingInterrupt
| CCmseNonSecureCall
| RustIntrinsic
| PlatformIntrinsic
| Unadjusted => true,
}
}
}
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ symbols! {
bridge,
bswap,
c_str,
c_unwind,
c_variadic,
call,
call_mut,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_symbol_mangling/src/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
}
match sig.abi {
Abi::Rust => {}
Abi::C { unwind: false } => cx.push("KC"),
Abi::C => cx.push("KC"),
abi => {
cx.push("K");
let name = abi.name();
Expand Down
76 changes: 14 additions & 62 deletions compiler/rustc_target/src/spec/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,24 @@ 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,
C { unwind: bool },
Rust = 0,
C = 1,

// Single platform ABIs
Cdecl,
Stdcall { unwind: bool },
Stdcall,
Fastcall,
Vectorcall,
Thiscall { unwind: bool },
Thiscall,
Aapcs,
Win64,
SysV64,
Expand All @@ -36,7 +39,7 @@ pub enum Abi {
CCmseNonSecureCall,

// Multiplatform / generic ABIs
System { unwind: bool },
System,
RustIntrinsic,
RustCall,
PlatformIntrinsic,
Expand All @@ -58,16 +61,13 @@ pub struct AbiData {
const AbiDatas: &[AbiData] = &[
// Cross-platform ABIs
AbiData { abi: Abi::Rust, name: "Rust", generic: true },
AbiData { abi: Abi::C { unwind: false }, name: "C", generic: true },
AbiData { abi: Abi::C { unwind: true }, name: "C-unwind", generic: true },
AbiData { abi: Abi::C, name: "C", generic: true },
// Platform-specific ABIs
AbiData { abi: Abi::Cdecl, name: "cdecl", 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::Stdcall, name: "stdcall", generic: false },
AbiData { abi: Abi::Fastcall, name: "fastcall", generic: false },
AbiData { abi: Abi::Vectorcall, name: "vectorcall", 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::Thiscall, name: "thiscall", 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 },
Expand All @@ -84,8 +84,7 @@ const AbiDatas: &[AbiData] = &[
},
AbiData { abi: Abi::CCmseNonSecureCall, name: "C-cmse-nonsecure-call", generic: false },
// Cross-platform ABIs
AbiData { abi: Abi::System { unwind: false }, name: "system", generic: true },
AbiData { abi: Abi::System { unwind: true }, name: "system-unwind", generic: true },
AbiData { abi: Abi::System, name: "system", 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 },
Expand All @@ -104,52 +103,7 @@ pub fn all_names() -> Vec<&'static str> {
impl Abi {
#[inline]
pub fn index(self) -> usize {
// N.B., this ordering MUST match the AbiDatas array above.
// (This is ensured by the test indices_are_correct().)
use Abi::*;
let i = 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,
};
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
self as usize
}

#[inline]
Expand All @@ -168,8 +122,6 @@ impl Abi {

impl fmt::Display for Abi {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
abi => write!(f, "\"{}\"", abi.name()),
}
write!(f, "\"{}\"", self.name())
}
}
Loading