Error handling without UB

[gnzlbg]

IIUC, you can pass mozjpeg a function that gets called when an error happens. That function currently panic!. Is that the only source of panics from Rust callbacks ?

If so, it would be easy to change that into a C++ function that calls some Rust function and then throws some C++ exception containing whatever information would be interesting for Rust code (e.g. an error string):

extern "C" ErrorType error_function_rust();
extern "C" void error_function_cpp() {
    ErrorType e = error_function_rust();
    if (ErrorType.error()) { throw e.exception(); }
}

Otherwise, we could provide a small proc macro that does this automatically.

To catch that exception before unwinding into Rust and invoking UB, we would need to write one wrapper per extern "C" function declaration in the library, with one or two exceptions.

It wouldn't be hard to write a small proc macro that does this automatically. You'd just put it in the top-level of the crate #![try_catch_extern_decls]. This proc macro would do two things. First, for all functions within a extern "C" { ... } block. It would change a function like:

extern "C" {
    pub fn jpeg_destroy_compress(cinfo: &mut jpeg_compress_struct);
}

into:

pub unsafe fn jpeg_destroy_compress(cinfo: &mut jpeg_compress_struct) {
     extern "C" { 
        pub fn jpeg_destroy_compress_rust_shim(cinfo: &mut jpeg_compress_struct) -> ErrorType;
     } 
    if let Some(string) = jpeg_destroy_compress_rust_shim(cinfo) {
        panic!(string)
    }
}

These functions map an optional error message received from C into a Rust panic. Most functions in the jpeg API don't have a return type, so we can use that, but we could also use an out parameter.

Simultaneously, the proc macro would generate a rust_shim.cpp file containing the Rust shims:

extern "C" CErrorType jpeg_destroy_compress_rust_shim(jpeg_compress_struct* cinfo) {
     try {
           jpeg_destroy_compress(cinfo);
           return CErrorType::ok();
     } catch (const runtime_error& e) {
            return CErrorType::error(e);
     }
}

We'd compile this shim using the cc crate and link it on the fly to the current crate.

[gnzlbg]

This approach is as fast as the current one for the non-exceptional path. When an exception does occur, this approach has the extra overhead of catching a C++ exception in the C++ side, and raising a panic in the Rust side. If this performance impact is not acceptable, we could, instead of throwing a C++ exception, do a longjmp, raising a panic only once.

[kornelski]

I feel like we're going in circles and not making any progress. This exact solution has been proposed a few times already, even in the original internals thread that started the rfc, and I've already explained a few times why it makes Rust suck:

https://github.com/rust-lang/rust/issues/58794#issuecomment-480241466

Re-iterating:

1. These solutions require a hundred wrappers for libjpeg functions, and change of the API to a custom one at the Rust<>C boundary. This is laborious, because C/C++ doesn't have proc macros, and try/catch wrapper has to be appropriate for each function type (some have meaningful return values, so can't simply return the error object).

2. I could still do 1. despite it being PITA, but it feels like it's for nothing. Using C++ to unwind through C stack frames seems like doing the exact same thing I do with Rust, and the only thing it does is shifting the problem of is-it-compatible-or-UB to be a C++'s problem.

Rust doesn't want to guarantee that all current and future implementations of unwinding, on all platforms existing and not yet existing, will be compatible with C code. I don't think C++ gives such guarantee either, so both are theoretically UB. So if I cared about theoretical possibility of UB, then this rewrite would not change anything for me.

I know that with -fexceptions it's not UB in practice, because that particular C/C++ implementation makes it work for currently supported platforms. Great! But if Rust stops adding nounwind on purpose, then for all platforms mozjpeg currently supports, it'll be implementation-defined happens-to-work thing too.

[gnzlbg]

Did you try this solution out ? Because I have a prototype that generates the wrappers in 100-200 LOC.

[gnzlbg]

It doesn't even need proc macros, my prototype does this in the build.rs.

[gnzlbg]

In the build.rs. I can submit a PR if you want, so that you can see it.

[kornelski]

Yes, please submit a PR. I haven't considered using Rust's bindings to generate C++ code, and tried to use C headers as the input, which was a horror.

[gnzlbg]

I'm at lunch, but will do so later.

[Boscop]

Any update on this? :)