Upgrade to FLINT 3.5, memory bugs remain

Author: jnsiemer

Cargo.toml

@@ -14,7 +14,7 @@ autobenches = false
 
 [dependencies]
 criterion = { version = "0.8", features = ["html_reports"] }
-flint-sys = "0.7"
+flint-sys = "0.9.0"
 libc = "0"
 paste = "1"
 rand = "0.10"

src/integer.rs

@@ -26,3 +26,6 @@ pub use poly_over_z::PolyOverZ;
 pub(crate) use poly_over_z::fmpz_poly_helpers;
 pub use z::Z;
 pub(crate) use z::fmpz_helpers;
+
+pub(crate) use mat_z::debug_fmpz_mat_struct;
+pub(crate) use z::debug_fmpz;

src/integer/mat_poly_over_z.rs

@@ -10,7 +10,7 @@
 //! This implementation uses the [FLINT](https://flintlib.org/) library.
 
 use crate::utils::parse::partial_string;
-use flint_sys::fmpz_poly_mat::fmpz_poly_mat_struct;
+use flint_sys::fmpz_types::fmpz_poly_mat_struct;
 use std::fmt;
 
 mod arithmetic;
@@ -88,10 +88,20 @@ impl fmt::Debug for MatPolyOverZ {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "MatPolyOverZ: {{matrix: {}, storage: {:?}}}",
+            "MatPolyOverZ: {{matrix: {}, storage: {{ matrix: {}}}}}",
             // printing the entire matrix is not meaningful for large matrices
             partial_string(self, 3, 3),
-            self.matrix
+            debug_fmpz_poly_mat_struct(&self.matrix)
         )
     }
 }
+
+pub(crate) fn debug_fmpz_poly_mat_struct(value: &fmpz_poly_mat_struct) -> String {
+    format!(
+        "fmpz_poly_mat_struct {{ entries: {:#x}, r: {}, c: {}, stride: {} }}",
+        value.entries.addr(),
+        value.r,
+        value.c,
+        value.stride
+    )
+}

src/integer/mat_poly_over_z/get.rs

@@ -14,11 +14,12 @@ use crate::{
     traits::{MatrixDimensions, MatrixGetEntry, MatrixGetSubmatrix},
 };
 use flint_sys::{
-    fmpz_poly::{fmpz_poly_set, fmpz_poly_struct},
+    fmpz_poly::fmpz_poly_set,
     fmpz_poly_mat::{
         fmpz_poly_mat_entry, fmpz_poly_mat_init_set, fmpz_poly_mat_window_clear,
         fmpz_poly_mat_window_init,
     },
+    fmpz_types::fmpz_poly_struct,
 };
 use std::mem::MaybeUninit;
 
@@ -178,7 +179,7 @@ impl MatPolyOverZ {
         for row in 0..self.get_num_rows() {
             for col in 0..self.get_num_columns() {
                 // efficiently get entry without cloning the entry itself
-                let entry = unsafe { *fmpz_poly_mat_entry(&self.matrix, row, col) };
+                let entry = unsafe { std::ptr::read(fmpz_poly_mat_entry(&self.matrix, row, col)) };
                 entries.push(entry);
             }
         }
@@ -592,16 +593,16 @@ mod test_collect_entries {
         let entries_2 = mat_2.collect_entries();
 
         assert_eq!(entries_1.len(), 6);
-        assert_eq!(unsafe { *entries_1[0].coeffs }.0, 1);
-        assert!(unsafe { *entries_1[2].coeffs }.0 >= 2_i64.pow(62));
-        assert!(unsafe { *entries_1[3].coeffs }.0 >= 2_i64.pow(62));
-        assert_eq!(unsafe { *entries_1[4].coeffs }.0, -3);
+        assert_eq!(unsafe { *entries_1[0].coeffs }, 1);
+        assert!(unsafe { *entries_1[2].coeffs } >= 2_i64.pow(62));
+        assert!(unsafe { *entries_1[3].coeffs } >= 2_i64.pow(62));
+        assert_eq!(unsafe { *entries_1[4].coeffs }, -3);
 
         assert_eq!(entries_2.len(), 2);
-        assert_eq!(unsafe { *entries_2[0].coeffs.offset(0) }.0, -1);
+        assert_eq!(unsafe { *entries_2[0].coeffs.offset(0) }, -1);
         assert_eq!(entries_2[0].length, 1);
-        assert_eq!(unsafe { *entries_2[1].coeffs.offset(0) }.0, 1);
-        assert_eq!(unsafe { *entries_2[1].coeffs.offset(1) }.0, 2);
+        assert_eq!(unsafe { *entries_2[1].coeffs.offset(0) }, 1);
+        assert_eq!(unsafe { *entries_2[1].coeffs.offset(1) }, 2);
         assert_eq!(entries_2[1].length, 2);
     }
 }

src/integer/mat_poly_over_z/ownership.rs

