On 3/4/25 10:13, Zhao Liu wrote:
- const fn as_mut_ptr(&self) -> *mut Self {
- self as *const Timer as *mut _
+ /// Create a `Timer` struct without initializing it.
+ ///
+ /// # Safety
+ ///
+ /// The timer must be initialized before it is armed with
+ /// [`modify`](Self::modify).
+ pub unsafe fn new() -> Self {
+ // SAFETY: requirements relayed to callers of Timer::new
+ Self(unsafe { Opaque::zeroed() })
We should use Opaque::uninit()? Because
MaybeUninit::<bindings::QEMUTimer>::zeroed()
marks the timer as initialized, which disables MaybeUninit's ability to check
for
initialization. e.g.,
While neither is good, a zeroed area of memory behaves better than an
uninitialized one... In particular, Drop calls timer_del() which works
fine with a zeroed QEMUTimer. With Opaque::uninit() you could have a
crash just with
drop(Timer::new());
// No compiling error or runtime panic
let t: MaybeUninit<bindings::QEMUTimer> = MaybeUninit::zeroed();
let _t = unsafe { t.assume_init() };
Further more, I spent some time trying to figure out if MaybeUninit in
Opaque<> could help identify UB caused by uninitialized Timer, but I found
it doesn't work. :-(
// No compiling error or runtime panic
let mut v: UnsafeCell<MaybeUninit<bindings::QEMUTimer>> =
UnsafeCell::new(MaybeUninit::uninit());
let _v = unsafe { v.get_mut().assume_init() };
But when I adjust MaybeUninit as the outer wrapper, the UB check can
work:
// Runtime panic: Illegal instruction
let v: MaybeUninit<UnsafeCell<bindings::QEMUTimer>> = MaybeUninit::uninit();
let _v = unsafe { v.assume_init() };
Compared with linux's Opaque, it also puts MaybeUninit on the outermost
layer.
Yes, I admit I just copied what Linux does. :)
And there's another example:
https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html#method.raw_get
Emm, I guess now we have UnsafeCell<MaybeUninit<>> because interior
mutability is expected... but this layout breaks MaybeUninit's functionality.
Thanks for the example from the documentation! Indeed it should be
possible to do
/// Returns a raw mutable pointer to the opaque data.
pub const fn as_mut_ptr(&self) -> *mut T {
UnsafeCell::raw_get(self.value.as_ptr())
}
/// Returns a raw pointer to the opaque data that can be passed to a
/// C function as `void *`.
pub const fn as_void_ptr(&self) -> *mut std::ffi::c_void {
self.as_mut_ptr().cast()
}
pub const fn raw_get(slot: *const Self) -> *mut T {
// SAFETY: even if uninitialized, slot points to a MaybeUninit
let slot = slot.cast::<MaybeUninit<UnsafeCell<T>>>;
UnsafeCell::raw_get(slot.as_ptr())
}
if Opaque<> uses a MaybeUninit<UnsafeCell<T>>. I'm a bit worried of
deviating from what Linux does though...
Paolo
