From 93e3c00670a8362ce384ed7911e547f87982af77 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Sat, 6 Jul 2024 22:24:00 -0500 Subject: [PATCH 1/7] Remove non-focused memory leaks in `alloc` doctests for Miri. --- library/alloc/src/rc.rs | 6 ++++++ library/alloc/src/sync.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 3745ecb48c18e..9f2bff9844906 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1287,6 +1287,8 @@ impl Rc { /// /// let five = Rc::from_raw(ptr); /// assert_eq!(2, Rc::strong_count(&five)); + /// # // Prevent leaks for Miri. + /// # Rc::decrement_strong_count(ptr); /// } /// ``` #[inline] @@ -1344,6 +1346,8 @@ impl Rc { /// let x = Rc::new("hello".to_owned()); /// let x_ptr = Rc::into_raw(x); /// assert_eq!(unsafe { &*x_ptr }, "hello"); + /// # // Prevent leaks for Miri. + /// # drop(unsafe { Rc::from_raw(x_ptr) }); /// ``` #[must_use = "losing the pointer will leak memory"] #[stable(feature = "rc_raw", since = "1.17.0")] @@ -1571,6 +1575,8 @@ impl Rc { /// /// let five = Rc::from_raw_in(ptr, System); /// assert_eq!(2, Rc::strong_count(&five)); + /// # // Prevent leaks for Miri. + /// # Rc::decrement_strong_count_in(ptr, System); /// } /// ``` #[inline] diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index 90672164cb932..6a591d24065c6 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -1428,6 +1428,8 @@ impl Arc { /// // the `Arc` between threads. /// let five = Arc::from_raw(ptr); /// assert_eq!(2, Arc::strong_count(&five)); + /// # // Prevent leaks for Miri. + /// # Arc::decrement_strong_count(ptr); /// } /// ``` #[inline] @@ -1487,6 +1489,8 @@ impl Arc { /// let x = Arc::new("hello".to_owned()); /// let x_ptr = Arc::into_raw(x); /// assert_eq!(unsafe { &*x_ptr }, "hello"); + /// # // Prevent leaks for Miri. + /// # drop(unsafe { Arc::from_raw(x_ptr) }); /// ``` #[must_use = "losing the pointer will leak memory"] #[stable(feature = "rc_raw", since = "1.17.0")] @@ -1769,6 +1773,8 @@ impl Arc { /// // the `Arc` between threads. /// let five = Arc::from_raw_in(ptr, System); /// assert_eq!(2, Arc::strong_count(&five)); + /// # // Prevent leaks for Miri. + /// # Arc::decrement_strong_count_in(ptr, System); /// } /// ``` #[inline] From e0ed696d2f1ce49b28be12879570dd402a354e2e Mon Sep 17 00:00:00 2001 From: Zachary S Date: Sat, 6 Jul 2024 22:30:37 -0500 Subject: [PATCH 2/7] Mitigate focused memory leaks in `alloc` doctests for Miri. If/when `-Zmiri-disable-leak-check` is able to be used at test-granularity, it should applied to these tests instead of unleaking. --- library/alloc/src/boxed.rs | 6 ++++++ library/alloc/src/string.rs | 3 +++ library/alloc/src/vec/into_iter.rs | 7 ++++++- library/alloc/src/vec/mod.rs | 6 ++++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 1ec095a46f704..ba157dc1a379b 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1213,6 +1213,9 @@ impl Box { /// let static_ref: &'static mut usize = Box::leak(x); /// *static_ref += 1; /// assert_eq!(*static_ref, 42); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # drop(unsafe { Box::from_raw(static_ref) }); /// ``` /// /// Unsized data: @@ -1222,6 +1225,9 @@ impl Box { /// let static_ref = Box::leak(x); /// static_ref[0] = 4; /// assert_eq!(*static_ref, [4, 2, 3]); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # drop(unsafe { Box::from_raw(static_ref) }); /// ``` #[stable(feature = "box_leak", since = "1.26.0")] #[inline] diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 36078da7c35a6..07ffd3e151914 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -1984,6 +1984,9 @@ impl String { /// let x = String::from("bucket"); /// let static_ref: &'static mut str = x.leak(); /// assert_eq!(static_ref, "bucket"); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # drop(unsafe { Box::from_raw(static_ref) }); /// ``` #[stable(feature = "string_leak", since = "1.72.0")] #[inline] diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs index 3bd89eaa6cb5a..10f62e4bb62d8 100644 --- a/library/alloc/src/vec/into_iter.rs +++ b/library/alloc/src/vec/into_iter.rs @@ -120,10 +120,15 @@ impl IntoIter { /// This is roughly equivalent to the following, but more efficient /// /// ``` - /// # let mut into_iter = Vec::::with_capacity(10).into_iter(); + /// # let mut vec = Vec::::with_capacity(10); + /// # let ptr = vec.as_mut_ptr(); + /// # let mut into_iter = vec.into_iter(); /// let mut into_iter = std::mem::replace(&mut into_iter, Vec::new().into_iter()); /// (&mut into_iter).for_each(drop); /// std::mem::forget(into_iter); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # drop(unsafe { Vec::::from_raw_parts(ptr, 0, 10) }); /// ``` /// /// This method is used by in-place iteration, refer to the vec::in_place_collect diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index f1706e31bb80a..0b91151c38613 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1473,6 +1473,9 @@ impl Vec { /// // 2. `0 <= capacity` always holds whatever `capacity` is. /// unsafe { /// vec.set_len(0); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # vec.set_len(3); /// } /// ``` /// @@ -2391,6 +2394,9 @@ impl Vec { /// let static_ref: &'static mut [usize] = x.leak(); /// static_ref[0] += 1; /// assert_eq!(static_ref, &[2, 2, 3]); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # drop(unsafe { Box::from_raw(static_ref) }); /// ``` #[stable(feature = "vec_leak", since = "1.47.0")] #[inline] From e4c064d81316a7041c50ac2cfd836247ce02ed37 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Sat, 6 Jul 2024 22:53:31 -0500 Subject: [PATCH 3/7] Remove non-focused memory leaks in `core` doctests for Miri. --- library/core/src/mem/maybe_uninit.rs | 12 ++++++++++++ library/core/src/ptr/non_null.rs | 2 ++ 2 files changed, 14 insertions(+) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 24ebe33bb2c1f..a8d03ef2611a4 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -274,6 +274,8 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let v: MaybeUninit> = MaybeUninit::new(vec![42]); + /// # // Prevent leaks for Miri + /// # unsafe { let _ = MaybeUninit::assume_init(v); } /// ``` /// /// [`assume_init`]: MaybeUninit::assume_init @@ -506,6 +508,8 @@ impl MaybeUninit { /// // Create a reference into the `MaybeUninit`. This is okay because we initialized it. /// let x_vec = unsafe { &*x.as_ptr() }; /// assert_eq!(x_vec.len(), 3); + /// # // Prevent leaks for Miri + /// # unsafe { MaybeUninit::assume_init_drop(&mut x); } /// ``` /// /// *Incorrect* usage of this method: @@ -545,6 +549,8 @@ impl MaybeUninit { /// let x_vec = unsafe { &mut *x.as_mut_ptr() }; /// x_vec.push(3); /// assert_eq!(x_vec.len(), 4); + /// # // Prevent leaks for Miri + /// # unsafe { MaybeUninit::assume_init_drop(&mut x); } /// ``` /// /// *Incorrect* usage of this method: @@ -746,6 +752,8 @@ impl MaybeUninit { /// use std::mem::MaybeUninit; /// /// let mut x = MaybeUninit::>::uninit(); + /// # let mut x_mu = x; + /// # let mut x = &mut x_mu; /// // Initialize `x`: /// x.write(vec![1, 2, 3]); /// // Now that our `MaybeUninit<_>` is known to be initialized, it is okay to @@ -755,6 +763,8 @@ impl MaybeUninit { /// x.assume_init_ref() /// }; /// assert_eq!(x, &vec![1, 2, 3]); + /// # // Prevent leaks for Miri + /// # unsafe { MaybeUninit::assume_init_drop(&mut x_mu); } /// ``` /// /// ### *Incorrect* usages of this method: @@ -1088,6 +1098,8 @@ impl MaybeUninit { /// let init = MaybeUninit::clone_from_slice(&mut dst, &src); /// /// assert_eq!(init, src); + /// # // Prevent leaks for Miri + /// # unsafe { std::ptr::drop_in_place(init); } /// ``` /// /// ``` diff --git a/library/core/src/ptr/non_null.rs b/library/core/src/ptr/non_null.rs index 75a99e14fdadc..1238061d68d5c 100644 --- a/library/core/src/ptr/non_null.rs +++ b/library/core/src/ptr/non_null.rs @@ -1663,6 +1663,8 @@ impl NonNull<[T]> { /// // Note that calling `memory.as_mut()` is not allowed here as the content may be uninitialized. /// # #[allow(unused_variables)] /// let slice: &mut [MaybeUninit] = unsafe { memory.as_uninit_slice_mut() }; + /// # // Prevent leaks for Miri. + /// # unsafe { Global.deallocate(memory.cast(), Layout::new::<[u8; 32]>()); } /// # Ok::<_, std::alloc::AllocError>(()) /// ``` #[inline] From 36258ed947c4013094b23029fb716f0ff715622c Mon Sep 17 00:00:00 2001 From: Zachary S Date: Sat, 6 Jul 2024 22:53:51 -0500 Subject: [PATCH 4/7] Mitigate focused memory leaks in `core` doctests for Miri. If/when `-Zmiri-disable-leak-check` is able to be used at test-granularity, it should applied to these tests instead of unleaking. --- library/core/src/mem/manually_drop.rs | 3 +++ library/core/src/mem/maybe_uninit.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/library/core/src/mem/manually_drop.rs b/library/core/src/mem/manually_drop.rs index e0c3b9f3b51da..997f088c6d687 100644 --- a/library/core/src/mem/manually_drop.rs +++ b/library/core/src/mem/manually_drop.rs @@ -62,6 +62,9 @@ impl ManuallyDrop { /// x.truncate(5); // You can still safely operate on the value /// assert_eq!(*x, "Hello"); /// // But `Drop` will not be run here + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # let _ = ManuallyDrop::into_inner(x); /// ``` #[must_use = "if you don't need the wrapper, you can use `mem::forget` instead"] #[stable(feature = "manually_drop", since = "1.20.0")] diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index a8d03ef2611a4..dd40f57dc8707 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -448,6 +448,9 @@ impl MaybeUninit { /// let mut x = MaybeUninit::::uninit(); /// /// x.write("Hello".to_string()); + /// # // FIXME(https://github.com/rust-lang/miri/issues/3670): + /// # // use -Zmiri-disable-leak-check instead of unleaking in tests meant to leak. + /// # unsafe { MaybeUninit::assume_init_drop(&mut x); } /// // This leaks the contained string: /// x.write("hello".to_string()); /// // x is initialized now: From a10c7a4b9b88606bf94348733f903d21276d8134 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Sat, 6 Jul 2024 23:35:31 -0500 Subject: [PATCH 5/7] Remove non-focused memory leak in `std` doctest for Miri. --- library/std/src/sync/condvar.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/std/src/sync/condvar.rs b/library/std/src/sync/condvar.rs index f9f83fb4f63c3..08d46f356d9f2 100644 --- a/library/std/src/sync/condvar.rs +++ b/library/std/src/sync/condvar.rs @@ -35,6 +35,7 @@ impl WaitTimeoutResult { /// let pair = Arc::new((Mutex::new(false), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// + /// # let handle = /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// @@ -58,6 +59,8 @@ impl WaitTimeoutResult { /// break /// } /// } + /// # // Prevent leaks for Miri. + /// # let _ = handle.join(); /// ``` #[must_use] #[stable(feature = "wait_timeout", since = "1.5.0")] From d8717cf780e795b12193934b16ebcd47021d4925 Mon Sep 17 00:00:00 2001 From: zachs18 <8355914+zachs18@users.noreply.github.com> Date: Sun, 7 Jul 2024 10:42:11 -0500 Subject: [PATCH 6/7] Re-enable Miri leak checking in CI. --- src/bootstrap/mk/Makefile.in | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/mk/Makefile.in b/src/bootstrap/mk/Makefile.in index cab37e0da4736..6267156e8e25d 100644 --- a/src/bootstrap/mk/Makefile.in +++ b/src/bootstrap/mk/Makefile.in @@ -58,9 +58,8 @@ check-aux: library/core \ library/alloc \ --no-doc - # Some doctests have intentional memory leaks. # Some use file system operations to demonstrate dealing with `Result`. - $(Q)MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-disable-isolation" \ + $(Q)MIRIFLAGS="-Zmiri-disable-isolation" \ $(BOOTSTRAP) miri --stage 2 \ library/core \ library/alloc \ @@ -70,7 +69,7 @@ check-aux: $(BOOTSTRAP) miri --stage 2 library/std \ --no-doc -- \ --skip fs:: --skip net:: --skip process:: --skip sys::pal:: - $(Q)MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-disable-isolation" \ + $(Q)MIRIFLAGS="-Zmiri-disable-isolation" \ $(BOOTSTRAP) miri --stage 2 library/std \ --doc -- \ --skip fs:: --skip net:: --skip process:: --skip sys::pal:: From 51727bae4bb4650d7fa297b983fdc13608ae6e17 Mon Sep 17 00:00:00 2001 From: zachs18 <8355914+zachs18@users.noreply.github.com> Date: Sun, 7 Jul 2024 11:23:13 -0500 Subject: [PATCH 7/7] Update src/bootstrap/mk/Makefile.in Co-authored-by: Ralf Jung --- src/bootstrap/mk/Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/mk/Makefile.in b/src/bootstrap/mk/Makefile.in index 6267156e8e25d..3d977bf305862 100644 --- a/src/bootstrap/mk/Makefile.in +++ b/src/bootstrap/mk/Makefile.in @@ -58,7 +58,7 @@ check-aux: library/core \ library/alloc \ --no-doc - # Some use file system operations to demonstrate dealing with `Result`. + # Some doctests use file system operations to demonstrate dealing with `Result`. $(Q)MIRIFLAGS="-Zmiri-disable-isolation" \ $(BOOTSTRAP) miri --stage 2 \ library/core \