@@ -30,7 +30,7 @@ impl Clone for MatPolyOverZ {
         // we can unwrap since we know, that the number of rows and columns is positive and fits into an [`i64`]
         let mut clone = MatPolyOverZ::new(self.get_num_rows(), self.get_num_columns());
 
-        unsafe { fmpz_poly_mat_set(&mut clone.matrix, &mut self.matrix.to_owned()) }
+        unsafe { fmpz_poly_mat_set(&mut clone.matrix, &self.matrix) }
 
         clone
     }
@@ -78,20 +78,26 @@ mod test_clone {
         // an i64, both should be a pointer and their values should differ
         unsafe {
             assert_ne!(
-                (*(*poly_1.matrix.entries).coeffs.offset(0)).0,
-                (*(*poly_2.matrix.entries).coeffs.offset(0)).0
+                (*(*poly_1.matrix.entries).coeffs.offset(0)),
+                (*(*poly_2.matrix.entries).coeffs.offset(0))
             );
         }
         unsafe {
             assert_ne!(
-                (*(*poly_1.matrix.entries).coeffs.offset(1)).0,
-                (*(*poly_2.matrix.entries).coeffs.offset(1)).0
+                (*(*poly_1.matrix.entries).coeffs.offset(1)),
+                (*(*poly_2.matrix.entries).coeffs.offset(1))
             );
         }
 
         // check if length of polynomial is correctly cloned
-        assert_eq!(unsafe { *poly_1.matrix.entries.offset(0) }.length, 2);
-        assert_eq!(unsafe { *poly_2.matrix.entries.offset(0) }.length, 2);
+        assert_eq!(
+            unsafe { std::ptr::read(poly_1.matrix.entries.offset(0)) }.length,
+            2
+        );
+        assert_eq!(
+            unsafe { std::ptr::read(poly_2.matrix.entries.offset(0)) }.length,
+            2
+        );
 
         assert_eq!(poly_1, poly_2);
     }
@@ -110,20 +116,26 @@ mod test_clone {
             // both should be stored directly on stack and their values should be equal
             unsafe {
                 assert_eq!(
-                    (*(*poly_1.matrix.entries).coeffs.offset(0)).0,
-                    (*(*poly_2.matrix.entries).coeffs.offset(0)).0
+                    (*(*poly_1.matrix.entries).coeffs.offset(0)),
+                    (*(*poly_2.matrix.entries).coeffs.offset(0))
                 );
             }
             unsafe {
                 assert_eq!(
-                    (*(*poly_1.matrix.entries).coeffs.offset(1)).0,
-                    (*(*poly_2.matrix.entries).coeffs.offset(1)).0
+                    (*(*poly_1.matrix.entries).coeffs.offset(1)),
+                    (*(*poly_2.matrix.entries).coeffs.offset(1))
                 );
             }
 
             // check if length of polynomial is correctly cloned
-            assert_eq!(unsafe { *poly_1.matrix.entries.offset(0) }.length, 2);
-            assert_eq!(unsafe { *poly_2.matrix.entries.offset(0) }.length, 2);
+            assert_eq!(
+                unsafe { std::ptr::read(poly_1.matrix.entries.offset(0)) }.length,
+                2
+            );
+            assert_eq!(
+                unsafe { std::ptr::read(poly_2.matrix.entries.offset(0)) }.length,
+                2
+            );
 
             assert_eq!(poly_1, poly_2);
         }
@@ -151,7 +163,7 @@ mod test_drop {
     /// Creates and drops a [`MatPolyOverZ`], and returns the storage points in memory
     fn create_and_drop_poly_over_z() -> i64 {
         let a = MatPolyOverZ::from_str(&format!("[[1  {}]]", u64::MAX)).unwrap();
-        unsafe { *(*a.matrix.entries).coeffs.offset(0) }.0
+        unsafe { *(*a.matrix.entries).coeffs.offset(0) }
     }
 
     /// Check whether freed memory is reused afterwards
@@ -166,13 +178,12 @@ mod test_drop {
         assert!(set.capacity() < 5);
 
         let a = MatPolyOverZ::from_str(&format!("[[2  {} {}]]", u64::MAX - 1, u64::MAX)).unwrap();
-        let storage_point = unsafe { *(*a.matrix.entries).coeffs.offset(0) }.0;
+        let storage_point = unsafe { *(*a.matrix.entries).coeffs.offset(0) };
 
         // memory slots differ due to previously created large integer
         let d = MatPolyOverZ::from_str(&format!("[[2  {} {}]]", u64::MAX - 1, u64::MAX)).unwrap();
-        assert_ne!(
-            storage_point,
-            unsafe { *(*d.matrix.entries).coeffs.offset(0) }.0
-        );
+        assert_ne!(storage_point, unsafe {
+            *(*d.matrix.entries).coeffs.offset(0)
+        });
     }
 }

src/integer/mat_poly_over_z/unsafe_functions.rs

@@ -11,7 +11,7 @@
 
 use super::MatPolyOverZ;
 use crate::macros::unsafe_passthrough::{unsafe_getter, unsafe_setter};
