Mark endian_scalar as unsafe. (#6588)

* Mark endian_scalar as unsafe.

Also
- removed the deprecated flexbuffer slice from example
- fixed some cargo warnings

* Assertions and read_scalar made unsafe

* Clippy lints

* Add to Safety

Co-authored-by: Casper Neo <[email protected]>

Author: Casper

rust/flatbuffers/src/array.rs

@@ -37,6 +37,8 @@ where
     }
 }
 
+#[allow(clippy::len_without_is_empty)]
+#[allow(clippy::from_over_into)]  // TODO(caspern): Go from From to Into.
 impl<'a, T: 'a, const N: usize> Array<'a, T, N> {
     #[inline(always)]
     pub fn new(buf: &'a [u8]) -> Self {
@@ -49,7 +51,7 @@ impl<'a, T: 'a, const N: usize> Array<'a, T, N> {
     }
 
     #[inline(always)]
-    pub fn len(&self) -> usize {
+    pub const fn len(&self) -> usize {
         N
     }
 }

rust/flatbuffers/src/builder.rs

@@ -564,12 +564,14 @@ impl<'fbb> FlatBufferBuilder<'fbb> {
 
         {
             let n = self.head + self.used_space() - object_revloc_to_vtable.value() as usize;
-            let saw = read_scalar_at::<UOffsetT>(&self.owned_buf, n);
+            let saw = unsafe { read_scalar_at::<UOffsetT>(&self.owned_buf, n) };
             debug_assert_eq!(saw, 0xF0F0_F0F0);
-            emplace_scalar::<SOffsetT>(
-                &mut self.owned_buf[n..n + SIZE_SOFFSET],
-                vt_use as SOffsetT - object_revloc_to_vtable.value() as SOffsetT,
-            );
+            unsafe {
+                emplace_scalar::<SOffsetT>(
+                    &mut self.owned_buf[n..n + SIZE_SOFFSET],
+                    vt_use as SOffsetT - object_revloc_to_vtable.value() as SOffsetT,
+                );
+            }
         }
 
         self.field_locs.clear();

rust/flatbuffers/src/endian_scalar.rs

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#![allow(clippy::wrong_self_convention)]
 
 use std::mem::size_of;
 
@@ -148,34 +149,36 @@ pub fn byte_swap_f64(x: f64) -> f64 {
 
 /// Place an EndianScalar into the provided mutable byte slice. Performs
 /// endian conversion, if necessary.
+/// # Safety
+/// Caller must ensure `s.len() > size_of::<T>()`
+/// and `x` does not overlap with `s`.
 #[inline]
-pub fn emplace_scalar<T: EndianScalar>(s: &mut [u8], x: T) {
+pub unsafe fn emplace_scalar<T: EndianScalar>(s: &mut [u8], x: T) {
     let x_le = x.to_little_endian();
-    unsafe {
-        core::ptr::copy_nonoverlapping(
-            &x_le as *const T as *const u8,
-            s.as_mut_ptr() as *mut u8,
-            size_of::<T>(),
-        );
-    }
+    core::ptr::copy_nonoverlapping(
+        &x_le as *const T as *const u8,
+        s.as_mut_ptr() as *mut u8,
+        size_of::<T>(),
+    );
 }
 
 /// Read an EndianScalar from the provided byte slice at the specified location.
 /// Performs endian conversion, if necessary.
+/// # Safety
+/// Caller must ensure `s.len() > loc + size_of::<T>()`.
 #[inline]
-pub fn read_scalar_at<T: EndianScalar>(s: &[u8], loc: usize) -> T {
+pub unsafe fn read_scalar_at<T: EndianScalar>(s: &[u8], loc: usize) -> T {
     read_scalar(&s[loc..])
 }
 
 /// Read an EndianScalar from the provided byte slice. Performs endian
 /// conversion, if necessary.
+/// # Safety
+/// Caller must ensure `s.len() > size_of::<T>()`.
 #[inline]
-pub fn read_scalar<T: EndianScalar>(s: &[u8]) -> T {
+pub unsafe fn read_scalar<T: EndianScalar>(s: &[u8]) -> T {
     let mut mem = core::mem::MaybeUninit::<T>::uninit();
     // Since [u8] has alignment 1, we copy it into T which may have higher alignment.
-    let x = unsafe {
-        core::ptr::copy_nonoverlapping(s.as_ptr(), mem.as_mut_ptr() as *mut u8, size_of::<T>());
-        mem.assume_init()
-    };
-    x.from_little_endian()
+    core::ptr::copy_nonoverlapping(s.as_ptr(), mem.as_mut_ptr() as *mut u8, size_of::<T>());
+    mem.assume_init().from_little_endian()
 }

rust/flatbuffers/src/primitives.rs

@@ -137,7 +137,9 @@ impl<T> Push for WIPOffset<T> {
     #[inline(always)]
     fn push(&self, dst: &mut [u8], rest: &[u8]) {
         let n = (SIZE_UOFFSET + rest.len() - self.value() as usize) as UOffsetT;
-        emplace_scalar::<UOffsetT>(dst, n);
+        unsafe {
+            emplace_scalar::<UOffsetT>(dst, n);
+        }
     }
 }
 
@@ -179,7 +181,7 @@ impl<'a, T: Follow<'a>> Follow<'a> for ForwardsUOffset<T> {
     #[inline(always)]
     fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
         let slice = &buf[loc..loc + SIZE_UOFFSET];
-        let off = read_scalar::<u32>(slice) as usize;
+        let off = unsafe { read_scalar::<u32>(slice) as usize };
         T::follow(buf, loc + off)
     }
 }
@@ -200,7 +202,7 @@ impl<'a, T: Follow<'a>> Follow<'a> for ForwardsVOffset<T> {
     #[inline(always)]
     fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
         let slice = &buf[loc..loc + SIZE_VOFFSET];
-        let off = read_scalar::<VOffsetT>(slice) as usize;
+        let off = unsafe { read_scalar::<VOffsetT>(slice) as usize };
         T::follow(buf, loc + off)
     }
 }
@@ -230,7 +232,7 @@ impl<'a, T: Follow<'a>> Follow<'a> for BackwardsSOffset<T> {
     #[inline(always)]
     fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
         let slice = &buf[loc..loc + SIZE_SOFFSET];
-        let off = read_scalar::<SOffsetT>(slice);
+        let off = unsafe { read_scalar::<SOffsetT>(slice) };
         T::follow(buf, (loc as SOffsetT - off) as usize)
     }
 }
