From d4e77856fe51ceea90258e40f6026d08e547d57f Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 15 Oct 2023 20:09:05 +0200 Subject: [PATCH 1/3] roc_std: fixes found by running miri --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- crates/roc_std/src/roc_list.rs | 26 +++++++++++++++++++++++ crates/roc_std/src/roc_str.rs | 13 ++++++------ crates/roc_std/tests/test_roc_std.rs | 31 +++++++++------------------- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ae8f504a06..7a7d8a53628 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/Cargo.toml b/Cargo.toml index 3c9b444a606..0139f8b2283 100644 --- a/Cargo.toml +++ b/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 diff --git a/crates/roc_std/src/roc_list.rs b/crates/roc_std/src/roc_list.rs index d3582e56112..d1207be25e4 100644 --- a/crates/roc_std/src/roc_list.rs +++ b/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_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]); + + 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::() as u32) } + } } diff --git a/crates/roc_std/src/roc_str.rs b/crates/roc_std/src/roc_str.rs index 57cfc549a88..b48be650aa1 100644 --- a/crates/roc_std/src/roc_str.rs +++ b/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::>()]; + 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::(); 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()) } diff --git a/crates/roc_std/tests/test_roc_std.rs b/crates/roc_std/tests/test_roc_std.rs index 347970153ac..a323d03e641 100644 --- a/crates/roc_std/tests/test_roc_std.rs +++ b/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) } @@ -358,23 +358,6 @@ mod test_roc_std { let roc_list = RocList::::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]); - } } #[cfg(test)] @@ -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 = bytes + .chunks(2) + .map(|c| c.try_into().unwrap()) + .map(|c| u16::from_ne_bytes(c)) + .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); From 72d5b64c6ffc3eeb81bf75f494c9a19beb4a974c Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 15 Oct 2023 20:26:02 +0200 Subject: [PATCH 2/3] clippy --- crates/roc_std/tests/test_roc_std.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/roc_std/tests/test_roc_std.rs b/crates/roc_std/tests/test_roc_std.rs index a323d03e641..15cbd31d5a8 100644 --- a/crates/roc_std/tests/test_roc_std.rs +++ b/crates/roc_std/tests/test_roc_std.rs @@ -341,22 +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::::empty(); - assert_eq!(roc_list.is_unique(), true); + assert!(roc_list.is_unique()); } } @@ -401,7 +401,7 @@ mod with_terminator { let items: Vec = bytes .chunks(2) .map(|c| c.try_into().unwrap()) - .map(|c| u16::from_ne_bytes(c)) + .map(u16::from_ne_bytes) .collect(); // Verify that it's nul-terminated From f717cb75a9796b6a9b2bea690b7e113c7d82f4a4 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 15 Oct 2023 21:06:23 +0200 Subject: [PATCH 3/3] clippy --- crates/roc_std/src/roc_list.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/roc_std/src/roc_list.rs b/crates/roc_std/src/roc_list.rs index d1207be25e4..5d454253dec 100644 --- a/crates/roc_std/src/roc_list.rs +++ b/crates/roc_std/src/roc_list.rs @@ -834,16 +834,16 @@ mod tests { 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); + 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_eq!(new_x.is_readonly(), true); - assert_eq!(y.is_readonly(), true); - assert_eq!(z.is_readonly(), true); + 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();