-
Notifications
You must be signed in to change notification settings - Fork 618
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
Undefined behavior detected by miri #1357
Comments
Did you find any other miri failures? Running all tests takes a long time and uses quite a lot of memory :) |
I did not try, but I am trying now to run all the tests with miri. 😉 |
Sorry if I did not reply, but it looks like using miri on some test is hardly doable: I went OOM with 16GiB of RAM, then asked for help to @lu-zero, and it nearly OOM with 32GiB of RAM. |
So far
|
I have a 96G amd64 machine, I'll spin the tests there as well. |
It also shouldn't be strictly necessary to run all tests. Only those which eventually might call into |
After 4h20 of CPU time (plus a single restart to add If there's an easy way to find out which tests resolve to |
@HeroicKatora You are totally right, I am guilty of being lazy not looking for Lets' summarize:
Is there anything I am missing? Otherwise we can close this 🎉 |
For those curious,
|
@dodomorandi Thanks for investigating. That should indeed cover most of it. There are some uses of I've tried running siderophile. It's not entirely accurate as it is using
Since running MIRI on all tests seems to be incredibly resource intensive, maybe we could work on a dedicate test suite that targets these functions in particular and directly. |
I can tell you that
|
@HeroicKatora I understand what you are saying, but on the other hand I would like to emphasize the good work your are doing in If a safe function in an external crate triggers UB, I totally understand that you can think "it's our fault, we chose that crate", but on the other hand you also cannot test the soundness of each dependency from image-rs. A good approach would hopefully be to track down which Let's take TL;DR: Using miri to check for UB correctness for all the project would be wonderful, but it looks like we are not still there. Manually tracking UBs requires huge efforts, I think that the best thing that can be done is cooperate with other crates' maintainers, especially in case of strange_ issues (like #1359). Moreover, when working with images and media in general, logic errors (i.e. wrapping behavior instead of saturating) are not safety issues but still very problematic, tools like miri won't help with these. Just assume that you safe code is really safe, otherwise you cannot rely on Rust safety abstractions 😊 |
Good points on bytemuck being MIRI tested without issue. Then we can close this for now. That said, any additional small and quickly executed tests that cover some of the @lu-zero Thanks for the dedication. Due to your efforts, we say that |
I think we can say that
I guess I can just kill this now. |
I skipped over |
No test failed with HEAD at e0261ce (but going back to 32bae1f reproduces the UB failure from the OP). Sanitised log (the tests I skipped because they OOMed are marked accordingly):
Raw log here, just in case. The truncation tests took over 12 hours to compile and OOMed immediately, so they remain unchecked. If there's anything else I can run miri on now or in future, I'm of course more than happy to. |
I just discovered an UB whilst testing another project with miri.
Primitive::from_slice
andPrimitive::from_slice_mut
perform an invalid unsafe cast, which is detected by miri.EDIT: #1358 showed that it is just a matter of a wrong
as_ptr
fn, thereforePrimitive::from_slice
is likely to be unaffected.Expected
cargo +nightly miri test
should not raise any problem.Actual behaviour
test buffer_::test::mut_iter
triggers UB.Reproduction steps
Just run miri on tests☺️
The text was updated successfully, but these errors were encountered: