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

Unaligned reads & incorrect encoded sizes #3

Open
mumbleskates opened this issue Jan 3, 2025 · 1 comment
Open

Unaligned reads & incorrect encoded sizes #3

mumbleskates opened this issue Jan 3, 2025 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@mumbleskates
Copy link

It looks like a number of methods, including at the very least UnsafeReader::unwire_u64, perform unsound unaligned reads which will cause the program to crash with assertions enabled. Additionally, structs which derive Wiring and Unwiring but have unused padding inside or outside the struct use mem::size_of::<T>() to calculate the encoded size of multiple values in <Vec<T> as Unwiring>::bytes_length, even though the actual byte size may be much shorter.

Here are a couple tests demonstrating the issues I found:

use wiring::prelude::*;

#[derive(Debug, PartialEq, Unwiring, Wiring)]
struct Syncopated {
    small: u8,
    big: u64,
}
#[derive(Debug, PartialEq, Unwiring, Wiring)]
struct Several {
    vec: Vec<Syncopated>,
}

#[test]
fn unaligned_read() {
    let a = Syncopated { small: 1, big: 2 };
    let mut buf = Vec::new();
    BufWire::new(&mut buf).wire(&a).unwrap();
    let b = BufUnWire::new(buf.as_slice()).unwire::<Syncopated>().unwrap();

    assert_eq!(a, b);
}

#[test]
fn incorrect_size_estimation() {
    let a = Several {
        vec: vec![Syncopated { small: 1, big: 2 }, Syncopated { small: 3, big: 4 }],
    };
    let mut buf = Vec::new();
    BufWire::new(&mut buf).wire(&a).unwrap();
    let b = BufUnWire::new(buf.as_slice()).unwire::<Several>().unwrap();

    assert_eq!(a, b);
}
mumbleskates added a commit to mumbleskates/rust_serialization_benchmark that referenced this issue Jan 3, 2025
djkoloski pushed a commit to djkoloski/rust_serialization_benchmark that referenced this issue Jan 15, 2025
* fix arithmetic overflow panic

* use vec instead of a boxed array to avoid blowing the stack in dev mode

* make it possible to disable compression to speed up `cargo test --benches`

* disable wiring: louaykamel/wiring#3

* add test step to build.yml

* also run build step in the master branch (& give it a workflow_dispatch button)

* organize & alphabetize deps; organize & alphabetize features; make compression a positive feature
@louaykamel
Copy link
Owner

Oh I see, thank you for highlighting the issues. I will dive into this during the weekend.

@louaykamel louaykamel added the bug Something isn't working label Jan 21, 2025
@louaykamel louaykamel self-assigned this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants