-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Converted StableVec to use u64 virtual addresses rather than physical #13
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,7 @@ | ||||||||||||||||
//! `Vec`, with a stable memory layout | ||||||||||||||||
|
||||||||||||||||
use std::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull}; | ||||||||||||||||
use std::ops::{Deref, DerefMut}; | ||||||||||||||||
use std::{marker::PhantomData, mem::ManuallyDrop}; | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
|
||||||||||||||||
/// `Vec`, with a stable memory layout | ||||||||||||||||
/// | ||||||||||||||||
|
@@ -24,27 +25,22 @@ use std::{marker::PhantomData, mem::ManuallyDrop, ptr::NonNull}; | |||||||||||||||
/// ``` | ||||||||||||||||
#[repr(C)] | ||||||||||||||||
pub struct StableVec<T> { | ||||||||||||||||
pub ptr: NonNull<T>, | ||||||||||||||||
pub cap: usize, | ||||||||||||||||
pub len: usize, | ||||||||||||||||
pub addr: u64, | ||||||||||||||||
pub cap: u64, | ||||||||||||||||
pub len: u64, | ||||||||||||||||
_marker: PhantomData<T>, | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// We shadow these slice methods of the same name to avoid going through | ||||||||||||||||
// `deref`, which creates an intermediate reference. | ||||||||||||||||
impl<T> StableVec<T> { | ||||||||||||||||
#[inline] | ||||||||||||||||
pub fn as_ptr(&self) -> *const T { | ||||||||||||||||
self.ptr.as_ptr() | ||||||||||||||||
pub fn as_vaddr(&self) -> u64 { | ||||||||||||||||
self.addr | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
#[inline] | ||||||||||||||||
pub fn as_mut_ptr(&mut self) -> *mut T { | ||||||||||||||||
self.ptr.as_ptr() | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
#[inline] | ||||||||||||||||
pub fn len(&self) -> usize { | ||||||||||||||||
pub fn len(&self) -> u64 { | ||||||||||||||||
self.len | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
@@ -56,13 +52,13 @@ impl<T> StableVec<T> { | |||||||||||||||
|
||||||||||||||||
impl<T> AsRef<[T]> for StableVec<T> { | ||||||||||||||||
fn as_ref(&self) -> &[T] { | ||||||||||||||||
self | ||||||||||||||||
} | ||||||||||||||||
self.deref() | ||||||||||||||||
} // === is this right? | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: leftover comment
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, left in a note by accident. Still, the question stands :). Deref and as_ref are .. fairly close in meaning I guess, and I think identical for our purposes here (?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed your comment before submitting my review, but did you hit errors just returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good question. Maybe that's unnecessary. Don't recall if I hit an error or just did it because it looked right to me. I'll have to check. |
||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
impl<T> AsMut<[T]> for StableVec<T> { | ||||||||||||||||
fn as_mut(&mut self) -> &mut [T] { | ||||||||||||||||
self | ||||||||||||||||
self.deref_mut() | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
@@ -71,14 +67,14 @@ impl<T> std::ops::Deref for StableVec<T> { | |||||||||||||||
|
||||||||||||||||
#[inline] | ||||||||||||||||
fn deref(&self) -> &[T] { | ||||||||||||||||
unsafe { core::slice::from_raw_parts(self.as_ptr(), self.len) } | ||||||||||||||||
unsafe { core::slice::from_raw_parts(self.addr as usize as *mut T, self.len as usize) } | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
impl<T> std::ops::DerefMut for StableVec<T> { | ||||||||||||||||
#[inline] | ||||||||||||||||
fn deref_mut(&mut self) -> &mut [T] { | ||||||||||||||||
unsafe { core::slice::from_raw_parts_mut(self.as_mut_ptr(), self.len) } | ||||||||||||||||
unsafe { core::slice::from_raw_parts_mut(self.addr as usize as *mut T, self.len as usize) } | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
@@ -121,9 +117,9 @@ impl<T> From<Vec<T>> for StableVec<T> { | |||||||||||||||
let mut other = ManuallyDrop::new(other); | ||||||||||||||||
Self { | ||||||||||||||||
// SAFETY: We have a valid Vec, so its ptr is non-null. | ||||||||||||||||
ptr: unsafe { NonNull::new_unchecked(other.as_mut_ptr()) }, | ||||||||||||||||
cap: other.capacity(), | ||||||||||||||||
len: other.len(), | ||||||||||||||||
addr: other.as_mut_ptr() as u64, // Problematic if other is in 32-bit physical address space | ||||||||||||||||
cap: other.capacity() as u64, | ||||||||||||||||
len: other.len() as u64, | ||||||||||||||||
_marker: PhantomData, | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
@@ -135,8 +131,16 @@ impl<T> From<StableVec<T>> for Vec<T> { | |||||||||||||||
// out of scope. | ||||||||||||||||
let other = ManuallyDrop::new(other); | ||||||||||||||||
// SAFETY: We have a valid StableVec, which we can only get from a Vec. Therefore it is | ||||||||||||||||
// safe to convert back to Vec. | ||||||||||||||||
unsafe { Vec::from_raw_parts(other.ptr.as_ptr(), other.len, other.cap) } | ||||||||||||||||
// safe to convert back to Vec. Assuming we're not starting with a vector in 64-bit virtual | ||||||||||||||||
// address space while building the app in 32-bit, and this vector is in that 32-bit physical | ||||||||||||||||
// space. | ||||||||||||||||
unsafe { | ||||||||||||||||
Vec::from_raw_parts( | ||||||||||||||||
other.addr as usize as *mut T, | ||||||||||||||||
other.len as usize, | ||||||||||||||||
other.cap as usize, | ||||||||||||||||
) | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
@@ -147,7 +151,13 @@ impl<T> Drop for StableVec<T> { | |||||||||||||||
// | ||||||||||||||||
// SAFETY: We have a valid StableVec, which we can only get from a Vec. Therefore it is | ||||||||||||||||
// safe to convert back to Vec. | ||||||||||||||||
let _vec = unsafe { Vec::from_raw_parts(self.ptr.as_ptr(), self.len, self.cap) }; | ||||||||||||||||
let _vec = unsafe { | ||||||||||||||||
Vec::from_raw_parts( | ||||||||||||||||
self.addr as usize as *mut T, | ||||||||||||||||
self.len as usize, | ||||||||||||||||
self.cap as usize, | ||||||||||||||||
) | ||||||||||||||||
}; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than these
as
casts which look dangerous and are only testable with e-to-e tests inside both 64 and 32 bit environments, everything looks sound.The logic is remaining the same, as well as the external API surface (apart from the move from
as_ptr
toas_vaddr
. So I don't have any code related issues with this.