From e5dfdccfe511eecdd9aad3bdc9a177b4ac388bc5 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Tue, 11 Oct 2022 19:27:23 +0200 Subject: [PATCH 1/5] Fix UB by not taking a shared reference --- src/ref_cnt.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ref_cnt.rs b/src/ref_cnt.rs index 5128f5c..d5decc1 100644 --- a/src/ref_cnt.rs +++ b/src/ref_cnt.rs @@ -92,7 +92,7 @@ unsafe impl RefCnt for Arc { Arc::into_raw(me) as *mut T } fn as_ptr(me: &Arc) -> *mut T { - me as &T as *const T as *mut T + Arc::as_ptr(me) as *mut T } unsafe fn from_ptr(ptr: *const T) -> Arc { Arc::from_raw(ptr) From 6c6093d1fb98e4ef5deeef6bc36951c2e903dd09 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Tue, 11 Oct 2022 19:36:46 +0200 Subject: [PATCH 2/5] Re-enable stacked borrows in CI Also improve performance of some tests in Miri --- .github/workflows/test.yaml | 3 +-- src/lib.rs | 13 ++++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e884e1b..b53e80d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -231,8 +231,7 @@ jobs: - name: Run miri env: PROPTEST_CASES: "10" - # TODO: Do something with the stacked borrows. Figure out what it means. - MIRIFLAGS: "-Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" + MIRIFLAGS: "-Zmiri-disable-isolation" run: cargo miri test --all-features thread_sanitizer-MacOS: diff --git a/src/lib.rs b/src/lib.rs index 7e2e6c2..c9526a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -883,7 +883,7 @@ macro_rules! t { const WRITER_CNT: usize = 2; const READER_CNT: usize = 3; #[cfg(miri)] - const ITERATIONS: usize = 10; + const ITERATIONS: usize = 5; #[cfg(not(miri))] const ITERATIONS: usize = 100; const SEQ: usize = 50; @@ -1160,6 +1160,9 @@ macro_rules! t { #[test] /// Make sure the reference count and compare_and_swap works as expected. fn cas_ref_cnt() { + #[cfg(miri)] + const ITERATIONS: usize = 10; + #[cfg(not(miri))] const ITERATIONS: usize = 50; let shared = ArcSwap::from(Arc::new(0)); for i in 0..ITERATIONS { @@ -1173,7 +1176,7 @@ macro_rules! t { // Fill up the slots sometimes let fillup = || { if i % 2 == 0 { - Some((0..50).map(|_| shared.load()).collect::>()) + Some((0..ITERATIONS).map(|_| shared.load()).collect::>()) } else { None } @@ -1272,10 +1275,14 @@ mod tests { /// created, but contain full Arcs. #[test] fn lease_overflow() { + #[cfg(miri)] + const GUARD_COUNT: usize = 100; + #[cfg(not(miri))] + const GUARD_COUNT: usize = 1000; let a = Arc::new(0); let shared = ArcSwap::from(Arc::clone(&a)); assert_eq!(2, Arc::strong_count(&a)); - let mut guards = (0..1000).map(|_| shared.load()).collect::>(); + let mut guards = (0..GUARD_COUNT).map(|_| shared.load()).collect::>(); let count = Arc::strong_count(&a); assert!(count > 2); let guard = shared.load(); From 802caca18a6058b24bb3f6f56039f0166cf63ea0 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Tue, 11 Oct 2022 20:46:22 +0200 Subject: [PATCH 3/5] Use permissive provenance --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index b53e80d..e313d2b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -231,7 +231,7 @@ jobs: - name: Run miri env: PROPTEST_CASES: "10" - MIRIFLAGS: "-Zmiri-disable-isolation" + MIRIFLAGS: "-Zmiri-disable-isolation -Zmiri-permissive-provenance" run: cargo miri test --all-features thread_sanitizer-MacOS: From c4998365d2e7253ba6ea9318b1064035ded1f0f2 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Wed, 12 Oct 2022 16:41:52 +0200 Subject: [PATCH 4/5] Use `into_raw` to retrieve the pointer --- src/ref_cnt.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/ref_cnt.rs b/src/ref_cnt.rs index d5decc1..a2fe583 100644 --- a/src/ref_cnt.rs +++ b/src/ref_cnt.rs @@ -92,7 +92,12 @@ unsafe impl RefCnt for Arc { Arc::into_raw(me) as *mut T } fn as_ptr(me: &Arc) -> *mut T { - Arc::as_ptr(me) as *mut T + // SAFETY: The refcount will be incremented then decremented + // so we force the compiler to remove the overflow check + if Arc::strong_count(me) == usize::max_value() { unsafe { std::hint::unreachable_unchecked(); } } + let ptr = Arc::into_raw(Arc::clone(me)); + let _ = unsafe { Arc::from_raw(ptr) }; + ptr as *mut T } unsafe fn from_ptr(ptr: *const T) -> Arc { Arc::from_raw(ptr) @@ -105,7 +110,12 @@ unsafe impl RefCnt for Rc { Rc::into_raw(me) as *mut T } fn as_ptr(me: &Rc) -> *mut T { - me as &T as *const T as *mut T + // SAFETY: The refcount will be incremented then decremented + // so we force the compiler to remove the overflow check + if Rc::strong_count(me) == usize::max_value() { unsafe { std::hint::unreachable_unchecked(); } } + let ptr = Rc::into_raw(Rc::clone(me)); + let _ = unsafe { Rc::from_raw(ptr) }; + ptr as *mut T } unsafe fn from_ptr(ptr: *const T) -> Rc { Rc::from_raw(ptr) From ce5d4df9dde28d2c41af3e0cafdf5c4d550196fa Mon Sep 17 00:00:00 2001 From: clubby789 Date: Wed, 19 Oct 2022 21:11:33 +0100 Subject: [PATCH 5/5] Remove UB in old approach --- src/ref_cnt.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/ref_cnt.rs b/src/ref_cnt.rs index a2fe583..2ea4ec6 100644 --- a/src/ref_cnt.rs +++ b/src/ref_cnt.rs @@ -92,11 +92,8 @@ unsafe impl RefCnt for Arc { Arc::into_raw(me) as *mut T } fn as_ptr(me: &Arc) -> *mut T { - // SAFETY: The refcount will be incremented then decremented - // so we force the compiler to remove the overflow check - if Arc::strong_count(me) == usize::max_value() { unsafe { std::hint::unreachable_unchecked(); } } - let ptr = Arc::into_raw(Arc::clone(me)); - let _ = unsafe { Arc::from_raw(ptr) }; + // SAFETY: &T cast to *const T will always be aligned, initialised and valid for reads + let ptr = Arc::into_raw(unsafe { std::ptr::read(me) }); ptr as *mut T } unsafe fn from_ptr(ptr: *const T) -> Arc { @@ -110,11 +107,8 @@ unsafe impl RefCnt for Rc { Rc::into_raw(me) as *mut T } fn as_ptr(me: &Rc) -> *mut T { - // SAFETY: The refcount will be incremented then decremented - // so we force the compiler to remove the overflow check - if Rc::strong_count(me) == usize::max_value() { unsafe { std::hint::unreachable_unchecked(); } } - let ptr = Rc::into_raw(Rc::clone(me)); - let _ = unsafe { Rc::from_raw(ptr) }; + // SAFETY: &T cast to *const T will always be aligned, initialised and valid for reads + let ptr = Rc::into_raw(unsafe { std::ptr::read(me) }); ptr as *mut T } unsafe fn from_ptr(ptr: *const T) -> Rc {