Skip to content
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

Merged
merged 3 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ impl<'a> InvokeContext<'a> {
// but performed on a very small slice and requires no heap allocations.
let instruction_context = self.transaction_context.get_current_instruction_context()?;
let mut deduplicated_instruction_accounts: Vec<InstructionAccount> = Vec::new();
let mut duplicate_indicies = Vec::with_capacity(instruction.accounts.len());
let mut duplicate_indicies = Vec::with_capacity(instruction.accounts.len() as usize);
for (instruction_account_index, account_meta) in instruction.accounts.iter().enumerate() {
let index_in_transaction = self
.transaction_context
Expand Down
8 changes: 4 additions & 4 deletions programs/bpf_loader/src/syscalls/cpi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,14 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust {
)?;
let account_metas = translate_slice::<AccountMeta>(
memory_mapping,
ix.accounts.as_ptr() as u64,
ix.accounts.len() as u64,
ix.accounts.as_vaddr(),
ix.accounts.len(),
invoke_context.get_check_aligned(),
)?;
let data = translate_slice::<u8>(
memory_mapping,
ix.data.as_ptr() as u64,
ix.data.len() as u64,
ix.data.as_vaddr(),
ix.data.len(),
invoke_context.get_check_aligned(),
)?
.to_vec();
Expand Down
54 changes: 32 additions & 22 deletions sdk/stable-layout/src/stable_vec.rs

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 to as_vaddr. So I don't have any code related issues with this.

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};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use std::ops::{Deref, DerefMut};
use std::{marker::PhantomData, mem::ManuallyDrop};
use std::{
marker::PhantomData,
mem::ManuallyDrop,
ops::{Deref, DerefMut},
};


/// `Vec`, with a stable memory layout
///
Expand All @@ -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
}

Expand All @@ -56,13 +52,13 @@ impl<T> StableVec<T> {

impl<T> AsRef<[T]> for StableVec<T> {
fn as_ref(&self) -> &[T] {
self
self.deref()
}
}

impl<T> AsMut<[T]> for StableVec<T> {
fn as_mut(&mut self) -> &mut [T] {
self
self.deref_mut()
}
}

Expand All @@ -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) }
}
}

Expand Down Expand Up @@ -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,
}
}
Expand All @@ -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,
)
}
}
}

Expand All @@ -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,
)
};
}
}

Expand Down