Skip to content

Commit

Permalink
Auto merge of #84722 - pnkfelix:address-c-unwind-via-backport-of-8415…
Browse files Browse the repository at this point in the history
…8, r=Mark-Simulacrum

backport: move new c abi abort behavior behind feature gate

This is a backport of PR #84158 to the beta branch.

The original T-compiler plan was to revert PR #76570 in its entirety, as was attempted in PR #84672. But the revert did not go smoothly (details below).

Therefore, we are backporting PR #84158 instead, which was our established backup plan if a revert did not go smoothly.

I have manually confirmed that this backport fixes the luajit issue described on issue #83541

<details>

<summary>Click for details as to why revert of PR #76570 did not go smoothly.</summary>

It turns out that Miri had been subsequently updated to reflect changes to `rustc_target` that landed in PR #76570. This meant that the attempt to land PR #84672 broke Miri builds.

Normally we allow tools to break when landing PR's (and just expect follow-up PR's to fix the tools), but we don't allow it for tools in the run-up to a release.

(We shouldn't be using that uniform policy for all tools. Miri should be allow to break during the week before a release; but currently we cannot express that, due to issue #74709.)

Therefore, its a lot of pain to try to revert PR #76570. And we're going with the backup plan.

</details>

Original commit message follows:

----

 *Background*

In #76570, new ABI strings including `C-unwind` were introduced. Their behavior is specified in RFC 2945 <sup>[1]</sup>.

However, it was reported in the #ffi-unwind stream of the Rust community Zulip that this had altered the way that `extern "C"` functions behaved even when the `c_unwind` feature gate was not active. <sup>[2]</sup>

 *Overview*

 This makes a small patch to `rustc_mir_build::build::should_abort_on_panic`, so that the same  behavior from before is in place when the `c_unwind` gate is not active.

`rustc_middle::ty::layout::fn_can_unwind` is not touched, as the visible behavior should not differ before/after #76570. <sup>[3]</sup>

 ### Footnotes

 1.: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
 2.: https://rust-lang.zulipchat.com/#narrow/stream/210922-project-ffi-unwind/topic/Is.20unwinding.20through.20extern.20C.20UB.3F/near/230112325
 3.: https://github.com/rust-lang/rust/pull/76570/files#diff-b0320c2b8868f325d83c027fc5d71732636e9763551e35895488f30fe057c6e9L2599-R2617

 [1]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
 [2]: https://rust-lang.zulipchat.com/#narrow/stream/210922-project-ffi-unwind/topic/Is.20unwinding.20through.20extern.20C.20UB.3F/near/230112325
 [3]: https://github.com/rust-lang/rust/pull/76570/files#diff-b0320c2b8868f325d83c027fc5d71732636e9763551e35895488f30fe057c6e9L2599-R2617
  • Loading branch information
bors committed Apr 30, 2021
2 parents f97769a + 8341f42 commit 9a1dfd2
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bo
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 => {
None if tcx.features().c_unwind => {
use Abi::*;
match abi {
// In the case of ABI's that have an `-unwind` equivalent, check whether the ABI
Expand Down Expand Up @@ -609,6 +609,10 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bo
| Unadjusted => true,
}
}
// If the `c_unwind` feature gate is not active, follow the behavior that was in place
// prior to #76570. This is a special case: some functions have a C ABI but are meant to
// unwind anyway. Don't stop them.
None => false, // FIXME(#58794); should be `!(abi == Abi::Rust || abi == Abi::RustCall)`
}
}

Expand Down

0 comments on commit 9a1dfd2

Please sign in to comment.