-use flint_sys::fmpz_poly_mat::{fmpz_poly_mat_clear, fmpz_poly_mat_struct};
+use flint_sys::{fmpz_poly_mat::fmpz_poly_mat_clear, fmpz_types::fmpz_poly_mat_struct};
 
 unsafe_getter!(MatPolyOverZ, matrix, fmpz_poly_mat_struct);
 unsafe_setter!(

src/integer/mat_z.rs

@@ -10,7 +10,7 @@
 //! This implementation uses the [FLINT](https://flintlib.org/) library.
 
 use crate::utils::parse::partial_string;
-use flint_sys::fmpz_mat::fmpz_mat_struct;
+use flint_sys::fmpz_types::fmpz_mat_struct;
 use std::fmt;
 
 mod arithmetic;
@@ -99,10 +99,20 @@ impl fmt::Debug for MatZ {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "MatZ: {{matrix: {}, storage: {:?}}}",
+            "MatZ: {{matrix: {}, storage: {{ matrix: {}}}}}",
             // printing the entire matrix is not meaningful for large matrices
             partial_string(self, 3, 3),
-            self.matrix
+            debug_fmpz_mat_struct(&self.matrix)
         )
     }
 }
+
+pub(crate) fn debug_fmpz_mat_struct(value: &fmpz_mat_struct) -> String {
+    format!(
+        "fmpz_mat_struct {{ entries: {:#x}, r: {}, c: {}, stride: {} }}",
+        value.entries.addr(),
+        value.r,
+        value.c,
+        value.stride
+    )
+}

src/integer/mat_z/arithmetic/mul.rs

@@ -97,8 +97,8 @@ impl Mul<&MatZq> for &MatZ {
             other.get_mod(),
         );
         unsafe {
-            fmpz_mat_mul(&mut new.matrix.mat[0], &self.matrix, &other.matrix.mat[0]);
-            _fmpz_mod_mat_reduce(&mut new.matrix)
+            fmpz_mat_mul(&mut new.matrix, &self.matrix, &other.matrix);
+            _fmpz_mod_mat_reduce(&mut new.matrix, other.modulus.get_fmpz_mod_ctx_struct())
         }
         new
     }

src/integer/mat_z/arithmetic/sub.rs

@@ -135,8 +135,8 @@ impl Sub<&MatZq> for &MatZ {
 
         let mut out = MatZq::new(self.get_num_rows(), self.get_num_columns(), other.get_mod());
         unsafe {
-            fmpz_mat_sub(&mut out.matrix.mat[0], &self.matrix, &other.matrix.mat[0]);
-            _fmpz_mod_mat_reduce(&mut out.matrix);
+            fmpz_mat_sub(&mut out.matrix, &self.matrix, &other.matrix);
+            _fmpz_mod_mat_reduce(&mut out.matrix, other.modulus.get_fmpz_mod_ctx_struct());
         }
         out
     }

src/integer/mat_z/get.rs

@@ -14,7 +14,8 @@ use crate::{
     traits::{MatrixDimensions, MatrixGetEntry, MatrixGetSubmatrix},
 };
 use flint_sys::{
-    fmpz::{fmpz, fmpz_init_set},
+    flint::fmpz,
+    fmpz::fmpz_init_set,
     fmpz_mat::{fmpz_mat_entry, fmpz_mat_init_set, fmpz_mat_window_clear, fmpz_mat_window_init},
 };
 use std::mem::MaybeUninit;
@@ -78,7 +79,7 @@ impl MatrixGetEntry<Z> for MatZ {
     /// assert_eq!(unsafe { matrix.get_entry_unchecked(2, 1) }, Z::from(8));
     /// ```
     unsafe fn get_entry_unchecked(&self, row: i64, column: i64) -> Z {
-        let mut copy = fmpz(0);
+        let mut copy: fmpz = 0;
         let entry = unsafe { fmpz_mat_entry(&self.matrix, row, column) };
         unsafe { fmpz_init_set(&mut copy, entry) };
 
@@ -685,15 +686,15 @@ mod test_collect_entries {
         let entries_2 = mat_2.collect_entries();
 
         assert_eq!(entries_1.len(), 6);
-        assert_eq!(entries_1[0].0, 1);
-        assert_eq!(entries_1[1].0, 2);
-        assert!(entries_1[2].0 >= 2_i64.pow(62));
-        assert!(entries_1[3].0 >= 2_i64.pow(62));
-        assert_eq!(entries_1[4].0, 3);
-        assert_eq!(entries_1[5].0, 4);
+        assert_eq!(entries_1[0], 1);
+        assert_eq!(entries_1[1], 2);
+        assert!(entries_1[2] >= 2_i64.pow(62));
+        assert!(entries_1[3] >= 2_i64.pow(62));
+        assert_eq!(entries_1[4], 3);
+        assert_eq!(entries_1[5], 4);
 
         assert_eq!(entries_2.len(), 2);
-        assert_eq!(entries_2[0].0, -1);
-        assert_eq!(entries_2[1].0, 2);
+        assert_eq!(entries_2[0], -1);
+        assert_eq!(entries_2[1], 2);
     }
 }