Skip to content

Checked swap methods may cause undefined behavior #919

@vihdzp

Description

@vihdzp

The core implementation of the swap method in a matrix looks like this:

/// Swaps two elements using their linear index without bound-checking.
#[inline]
unsafe fn swap_unchecked_linear(&mut self, i1: usize, i2: usize) {
let a = self.get_address_unchecked_linear_mut(i1);
let b = self.get_address_unchecked_linear_mut(i2);
mem::swap(&mut *a, &mut *b);
}

If i1 and i2 are equal, this method will cause mutable aliasing, which is undefined behavior under Rust's aliasing rules. However, the safe swap methods don't check this:

nalgebra/src/base/matrix.rs

Lines 1162 to 1175 in 38add0b

/// Swaps two entries.
#[inline]
pub fn swap(&mut self, row_cols1: (usize, usize), row_cols2: (usize, usize)) {
let (nrows, ncols) = self.shape();
assert!(
row_cols1.0 < nrows && row_cols1.1 < ncols,
"Matrix elements swap index out of bounds."
);
assert!(
row_cols2.0 < nrows && row_cols2.1 < ncols,
"Matrix elements swap index out of bounds."
);
unsafe { self.swap_unchecked(row_cols1, row_cols2) }
}

I believe that the best way to fix this would be to add more assertions to the safe method, and document these safety assumptions in the unsafe counterparts.

Edit: using ptr::swap instead of mem::swap fixes this.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions