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

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

Merged
merged 4 commits into from
Mar 10, 2021

Commits on Mar 9, 2021

  1. rustc_target: add "unwind" payloads to Abi

     ### Overview
    
        This commit begins the implementation work for RFC 2945. For more
        information, see the rendered RFC [1] and tracking issue [2].
    
        A boolean `unwind` payload is added to the `C`, `System`, `Stdcall`,
        and `Thiscall` variants, marking whether unwinding across FFI
        boundaries is acceptable. The cases where each of these variants'
        `unwind` member is true correspond with the `C-unwind`,
        `system-unwind`, `stdcall-unwind`, and `thiscall-unwind` ABI strings
        introduced in RFC 2945 [3].
    
     ### Feature Gate and Unstable Book
    
        This commit adds a `c_unwind` feature gate for the new ABI strings.
        Tests for this feature gate are included in `src/test/ui/c-unwind/`,
        which ensure that this feature gate works correctly for each of the
        new ABIs.
    
        A new language features entry in the unstable book is added as well.
    
     ### Further Work To Be Done
    
        This commit does not proceed to implement the new unwinding ABIs,
        and is intentionally scoped specifically to *defining* the ABIs and
        their feature flag.
    
     ### One Note on Test Churn
    
        This will lead to some test churn, in re-blessing hash tests, as the
        deleted comment in `src/librustc_target/spec/abi.rs` mentioned,
        because we can no longer guarantee the ordering of the `Abi`
        variants.
    
        While this is a downside, this decision was made bearing in mind
        that RFC 2945 states the following, in the "Other `unwind` Strings"
        section [3]:
    
        >  More unwind variants of existing ABI strings may be introduced,
        >  with the same semantics, without an additional RFC.
    
        Adding a new variant for each of these cases, rather than specifying
        a payload for a given ABI, would quickly become untenable, and make
        working with the `Abi` enum prone to mistakes.
    
        This approach encodes the unwinding information *into* a given ABI,
        to account for the future possibility of other `-unwind` ABI
        strings.
    
     ### Ignore Directives
    
        `ignore-*` directives are used in two of our `*-unwind` ABI test
        cases.
    
        Specifically, the `stdcall-unwind` and `thiscall-unwind` test cases
        ignore architectures that do not support `stdcall` and
        `thiscall`, respectively.
    
        These directives are cribbed from
        `src/test/ui/c-variadic/variadic-ffi-1.rs` for `stdcall`, and
        `src/test/ui/extern/extern-thiscall.rs` for `thiscall`.
    
        This would otherwise fail on some targets, see:
        rust-lang-ci@fcf697f
    
     ### Footnotes
    
    [1]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md
    [2]: rust-lang#74990
    [3]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md#other-unwind-abi-strings
    katelyn a. martin committed Mar 9, 2021
    Configuration menu
    Copy the full SHA
    df45c57 View commit details
    Browse the repository at this point in the history
  2. implement unwinding abi's (RFC 2945)

     ### Changes
    
        This commit implements unwind ABI's, specified in RFC 2945.
    
        We adjust the `rustc_middle::ty::layout::fn_can_unwind` function,
        used to compute whether or not a `FnAbi` object represents a
        function that should be able to unwind when `panic=unwind` is in
        use.
    
        Changes are also made to
        `rustc_mir_build::build::should_abort_on_panic` so that the
        function ABI is used to determind whether it should abort, assuming
        that the `panic=unwind` strategy is being used, and no explicit
        unwind attribute was provided.
    
     ### Tests
    
        Unit tests, checking that the behavior is correct for `C-unwind`,
        `stdcall-unwind`, `system-unwind`, and `thiscall-unwind`, are
        included. These alternative `unwind` ABI strings are specified in
        RFC 2945, in the "_Other `unwind` ABI strings_" section.
    
        Additionally, a test case is included to assert that the LLVM IR
        generated for an external function defined with the `C-unwind` ABI
        will be appropriately labeled with the `nounwind` LLVM attribute
        when the `panic=abort` compilation flag is used.
    
     ### Ignore Directives
    
        This commit uses `ignore-*` directives in two of our `*-unwind` ABI
        test cases.
    
        Specifically, the `stdcall-unwind` and `thiscall-unwind` test cases
        ignore architectures that do not support `stdcall` and `thiscall`,
        respectively.
    
        These directives are cribbed from
        `src/test/ui/c-variadic/variadic-ffi-1.rs` for `stdcall`, and
        `src/test/ui/extern/extern-thiscall.rs` for `thiscall`.
    katelyn a. martin committed Mar 9, 2021
    Configuration menu
    Copy the full SHA
    0f33e9f View commit details
    Browse the repository at this point in the history
  3. add integration tests, unwind across FFI boundary

     ### Integration Tests
    
        This commit introduces some new fixtures to the `run-make-fulldeps`
        test suite.
    
            * c-unwind-abi-catch-panic: Exercise unwinding a panic. This
              catches a panic across an FFI boundary and downcasts it into
              an integer.
    
            * c-unwind-abi-catch-lib-panic: This is similar to the previous
             `*catch-panic` test, however in this case the Rust code that
             panics resides in a separate crate.
    
     ### Add `rust_eh_personality` to `#[no_std]` alloc tests
    
        This commit addresses some test failures that now occur in the
        following two tests:
    
            * no_std-alloc-error-handler-custom.rs
            * no_std-alloc-error-handler-default.rs
    
        Each test now defines a `rust_eh_personality` extern function, in
        the same manner as shown in the "Writing an executable without
        stdlib" section of the `lang_items` documentation here:
        https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib
    
        Without this change, these tests would fail to compile due to a
        linking error explaining that there was an "undefined reference
        to `rust_eh_personality'."
    
     ### Updated hash
    
        * update 32-bit hash in `impl1` test
    
     ### Panics
    
        This commit uses `panic!` macro invocations that return a string,
        rather than using an integer as a panic payload.
    
        Doing so avoids the following warnings that were observed during
        rollup for the `*-msvc-1` targets:
    
        ```
        warning: panic message is not a string literal
          --> panic.rs:10:16
           |
        10 |         panic!(x); // That is too big!
           |                ^
           |
           = note: `#[warn(non_fmt_panic)]` on by default
           = note: this is no longer accepted in Rust 2021
        help: add a "{}" format string to Display the message
           |
        10 |         panic!("{}", x); // That is too big!
           |                ^^^^^
        help: or use std::panic::panic_any instead
           |
        10 |         std::panic::panic_any(x); // That is too big!
           |         ^^^^^^^^^^^^^^^^^^^^^
    
        warning: 1 warning emitted
        ```
    
        See: https://github.com/rust-lang-ci/rust/runs/1992118428
    
        As these errors imply, panicking without a format string will be
        disallowed in Rust 2021, per rust-lang#78500.
    katelyn a. martin committed Mar 9, 2021
    Configuration menu
    Copy the full SHA
    baf227e View commit details
    Browse the repository at this point in the history
  4. address pr review comments

     ### Add debug assertion to check `AbiDatas` ordering
    
        This makes a small alteration to `Abi::index`, so that we include a
        debug assertion to check that the index we are returning corresponds
        with the same abi in our data array.
    
        This will help prevent ordering bugs in the future, which can
        manifest in rather strange errors.
    
     ### Using exhaustive ABI matches
    
        This slightly modifies the changes from our previous commits,
        favoring exhaustive matches in place of `_ => ...` fall-through
        arms.
    
        This should help with maintenance in the future, when additional
        ABI's are added, or when existing ABI's are modified.
    
     ### List all `-unwind` ABI's in unstable book
    
        This updates the `c-unwind` page in the unstable book to list _all_
        of the other ABI strings that are introduced by this feature gate.
    
        Now, all of the ABI's specified by RFC 2945 are shown.
    
    Co-authored-by: Amanieu d'Antras <[email protected]>
    Co-authored-by: Niko Matsakis <[email protected]>
    3 people committed Mar 9, 2021
    Configuration menu
    Copy the full SHA
    05bf037 View commit details
    Browse the repository at this point in the history