@@ -293,7 +295,7 @@ impl<'a> Follow<'a> for bool {
     type Inner = bool;
     #[inline(always)]
     fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
-        read_scalar_at::<u8>(buf, loc) != 0
+        unsafe { read_scalar_at::<u8>(buf, loc) != 0 }
     }
 }
 
@@ -308,7 +310,7 @@ macro_rules! impl_follow_for_endian_scalar {
             type Inner = $ty;
             #[inline(always)]
             fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
-                read_scalar_at::<$ty>(buf, loc)
+                unsafe { read_scalar_at::<$ty>(buf, loc) }
             }
         }
     };

rust/flatbuffers/src/push.rs

@@ -61,7 +61,9 @@ macro_rules! impl_push_for_endian_scalar {
 
             #[inline]
             fn push(&self, dst: &mut [u8], _rest: &[u8]) {
-                emplace_scalar::<$ty>(dst, *self);
+                unsafe {
+                    emplace_scalar::<$ty>(dst, *self);
+                }
             }
         }
     };

rust/flatbuffers/src/vector.rs

@@ -73,7 +73,7 @@ impl<'a, T: 'a> Vector<'a, T> {
 
     #[inline(always)]
     pub fn len(&self) -> usize {
-        read_scalar_at::<UOffsetT>(&self.0, self.1) as usize
+        unsafe { read_scalar_at::<UOffsetT>(&self.0, self.1) as usize }
     }
     #[inline(always)]
     pub fn is_empty(&self) -> bool {
@@ -84,7 +84,7 @@ impl<'a, T: 'a> Vector<'a, T> {
 impl<'a, T: Follow<'a> + 'a> Vector<'a, T> {
     #[inline(always)]
     pub fn get(&self, idx: usize) -> T::Inner {
-        debug_assert!(idx < read_scalar_at::<u32>(&self.0, self.1) as usize);
+        debug_assert!(idx < self.len() as usize);
         let sz = size_of::<T>();
         debug_assert!(sz > 0);
         T::follow(self.0, self.1 as usize + SIZE_UOFFSET + sz * idx)
@@ -103,7 +103,7 @@ impl<'a, T: SafeSliceAccess + 'a> Vector<'a, T> {
         let loc = self.1;
         let sz = size_of::<T>();
         debug_assert!(sz > 0);
-        let len = read_scalar_at::<UOffsetT>(&buf, loc) as usize;
+        let len = unsafe { read_scalar_at::<UOffsetT>(&buf, loc) } as usize;
         let data_buf = &buf[loc + SIZE_UOFFSET..loc + SIZE_UOFFSET + len * sz];
         let ptr = data_buf.as_ptr() as *const T;
         let s: &'a [T] = unsafe { from_raw_parts(ptr, len) };
@@ -144,7 +144,7 @@ pub fn follow_cast_ref<'a, T: Sized + 'a>(buf: &'a [u8], loc: usize) -> &'a T {
 impl<'a> Follow<'a> for &'a str {
     type Inner = &'a str;
     fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
-        let len = read_scalar_at::<UOffsetT>(&buf, loc) as usize;
+        let len = unsafe { read_scalar_at::<UOffsetT>(&buf, loc) } as usize;
         let slice = &buf[loc + SIZE_UOFFSET..loc + SIZE_UOFFSET + len];
         unsafe { from_utf8_unchecked(slice) }
     }
@@ -154,7 +154,7 @@ impl<'a> Follow<'a> for &'a str {
 fn follow_slice_helper<T>(buf: &[u8], loc: usize) -> &[T] {
     let sz = size_of::<T>();
     debug_assert!(sz > 0);
-    let len = read_scalar_at::<UOffsetT>(&buf, loc) as usize;
+    let len = unsafe { read_scalar_at::<UOffsetT>(&buf, loc) as usize };
     let data_buf = &buf[loc + SIZE_UOFFSET..loc + SIZE_UOFFSET + len * sz];
     let ptr = data_buf.as_ptr() as *const T;
     let s: &[T] = unsafe { from_raw_parts(ptr, len) };

rust/flatbuffers/src/vtable.rs

@@ -40,28 +40,30 @@ impl<'a> VTable<'a> {
         (self.num_bytes() / SIZE_VOFFSET) - 2
     }
     pub fn num_bytes(&self) -> usize {
-        read_scalar_at::<VOffsetT>(self.buf, self.loc) as usize
+        unsafe { read_scalar_at::<VOffsetT>(self.buf, self.loc) as usize }
     }
     pub fn object_inline_num_bytes(&self) -> usize {
-        let n = read_scalar_at::<VOffsetT>(self.buf, self.loc + SIZE_VOFFSET);
+        let n = unsafe { read_scalar_at::<VOffsetT>(self.buf, self.loc + SIZE_VOFFSET) };
         n as usize
     }
     pub fn get_field(&self, idx: usize) -> VOffsetT {
         // TODO(rw): distinguish between None and 0?
         if idx > self.num_fields() {
             return 0;
         }
-        read_scalar_at::<VOffsetT>(
-            self.buf,
-            self.loc + SIZE_VOFFSET + SIZE_VOFFSET + SIZE_VOFFSET * idx,
-        )
+        unsafe {
+            read_scalar_at::<VOffsetT>(
+                self.buf,
+                self.loc + SIZE_VOFFSET + SIZE_VOFFSET + SIZE_VOFFSET * idx,
+            )
+        }
     }
     pub fn get(&self, byte_loc: VOffsetT) -> VOffsetT {
         // TODO(rw): distinguish between None and 0?
         if byte_loc as usize >= self.num_bytes() {
             return 0;
         }
-        read_scalar_at::<VOffsetT>(self.buf, self.loc + byte_loc as usize)
+        unsafe { read_scalar_at::<VOffsetT>(self.buf, self.loc + byte_loc as usize) }
     }
     pub fn as_bytes(&self) -> &[u8] {
         let len = self.num_bytes();

rust/flatbuffers/src/vtable_writer.rs

@@ -40,14 +40,18 @@ impl<'a> VTableWriter<'a> {
     /// to the provided value.
     #[inline(always)]
     pub fn write_vtable_byte_length(&mut self, n: VOffsetT) {
-        emplace_scalar::<VOffsetT>(&mut self.buf[..SIZE_VOFFSET], n);
+        unsafe {
+            emplace_scalar::<VOffsetT>(&mut self.buf[..SIZE_VOFFSET], n);
+        }
         debug_assert_eq!(n as usize, self.buf.len());
     }
 
     /// Writes an object length (in bytes) into the vtable.
     #[inline(always)]
     pub fn write_object_inline_size(&mut self, n: VOffsetT) {
-        emplace_scalar::<VOffsetT>(&mut self.buf[SIZE_VOFFSET..2 * SIZE_VOFFSET], n);
+        unsafe {
+            emplace_scalar::<VOffsetT>(&mut self.buf[SIZE_VOFFSET..2 * SIZE_VOFFSET], n);
+        }
     }
 
     /// Gets an object field offset from the vtable. Only used for debugging.
@@ -57,7 +61,7 @@ impl<'a> VTableWriter<'a> {
     #[inline(always)]
     pub fn get_field_offset(&self, vtable_offset: VOffsetT) -> VOffsetT {
         let idx = vtable_offset as usize;
-        read_scalar_at::<VOffsetT>(&self.buf, idx)
+        unsafe { read_scalar_at::<VOffsetT>(&self.buf, idx) }
     }
 
     /// Writes an object field offset into the vtable.
@@ -67,7 +71,9 @@ impl<'a> VTableWriter<'a> {
     #[inline(always)]
     pub fn write_field_offset(&mut self, vtable_offset: VOffsetT, object_data_offset: VOffsetT) {
         let idx = vtable_offset as usize;
-        emplace_scalar::<VOffsetT>(&mut self.buf[idx..idx + SIZE_VOFFSET], object_data_offset);
+        unsafe {
+            emplace_scalar::<VOffsetT>(&mut self.buf[idx..idx + SIZE_VOFFSET], object_data_offset);
+        }
     }
 
     /// Clears all data in this VTableWriter. Used to cleanly undo a

samples/monster_generated.rs

@@ -76,7 +76,9 @@ impl<'a> flatbuffers::Follow<'a> for Color {
   type Inner = Self;
   #[inline]
   fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
-    let b = flatbuffers::read_scalar_at::<i8>(buf, loc);
+    let b = unsafe {
+      flatbuffers::read_scalar_at::<i8>(buf, loc)
+    };
     Self(b)
   }
 }
@@ -85,7 +87,7 @@ impl flatbuffers::Push for Color {
     type Output = Color;
     #[inline]
     fn push(&self, dst: &mut [u8], _rest: &[u8]) {
-        flatbuffers::emplace_scalar::<i8>(dst, self.0);
+        unsafe { flatbuffers::emplace_scalar::<i8>(dst, self.0); }
     }
 }
 
@@ -161,7 +163,9 @@ impl<'a> flatbuffers::Follow<'a> for Equipment {
   type Inner = Self;
   #[inline]
   fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
-    let b = flatbuffers::read_scalar_at::<u8>(buf, loc);
+    let b = unsafe {
+      flatbuffers::read_scalar_at::<u8>(buf, loc)
+    };
     Self(b)
   }
 }
@@ -170,7 +174,7 @@ impl flatbuffers::Push for Equipment {
     type Output = Equipment;
     #[inline]
     fn push(&self, dst: &mut [u8], _rest: &[u8]) {
-        flatbuffers::emplace_scalar::<u8>(dst, self.0);
+        unsafe { flatbuffers::emplace_scalar::<u8>(dst, self.0); }
     }
 }
 

samples/sample_flexbuffers.rs

@@ -149,24 +149,6 @@ fn main() {
         .iter()
         .map(|r| r.as_u8())
         .eq(vec![5, 10, 25, 25, 25, 100].into_iter()));
-    // For very speed sensitive applications, you can directly read the slice if all of the
-    // following are true:
-    //
-    // *   The provided data buffer contains a valid flexbuffer.
-    // *   You correctly specify the flexbuffer type and width.
-    // *   The host machine is little endian.
-    // *   The provided data buffer itself is aligned in memory to 8 bytes.
-    //
-    // Vec<u8> has alignment 1 so special care is needed to get your buffer's alignment to 8.
-    #[cfg(target_endian = "little")]
-    {
-        if monster_coins.is_aligned() {
-            assert_eq!(
-                monster_coins.get_slice::<i8>().unwrap(),
-                &[5, 10, 25, 25, 25, 100]
-            );
-        }
-    }
 
     // Build the answer to life the universe and everything. Reusing a builder resets it. The
     // reused internals won't need to reallocate leading to a potential 2x speedup.