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

test edge cases in the public C api #150

Merged
merged 4 commits into from
Aug 16, 2024
Merged

test edge cases in the public C api #150

merged 4 commits into from
Aug 16, 2024

Conversation

folkertdev
Copy link
Collaborator

we now cover all branches with dedicated tests.

miri reported UB here. I'm not entirely sure why, I don't think there is actually anything wrong with the original code, and suspect miri is confused by lifetimes here? anyway, the fix is pretty simple, and miri is happy now
Comment on lines -2225 to -2226
let mut state = State::new(&[], ReadBuf::new(&mut []));
core::mem::swap(&mut state, stream.state);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

miri reported UB here. I think the problem was that state got dropped and/or got confused about lifetimes here?

error: Undefined Behavior: constructing invalid value at .writer.buf: encountered a dangling reference (use-after-free)
    --> /home/folkertdev/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:734:14
     |
734  |     unsafe { intrinsics::typed_swap(x, y) }
     |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .writer.buf: encountered a dangling reference (use-after-free)
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
     = note: BACKTRACE on thread `coverage::ub`:
     = note: inside `std::mem::swap::<zlib_rs::inflate::State<'_>>` at /home/folkertdev/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:734:14: 734:42
note: inside `zlib_rs::inflate::end`
    --> /home/folkertdev/rust/zlib-rs/zlib-rs/src/inflate.rs:2226:5
     |
2226 |     core::mem::swap(&mut state, stream.state);
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `libz_rs_sys::inflateEnd`
    --> /home/folkertdev/rust/zlib-rs/libz-rs-sys/src/lib.rs:370:13
     |
370  |             zlib_rs::inflate::end(stream);

but I'm not 100% sure. This fix makes miri happy, and still passes all tests so I think we're good.

@folkertdev folkertdev merged commit b7bf6ac into main Aug 16, 2024
17 checks passed
@folkertdev folkertdev deleted the lib-coverage branch August 16, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant