Fix soundness issue: avoid UB in swap_columns/swap_elements when indices alias

When swap_columns is called with a == b, the previous implementation created
two mutable references (&mut self[a] and &mut self[b]) to the same memory
location (aliased &mut UB). Additionally, even for distinct indices a != b,
using ptr::swap(&mut self[a], &mut self[b]) is UB under Miri (Stacked
Borrows): the second IndexMut call creates a new unique reborrow of all of
self, invalidating the tag from the first call.

The same class of bug existed in:
- Array::swap_elements (structure.rs)
- Matrix{2,3,4}::swap_columns
- Matrix{2,3,4}::swap_elements

Fix: since S: BaseFloat implies S: Copy, column vectors and scalar elements
are Copy. We can swap entirely without unsafe by using a temporary variable:

    let tmp = self[a];
    self[a] = self[b];
    self[b] = tmp;

This avoids creating multiple mutable references simultaneously and requires
no unsafe code at all. Verified clean under Miri.

Tests added in tests/matrix.rs covering all three matrix sizes for:
- swap_columns with equal indices (no-op, was UB)
- swap_columns with distinct indices (correct swap)
- swap_elements with same position (no-op, was UB)
- swap_elements with same column, different rows

Note: the tests verify correctness but do not catch the soundness bug in a
regular cargo test run. Use cargo +nightly miri test to detect aliasing UB.

Fixes: #565

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: haraldreingruber-dedalus

src/matrix.rs

