From 427a079d318e68686cc7512838a23ed71315d8de Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 10:40:15 +0900 Subject: [PATCH 1/4] kmc-solid: Use `expose_addr` and `from_exposed_addr` for pointer-integer casts Pointer-integer casts are required for conversion between `EXINF` (ITRON task entry point parameter) and `*const ThreadInner`. Addresses the deny-level lint `fuzzy_provenance_casts`. --- library/std/src/sys/itron/thread.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/itron/thread.rs b/library/std/src/sys/itron/thread.rs index d28f57f33be20..573a50b42de3c 100644 --- a/library/std/src/sys/itron/thread.rs +++ b/library/std/src/sys/itron/thread.rs @@ -91,7 +91,7 @@ impl Thread { unsafe extern "C" fn trampoline(exinf: isize) { // Safety: `ThreadInner` is alive at this point - let inner = unsafe { &*(exinf as *const ThreadInner) }; + let inner: &ThreadInner = unsafe { &*crate::ptr::from_exposed_addr(exinf as usize) }; // Safety: Since `trampoline` is called only once for each // `ThreadInner` and only `trampoline` touches `start`, @@ -168,7 +168,7 @@ impl Thread { abi::acre_tsk(&abi::T_CTSK { // Activate this task immediately tskatr: abi::TA_ACT, - exinf: inner_ptr as abi::EXINF, + exinf: inner_ptr.expose_addr() as abi::EXINF, // The entry point task: Some(trampoline), // Inherit the calling task's base priority From 47f2f6d615a70b29a4ef76ebf480703655a0ea05 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 10:57:33 +0900 Subject: [PATCH 2/4] kmc-solid: Add a stub implementation of `is_terminal` Copied from `unsupported/io.rs`. Fixes build failure. --- library/std/src/sys/solid/io.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/sys/solid/io.rs b/library/std/src/sys/solid/io.rs index 9eb17a10daa28..a862bb7870264 100644 --- a/library/std/src/sys/solid/io.rs +++ b/library/std/src/sys/solid/io.rs @@ -75,3 +75,7 @@ impl<'a> IoSliceMut<'a> { unsafe { slice::from_raw_parts_mut(self.vec.iov_base as *mut u8, self.vec.iov_len) } } } + +pub fn is_terminal(_: &T) -> bool { + false +} From f482e55adf3fb76860d304c9b7f5c7f3f9a40717 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 11:03:43 +0900 Subject: [PATCH 3/4] kmc-solid: Address compiler warnings Addresses the warn-by-default lints `unused_imports` and `unused_unsafe`. --- library/std/src/sys/itron/condvar.rs | 4 ++-- library/std/src/sys/itron/mutex.rs | 2 +- library/std/src/sys/solid/os.rs | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/itron/condvar.rs b/library/std/src/sys/itron/condvar.rs index f70aa434e4834..7a47cc6696a34 100644 --- a/library/std/src/sys/itron/condvar.rs +++ b/library/std/src/sys/itron/condvar.rs @@ -71,7 +71,7 @@ impl Condvar { } } - unsafe { mutex.lock() }; + mutex.lock(); } pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool { @@ -109,7 +109,7 @@ impl Condvar { // we woke up because of `notify_*`. let success = self.waiters.with_locked(|waiters| unsafe { !waiters.remove(waiter) }); - unsafe { mutex.lock() }; + mutex.lock(); success } } diff --git a/library/std/src/sys/itron/mutex.rs b/library/std/src/sys/itron/mutex.rs index f2eed8e771c40..1f6cc41947602 100644 --- a/library/std/src/sys/itron/mutex.rs +++ b/library/std/src/sys/itron/mutex.rs @@ -72,7 +72,7 @@ pub(super) struct MutexGuard<'a>(&'a Mutex); impl<'a> MutexGuard<'a> { #[inline] pub(super) fn lock(x: &'a Mutex) -> Self { - unsafe { x.lock() }; + x.lock(); Self(x) } } diff --git a/library/std/src/sys/solid/os.rs b/library/std/src/sys/solid/os.rs index 4906c62689d4c..6135921f0b5a8 100644 --- a/library/std/src/sys/solid/os.rs +++ b/library/std/src/sys/solid/os.rs @@ -1,7 +1,6 @@ use super::unsupported; -use crate::convert::TryFrom; use crate::error::Error as StdError; -use crate::ffi::{CStr, CString, OsStr, OsString}; +use crate::ffi::{CStr, OsStr, OsString}; use crate::fmt; use crate::io; use crate::os::{ From ae7633f434011fe1829b9b235a20d91634479eb5 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 11:56:31 +0900 Subject: [PATCH 4/4] kmc-solid: Don't do `Box::from_raw(&*(x: Box) as *const T as *mut T)` This pattern seems to be considered illegal by Miri. --- library/std/src/sys/itron/thread.rs | 48 ++++++++++++++++++----------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/library/std/src/sys/itron/thread.rs b/library/std/src/sys/itron/thread.rs index 573a50b42de3c..c2b3668087225 100644 --- a/library/std/src/sys/itron/thread.rs +++ b/library/std/src/sys/itron/thread.rs @@ -11,18 +11,25 @@ use crate::{ ffi::CStr, hint, io, mem::ManuallyDrop, + ptr::NonNull, sync::atomic::{AtomicUsize, Ordering}, sys::thread_local_dtor::run_dtors, time::Duration, }; pub struct Thread { - inner: ManuallyDrop>, + p_inner: NonNull, /// The ID of the underlying task. task: abi::ID, } +// Safety: There's nothing in `Thread` that ties it to the original creator. It +// can be dropped by any threads. +unsafe impl Send for Thread {} +// Safety: `Thread` provides no methods that take `&self`. +unsafe impl Sync for Thread {} + /// State data shared between a parent thread and child thread. It's dropped on /// a transition to one of the final states. struct ThreadInner { @@ -90,8 +97,9 @@ impl Thread { }); unsafe extern "C" fn trampoline(exinf: isize) { + let p_inner: *mut ThreadInner = crate::ptr::from_exposed_addr_mut(exinf as usize); // Safety: `ThreadInner` is alive at this point - let inner: &ThreadInner = unsafe { &*crate::ptr::from_exposed_addr(exinf as usize) }; + let inner = unsafe { &*p_inner }; // Safety: Since `trampoline` is called only once for each // `ThreadInner` and only `trampoline` touches `start`, @@ -119,13 +127,13 @@ impl Thread { // No one will ever join, so we'll ask the collector task to // delete the task. - // In this case, `inner`'s ownership has been moved to us, - // And we are responsible for dropping it. The acquire + // In this case, `*p_inner`'s ownership has been moved to + // us, and we are responsible for dropping it. The acquire // ordering is not necessary because the parent thread made // no memory access needing synchronization since the call // to `acre_tsk`. // Safety: See above. - let _ = unsafe { Box::from_raw(inner as *const _ as *mut ThreadInner) }; + let _ = unsafe { Box::from_raw(p_inner) }; // Safety: There are no pinned references to the stack unsafe { terminate_and_delete_current_task() }; @@ -162,13 +170,14 @@ impl Thread { } } - let inner_ptr = (&*inner) as *const ThreadInner; + // Safety: `Box::into_raw` returns a non-null pointer + let p_inner = unsafe { NonNull::new_unchecked(Box::into_raw(inner)) }; let new_task = ItronError::err_if_negative(unsafe { abi::acre_tsk(&abi::T_CTSK { // Activate this task immediately tskatr: abi::TA_ACT, - exinf: inner_ptr.expose_addr() as abi::EXINF, + exinf: p_inner.as_ptr().expose_addr() as abi::EXINF, // The entry point task: Some(trampoline), // Inherit the calling task's base priority @@ -180,7 +189,7 @@ impl Thread { }) .map_err(|e| e.as_io_error())?; - Ok(Self { inner: ManuallyDrop::new(inner), task: new_task }) + Ok(Self { p_inner, task: new_task }) } pub fn yield_now() { @@ -197,8 +206,9 @@ impl Thread { } } - pub fn join(mut self) { - let inner = &*self.inner; + pub fn join(self) { + // Safety: `ThreadInner` is alive at this point + let inner = unsafe { self.p_inner.as_ref() }; // Get the current task ID. Panicking here would cause a resource leak, // so just abort on failure. let current_task = task::current_task_id_aborting(); @@ -243,8 +253,8 @@ impl Thread { unsafe { terminate_and_delete_task(self.task) }; // In either case, we are responsible for dropping `inner`. - // Safety: The contents of `self.inner` will not be accessed hereafter - let _inner = unsafe { ManuallyDrop::take(&mut self.inner) }; + // Safety: The contents of `*p_inner` will not be accessed hereafter + let _inner = unsafe { Box::from_raw(self.p_inner.as_ptr()) }; // Skip the destructor (because it would attempt to detach the thread) crate::mem::forget(self); @@ -253,13 +263,16 @@ impl Thread { impl Drop for Thread { fn drop(&mut self) { + // Safety: `ThreadInner` is alive at this point + let inner = unsafe { self.p_inner.as_ref() }; + // Detach the thread. - match self.inner.lifecycle.swap(LIFECYCLE_DETACHED_OR_JOINED, Ordering::Acquire) { + match inner.lifecycle.swap(LIFECYCLE_DETACHED_OR_JOINED, Ordering::Acquire) { LIFECYCLE_INIT => { // [INIT → DETACHED] // When the time comes, the child will figure out that no // one will ever join it. - // The ownership of `self.inner` is moved to the child thread. + // The ownership of `*p_inner` is moved to the child thread. // However, the release ordering is not necessary because we // made no memory access needing synchronization since the call // to `acre_tsk`. @@ -278,10 +291,9 @@ impl Drop for Thread { // delete by entering the `FINISHED` state. unsafe { terminate_and_delete_task(self.task) }; - // Wwe are responsible for dropping `inner`. - // Safety: The contents of `self.inner` will not be accessed - // hereafter - unsafe { ManuallyDrop::drop(&mut self.inner) }; + // Wwe are responsible for dropping `*p_inner`. + // Safety: The contents of `*p_inner` will not be accessed hereafter + let _ = unsafe { Box::from_raw(self.p_inner.as_ptr()) }; } _ => unsafe { hint::unreachable_unchecked() }, }