rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
161 stars 63 forks source link

serialize alignment issue for `SendEventDest` #229

Closed wez closed 1 year ago

wez commented 1 year ago

Over in https://github.com/wez/wezterm/issues/3838 a user reported this panic:

thread 'main' panicked at 'misaligned pointer dereference: address must be a multiple of 0x4 but is 0x7ffeca6b441f', /home/riffraff/src/wezterm/target/debug/build/xcb-742adc88e75b2c36/out/xproto.rs:8103:13
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
19:29:54.341 ERROR env_bootstrap > panic at library/core/src/panicking.rs:126:5 - panic in a function that cannot unwind
0: env_bootstrap::register_panic_hook::{{closure}}
at env-bootstrap/src/lib.rs:176:18
1: call<(&core::panic::panic_info::PanicInfo), (dyn core::ops::function::Fn<(&core::panic::panic_info::PanicInfo), Output=()> + core::marker::Send + core::marker::Sync), alloc::alloc::Global>
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/alloc/src/boxed.rs:1999:9
rust_panic_with_hook
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/std/src/panicking.rs:709:13
2: {closure#0}
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/std/src/panicking.rs:595:13
3: __rust_end_short_backtrace<std::panicking::begin_panic_handler::{closure_env#0}, !>
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/std/src/sys_common/backtrace.rs:151:18
4: begin_panic_handler
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/std/src/panicking.rs:593:5
5: panic_nounwind_fmt
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/core/src/panicking.rs:96:14
6: panic_nounwind
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/core/src/panicking.rs:126:5
7: core::panicking::panic_cannot_unwind
at /rustc/b2b34bd83192c3d16c88655158f7d8d612513e88/library/core/src/panicking.rs:188:5
8: xcb::xproto::SendEventDest::serialize
at target/debug/build/xcb-742adc88e75b2c36/out/xproto.rs:8100:5
9: <xcb::xproto::SendEvent as xcb::base::RawRequest>::raw_request
at target/debug/build/xcb-742adc88e75b2c36/out/xproto.rs:14645:9
10: xcb::base::Connection::send_request_checked
at /home/riffraff/.cargo/registry/src/index.crates.io-6f17d22bba15001f/xcb-1.2.1/src/base.rs:1442:51
11: xcb::base::Connection::send_and_check_request
at /home/riffraff/.cargo/registry/src/index.crates.io-6f17d22bba15001f/xcb-1.2.1/src/base.rs:1533:28
12: window::os::x11::connection::XConnection::send_request_no_reply
at window/src/os/x11/connection.rs:803:9
13: window::os::x11:window:XWindowInner::selection_request
at window/src/os/x11/window.rs:781:9

the relevant section of xproto.rs is:

impl SendEventDest {
    pub fn resource_id(&self) -> u32 {
        match self {
            SendEventDest::PointerWindow => 0,
            SendEventDest::ItemFocus => 1,
            SendEventDest::Window(w) => w.resource_id(),
        }
    }

    pub fn serialize(&self, wire_buf: &mut [u8]) -> usize {
        debug_assert!(wire_buf.len() >= 4);
        unsafe {
            *(wire_buf.as_mut_ptr() as *mut u32) = self.resource_id(); //  <--- this is the line from the panic
        }
        4
    }
}

I believe that we need a similar treatment as that in https://github.com/rust-x-bindings/rust-xcb/pull/225 for the generated serialization logic to avoid unaligned memory accesses