Merge pull request #5908 from roc-lang/miri-fixes

roc_std: fixes found by running miri

Cargo.lock

@@ -1874,9 +1874,9 @@ dependencies = [
 
 [[package]]
 name = "proc-macro2"
-version = "1.0.57"
+version = "1.0.69"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c4ec6d5fe0b140acb27c9a0444118cf55bfbb4e0b259739429abb4521dd67c16"
+checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da"
 dependencies = [
  "unicode-ident",
 ]

Cargo.toml

@@ -128,7 +128,7 @@ perfcnt = "0.8.0"
 pest = "2.5.6"
 pest_derive = "2.5.6"
 pretty_assertions = "1.3.0" # update roc_std/Cargo.toml on change
-proc-macro2 = "1.0.51"
+proc-macro2 = "1.0.63"
 proptest = "1.1.0"
 pulldown-cmark = { version = "0.9.2", default-features = false }
 quickcheck = "1.0.3" # update roc_std/Cargo.toml on change

crates/roc_std/src/roc_list.rs

@@ -829,4 +829,30 @@ mod tests {
         drop(a);
         drop(b);
     }
+
+    #[test]
+    fn readonly_list_is_sendsafe() {
+        let x = RocList::from_slice(&[1, 2, 3, 4, 5]);
+        unsafe { x.set_readonly() };
+        assert!(x.is_readonly());
+
+        let y = x.clone();
+        let z = y.clone();
+
+        let safe_x = SendSafeRocList::from(x);
+        let new_x = RocList::from(safe_x);
+        assert!(new_x.is_readonly());
+        assert!(y.is_readonly());
+        assert!(z.is_readonly());
+        assert_eq!(new_x.as_slice(), &[1, 2, 3, 4, 5]);
+
+        let ptr = new_x.ptr_to_allocation();
+
+        drop(y);
+        drop(z);
+        drop(new_x);
+
+        // free the underlying memory
+        unsafe { crate::roc_dealloc(ptr, std::mem::align_of::<usize>() as u32) }
+    }
 }

crates/roc_std/src/roc_str.rs

@@ -325,7 +325,9 @@ impl RocStr {
                 }
             }
             RocStrInnerRef::SmallString(small_str) => {
-                let mut bytes = small_str.bytes;
+                let mut bytes = [0; size_of::<RocList<u8>>()];
+                let mut it = small_str.bytes.iter();
+                bytes = bytes.map(|_| it.next().copied().unwrap_or_default());
 
                 // Even if the small string is at capacity, there will be room to write
                 // a terminator in the byte that's used to store the length.
@@ -380,9 +382,7 @@ impl RocStr {
         self.with_terminator(terminator, |dest_ptr: *mut u16, str_slice: &str| {
             // Translate UTF-8 source bytes into UTF-16 and write them into the destination.
             for (index, wchar) in str_slice.encode_utf16().enumerate() {
-                unsafe {
-                    *(dest_ptr.add(index)) = wchar;
-                }
+                unsafe { std::ptr::write_unaligned(dest_ptr.add(index), wchar) };
             }
 
             func(dest_ptr, str_slice.len())
@@ -467,7 +467,7 @@ impl RocStr {
         use core::mem::align_of;
 
         let terminate = |alloc_ptr: *mut E, str_slice: &str| unsafe {
-            *(alloc_ptr.add(str_slice.len())) = terminator;
+            std::ptr::write_unaligned(alloc_ptr.add(str_slice.len()), terminator);
 
             func(alloc_ptr, str_slice)
         };
@@ -548,7 +548,8 @@ impl RocStr {
                 let available_bytes = size_of::<SmallString>();
 
                 if needed_bytes < available_bytes {
-                    terminate(small_str.bytes.as_ptr() as *mut E, self.as_str())
+                    let mut bytes = small_str.bytes;
+                    terminate(&mut bytes as *mut u8 as *mut E, self.as_str())
                 } else {
                     fallback(self.as_str())
                 }

crates/roc_std/tests/test_roc_std.rs

@@ -54,7 +54,7 @@ pub unsafe extern "C" fn roc_memset(dst: *mut c_void, c: i32, n: usize) -> *mut
 
 #[cfg(test)]
 mod test_roc_std {
-    use roc_std::{RocBox, RocDec, RocList, RocResult, RocStr, SendSafeRocList, SendSafeRocStr};
+    use roc_std::{RocBox, RocDec, RocList, RocResult, RocStr, SendSafeRocStr};
 
     fn roc_str_byte_representation(string: &RocStr) -> [u8; RocStr::SIZE] {
         unsafe { core::mem::transmute_copy(string) }
@@ -341,39 +341,22 @@ mod test_roc_std {
         let x = RocStr::from("short");
         let y = x.clone();
         let z = y.clone();
-        assert_eq!(x.is_unique(), true);
-        assert_eq!(y.is_unique(), true);
-        assert_eq!(z.is_unique(), true);
+        assert!(x.is_unique());
+        assert!(y.is_unique());
+        assert!(z.is_unique());
 
         let safe_x = SendSafeRocStr::from(x);
         let new_x = RocStr::from(safe_x);
-        assert_eq!(new_x.is_unique(), true);
-        assert_eq!(y.is_unique(), true);
-        assert_eq!(z.is_unique(), true);
+        assert!(new_x.is_unique());
+        assert!(y.is_unique(),);
+        assert!(z.is_unique(),);
         assert_eq!(new_x.as_str(), "short");
     }
 
     #[test]
     fn empty_list_is_unique() {
         let roc_list = RocList::<RocStr>::empty();
-        assert_eq!(roc_list.is_unique(), true);
-    }
-
-    #[test]
-    fn readonly_list_is_sendsafe() {
-        let x = RocList::from_slice(&[1, 2, 3, 4, 5]);
-        unsafe { x.set_readonly() };
-        assert_eq!(x.is_readonly(), true);
-
-        let y = x.clone();
-        let z = y.clone();
-
-        let safe_x = SendSafeRocList::from(x);
-        let new_x = RocList::from(safe_x);
-        assert_eq!(new_x.is_readonly(), true);
-        assert_eq!(y.is_readonly(), true);
-        assert_eq!(z.is_readonly(), true);
-        assert_eq!(new_x.as_slice(), &[1, 2, 3, 4, 5]);
+        assert!(roc_list.is_unique());
     }
 }
 
@@ -413,12 +396,18 @@ mod with_terminator {
         // utf16_nul_terminated
         {
             let answer = roc_str.utf16_nul_terminated(|ptr, len| {
-                let bytes: &[u16] = unsafe { slice::from_raw_parts(ptr.cast(), len + 1) };
+                let bytes: &[u8] = unsafe { slice::from_raw_parts(ptr as *mut u8, 2 * (len + 1)) };
+
+                let items: Vec<u16> = bytes
+                    .chunks(2)
+                    .map(|c| c.try_into().unwrap())
+                    .map(u16::from_ne_bytes)
+                    .collect();
 
                 // Verify that it's nul-terminated
-                assert_eq!(bytes[len], 0);
+                assert_eq!(items[len], 0);
 
-                let string = String::from_utf16(&bytes[0..len]).unwrap();
+                let string = String::from_utf16(&items[0..len]).unwrap();
 
                 assert_eq!(string.as_str(), string);