@@ -580,14 +580,26 @@ impl<S: BaseFloat> Matrix for Matrix2<S> {
 
     #[inline]
     fn swap_columns(&mut self, a: usize, b: usize) {
-        unsafe { ptr::swap(&mut self[a], &mut self[b]) };
+        if a == b {
+            return;
+        }
+        // S: BaseFloat implies S: Copy, so column vectors are Copy — no unsafe needed.
+        let tmp = self[a];
+        self[a] = self[b];
+        self[b] = tmp;
     }
 
     #[inline]
     fn swap_elements(&mut self, a: (usize, usize), b: (usize, usize)) {
+        if a == b {
+            return;
+        }
         let (ac, ar) = a;
         let (bc, br) = b;
-        unsafe { ptr::swap(&mut self[ac][ar], &mut self[bc][br]) };
+        // S: BaseFloat implies S: Copy — swap by value without unsafe.
+        let tmp = self[ac][ar];
+        self[ac][ar] = self[bc][br];
+        self[bc][br] = tmp;
     }
 
     fn transpose(&self) -> Matrix2<S> {
@@ -679,14 +691,26 @@ impl<S: BaseFloat> Matrix for Matrix3<S> {
 
     #[inline]
     fn swap_columns(&mut self, a: usize, b: usize) {
-        unsafe { ptr::swap(&mut self[a], &mut self[b]) };
+        if a == b {
+            return;
+        }
+        // S: BaseFloat implies S: Copy, so column vectors are Copy — no unsafe needed.
+        let tmp = self[a];
+        self[a] = self[b];
+        self[b] = tmp;
     }
 
     #[inline]
     fn swap_elements(&mut self, a: (usize, usize), b: (usize, usize)) {
+        if a == b {
+            return;
+        }
         let (ac, ar) = a;
         let (bc, br) = b;
-        unsafe { ptr::swap(&mut self[ac][ar], &mut self[bc][br]) };
+        // S: BaseFloat implies S: Copy — swap by value without unsafe.
+        let tmp = self[ac][ar];
+        self[ac][ar] = self[bc][br];
+        self[bc][br] = tmp;
     }
 
     fn transpose(&self) -> Matrix3<S> {
@@ -795,14 +819,26 @@ impl<S: BaseFloat> Matrix for Matrix4<S> {
 
     #[inline]
     fn swap_columns(&mut self, a: usize, b: usize) {
-        unsafe { ptr::swap(&mut self[a], &mut self[b]) };
+        if a == b {
+            return;
+        }
+        // S: BaseFloat implies S: Copy, so column vectors are Copy — no unsafe needed.
+        let tmp = self[a];
+        self[a] = self[b];
+        self[b] = tmp;
     }
 
     #[inline]
     fn swap_elements(&mut self, a: (usize, usize), b: (usize, usize)) {
+        if a == b {
+            return;
+        }
         let (ac, ar) = a;
         let (bc, br) = b;
-        unsafe { ptr::swap(&mut self[ac][ar], &mut self[bc][br]) };
+        // S: BaseFloat implies S: Copy — swap by value without unsafe.
+        let tmp = self[ac][ar];
+        self[ac][ar] = self[bc][br];
+        self[bc][br] = tmp;
     }
 
     fn transpose(&self) -> Matrix4<S> {

src/structure.rs

@@ -72,10 +72,13 @@ where
     /// Swap the elements at indices `i` and `j` in-place.
     #[inline]
     fn swap_elements(&mut self, i: usize, j: usize) {
-        use std::ptr;
-
-        // Yeah, ok borrow checker – I know what I'm doing here
-        unsafe { ptr::swap(&mut self[i], &mut self[j]) };
+        if i == j {
+            return;
+        }
+        // Element: Copy is a supertrait bound, so we can swap without unsafe.
+        let tmp = self[i];
+        self[i] = self[j];
+        self[j] = tmp;
     }
 
     /// The sum of the elements of the array.

tests/matrix.rs

@@ -16,6 +16,134 @@
 extern crate approx;
 extern crate cgmath;
 
+/// Tests for swap_columns, swap_rows, and swap_elements correctness and soundness
+/// (see https://github.com/rustgd/cgmath/issues/565).
+pub mod swap {
+    use cgmath::*;
+
+    #[test]
+    fn matrix2_swap_columns_same_index() {
+        let original = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
+        let mut m = original;
+        m.swap_columns(0, 0);
+        assert_eq!(m, original);
+        m.swap_columns(1, 1);
+        assert_eq!(m, original);
+    }
+
+    #[test]
+    fn matrix2_swap_columns_different_indices() {
+        let mut m = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
+        m.swap_columns(0, 1);
+        assert_eq!(m, Matrix2::new(3.0, 4.0, 1.0, 2.0));
+        // swapping back gives original
+        m.swap_columns(1, 0);
+        assert_eq!(m, Matrix2::new(1.0, 2.0, 3.0, 4.0));
+    }
+
+    #[test]
+    fn matrix2_swap_elements_same_position() {
+        let original = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
+        let mut m = original;
+        m.swap_elements((0, 0), (0, 0));
+        assert_eq!(m, original);
+        m.swap_elements((1, 1), (1, 1));
+        assert_eq!(m, original);
+    }
+
+    #[test]
+    fn matrix2_swap_elements_same_column_different_row() {
+        // column 0 has elements [1.0 (row0), 2.0 (row1)]
+        let mut m = Matrix2::new(1.0f64, 2.0, 3.0, 4.0);
+        m.swap_elements((0, 0), (0, 1));
+        assert_eq!(m, Matrix2::new(2.0, 1.0, 3.0, 4.0));
+    }
+
+    #[test]
+    fn matrix3_swap_columns_same_index() {
+        let original = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
+        let mut m = original;
+        m.swap_columns(0, 0);
+        assert_eq!(m, original);
+        m.swap_columns(1, 1);
+        assert_eq!(m, original);
+        m.swap_columns(2, 2);
+        assert_eq!(m, original);
+    }
+
+    #[test]
+    fn matrix3_swap_columns_different_indices() {
+        let mut m = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
+        let col0_before = m[0];
+        let col2_before = m[2];
+        m.swap_columns(0, 2);
+        assert_eq!(m[0], col2_before);
+        assert_eq!(m[2], col0_before);
+    }
+
+    #[test]
+    fn matrix3_swap_elements_same_position() {
+        let original = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
+        let mut m = original;
+        m.swap_elements((1, 1), (1, 1));
+        assert_eq!(m, original);
+    }
+
+    #[test]
+    fn matrix3_swap_elements_same_column_different_row() {
+        let mut m = Matrix3::new(1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
+        m.swap_elements((1, 0), (1, 2));
+        // column 1 had [4, 5, 6]; after swap row0 and row2: [6, 5, 4]
+        assert_eq!(m[1], Vector3::new(6.0, 5.0, 4.0));
+    }
+
+    #[test]
+    fn matrix4_swap_columns_same_index() {
+        let original = Matrix4::new(
+            1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
+            16.0,
+        );
+        let mut m = original;
+        m.swap_columns(2, 2);
+        assert_eq!(m, original);
+    }
+
+    #[test]
+    fn matrix4_swap_columns_different_indices() {
+        let mut m = Matrix4::new(
+            1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
+            16.0,
+        );
+        let col1_before = m[1];
+        let col3_before = m[3];
+        m.swap_columns(1, 3);
+        assert_eq!(m[1], col3_before);
+        assert_eq!(m[3], col1_before);
+    }
+
+    #[test]
+    fn matrix4_swap_elements_same_position() {
+        let original = Matrix4::new(
+            1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
+            16.0,
+        );
+        let mut m = original;
+        m.swap_elements((2, 2), (2, 2));
+        assert_eq!(m, original);
+    }
+
+    #[test]
+    fn matrix4_swap_elements_same_column_different_row() {
+        let mut m = Matrix4::new(
+            1.0f64, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
+            16.0,
+        );
+        // column 2 has rows [9, 10, 11, 12]; swap row 0 and row 3
+        m.swap_elements((2, 0), (2, 3));
+        assert_eq!(m[2], Vector4::new(12.0, 10.0, 11.0, 9.0));
+    }
+}
+
 pub mod matrix2 {
     use std::f64;