Fix UB in Link and Link logic

linksplatform · Jul 1, 2022 · f7dfc0e · f7dfc0e
1 parent 2635c04
commit f7dfc0e
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 30 deletions.
diff --git a/rust/doublets/src/data/link.rs b/rust/doublets/src/data/link.rs
@@ -1,15 +1,17 @@
-use std::default::default;
-use std::fmt::{Debug, Display, Formatter};
-use std::ops::Index;
-use std::slice::{from_raw_parts, SliceIndex};
+use std::{
+    fmt,
+    fmt::{Debug, Formatter},
+    ops::Index,
+    slice::{from_raw_parts, SliceIndex},
+};
 
 use num_traits::zero;
 
-use data::Query;
-use data::ToQuery;
+use data::{Query, ToQuery};
 use num::LinkType;
 
-#[derive(Default, Debug, Eq, PartialEq, Clone, Hash)]
+#[derive(Default, Eq, PartialEq, Clone, Hash)]
+#[repr(C)]
 pub struct Link<T: LinkType> {
     pub index: T,
     pub source: T,
@@ -18,7 +20,7 @@ pub struct Link<T: LinkType> {
 
 impl<T: LinkType> Link<T> {
     pub fn nothing() -> Self {
-        default()
+        Self::default()
     }
 
     pub fn new(index: T, source: T, target: T) -> Self {
@@ -44,25 +46,21 @@ impl<T: LinkType> Link<T> {
     pub fn is_partial(&self) -> bool {
         self.index == self.source || self.index == self.target
     }
-}
-
-impl<T: LinkType, I: SliceIndex<[T]>> Index<I> for Link<T> {
-    type Output = I::Output;
 
-    fn index(&self, index: I) -> &Self::Output {
-        let slice = unsafe { from_raw_parts(&self.index, 3) };
-        Index::index(slice, index)
+    pub fn as_slice(&self) -> &[T] {
+        // SAFETY: Link is repr(C) and therefore is safe to transmute to a slice
+        unsafe { from_raw_parts(&self.index, 3) }
     }
 }
 
-impl<T: LinkType> Display for Link<T> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+impl<T: LinkType> Debug for Link<T> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         write!(f, "({}: {} {})", self.index, self.source, self.target)
     }
 }
 
 impl<T: LinkType> ToQuery<T> for Link<T> {
     fn to_query(&self) -> Query<'_, T> {
-        Query::new(unsafe { from_raw_parts(&self.index, 3) })
+        self.as_slice().to_query()
     }
 }
diff --git a/rust/doublets/src/mem/split/store.rs b/rust/doublets/src/mem/split/store.rs
@@ -810,12 +810,14 @@ impl<
         query: &[T],
         handler: WriteHandler<T>,
     ) -> Result<Flow, Box<dyn Error>> {
-        self.create_by_with(query, |before, after| handler(&before[..], &after[..]))
-            .map_err(|err| err.into())
+        self.create_by_with(query, |before, after| {
+            handler(before.as_slice(), after.as_slice())
+        })
+        .map_err(|err| err.into())
     }
 
     fn each_links(&self, query: &[T], handler: ReadHandler<T>) -> Result<Flow, Box<dyn Error>> {
-        Ok(self.each_by(query, |link| handler(&link[..])))
+        Ok(self.each_by(query, |link| handler(link.as_slice())))
     }
 
     fn update_links(
@@ -825,7 +827,7 @@ impl<
         handler: WriteHandler<T>,
     ) -> Result<Flow, Box<dyn Error>> {
         self.update_by_with(query, change, |before, after| {
-            handler(&before[..], &after[..])
+            handler(before.as_slice(), after.as_slice())
         })
         .map_err(|err| err.into())
     }
@@ -835,8 +837,10 @@ impl<
         query: &[T],
         handler: WriteHandler<T>,
     ) -> Result<Flow, Box<dyn Error>> {
-        self.delete_by_with(query, |before, after| handler(&before[..], &after[..]))
-            .map_err(|err| err.into())
+        self.delete_by_with(query, |before, after| {
+            handler(before.as_slice(), after.as_slice())
+        })
+        .map_err(|err| err.into())
     }
 }
 

diff --git a/rust/doublets/src/mem/unit/store.rs b/rust/doublets/src/mem/unit/store.rs
@@ -603,12 +603,14 @@ impl<T: LinkType, M: RawMem<LinkPart<T>>, TS: UnitTree<T>, TT: UnitTree<T>, TU:
         query: &[T],
         handler: WriteHandler<T>,
     ) -> Result<Flow, Box<dyn Error>> {
-        self.create_by_with(query, |before, after| handler(&before[..], &after[..]))
-            .map_err(|err| err.into())
+        self.create_by_with(query, |before, after| {
+            handler(before.as_slice(), after.as_slice())
+        })
+        .map_err(|err| err.into())
     }
 
     fn each_links(&self, query: &[T], handler: ReadHandler<T>) -> Result<Flow, Box<dyn Error>> {
-        Ok(self.each_by(query, |link| handler(&link[..])))
+        Ok(self.each_by(query, |link| handler(link.as_slice())))
     }
 
     fn update_links(
@@ -618,7 +620,7 @@ impl<T: LinkType, M: RawMem<LinkPart<T>>, TS: UnitTree<T>, TT: UnitTree<T>, TU:
         handler: WriteHandler<T>,
     ) -> Result<Flow, Box<dyn Error>> {
         self.update_by_with(query, change, |before, after| {
-            handler(&before[..], &after[..])
+            handler(before.as_slice(), after.as_slice())
         })
         .map_err(|err| err.into())
     }
@@ -628,8 +630,10 @@ impl<T: LinkType, M: RawMem<LinkPart<T>>, TS: UnitTree<T>, TT: UnitTree<T>, TU:
         query: &[T],
         handler: WriteHandler<T>,
     ) -> Result<Flow, Box<dyn Error>> {
-        self.delete_by_with(query, |before, after| handler(&before[..], &after[..]))
-            .map_err(|err| err.into())
+        self.delete_by_with(query, |before, after| {
+            handler(before.as_slice(), after.as_slice())
+        })
+        .map_err(|err| err.into())
     }
 }
 

diff --git a/rust/doublets/tests/link.rs b/rust/doublets/tests/link.rs
@@ -0,0 +1,8 @@
+use data::ToQuery;
+use doublets::Link;
+
+#[test]
+fn link_to_query() {
+    let link = Link::<usize>::new(1, 2, 3);
+    assert_eq!([1, 2, 3], &link.to_query()[..]);
+}