nalgebra
nalgebra copied to clipboard
Fix UB in `RawStorageMut::swap_unchecked_linear`
According to Miri, RawStorageMut::swap_unchecked_linear introduces a stacked borrows violation:
➜ nalgebra git:(dev) ✗ cargo miri test -- edition::swap
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
Compiling nalgebra-macros v0.2.1 (/home/yotam/nalgebra/nalgebra-macros)
Compiling nalgebra v0.32.3 (/home/yotam/nalgebra)
Running tests/lib.rs (target/miri/x86_64-unknown-linux-gnu/debug/deps/lib-85cd9506b7860f9a)
running 2 tests
test core::edition::swap_columns ... error: Undefined Behavior: attempting a read access using <268833> at alloc82401[0x18], but that tag does not exist in the borrow stack for this location
--> /home/yotam/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:878:9
|
878 | copy_nonoverlapping(x, tmp.as_mut_ptr(), 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| attempting a read access using <268833> at alloc82401[0x18], but that tag does not exist in the borrow stack for this location
| this error occurs as part of an access at alloc82401[0x18..0x20]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <268833> was created by a SharedReadWrite retag at offsets [0x0..0x78]
--> /home/yotam/nalgebra/src/base/array_storage.rs:141:9
|
141 | self.0.as_mut_ptr() as *mut T
| ^^^^^^^^^^^^^^^^^^^
help: <268833> was later invalidated at offsets [0x0..0x78] by a Unique function-entry retag inside this call
--> /home/yotam/nalgebra/src/base/storage.rs:200:17
|
200 | let b = self.get_address_unchecked_linear_mut(i2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: BACKTRACE (of the first span):
= note: inside `std::ptr::swap::<f64>` at /home/yotam/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:878:9: 878:52
note: inside `<na::ArrayStorage<f64, 3, 5> as na::RawStorageMut<f64, na::Const<3>, na::Const<5>>>::swap_unchecked_linear`
--> /home/yotam/nalgebra/src/base/storage.rs:202:9
|
202 | ptr::swap(a, b);
| ^^^^^^^^^^^^^^^
note: inside `<na::ArrayStorage<f64, 3, 5> as na::RawStorageMut<f64, na::Const<3>, na::Const<5>>>::swap_unchecked`
--> /home/yotam/nalgebra/src/base/storage.rs:214:9
|
214 | self.swap_unchecked_linear(lid1, lid2)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `na::Matrix::<f64, na::Const<3>, na::Const<5>, na::ArrayStorage<f64, 3, 5>>::swap_unchecked`
--> /home/yotam/nalgebra/src/base/matrix.rs:1201:9
|
1201 | self.data.swap_unchecked(row_cols1, row_cols2)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `na::base::edition::<impl na::Matrix<f64, na::Const<3>, na::Const<5>, na::ArrayStorage<f64, 3, 5>>>::swap_columns`
--> /home/yotam/nalgebra/src/base/edition.rs:331:26
|
331 | unsafe { self.swap_unchecked((i, icol1), (i, icol2)) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `core::edition::swap_columns`
--> tests/core/edition.rs:208:5
|
208 | m.swap_columns(1, 3);
| ^^^^^^^^^^^^^^^^^^^^
note: inside closure
--> tests/core/edition.rs:197:18
|
195 | #[test]
| ------- in this procedural macro expansion
196 | #[rustfmt::skip]
197 | fn swap_columns() {
| ^
= note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to previous error
error: test failed, to rerun pass `--test lib`
Caused by:
process didn't exit successfully: `/home/yotam/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner /home/yotam/nalgebra/target/miri/x86_64-unknown-linux-gnu/debug/deps/lib-85cd9506b7860f9a 'edition::swap'` (exit status: 1)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
My fix is possibly over-conservative, but I couldn't think of anything else that wouldn't be a breaking change. Granted, for a user to be affected by the change would require that they implemented the unsafe RawStorageMut trait on their own type, and overrode the default get_address_unchecked_linear_mut in a weird way. Still, I think it's better to err on the cautious side.
(BTW, a lot of the tests in edition.rs are currently crashing under miri due to UB, I'm investigating how to fix the rest of the issues but they seem less straight-forward than this one)