Implement `mrb_raise` with Rust `panic!` #1904

lopopolo · 2022-06-12T22:12:13Z

Fixes #35.

WIP, blocked on stabilization of the c_unwind feature.

Compatibility Notes

This PR changes Artichoke to compile mruby as C++ instead of C. A compiler that supports c++17 is required.

By compiling mruby as C++, artichoke-backend's build.rs is able to set -DMRB_USE_CXX_EXCEPTION which uses C++ try/catch instead of setjmp/longjmp for unwinding in the parser.

C++ exceptions are possible now with the C-unwind ABI; whereas before they would be UB if passing through Rust stack frames.

Hopefully this improves the state of the playground's Emscripten bundle when raising Ruby Exceptions in native code:

Exceptions raised from Rust with error::raise crash the playground with an unreachable error playground#389

- mruby/mruby#5728 - artichoke/mruby@d4d902b - artichoke/mruby#11 - artichoke/mruby#12 - https://github.com/artichoke/mruby/releases/tag/artichoke-vendor.10 This fix is required to make progress on: - #1904

lopopolo · 2022-07-20T17:58:19Z

Blocked on c_unwind feature stabilization which is blocked on this ticket which is the source of the compilation errors on this branch:

Implement Debug, Pointer, etc on function pointers for all calling conventions rust-lang/rust#92964

lopopolo · 2022-07-24T00:49:22Z

Errors with the latest nightly:

error[E0277]: `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, sys::ffi::mrb_value) -> sys::ffi::mrb_value` doesn't implement `std::fmt::Debug`
  --> artichoke-backend/src/method.rs:22:5
   |
17 | #[derive(Debug, Clone)]
   |          ----- in this derive macro expansion
...
22 |     method: Method,
   |     ^^^^^^^^^^^^^^ `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, sys::ffi::mrb_value) -> sys::ffi::mrb_value` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
   |
   = help: the trait `std::fmt::Debug` is not implemented for `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, sys::ffi::mrb_value) -> sys::ffi::mrb_value`
   = help: the following other types implement trait `std::fmt::Debug`:
             extern "C" fn() -> Ret
             extern "C" fn(A) -> Ret
             extern "C" fn(A, ...) -> Ret
             extern "C" fn(A, B) -> Ret
             extern "C" fn(A, B, ...) -> Ret
             extern "C" fn(A, B, C) -> Ret
             extern "C" fn(A, B, C, ...) -> Ret
             extern "C" fn(A, B, C, D) -> Ret
           and 68 others
   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, *mut c_void, usize, *mut c_void) -> *mut c_void` doesn't implement `std::fmt::Debug`
    --> /Users/lopopolo/dev/artichoke/artichoke/target/debug/build/artichoke-backend-6cd3d86938d9a17a/out/ffi.rs:1612:5
     |
1609 | #[derive(Debug, Copy, Clone)]
     |          ----- in this derive macro expansion
...
1612 |     pub allocf: mrb_allocf,
     |     ^^^^^^^^^^^^^^^^^^^^^^ `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, *mut c_void, usize, *mut c_void) -> *mut c_void` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
     |
     = help: the trait `std::fmt::Debug` is not implemented for `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, *mut c_void, usize, *mut c_void) -> *mut c_void`
     = help: the following other types implement trait `std::fmt::Debug`:
               extern "C" fn() -> Ret
               extern "C" fn(A) -> Ret
               extern "C" fn(A, ...) -> Ret
               extern "C" fn(A, B) -> Ret
               extern "C" fn(A, B, ...) -> Ret
               extern "C" fn(A, B, C) -> Ret
               extern "C" fn(A, B, C, ...) -> Ret
               extern "C" fn(A, B, C, D) -> Ret
             and 68 others
     = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `unsafe extern "C-unwind" fn(*mut mrb_parser_state) -> i32` doesn't implement `std::fmt::Debug`
    --> /Users/lopopolo/dev/artichoke/artichoke/target/debug/build/artichoke-backend-6cd3d86938d9a17a/out/ffi.rs:3546:5
     |
3540 |   #[derive(Debug, Copy, Clone)]
     |            ----- in this derive macro expansion
...
3546 | /     pub partial_hook:
3547 | |         ::std::option::Option<unsafe extern "C-unwind" fn(arg1: *mut mrb_parser_state) -> ::std::os::raw::c_int>,
     | |________________________________________________________________________________________________________________^ `unsafe extern "C-unwind" fn(*mut mrb_parser_state) -> i32` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
     |
     = help: the trait `std::fmt::Debug` is not implemented for `unsafe extern "C-unwind" fn(*mut mrb_parser_state) -> i32`
     = help: the following other types implement trait `std::fmt::Debug`:
               extern "C" fn() -> Ret
               extern "C" fn(A) -> Ret
               extern "C" fn(A, ...) -> Ret
               extern "C" fn(A, B) -> Ret
               extern "C" fn(A, B, ...) -> Ret
               extern "C" fn(A, B, C) -> Ret
               extern "C" fn(A, B, C, ...) -> Ret
               extern "C" fn(A, B, C, D) -> Ret
             and 68 others
     = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, *mut c_void)` doesn't implement `std::fmt::Debug`
    --> /Users/lopopolo/dev/artichoke/artichoke/target/debug/build/artichoke-backend-6cd3d86938d9a17a/out/ffi.rs:5007:5
     |
