From 004961c1df6647b8fb30181f709e4cd94bdbd742 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 6 Jul 2022 14:18:00 +0200 Subject: [PATCH 1/3] miri: don't use int2ptr casts for invalid pointers --- src/bytes_mut.rs | 35 ++++++++++++++++++++++++++--------- tests/test_bytes_vec_alloc.rs | 19 ++++++++++++++++++- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index cc1a3ba0d..09f63df10 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -253,7 +253,7 @@ impl BytesMut { let ptr = self.ptr.as_ptr(); let len = self.len; - let data = AtomicPtr::new(self.data as _); + let data = AtomicPtr::new(self.data.cast()); mem::forget(self); unsafe { Bytes::with_vtable(ptr, len, data, &SHARED_VTABLE) } } @@ -613,7 +613,7 @@ impl BytesMut { } debug_assert_eq!(kind, KIND_ARC); - let shared: *mut Shared = self.data as _; + let shared: *mut Shared = self.data; // Reserving involves abandoning the currently shared buffer and // allocating a new vector with the requested capacity. @@ -692,7 +692,7 @@ impl BytesMut { // Update self let data = (original_capacity_repr << ORIGINAL_CAPACITY_OFFSET) | KIND_VEC; - self.data = data as _; + self.data = invalid_ptr(data); self.ptr = vptr(v.as_mut_ptr()); self.len = v.len(); self.cap = v.capacity(); @@ -723,7 +723,7 @@ impl BytesMut { // Reserved above debug_assert!(dst.len() >= cnt); - ptr::copy_nonoverlapping(extend.as_ptr(), dst.as_mut_ptr() as *mut u8, cnt); + ptr::copy_nonoverlapping(extend.as_ptr(), dst.as_mut_ptr(), cnt); } unsafe { @@ -788,7 +788,7 @@ impl BytesMut { ptr, len, cap, - data: data as *mut _, + data: invalid_ptr(data), } } @@ -909,7 +909,7 @@ impl BytesMut { // always succeed. debug_assert_eq!(shared as usize & KIND_MASK, KIND_ARC); - self.data = shared as _; + self.data = shared; } /// Makes an exact shallow clone of `self`. @@ -942,7 +942,7 @@ impl BytesMut { debug_assert_eq!(self.kind(), KIND_VEC); debug_assert!(pos <= MAX_VEC_POS); - self.data = ((pos << VEC_POS_OFFSET) | (prev & NOT_VEC_POS_MASK)) as *mut _; + self.data = invalid_ptr((pos << VEC_POS_OFFSET) | (prev & NOT_VEC_POS_MASK)); } #[inline] @@ -968,7 +968,7 @@ impl Drop for BytesMut { let _ = rebuild_vec(self.ptr.as_ptr(), self.len, self.cap, off); } } else if kind == KIND_ARC { - unsafe { release_shared(self.data as _) }; + unsafe { release_shared(self.data) }; } } } @@ -1549,6 +1549,23 @@ fn vptr(ptr: *mut u8) -> NonNull { } } +/// Returns a dangling pointer with the given address. This is used to store +/// integer data in pointer fields. +#[inline] +fn invalid_ptr(addr: usize) -> *mut T { + #[cfg(miri)] + { + let ptr = std::ptr::null_mut::().wrapping_add(addr); + assert_eq!(ptr as usize, addr); + ptr.cast::() + } + + #[cfg(not(miri))] + { + addr as *mut T + } +} + unsafe fn rebuild_vec(ptr: *mut u8, mut len: usize, mut cap: usize, off: usize) -> Vec { let ptr = ptr.offset(-(off as isize)); len += off; @@ -1568,7 +1585,7 @@ unsafe fn shared_v_clone(data: &AtomicPtr<()>, ptr: *const u8, len: usize) -> By let shared = data.load(Ordering::Relaxed) as *mut Shared; increment_shared(shared); - let data = AtomicPtr::new(shared as _); + let data = AtomicPtr::new(shared as *mut ()); Bytes::with_vtable(ptr, len, data, &SHARED_VTABLE) } diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index 9d9823284..f24d4a55c 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -45,7 +45,7 @@ impl Ledger { if entry_ptr .compare_exchange( ptr, - usize::MAX as *mut u8, + invalid_ptr(usize::MAX), Ordering::SeqCst, Ordering::SeqCst, ) @@ -103,3 +103,20 @@ fn test_bytes_truncate_and_advance() { bytes.advance(1); drop(bytes); } + +/// Returns a dangling pointer with the given address. This is used to store +/// integer data in pointer fields. +#[inline] +fn invalid_ptr(addr: usize) -> *mut T { + #[cfg(miri)] + { + let ptr = std::ptr::null_mut::().wrapping_add(addr); + assert_eq!(ptr as usize, addr); + ptr.cast::() + } + + #[cfg(not(miri))] + { + addr as *mut T + } +} From 6ced2a983b9de7c748dc99704e54ebb83e3b0cfb Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Sat, 9 Jul 2022 13:50:55 +0200 Subject: [PATCH 2/3] Always use null+addr for `invalid_ptr` --- src/bytes_mut.rs | 17 ++++++----------- tests/test_bytes_vec_alloc.rs | 14 +++----------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 09f63df10..31d3c2817 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -1551,19 +1551,14 @@ fn vptr(ptr: *mut u8) -> NonNull { /// Returns a dangling pointer with the given address. This is used to store /// integer data in pointer fields. +/// +/// It is equivalent to `addr as *mut T`, but this fails on miri when strict +/// provenance checking is enabled. #[inline] fn invalid_ptr(addr: usize) -> *mut T { - #[cfg(miri)] - { - let ptr = std::ptr::null_mut::().wrapping_add(addr); - assert_eq!(ptr as usize, addr); - ptr.cast::() - } - - #[cfg(not(miri))] - { - addr as *mut T - } + let ptr = std::ptr::null_mut::().wrapping_add(addr); + debug_assert_eq!(ptr as usize, addr); + ptr.cast::() } unsafe fn rebuild_vec(ptr: *mut u8, mut len: usize, mut cap: usize, off: usize) -> Vec { diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index f24d4a55c..752009f38 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -108,15 +108,7 @@ fn test_bytes_truncate_and_advance() { /// integer data in pointer fields. #[inline] fn invalid_ptr(addr: usize) -> *mut T { - #[cfg(miri)] - { - let ptr = std::ptr::null_mut::().wrapping_add(addr); - assert_eq!(ptr as usize, addr); - ptr.cast::() - } - - #[cfg(not(miri))] - { - addr as *mut T - } + let ptr = std::ptr::null_mut::().wrapping_add(addr); + debug_assert_eq!(ptr as usize, addr); + ptr.cast::() } From c48c72e253a2ce858bf8c657579f96212d352334 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Sat, 9 Jul 2022 14:11:17 +0200 Subject: [PATCH 3/3] Use `core::ptr` --- src/bytes_mut.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bytes_mut.rs b/src/bytes_mut.rs index 31d3c2817..4f9a8851c 100644 --- a/src/bytes_mut.rs +++ b/src/bytes_mut.rs @@ -1556,7 +1556,7 @@ fn vptr(ptr: *mut u8) -> NonNull { /// provenance checking is enabled. #[inline] fn invalid_ptr(addr: usize) -> *mut T { - let ptr = std::ptr::null_mut::().wrapping_add(addr); + let ptr = core::ptr::null_mut::().wrapping_add(addr); debug_assert_eq!(ptr as usize, addr); ptr.cast::() }