From 698e130c3b7bbc69add13b1325001fd45c67b737 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 5 Feb 2022 17:05:59 -0500 Subject: [PATCH 1/2] Remove abomonation support Abomonation has numerous soundness problems which have been well-documented in its issue tracker for over 2 years. Some of them could be fixed, but some are fundamental to its design. If a user wants super-fast ser/de, they should use rkyv. --- .github/workflows/nalgebra-ci-build.yml | 4 +- Cargo.toml | 2 - nalgebra-glm/Cargo.toml | 1 - src/base/array_storage.rs | 31 --------------- src/base/matrix.rs | 20 ---------- src/base/unit.rs | 20 ---------- src/base/vec_storage.rs | 20 ---------- src/geometry/isometry.rs | 28 -------------- src/geometry/point.rs | 25 ------------ src/geometry/quaternion.rs | 23 ----------- src/geometry/rotation.rs | 24 ------------ src/geometry/scale.rs | 24 ------------ src/geometry/similarity.rs | 24 ------------ src/geometry/translation.rs | 24 ------------ tests/core/abomonation.rs | 51 ------------------------- tests/core/mod.rs | 2 - tests/lib.rs | 2 - 17 files changed, 2 insertions(+), 323 deletions(-) delete mode 100644 tests/core/abomonation.rs diff --git a/.github/workflows/nalgebra-ci-build.yml b/.github/workflows/nalgebra-ci-build.yml index b9df85da..bc2f9ca6 100644 --- a/.github/workflows/nalgebra-ci-build.yml +++ b/.github/workflows/nalgebra-ci-build.yml @@ -61,13 +61,13 @@ jobs: steps: - uses: actions/checkout@v2 - name: test - run: cargo test --features arbitrary,rand,serde-serialize,abomonation-serialize,sparse,debug,io,compare,libm,proptest-support,slow-tests; + run: cargo test --features arbitrary,rand,serde-serialize,sparse,debug,io,compare,libm,proptest-support,slow-tests; test-nalgebra-glm: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - name: test nalgebra-glm - run: cargo test -p nalgebra-glm --features arbitrary,serde-serialize,abomonation-serialize; + run: cargo test -p nalgebra-glm --features arbitrary,serde-serialize; test-nalgebra-sparse: runs-on: ubuntu-latest steps: diff --git a/Cargo.toml b/Cargo.toml index 13f0584c..15a24d41 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,7 +53,6 @@ convert-glam020 = [ "glam020" ] ## `serde-serialize`. serde-serialize-no-std = [ "serde", "num-complex/serde" ] serde-serialize = [ "serde-serialize-no-std", "serde/std" ] -abomonation-serialize = [ "abomonation" ] rkyv-serialize-no-std = [ "rkyv" ] rkyv-serialize = [ "rkyv-serialize-no-std", "rkyv/std" ] @@ -81,7 +80,6 @@ alga = { version = "0.9", default-features = false, optional = true } rand_distr = { version = "0.4", default-features = false, optional = true } matrixmultiply = { version = "0.3", optional = true } serde = { version = "1.0", default-features = false, features = [ "derive" ], optional = true } -abomonation = { version = "0.7", optional = true } rkyv = { version = "~0.6.4", default-features = false, features = ["const_generics"], optional = true } mint = { version = "0.5", optional = true } quickcheck = { version = "1", optional = true } diff --git a/nalgebra-glm/Cargo.toml b/nalgebra-glm/Cargo.toml index 287bb8c7..f8087581 100644 --- a/nalgebra-glm/Cargo.toml +++ b/nalgebra-glm/Cargo.toml @@ -21,7 +21,6 @@ default = [ "std" ] std = [ "nalgebra/std", "simba/std" ] arbitrary = [ "nalgebra/arbitrary" ] serde-serialize = [ "nalgebra/serde-serialize-no-std" ] -abomonation-serialize = [ "nalgebra/abomonation-serialize" ] cuda = [ "nalgebra/cuda" ] # Conversion diff --git a/src/base/array_storage.rs b/src/base/array_storage.rs index b46d442f..6851c381 100644 --- a/src/base/array_storage.rs +++ b/src/base/array_storage.rs @@ -1,7 +1,5 @@ use std::fmt::{self, Debug, Formatter}; // use std::hash::{Hash, Hasher}; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; use std::ops::Mul; #[cfg(feature = "serde-serialize-no-std")] @@ -13,9 +11,6 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde-serialize-no-std")] use std::marker::PhantomData; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use crate::base::allocator::Allocator; use crate::base::default_allocator::DefaultAllocator; use crate::base::dimension::{Const, ToTypenum}; @@ -282,32 +277,6 @@ unsafe impl by { } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for ArrayStorage -where - T: Scalar + Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - for element in self.as_slice() { - element.entomb(writer)?; - } - - Ok(()) - } - - unsafe fn exhume<'a, 'b>(&'a mut self, mut bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - for element in self.as_mut_slice() { - let temp = bytes; - bytes = element.exhume(temp)? - } - Some(bytes) - } - - fn extent(&self) -> usize { - self.as_slice().iter().fold(0, |acc, e| acc + e.extent()) - } -} - #[cfg(feature = "rkyv-serialize-no-std")] mod rkyv_impl { use super::ArrayStorage; diff --git a/src/base/matrix.rs b/src/base/matrix.rs index 652eace1..bdf3a8c7 100644 --- a/src/base/matrix.rs +++ b/src/base/matrix.rs @@ -1,6 +1,4 @@ use num::{One, Zero}; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use std::any::TypeId; @@ -13,9 +11,6 @@ use std::mem; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::scalar::{ClosedAdd, ClosedMul, ClosedSub, Field, SupersetOf}; use simba::simd::SimdPartialOrd; @@ -254,21 +249,6 @@ where } } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Matrix { - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.data.entomb(writer) - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.data.exhume(bytes) - } - - fn extent(&self) -> usize { - self.data.extent() - } -} - #[cfg(feature = "compare")] impl> matrixcompare_core::Matrix for Matrix diff --git a/src/base/unit.rs b/src/base/unit.rs index 60281b8f..9336a5e5 100644 --- a/src/base/unit.rs +++ b/src/base/unit.rs @@ -1,14 +1,9 @@ use std::fmt; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; use std::ops::Deref; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use crate::allocator::Allocator; use crate::base::DefaultAllocator; use crate::storage::RawStorage; @@ -66,21 +61,6 @@ impl<'de, T: Deserialize<'de>> Deserialize<'de> for Unit { } } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Unit { - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.value.entomb(writer) - } - - fn extent(&self) -> usize { - self.value.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.value.exhume(bytes) - } -} - #[cfg(feature = "rkyv-serialize-no-std")] mod rkyv_impl { use super::Unit; diff --git a/src/base/vec_storage.rs b/src/base/vec_storage.rs index bf73661d..414354cd 100644 --- a/src/base/vec_storage.rs +++ b/src/base/vec_storage.rs @@ -1,6 +1,3 @@ -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; - #[cfg(all(feature = "alloc", not(feature = "std")))] use alloc::vec::Vec; @@ -18,8 +15,6 @@ use serde::{ }; use crate::Storage; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; use std::mem::MaybeUninit; /* @@ -402,21 +397,6 @@ where } } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for VecStorage { - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.data.entomb(writer) - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.data.exhume(bytes) - } - - fn extent(&self) -> usize { - self.data.extent() - } -} - impl Extend for VecStorage { /// Extends the number of columns of the `VecStorage` with elements /// from the given iterator. diff --git a/src/geometry/isometry.rs b/src/geometry/isometry.rs index f020c0e9..8cdd1bfc 100755 --- a/src/geometry/isometry.rs +++ b/src/geometry/isometry.rs @@ -1,15 +1,10 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use std::fmt; use std::hash; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Serialize}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::scalar::{RealField, SubsetOf}; use simba::simd::SimdRealField; @@ -81,29 +76,6 @@ pub struct Isometry { pub translation: Translation, } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Isometry -where - T: SimdRealField, - R: Abomonation, - Translation: Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.rotation.entomb(writer)?; - self.translation.entomb(writer) - } - - fn extent(&self) -> usize { - self.rotation.extent() + self.translation.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.rotation - .exhume(bytes) - .and_then(|bytes| self.translation.exhume(bytes)) - } -} - #[cfg(feature = "rkyv-serialize-no-std")] mod rkyv_impl { use super::Isometry; diff --git a/src/geometry/point.rs b/src/geometry/point.rs index 0953a9ce..b62998c3 100644 --- a/src/geometry/point.rs +++ b/src/geometry/point.rs @@ -3,15 +3,10 @@ use num::One; use std::cmp::Ordering; use std::fmt; use std::hash; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::simd::SimdPartialOrd; use crate::base::allocator::Allocator; @@ -130,26 +125,6 @@ where } } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for OPoint -where - T: Scalar, - OVector: Abomonation, - DefaultAllocator: Allocator, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.coords.entomb(writer) - } - - fn extent(&self) -> usize { - self.coords.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.coords.exhume(bytes) - } -} - impl OPoint where DefaultAllocator: Allocator, diff --git a/src/geometry/quaternion.rs b/src/geometry/quaternion.rs index cae06251..0aa7f3d3 100755 --- a/src/geometry/quaternion.rs +++ b/src/geometry/quaternion.rs @@ -2,17 +2,12 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use num::Zero; use std::fmt; use std::hash::{Hash, Hasher}; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; #[cfg(feature = "serde-serialize-no-std")] use crate::base::storage::Owned; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::scalar::{ClosedNeg, RealField}; use simba::simd::{SimdBool, SimdOption, SimdRealField}; @@ -77,24 +72,6 @@ where { } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Quaternion -where - Vector4: Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.coords.entomb(writer) - } - - fn extent(&self) -> usize { - self.coords.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.coords.exhume(bytes) - } -} - #[cfg(feature = "serde-serialize-no-std")] impl Serialize for Quaternion where diff --git a/src/geometry/rotation.rs b/src/geometry/rotation.rs index 2d5e5d24..69c4a355 100755 --- a/src/geometry/rotation.rs +++ b/src/geometry/rotation.rs @@ -2,8 +2,6 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use num::{One, Zero}; use std::fmt; use std::hash; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -11,9 +9,6 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "serde-serialize-no-std")] use crate::base::storage::Owned; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::scalar::RealField; use simba::simd::SimdRealField; @@ -94,25 +89,6 @@ where { } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Rotation -where - T: Scalar, - SMatrix: Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.matrix.entomb(writer) - } - - fn extent(&self) -> usize { - self.matrix.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.matrix.exhume(bytes) - } -} - #[cfg(feature = "serde-serialize-no-std")] impl Serialize for Rotation where diff --git a/src/geometry/scale.rs b/src/geometry/scale.rs index b1d278d6..064e0075 100755 --- a/src/geometry/scale.rs +++ b/src/geometry/scale.rs @@ -2,15 +2,10 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use num::{One, Zero}; use std::fmt; use std::hash; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use crate::base::allocator::Allocator; use crate::base::dimension::{DimNameAdd, DimNameSum, U1}; use crate::base::storage::Owned; @@ -64,25 +59,6 @@ where { } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Scale -where - T: Scalar, - SVector: Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.vector.entomb(writer) - } - - fn extent(&self) -> usize { - self.vector.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.vector.exhume(bytes) - } -} - #[cfg(feature = "serde-serialize-no-std")] impl Serialize for Scale where diff --git a/src/geometry/similarity.rs b/src/geometry/similarity.rs index fd6b1d36..46c86f5d 100755 --- a/src/geometry/similarity.rs +++ b/src/geometry/similarity.rs @@ -3,15 +3,9 @@ use num::Zero; use std::fmt; use std::hash; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; - #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Serialize}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::scalar::{RealField, SubsetOf}; use simba::simd::SimdRealField; @@ -49,24 +43,6 @@ pub struct Similarity { scaling: T, } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Similarity -where - Isometry: Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.isometry.entomb(writer) - } - - fn extent(&self) -> usize { - self.isometry.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.isometry.exhume(bytes) - } -} - impl hash::Hash for Similarity where Owned>: hash::Hash, diff --git a/src/geometry/translation.rs b/src/geometry/translation.rs index 1ce8cba3..b07cce20 100755 --- a/src/geometry/translation.rs +++ b/src/geometry/translation.rs @@ -2,15 +2,10 @@ use approx::{AbsDiffEq, RelativeEq, UlpsEq}; use num::{One, Zero}; use std::fmt; use std::hash; -#[cfg(feature = "abomonation-serialize")] -use std::io::{Result as IOResult, Write}; #[cfg(feature = "serde-serialize-no-std")] use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[cfg(feature = "abomonation-serialize")] -use abomonation::Abomonation; - use simba::scalar::{ClosedAdd, ClosedNeg, ClosedSub}; use crate::base::allocator::Allocator; @@ -64,25 +59,6 @@ where { } -#[cfg(feature = "abomonation-serialize")] -impl Abomonation for Translation -where - T: Scalar, - SVector: Abomonation, -{ - unsafe fn entomb(&self, writer: &mut W) -> IOResult<()> { - self.vector.entomb(writer) - } - - fn extent(&self) -> usize { - self.vector.extent() - } - - unsafe fn exhume<'a, 'b>(&'a mut self, bytes: &'b mut [u8]) -> Option<&'b mut [u8]> { - self.vector.exhume(bytes) - } -} - #[cfg(feature = "serde-serialize-no-std")] impl Serialize for Translation where diff --git a/tests/core/abomonation.rs b/tests/core/abomonation.rs deleted file mode 100644 index 5148db81..00000000 --- a/tests/core/abomonation.rs +++ /dev/null @@ -1,51 +0,0 @@ -use abomonation::{decode, encode, Abomonation}; -use na::{ - DMatrix, Isometry3, IsometryMatrix3, Matrix3x4, Point3, Quaternion, Rotation3, Similarity3, - SimilarityMatrix3, Translation3, -}; -use rand::random; - -#[test] -fn abomonate_dmatrix() { - assert_encode_and_decode(DMatrix::::new_random(3, 5)); -} - -macro_rules! test_abomonation( - ($($test: ident, $ty: ty);* $(;)*) => {$( - #[test] - fn $test() { - assert_encode_and_decode(random::<$ty>()); - } - )*} -); - -test_abomonation! { - abomonate_matrix3x4, Matrix3x4; - abomonate_point3, Point3; - abomonate_translation3, Translation3; - abomonate_rotation3, Rotation3; - abomonate_isometry3, Isometry3; - abomonate_isometry_matrix3, IsometryMatrix3; - abomonate_similarity3, Similarity3; - abomonate_similarity_matrix3, SimilarityMatrix3; - abomonate_quaternion, Quaternion; -} - -fn assert_encode_and_decode(original_data: T) { - // Hold on to a clone for later comparison - let data = original_data.clone(); - - // Encode - let mut bytes = Vec::new(); - unsafe { - let _ = encode(&original_data, &mut bytes); - } - - // Drop the original, so that dangling pointers are revealed by the test - drop(original_data); - - if let Some((result, rest)) = unsafe { decode::(&mut bytes) } { - assert!(result == &data); - assert!(rest.len() == 0, "binary data was not decoded completely"); - } -} diff --git a/tests/core/mod.rs b/tests/core/mod.rs index c03461dc..5fb8c82b 100644 --- a/tests/core/mod.rs +++ b/tests/core/mod.rs @@ -1,5 +1,3 @@ -#[cfg(feature = "abomonation-serialize")] -mod abomonation; mod blas; mod cg; mod conversion; diff --git a/tests/lib.rs b/tests/lib.rs index 11c1e23f..546aa8a7 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -9,8 +9,6 @@ compile_error!( Example: `cargo test --features debug,compare,rand,macros`" ); -#[cfg(feature = "abomonation-serialize")] -extern crate abomonation; #[cfg(all(feature = "debug", feature = "compare", feature = "rand"))] #[macro_use] extern crate approx; From 18730dd9868c326cba4afa981527fae5c8541dd3 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 6 Feb 2022 10:03:22 -0500 Subject: [PATCH 2/2]