5004 | #[derive(Debug, Copy, Clone)]
     |          ----- in this derive macro expansion
...
5007 |     pub dfree: ::std::option::Option<unsafe extern "C-unwind" fn(mrb: *mut mrb_state, arg1: *mut ::std::os::raw::c_void)>,
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, *mut c_void)` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
     |
     = help: the trait `std::fmt::Debug` is not implemented for `unsafe extern "C-unwind" fn(*mut sys::ffi::mrb_state, *mut c_void)`
     = help: the following other types implement trait `std::fmt::Debug`:
               extern "C" fn() -> Ret
               extern "C" fn(A) -> Ret
               extern "C" fn(A, ...) -> Ret
               extern "C" fn(A, B) -> Ret
               extern "C" fn(A, B, ...) -> Ret
               extern "C" fn(A, B, C) -> Ret
               extern "C" fn(A, B, C, ...) -> Ret
               extern "C" fn(A, B, C, D) -> Ret
             and 68 others
     = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `artichoke-backend` due to 4 previous errors

Nightly version

$ rustc -v --version
rustc 1.64.0-nightly (f8588549c 2022-07-18)
binary: rustc
commit-hash: f8588549c3c3d45c32b404210cada01e2a45def3
commit-date: 2022-07-18
host: x86_64-apple-darwin
release: 1.64.0-nightly
LLVM version: 14.0.6

lopopolo · 2022-07-24T01:53:53Z

See:

builtin trait to abstract over function pointers rust-lang/rust#99531

lopopolo · 2022-10-22T00:09:13Z

Unblocked on nightly I believe now that fmt::Debug impls are merged:

Add default trait implementations for "c-unwind" ABI function pointers rust-lang/rust#101263

lopopolo · 2022-10-24T06:11:00Z

The nightly changes made it, but now blocked on linker failures in mruby:

Linker failures when building mruby as C++ mruby/mruby#5818

The Rust team has put out a call for testing on the `c_unwind` feature and associated `C-unwind` ABI: - rust-lang/rfcs#2945 (comment) I intend to implement mruby's exception unwinding using Rust's panic infrastructure: - #35 This commit changes the Rust toolchain to nightly for this test. Merging this experiment will be blocked on the `C-unwind` ABI making it to stable.

`mruby-error` implements `mrb_protect` and `mrb_ensure`. These two functions are used to stop the propagation of exception unwinding in mruby, which natively uses either `setjmp`/`longjmp` or C++'s `catch` and `throw`. I intend to reimplement these functions in Rust using Rust panics: `std::panic::catch_unwind` and `std::panic::panic_any`. See: - #35

When `-DARTICHOKE` is passed when compiling mruby, `#ifndef` out the definition and implementation of the `exc_throw` static function and replace it with a forward declaration. This function normally uses the `MRB_THROW` macro to initiate an unwind. This prepares us to implement this function in Rust with a `#[no_mangle] unsafe extern "C-unwind" fn` item that unwinds with a Rust panic. The previous commit disables the implementations of `mrb_protect` and `mrb_ensure` which pair usage of `MRB_TRY` and `MRB_CATCH` with the `MRB_THROW` in `exc_throw`.

