jam1garner / binrw

A Rust crate for helping parse and rebuild binary data using ✨macro magic✨.
https://binrw.rs
MIT License
545 stars 35 forks source link

Stack overflow issue in test #257

Closed jkorinth closed 2 months ago

jkorinth commented 2 months ago

MVE:

use binrw::*;

#[derive(Eq, PartialEq)]
#[repr(u8)]
#[binrw]
#[brw(repr = u8)]
pub enum BlockType {
    Empty = 0_u8,
}

impl PartialEq<BlockType> for &BlockType {
    fn eq(&self, other: &BlockType) -> bool {
        self == other
    }
}

#[binrw]
pub struct Padding([u8; 54]);

impl Default for Padding {
    fn default() -> Self {
        Self([0xde_u8; 54])
    }
}

#[binrw]
pub struct Version {
    major: u8,
    minor: u8,
    patch: u8,
}

#[binrw]
#[brw(little)]
pub struct Block {
    crc32: u32,
    ty: BlockType,
    version: Version,
    #[brw(if(ty == BlockType::Empty))]
    empty: Padding,
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_problem() {
        let mut data = [0_u8; 64];
        let b = Block {
            crc32: 0x78563412,
            ty: BlockType::Empty,
            version: Version { major: 1, minor: 2, patch: 3 },
            empty: Padding::default(),
        };
        b.write(&mut binrw::io::Cursor::new(&mut data as &mut [u8])).unwrap();
    }
}

Would expect this to pass, instead get:

   Compiling bug v0.1.0 (/bug)
    Finished test [unoptimized + debuginfo] target(s) in 0.22s
     Running unittests src/lib.rs (target/debug/deps/bug-eef9c9490de6d757)

running 1 test

thread 'tests::test_problem' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/bug/target/debug/deps/bug-eef9c9490de6d757` (signal: 6, SIGABRT: process abort signal)

I also tried to wrap the Block in a Box, but that didn't work either.

Output of cargo tree:

bug v0.1.0 (/bug)
└── binrw v0.13.3
    ├── array-init v2.1.0
    ├── binrw_derive v0.13.3 (proc-macro)
    │   ├── either v1.11.0
    │   ├── proc-macro2 v1.0.82
    │   │   └── unicode-ident v1.0.12
    │   ├── quote v1.0.36
    │   │   └── proc-macro2 v1.0.82 (*)
    │   └── syn v1.0.109
    │       ├── proc-macro2 v1.0.82 (*)
    │       ├── quote v1.0.36 (*)
    │       └── unicode-ident v1.0.12
    └── bytemuck v1.15.0
csnover commented 2 months ago

Thanks for your report!

This code is recursing infinitely in the impl PartialEq<BlockType> for &BlockType implementation, not inside binrw.

Dereference inside that comparison function so it is actually comparing two BlockType rather than comparing references (which is where the recursion comes from), or separate #[brw(if)] into #[br] (without a deref) and #[bw] (with a deref) to eliminate the PartialEq<T> for &T implementation.

See also https://github.com/jam1garner/binrw/issues/179.

jkorinth commented 2 months ago

Argh, you’re right, sorry! 🤦