From 3d32569e2fdc36a1b46e0173fda6f157a59ce095 Mon Sep 17 00:00:00 2001 From: Janis Date: Sat, 21 Jun 2025 00:28:18 +0200 Subject: [PATCH] almost runs thru miri..? --- src/lib.rs | 1 + src/praetor/mod.rs | 95 ++++++++++++++++++++++++-------------------- src/praetor/tests.rs | 59 ++++++++++----------------- 3 files changed, 76 insertions(+), 79 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7444558..5dcdaf2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ cold_path, fn_align, box_vec_non_null, + box_as_ptr, atomic_try_update, let_chains )] diff --git a/src/praetor/mod.rs b/src/praetor/mod.rs index dd04590..568884a 100644 --- a/src/praetor/mod.rs +++ b/src/praetor/mod.rs @@ -432,8 +432,9 @@ mod job { // for some reason I confused head and tail here and the list is something like this: // tail <-> job1 <-> job2 <-> ... <-> head pub struct JobList { - head: Box, - tail: Box, + // these cannot be boxes because boxes are noalias. + head: NonNull, + tail: NonNull, } impl Debug for JobList { @@ -471,73 +472,70 @@ mod job { impl JobList { pub fn new() -> JobList { - let head = Box::new(Job::empty()); - let tail = Box::new(Job::empty()); + let head = Box::into_raw(Box::new(Job::empty())); + let tail = Box::into_raw(Box::new(Job::empty())); // head and tail point at themselves unsafe { - (&mut *head.err_or_link.get()).link.next = None; - (&mut *head.err_or_link.get()).link.prev = - Some(NonNull::new_unchecked((&raw const *tail).cast_mut())); + (&mut *(&mut *head).err_or_link.get()).link.next = None; + (&mut *(&mut *head).err_or_link.get()).link.prev = + Some(NonNull::new_unchecked(tail)); - (&mut *tail.err_or_link.get()).link.next = - Some(NonNull::new_unchecked((&raw const *head).cast_mut())); - (&mut *tail.err_or_link.get()).link.prev = None; + (&mut *(&mut *tail).err_or_link.get()).link.prev = None; + (&mut *(&mut *tail).err_or_link.get()).link.next = + Some(NonNull::new_unchecked(head)); + + Self { + head: NonNull::new_unchecked(head), + tail: NonNull::new_unchecked(tail), + } } - - Self { head, tail } } - fn head_ptr(&self) -> *const Job { - &raw const *self.head - } - fn tail_ptr(&self) -> *const Job { - &raw const *self.tail - } fn head(&self) -> NonNull { - unsafe { NonNull::new_unchecked(self.head_ptr().cast_mut()) } + self.head } fn tail(&self) -> NonNull { - unsafe { NonNull::new_unchecked(self.tail_ptr().cast_mut()) } + self.tail } /// elem must be valid until it is popped. - pub unsafe fn push_front(&mut self, elem: &Job) { - let head_link = unsafe { self.head.link_mut() }; + pub unsafe fn push_front(&mut self, elem: *const Job) { + let head_link = unsafe { self.head.as_ref().link_mut() }; // SAFETY: head will always have a previous element. let prev = head_link.prev.unwrap(); let prev_link = unsafe { prev.as_ref().link_mut() }; - let elem_ptr = unsafe { NonNull::new_unchecked(&*elem as *const Job as *mut Job) }; + let elem_ptr = unsafe { NonNull::new_unchecked(elem as _) }; head_link.prev = Some(elem_ptr); prev_link.next = Some(elem_ptr); - let elem_link = unsafe { elem.link_mut() }; + let elem_link = unsafe { (*elem).link_mut() }; elem_link.prev = Some(prev); elem_link.next = Some(self.head()); } /// elem must be valid until it is popped. - pub unsafe fn push_back(&mut self, elem: &Job) { - let tail_link = unsafe { self.tail.link_mut() }; + pub unsafe fn push_back(&mut self, elem: *const Job) { + let tail_link = unsafe { self.tail.as_ref().link_mut() }; // SAFETY: tail will always have a previous element. let next = tail_link.next.unwrap(); let next_link = unsafe { next.as_ref().link_mut() }; - let elem_ptr = unsafe { NonNull::new_unchecked(&*elem as *const Job as *mut Job) }; + let elem_ptr = unsafe { NonNull::new_unchecked(elem as _) }; tail_link.next = Some(elem_ptr); next_link.prev = Some(elem_ptr); - let elem_link = unsafe { elem.link_mut() }; + let elem_link = unsafe { (*elem).link_mut() }; elem_link.next = Some(next); elem_link.prev = Some(self.tail()); } #[allow(dead_code)] pub fn pop_front(&mut self) -> Option> { - let head_link = unsafe { self.head.link_mut() }; + let head_link = unsafe { self.head.as_ref().link_mut() }; // SAFETY: head will always have a previous element. let elem = head_link.prev.unwrap(); @@ -554,7 +552,7 @@ mod job { pub fn pop_back(&mut self) -> Option> { // TODO: next and elem might be the same - let tail_link = unsafe { self.tail.link_mut() }; + let tail_link = unsafe { self.tail.as_ref().link_mut() }; // SAFETY: head will always have a previous element. let elem = tail_link.next.unwrap(); @@ -570,6 +568,16 @@ mod job { } } + impl Drop for JobList { + fn drop(&mut self) { + // Need to drop the head and tail, which were allocated on the heap. + // elements of the list are managed externally. + unsafe { + drop((Box::from_non_null(self.head), Box::from_non_null(self.tail))); + }; + } + } + union ValueOrThis { uninit: (), value: ManuallyDrop>, @@ -912,7 +920,7 @@ mod job { } #[allow(dead_code)] - pub fn into_boxed_job(self: Box) -> Box> + pub fn into_boxed_job(self: Box) -> *mut Job<()> where F: FnOnce() -> T + Send, T: Send, @@ -923,10 +931,12 @@ mod job { F: FnOnce() -> T + Send, T: Sized + Send, { - eprintln!("heapjob harness"); let job = job.cast_mut(); + + // turn `this`, which was allocated at (2), into box. + // miri complains this is a use-after-free, but it isn't? silly miri... let this = unsafe { Box::from_raw(this.cast::>().cast_mut()) }; - let f = this.f; + let f = this.into_inner(); _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f())); @@ -938,9 +948,10 @@ mod job { } // (1) allocate box for job - Box::new(Job::new(harness::, unsafe { + Box::into_raw(Box::new(Job::new(harness::, unsafe { + // (2) convert self into a pointer NonNull::new_unchecked(Box::into_raw(self)).cast() - })) + }))) } } @@ -1027,7 +1038,7 @@ use std::{ future::Future, hint::cold_path, marker::PhantomData, - mem::{self, MaybeUninit}, + mem::MaybeUninit, ptr::{self, NonNull}, sync::{ atomic::{AtomicBool, AtomicPtr, AtomicUsize, Ordering}, @@ -1179,13 +1190,13 @@ impl WorkerThread { unsafe { (*WORKER.with(UnsafeCell::get)).map(|ptr| ptr.as_ref()) } } - fn push_front(&self, job: &Job) { + fn push_front(&self, job: *const Job) { unsafe { self.queue.as_mut_unchecked().push_front(job); } } #[allow(dead_code)] - fn push_back(&self, job: &Job) { + fn push_back(&self, job: *const Job) { unsafe { self.queue.as_mut_unchecked().push_back(job); } @@ -1534,8 +1545,8 @@ impl<'scope> Scope<'scope> { tracing::trace!("allocated heapjob"); - worker.push_front(&job); - Box::leak(job); + worker.push_front(job); + tracing::trace!("leaked heapjob"); }); } @@ -1564,13 +1575,13 @@ impl<'scope> Scope<'scope> { Runnable::<()>::from_raw(NonNull::new_unchecked(this.cast_mut())); runnable.run(); + // SAFETY: job was turned into raw drop(Box::from_raw(job.cast_mut())); } let job = Box::new(Job::::new(harness::, runnable.into_raw())); - worker.push_front(job.as_ref()); - mem::forget(job); + worker.push_front(Box::into_raw(job)); }; let (runnable, task) = unsafe { async_task::spawn_unchecked(future, schedule) }; diff --git a/src/praetor/tests.rs b/src/praetor/tests.rs index efe1abf..f136778 100644 --- a/src/praetor/tests.rs +++ b/src/praetor/tests.rs @@ -1,5 +1,5 @@ use std::{ - mem::MaybeUninit, + mem::{self, MaybeUninit}, pin::{pin, Pin}, }; @@ -42,14 +42,14 @@ fn job_list_pop_back() { let c = pin!(Job::empty()); unsafe { - list.push_front(&a); - list.push_front(&b); - list.push_back(&c); + list.push_front(&*a); + list.push_front(&*b); + list.push_back(&*c); } assert_eq!(list.pop_back(), Some(pin_ptr(&c))); unsafe { - list.push_front(&c); + list.push_front(&*c); } assert_eq!(list.pop_back(), Some(pin_ptr(&a))); assert_eq!(list.pop_back(), Some(pin_ptr(&b))); @@ -66,14 +66,14 @@ fn job_list_pop_front() { let c = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); - list.push_front(&b); - list.push_back(&c); + list.push_front(&*a); + list.push_front(&*b); + list.push_back(&*c); } assert_eq!(list.pop_front(), Some(pin_ptr(&b))); unsafe { - list.push_back(&b); + list.push_back(&*b); } assert_eq!(list.pop_front(), Some(pin_ptr(&a))); assert_eq!(list.pop_front(), Some(pin_ptr(&c))); @@ -90,9 +90,9 @@ fn unlink_job_middle() { let c = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); - list.push_front(&b); - list.push_front(&c); + list.push_front(&*a); + list.push_front(&*b); + list.push_front(&*c); } unsafe { @@ -113,9 +113,9 @@ fn unlink_job_front() { let c = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); - list.push_front(&b); - list.push_front(&c); + list.push_front(&*a); + list.push_front(&*b); + list.push_front(&*c); } unsafe { @@ -136,9 +136,9 @@ fn unlink_job_back() { let c = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); - list.push_front(&b); - list.push_front(&c); + list.push_front(&*a); + list.push_front(&*b); + list.push_front(&*c); } unsafe { @@ -157,7 +157,7 @@ fn unlink_job_single() { let a = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); + list.push_front(&*a); } unsafe { @@ -347,7 +347,7 @@ fn job_list_pop_back_emptied() { let a = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); + list.push_front(&*a); } assert_eq!(list.pop_back(), Some(pin_ptr(&a))); @@ -362,7 +362,7 @@ fn job_list_pop_front_emptied() { let a = pin!(Job::<()>::empty()); unsafe { - list.push_front(&a); + list.push_front(&*a); } assert_eq!(list.pop_front(), Some(pin_ptr(&a))); @@ -372,7 +372,6 @@ fn job_list_pop_front_emptied() { } #[test] -#[tracing_test::traced_test] fn spawn() { let pool = ThreadPool::new(); @@ -390,19 +389,6 @@ fn spawn() { eprintln!("x: {x}"); } -#[test] -fn rayon_spawn() { - let pool = rayon::ThreadPoolBuilder::new().build().unwrap(); - let mut x = 0; - pool.scope(|s| { - s.spawn(|_| { - x += 1; - }); - }); - - eprintln!("x: {x}"); -} - #[test] fn spawn_borrow() { let pool = ThreadPool::new(); @@ -449,13 +435,12 @@ fn join() { } #[test] -#[traced_test] fn join_many() { use crate::util::tree::Tree; let pool = ThreadPool::new(); - let tree = Tree::new(16, 1u32); + let tree = Tree::new(4, 1u32); fn sum(tree: &Tree, node: usize, scope: &Scope) -> u32 { let node = tree.get(node);