- Add a custom panic payload which wraps a `sys::mrb_value` and implements the necessary traits to be used with `panic!`. - Implement `exc_throw` with `std::panic::panic_any` and the custom panic payload. - Implement `mrb_protect` with `std::panic::catch_unwind` and downcasting to check for the custom panic payload. When using `std::panic::panic_any`, the built-in Rust panic hook is triggered which prints the panic to stderr and enables the `RUST_BACKTRACE` handler. This commit includes some code that experiments with disabling these built-in behaviors. Per feedback from upstream, `panic_any` should be replaced with `std::panic::resume_unwind` which initiates the panic with a custom payload and avoids running the registered panic hook.

Per feedback from upstream, to avoid printing to stderr when panicking, I can use `std::panic::resume_unwind` instead of `std::panic::panic_any`.

bindgen does not support customizing the ABI of function and function pointer bindings. This commit hacks support for `C-unwind` ABI into the generated bindings by doing a find and replace.

- Update `Free` function type alias. - Update `Method` function type alias. - Update `box_unbox_free` to use `C-unwind` ABI.

mruby supports compilation as a C++ library with a C++ compiler by defining the `MRB_USE_CXX_EXCEPTION` macro. Per upstream, it is always UB to unwind past stack frames that assume that no unwinding will occur. On most compilers, C code is compiled by default with this assumption. In Rust, C code compiled this way corresponds to the `unsafe extern "C" fn` ABI. This branch changes Rust functions and the mruby bindings to use the `C-unwind` ABI, which corresponds to C compiled with support for exceptions and unwinding. The unwinding mechanism used by Rust and its `C-unwind` ABI is the same mechanism used by C++ for its `try`/`catch` exception handling. These changes are similar to if mruby was compiled by a C compiler with `-fexceptions`. However, several parts of mruby, like the codegen module, use the `MRB_TRY` and `MRB_CATCH` macros still. Compiling mruby as C++ means `setjmp`/`longjmp` will no longer be used for these internal unwinding cases. `-x c++` is passed to the `cc` crate and to `clang` via `bindgen` to avoid this deprecation warning: warning: clang: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]

Neither Artichoke nor its vendored mruby sources call `mrb_ensure`, but implementing it ensures we have maintain an API/ABI compatible mruby implementation.

- Ensure Rust panics are propagated and ignored by `mrb_protect` and `mrb_ensure`. - Add safety comments to unsafe trait impls. - Add documentation to panic payload. - Add inline comments. - Properly forward Artichoke `Error`s.

Instead of disabling the static function `exc_throw` in C code when compiling for Artichoke and implementing the function with global linkage in Rust as a `#[no_mangle] extern "C" fn`, implement an Artichoke-specific `extern "C" fn` and make `exc_throw` an alias to it with a macro.

This commit includes changes to parse.y. The parser is regenerated with: ``` /usr/local/opt/bison/bin/bison -o artichoke-backend/vendor/mruby/mrbgems/mruby-compiler/core/y.tab.c artichoke-backend/vendor/mruby/mrbgems/mruby-compiler/core/parse.y sed -i '' -e 's#artichoke-backend/vendor/mruby/##g' artichoke-backend/vendor/mruby/mrbgems/mruby-compiler/core/y.tab.c ```

- Improve safety comment on `ExceptionPayload` unsafe impl - Document why `panic::resume_unwind` is used instead of `panic_any!`

lopopolo · 2022-11-09T03:30:56Z

I think this PR is stuck. The VM relies on being able to catch unwinds so it can enter the rescue flow:

artichoke/artichoke-backend/vendor/mruby/src/vm.c

Lines 3034 to 3042 in 4018f41

    
           MRB_CATCH(&c_jmp) { 
        
             mrb_callinfo *ci = mrb->c->ci; 
        
             while (ci > mrb->c->cibase && ci->cci == CINFO_DIRECT) { 
        
               ci = cipop(mrb); 
        
             } 
        
             exc_catched = TRUE; 
        
             pc = ci->pc; 
        
             goto RETRY_TRY_BLOCK; 
        
           }

