Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support/borsh: Replace the unsafety with generic_const_exprs when #425

Open
github-actions bot opened this issue Dec 30, 2024 · 3 comments
Open

support/borsh: Replace the unsafety with generic_const_exprs when #425

github-actions bot opened this issue Dec 30, 2024 · 3 comments
Assignees
Labels
to do To be done tracker Issue tracked by bot

Comments

@github-actions
Copy link

On 2024-12-30 @prestwich wrote in 6a513a0 “Merge pull request #424 from Evalir/evalir/copy-to-buf”:

Replace the unsafety with generic_const_exprs when
available

    #[inline]
    fn serialize<W: io::Write>(&self, writer: &mut W) -> io::Result<()> {
        #[cfg(target_endian = "little")]
        return writer.write_all(self.as_le_slice());

        // TODO: Replace the unsafety with `generic_const_exprs` when
        // available
        #[cfg(target_endian = "big")]
        {
            let mut limbs = [0u64; LIMBS];
            // SAFETY: `limbs` is known to have identical memory layout and
            // alignment to `[u8; LIMBS * 8]`, which is guaranteed to safely

From src/support/borsh.rs:47

@github-actions github-actions bot added to do To be done tracker Issue tracked by bot labels Dec 30, 2024
@kpp
Copy link
Contributor

kpp commented Feb 28, 2025

This unsafety can be fixed with bytemuck, no?

@prestwich
Copy link
Collaborator

that's pushing safety management into a dep, which is not exactly the same thing as fixing it

@kpp
Copy link
Contributor

kpp commented Mar 1, 2025

Yes that's true. Same discussion happened here rust-random/rand#1574 with the zerocopy crate in rand. Unfortunately, this unsafety can be also managed with the https://docs.rs/zerocopy/latest/zerocopy/macro.transmute_mut.html macro for the same generic_const_exprs reason.

It can be achieved like this though:

diff --git a/src/support/borsh.rs b/src/support/borsh.rs
index 00dec73..b52bf2b 100644
--- a/src/support/borsh.rs
+++ b/src/support/borsh.rs
@@ -5,26 +5,17 @@
 
 use crate::{Bits, Uint};
 use borsh::{io, BorshDeserialize, BorshSerialize};
+use zerocopy::IntoBytes;
 
 impl<const BITS: usize, const LIMBS: usize> BorshDeserialize for Uint<BITS, LIMBS> {
     #[inline]
     fn deserialize_reader<R: io::Read>(reader: &mut R) -> io::Result<Self> {
-        // This is a bit of an end-run around missing `generic_const_exprs`
-        // We cannot declare a `[u8; Self::BYTES]` or `[u8; LIMBS * 8]`,
-        // so we declare a `[u8; LIMBS]` and use unsafe to write to it.
+        assert!(LIMBS * 8 >= Self::BYTES);
 
-        // TODO: Replace the unsafety with `generic_const_exprs` when
-        // available
         let mut limbs = [0u64; LIMBS];
+        let bytes = limbs.as_mut_bytes();
+        let (target, _rest) = bytes.split_at_mut(Self::BYTES);
 
-        // SAFETY: `limbs` is known to have identical memory layout and
-        // alignment to `[u8; LIMBS * 8]`, which is guaranteed to safely
-        // contain  [u8; Self::BYTES]`, as `LIMBS * 8 >= Self::BYTES`.
-        // Reference:
-        // https://doc.rust-lang.org/reference/type-layout.html#array-layout
-        let target = unsafe {
-            core::slice::from_raw_parts_mut(limbs.as_mut_ptr().cast::<u8>(), Self::BYTES)
-        };
         reader.read_exact(target)?;
 
         // Using `Self::from_limbs(limbs)` would be incorrect here, as the
@@ -44,21 +35,16 @@ impl<const BITS: usize, const LIMBS: usize> BorshSerialize for Uint<BITS, LIMBS>
         #[cfg(target_endian = "little")]
         return writer.write_all(self.as_le_slice());
 
-        // TODO: Replace the unsafety with `generic_const_exprs` when
-        // available
         #[cfg(target_endian = "big")]
         {
+            assert!(LIMBS * 8 >= Self::BYTES);
+
             let mut limbs = [0u64; LIMBS];
-            // SAFETY: `limbs` is known to have identical memory layout and
-            // alignment to `[u8; LIMBS * 8]`, which is guaranteed to safely
-            // contain  [u8; Self::BYTES]`, as `LIMBS * 8 >= Self::BYTES`.
-            // Reference:
-            // https://doc.rust-lang.org/reference/type-layout.html#array-layout
-            let mut buf = unsafe {
-                core::slice::from_raw_parts_mut(limbs.as_mut_ptr().cast::<u8>(), Self::BYTES)
-            };
-            self.copy_le_bytes_to(&mut buf);
-            writer.write_all(&buf)
+            let bytes = limbs.as_mut_bytes();
+            let (buf, _rest) = bytes.split_at_mut(Self::BYTES);
+
+            self.copy_le_bytes_to(buf);
+            writer.write_all(buf)
         }
     }
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to do To be done tracker Issue tracked by bot
Projects
None yet
Development

No branches or pull requests

2 participants