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

ffi-unwind #19

Closed
nikomatsakis opened this issue Jun 11, 2020 · 22 comments
Closed

ffi-unwind #19

nikomatsakis opened this issue Jun 11, 2020 · 22 comments
Assignees
Labels
lang-initiative An active lang team initiative

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 11, 2020

Summary

Working out the rules for cross-language unwinding as well as longjmp.

Status

This section lists the efforts that we are working towards.

"C-unwind" ABI

The "C-unwind" ABI allows you to invoke foreign functions that may unwind using the native ABI.

Interaction with catch_unwind

We need to define what happens when foreign exceptions interact with catch_unwind.

longjmp compatibility

We need to define the conditions in which it is legal to longjmp through Rust frames. We've done some initial conversation about this.

Links

@nikomatsakis nikomatsakis added the lang-initiative An active lang team initiative label Jun 11, 2020
@nikomatsakis nikomatsakis changed the title ffi-unwind project group ffi-unwind Jun 11, 2020
@nikomatsakis
Copy link
Contributor Author

2020-04-02: Working towards RFC

@nikomatsakis
Copy link
Contributor Author

2020-04-09: BatmanAod opened a PR with draft of RFC

@nikomatsakis
Copy link
Contributor Author

2020-04-30:
- The only question is whether to make it “defined behavior” if a foreign except propagates across a “C unwind” boundary (and there is no catch_unwind)
- Advantage: we probably want to support this
- Disadvantage: it will require a shim to catch C++ exceptions — this is probably necessary later, unless we reverse the “two ABIs” proposal
- Interesting downside of the “two ABI” design
- If you have a C function but it takes callbacks, then there could potentially be pressure to make callbacks into “C unwind” and the function too (for flexibility)
- but note that this is not a trivial change, necessarily, and indeed many C libraries are not ready for this to happen “just because”
- True, but just a downside of this approach.

@nikomatsakis
Copy link
Contributor Author

2020-06-01:
- Plain Old Frames — for frames that don’t have a pending destructor or catch_unwind

@nikomatsakis
Copy link
Contributor Author

2020-06-11:

We uncovered a few interesting things in discussing the latest draft. The most important is that we are not comfortable saying that longjmp functions can unwind Plain Old Frames (POF) without any further annotation, because that will hinder our efforts to optimize. For the time being, we plan to edit the RFC to leave the details of when longjmp is legal as TBD, but we discussed some possible alternative proposals here rust-lang/project-ffi-unwind#30.

@nikomatsakis
Copy link
Contributor Author

2020-06-22:

  • RFC 2945 opened
  • Not just about "C" vs "C unwind" ABIs, but we currently use #[unwind(allowed)] for "system":
    extern "system" {
        #[unwind(allowed)]
        pub fn _CxxThrowException(pExceptionObject: *mut c_void, pThrowInfo: *mut u8) -> !;
    }
  • Options:
    • add "system unwind", "stdcall unwind", etc
    • maybe "system" itself is redundant

@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Jul 6, 2020

2020-07-06:

  • RFC is believed to be ready for FCP, needs someone from lang-team to review and fcp merge.
  • We clarified that other ABIs (e.g., system) may have unwind variants (e.g., extern "system unwind").
  • Josh: If there were an ABI that is used in some context where unwinding is always used, it's not useful to have a non-unwind variant of it, in practice there is often a "defacto no-unwind" guarantee. (Niko: but we may want to call it "foo unwind", even if there is no "foo").

@nikomatsakis
Copy link
Contributor Author

2020-07-20:

  • RFC has just entered FCP.
  • Bikeshedding now about "C unwind" vs "C-unwind"

@nikomatsakis
Copy link
Contributor Author

2020-07-27:

  • Bikeshed resolved ABI name to "C-unwind".

@nikomatsakis
Copy link
Contributor Author

2020-07-31:

@nikomatsakis
Copy link
Contributor Author

2020-09-14:

  • Implementation work is ongoing, we may start looking at the longjmp question.

@nikomatsakis
Copy link
Contributor Author

2020-09-21:

No update.

@joshtriplett
Copy link
Member

Discussion in today's meeting:

  • New discussion and proposals for handling "target ABI"
    • e.g. target for native iOS vs target for iOS simulator, which have different ABIs
    • Rust doesn't have a way to express this today

This isn't directly in the current ffi-unwind charter. Rather than expanding it (which we'd do to cover c++/longjmp/etc), we may need to consider chartering an additional project for new FFI issues.

@joshtriplett
Copy link
Member

joshtriplett commented Sep 28, 2020

(Not sure why this unassigned @BatmanAoD; I didn't change anything there.)

@nikomatsakis
Copy link
Contributor Author

Update:

@nikomatsakis
Copy link
Contributor Author

2020-11-17:

  • Previous comment still basically accurate.

@nikomatsakis
Copy link
Contributor Author

Update: PR is ready for review, and we'll have a longjmp blog post going out soon

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jan 28, 2021
…945-c-unwind-abi, r=Amanieu

Implement RFC 2945: "C-unwind" ABI

## Implement RFC 2945: "C-unwind" ABI

This branch implements [RFC 2945]. The tracking issue for this RFC is rust-lang#74990.

The feature gate for the issue is `#![feature(c_unwind)]`.

This RFC was created as part of the ffi-unwind project group tracked at rust-lang/lang-team#19.

### Changes

Further details will be provided in commit messages, but a high-level overview
of the changes follows:

* 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].