lopopolo mentioned this pull request Jun 28, 2022

Cherry-pick C++ with C ABI fixes artichoke/mruby#11

Closed

lopopolo mentioned this pull request Jun 28, 2022

Pull in upstream mruby fix to compile mruby as C++ #1923

Merged

lopopolo force-pushed the lopopolo/c-unwind-mrb_raise-panic branch 3 times, most recently from f0019ad to ee80042 Compare June 28, 2022 02:20

lopopolo mentioned this pull request Jul 23, 2022

Investigate wasm32-wasi build support #1966

Open

lopopolo force-pushed the lopopolo/c-unwind-mrb_raise-panic branch from ee80042 to f6a053b Compare July 24, 2022 00:48

lopopolo mentioned this pull request Jul 24, 2022

Cleanup mrbsys C extension #1968

Merged

lopopolo force-pushed the lopopolo/c-unwind-mrb_raise-panic branch from f6a053b to 20157e8 Compare July 24, 2022 01:50

lopopolo force-pushed the lopopolo/c-unwind-mrb_raise-panic branch from 20157e8 to 84506ca Compare October 24, 2022 06:11

lopopolo force-pushed the lopopolo/c-unwind-mrb_raise-panic branch from 84506ca to 59e1fb4 Compare November 8, 2022 04:19

lopopolo added 5 commits November 7, 2022 20:29

Sync toolchain to nightly in CI

f270ae6

lopopolo added 14 commits November 7, 2022 20:30

Use C-unwind ABI in sys::protect internals

69344b3

Skip panic hook and RUST_BACKTRACE handling when unwinding

e7b2280

Per feedback from upstream, to avoid printing to stderr when panicking, I can use `std::panic::resume_unwind` instead of `std::panic::panic_any`.

Generate C-unwind ABI functions and function pointers from bindgen

03b3219

bindgen does not support customizing the ABI of function and function pointer bindings. This commit hacks support for `C-unwind` ABI into the generated bindings by doing a find and replace.

Use C-unwind ABI in def mruby FFI glue

2e0387a

- Update `Free` function type alias. - Update `Method` function type alias. - Update `box_unbox_free` to use `C-unwind` ABI.

Use C-unwind ABI for all native method impls in extn

59b9f8c

Implement mrb_ensure

317096d

Neither Artichoke nor its vendored mruby sources call `mrb_ensure`, but implementing it ensures we have maintain an API/ABI compatible mruby implementation.

Add note about mrb_protect_error

7f03bf6

Update docs to reference C-unwind ABI

c1b865b

Fix compilation errors in panic payload downcasting

c358db4

Improve documentation in raise module

958bece

- Improve safety comment on `ExceptionPayload` unsafe impl - Document why `panic::resume_unwind` is used instead of `panic_any!`

lopopolo force-pushed the lopopolo/c-unwind-mrb_raise-panic branch from 59e1fb4 to 958bece Compare November 8, 2022 04:37

Migrate tests to C-unwind ABI

bc693e2

lopopolo added 2 commits November 8, 2022 19:31

Handle stack frame munging in protect flows

d20fbb5

cargo fmt

1171d9f

lopopolo mentioned this pull request Nov 9, 2022

Fix clippy lint violations on nightly #2266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `mrb_raise` with Rust `panic!` #1904

Implement `mrb_raise` with Rust `panic!` #1904

lopopolo commented Jun 12, 2022

lopopolo commented Jul 20, 2022

lopopolo commented Jul 24, 2022

lopopolo commented Jul 24, 2022

lopopolo commented Oct 22, 2022

lopopolo commented Oct 24, 2022

lopopolo commented Nov 9, 2022

Implement mrb_raise with Rust panic! #1904

Are you sure you want to change the base?

Implement mrb_raise with Rust panic! #1904

Conversation

lopopolo commented Jun 12, 2022

Compatibility Notes

lopopolo commented Jul 20, 2022

lopopolo commented Jul 24, 2022

lopopolo commented Jul 24, 2022

lopopolo commented Oct 22, 2022

lopopolo commented Oct 24, 2022

lopopolo commented Nov 9, 2022

Implement `mrb_raise` with Rust `panic!` #1904

Implement `mrb_raise` with Rust `panic!` #1904