opencv-rust: Possible bug: OpenCV Mat's inner mutability + Rust's aliasing optimization => problems?

Hi thanks for this binding! I am thinking about the following situation: We know multiple Mat can share the same underlying uchar* buffer. Therefore, we can write:

fn copy(dst: &mut [u8], src: &[u8]) {
  for i in 0..dst.len() { dst[i] = src[i]; }
}

fn main() {
  let a = Mat::zeros(...);
  let b = use_some_opencv_function_that_makes_a_and_b_share_same_underlying_buffer(a);
  copy(a.data_bytes_mut(), b.data_bytes()); // OOPS!
}

With assumptions on aliasing, copy function’s dst and src must be two different memory areas, so Rust may optimize based on this assumption. But we know that is not true. So problem occurs!

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 18 (11 by maintainers)

Most upvoted comments

In the 0.89 version there is now BoxedRef and BoxedRefMut wrappers that address this issue, some of the functions like roi are already converted to using those, but there are probably more functions that borrow data behind the scenes that I missed. I would be glad to receive reports about them!

twistedfall on Apr 14, 2024

This indeed fixes it, thanks a lot and sorry for the issue I guess since this crate wasn’t at fault 😃

AntoniosBarotsis on May 15, 2023

The cv-bridge-rs crate uses the unsafe function new_rows_cols_with_data() to create a Mat in as_cvmat(): https://github.com/OmkarKabadagi5823/cv-bridge-rs/blob/main/src/cv_image.rs#LL198C17-L198C17 This makes the resulting Mat a reference to the data contained in CvImage. As soon as the CvImage is destroyed the memory is freed and Mat is now referring to that freed memory. The artifacts you’re seeing could really likely be caused by this. I suggest you try calling a clone() right after you call as_cvmat(), it should fix the issue.

twistedfall on May 15, 2023

That could be the issue but it’s weird that cloning it seems to solve the issue.

I do want to add that for me, cloning is fine as the resolution is pretty low so it shouldn’t add too much overhead. At this point, I’m not too sure if this is indeed an issue with this crate since there are so many moving parts to get this specific use case to break successfully.

I think that explaining all of the involved code would just go a bit too much off-topic. I will reply here if I figure something out.

Again, really appreciate your responses and the bindings themselves!

AntoniosBarotsis on May 15, 2023

You’re saying that image is coming from some external source and when you read the same image from the disk you can’t reproduce the problem, right? If so, it looks like the source of the image is somehow problematic, so I suggest we look there.

twistedfall on May 15, 2023

For now, be careful when writing the code 😃 In the future those function that create a reference to the Mat (like Mat::roi) should be identified and marked manually and the they should return something like MatRef<'m> instead of just Mat.

twistedfall on Oct 8, 2021