rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
164 stars 64 forks source link

Segfault due to unknown xcb::damage::ReportLevel value #228

Open Maxdamantus opened 1 year ago

Maxdamantus commented 1 year ago

xcb = { version = "1.2.1", features = ["damage"] }

The ReportLevel enum is only defined for values 0, 1, 2, 3, but the server will sometimes set an extra bit (DamageNotifyMore, 0x80) to denote that there are more damage events to follow.

Sample reproduction code (usually segfaults for me within a couple of seconds of updates, though I haven't tried with other setups involving compositors etc):

fn main() {
    let (conn, screen_num) = xcb::Connection::connect_with_extensions(None, &[xcb::Extension::Damage], &[]).unwrap();
    let root_window = conn.get_setup().roots().nth(0).unwrap().root();
    conn.wait_for_reply(conn.send_request(&xcb::damage::QueryVersion {
        client_major_version: xcb::damage::MAJOR_VERSION,
        client_minor_version: xcb::damage::MINOR_VERSION,
    })).unwrap();
    conn.check_request(conn.send_request_checked(&xcb::damage::Create {
        damage: conn.generate_id(),
        drawable: xcb::x::Drawable::Window(root_window),
        level: xcb::damage::ReportLevel::RawRectangles,
    })).unwrap();
    loop {
        let e = conn.wait_for_event().unwrap();
        println!("e = {:#?}", e);
    }
}
Maxdamantus commented 1 year ago

Just thought I'd note some observations I've made:

While trying with a compositor (particularly, xcompmgr), the issue doesn't occur, because something seems to combine multiple damages into a single bounding box.

I'm not too familiar with the semantics of xcbproto, but the definition for the event (https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/blob/master/src/damage.xml) does indeed seem to omit this detail. Maybe it's an xcbproto bug?

Even if it is an xcbproto bug, is it expected that this library introduce memory issues due to unexpected data from the server? I'm guessing std::mem::transmute is used for all enums, but this seems a bit unsafe since afaik Rust assumes that the Rust enum members cover all values when performing exhaustiveness checks. I'm not sure what guarantees the xcb C API provides, but at least in this case it's exposed as a C enum type, which is of course just a regular integer type, so values are not limited to the ones listed.

rtbo commented 1 year ago

Hi, In the XML documentation, the 0x80 bit is mentionned in the Notify event, but it's not in the type definition. The thing is that ReportLevel is not really a bitfield. It is an enum with 4 possible values (RawRectangles as value 0 which can't be expressed in a bit field). It is only in the Notify event that we can have an additional bit set on top of this value. The easiest way to deal with this IMO is to hand-write a definition for ReportLevel according the semantics I've just described and to bypass it in the code generation. It was already done for xinput::Device