* 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.

* 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.

[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 4, 2021
…d-abi, r=Amanieu

Implement RFC 2945: "C-unwind" ABI

## Implement RFC 2945: "C-unwind" ABI

This branch implements [RFC 2945]. The tracking issue for this RFC is rust-lang#74990.

The feature gate for the issue is `#![feature(c_unwind)]`.

This RFC was created as part of the ffi-unwind project group tracked at rust-lang/lang-team#19.

### Changes

Further details will be provided in commit messages, but a high-level overview
of the changes follows:

* 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].

* 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.

* 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.

[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 5, 2021
…d-abi, r=Amanieu

Implement RFC 2945: "C-unwind" ABI

## Implement RFC 2945: "C-unwind" ABI

This branch implements [RFC 2945]. The tracking issue for this RFC is rust-lang#74990.

The feature gate for the issue is `#![feature(c_unwind)]`.

This RFC was created as part of the ffi-unwind project group tracked at rust-lang/lang-team#19.

### Changes

Further details will be provided in commit messages, but a high-level overview
of the changes follows:

* 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].

* 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.

* 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.

[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 6, 2021
…d-abi, r=Amanieu

Implement RFC 2945: "C-unwind" ABI

## Implement RFC 2945: "C-unwind" ABI

This branch implements [RFC 2945]. The tracking issue for this RFC is rust-lang#74990.

The feature gate for the issue is `#![feature(c_unwind)]`.

This RFC was created as part of the ffi-unwind project group tracked at rust-lang/lang-team#19.

### Changes

Further details will be provided in commit messages, but a high-level overview
of the changes follows:

* 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].

* 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.

* 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.

[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 10, 2021
…abi, r=Amanieu

Implement RFC 2945: "C-unwind" ABI

## Implement RFC 2945: "C-unwind" ABI

This branch implements [RFC 2945]. The tracking issue for this RFC is rust-lang#74990.

The feature gate for the issue is `#![feature(c_unwind)]`.

This RFC was created as part of the ffi-unwind project group tracked at rust-lang/lang-team#19.

### Changes

Further details will be provided in commit messages, but a high-level overview
of the changes follows:

* 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].

* 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.

* 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.

[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Mar 18, 2021
…anieu

Implement RFC 2945: "C-unwind" ABI

## Implement RFC 2945: "C-unwind" ABI

This branch implements [RFC 2945]. The tracking issue for this RFC is #74990.

The feature gate for the issue is `#![feature(c_unwind)]`.

This RFC was created as part of the ffi-unwind project group tracked at rust-lang/lang-team#19.

### Changes

Further details will be provided in commit messages, but a high-level overview
of the changes follows:

* 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].

* 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.

* 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.

[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
@BatmanAoD
Copy link
Member

2021-04-06:

  • "C-unwind" PR was merged!
    • This accidentally introduced a change to "C" unwind behavior even without the feature flag; we will need to fix this before the next stable release. We have alerted the release team and are working on a fix.
    • There are a few more follow-up issues to address, listed here.
  • a longjmp blog post was published, but we are prioritizing the "C-unwind" issues for now

@nikomatsakis
Copy link
Contributor Author

Lang team planning meeting update:

  • We should promote this project to "evaluation"
  • On the topic of longjmp:
    • Josh: Fairly unusual to see people using longjmp these days, should not sacrifice optimizablity
    • Josh: But it should be possible -- with enough care -- to longjmp without UB
    • Amanieu: this also affects functions that call pthread_exit
  • Would be nice to open a PR and update the charter to include new scope (can link to the blog post)

@BatmanAoD
Copy link
Member

  • updated charter
  • Follow-up issues for "C-unwind" remain our top priority.
    • In particular, we need to implement the specified "abort-on-unwind" behavior for "C-unwind" FFI calls in panic=abort mode.
  • Regarding longjmp, we believe there's an LLVM bug causing incorrect code-gen when strict-aliasing is enabled in a frame that gets discarded by longjmp. We haven't yet discussed how to move forward, though we plan to open a bug report for it.

@nikomatsakis
Copy link
Contributor Author

Update:

  • @alexcrichton has a PR fixing various bugs and implementing the intended semantics, under review
  • Plan of record is to
    • remove the UB from panics in "C" while adding "C-unwind" as a target
    • once "C-unwind" is stabilized -- hopefully soon! -- add back in the UB for panics in "C"

@nikomatsakis
Copy link
Contributor Author

Closing in favor of rust-lang/project-ffi-unwind#36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-initiative An active lang team initiative
Projects
None yet
Development

No branches or pull requests

3 participants