Skip to content

Commit

Permalink
Merge #2053
Browse files Browse the repository at this point in the history
2053: Add utilities for allocating dynamic grants. r=bradjc a=daboross

### Pull Request Overview

This pull request is intended to expand the basic functionality for dynamically allocating values. It:

- changes the existing `alloc` method to accept non-`Default` values
- adds a new method, `alloc_n_with`, for allocating a runtime-determined number of contiguous values
- splits the internal `alloc_unowned` into `alloc_raw` and `alloc_default_unowned` to allow for allocating non-`Default` values

I believe all of these changes will be useful, but the one which motivated this PR was `alloc_n_with`. To support connections in the Rubble BLE capsule, we'll need to support attributes, and the capsule won't know ahead of time how many attributes any given app will want to have in a connection. Attributes are most naturally stored in an array, and since the kernel won't know at compile time how many attributes an app has, we need dynamic allocation. Hence, `alloc_n_with`.

Note that this PR leaves the usage of `Owned`, which I believe is unsound. Once we re-export `Owned` so that applications can store them, those applications could potentially store them outside of app-specific grant memory, and then access them after the app has been restarted (thus accessing deallocated memory). I have a fix for this, but since the changes are fairly separate, I've submitted it as a separate PR - #2052.

If one PR is merged first, the other will need a rebase, but it shouldn't matter which one that is.

CC @alevy 

### Testing Strategy

I created a temporary test capsule and app which calls `alloc` and `alloc_n_with` every other tick until we hit `OutOfMemory`, keeps each allocation one tick, and prints the data out after retrieving it.

### TODO or Help Wanted

While this was manually tested, automated testing would be better. Do we have a place for unit tests for kernel functionality exposed to capsules?

### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make prepush`.


Co-authored-by: David Ross <David.Ross@wdc.com>
  • Loading branch information
bors[bot] and David Ross committed Sep 17, 2020
2 parents 14dda51 + 1dd5754 commit 3bfb711
Showing 1 changed file with 92 additions and 16 deletions.
108 changes: 92 additions & 16 deletions kernel/src/grant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use core::marker::PhantomData;
use core::mem::{align_of, size_of};
use core::ops::{Deref, DerefMut};
use core::ptr::{write, NonNull};
use core::ptr::{slice_from_raw_parts_mut, write, NonNull};

use crate::callback::AppId;
use crate::process::{Error, ProcessType};
Expand Down Expand Up @@ -70,32 +70,108 @@ impl<T: ?Sized> DerefMut for Owned<T> {
}

impl Allocator {
pub fn alloc<T: Default>(&mut self) -> Result<Owned<T>, Error> {
/// Allocates a new owned grant initialized using the given closure.
///
/// The closure will be called exactly once, and the result will be used to
/// initialize the owned value.
///
/// This interface was chosen instead of a simple `alloc(val)` as it's
/// much more likely to optimize out all stack intermediates. This
/// helps to prevent stack overflows when allocating large values.
///
/// # Panic Safety
///
/// If `init` panics, the freshly allocated memory may leak.
pub fn alloc_with<T, F>(&mut self, init: F) -> Result<Owned<T>, Error>
where
F: FnOnce() -> T,
{
unsafe {
let ptr = self.alloc_unowned()?;
let ptr = self.alloc_raw()?;

// We use `ptr::write` to avoid `Drop`ping the uninitialized memory in
// case `T` implements the `Drop` trait.
write(ptr.as_ptr(), init());

Ok(Owned::new(ptr, self.appid))
}
}

// Like `alloc`, but the caller is responsible for free-ing the allocated
// memory, as it is not wrapped in a type that implements `Drop`
unsafe fn alloc_unowned<T: Default>(&mut self) -> Result<NonNull<T>, Error> {
/// Allocates a slice of n instances of a given type. Each instance is
/// initialized using the provided function.
///
/// The provided function will be called exactly `n` times, and will be
/// passed the index it's initializing, from `0` through `num_items - 1`.
///
/// # Panic Safety
///
/// If `val_func` panics, the freshly allocated memory and any values
/// already written will be leaked.
pub fn alloc_n_with<T, F>(
&mut self,
num_items: usize,
mut val_func: F,
) -> Result<Owned<[T]>, Error>
where
F: FnMut(usize) -> T,
{
unsafe {
let ptr = self.alloc_n_raw::<T>(num_items)?;

for i in 0..num_items {
write(ptr.as_ptr().add(i), val_func(i));
}

// convert `NonNull<T>` to a fat pointer `NonNull<[T]>` which includes
// the length information. We do this here as initialization is more
// convenient with the non-slice ptr.
let slice_ptr =
NonNull::new(slice_from_raw_parts_mut(ptr.as_ptr(), num_items)).unwrap();

Ok(Owned::new(slice_ptr, self.appid))
}
}

/// Like `alloc`, but the caller is responsible for free-ing the allocated
/// memory, as it is not wrapped in a type that implements `Drop`.
///
/// In contrast to `alloc_raw`, this method does initialize the returned
/// memory.
unsafe fn alloc_default_unowned<T: Default>(&mut self) -> Result<NonNull<T>, Error> {
let ptr = self.alloc_raw()?;

// We use `ptr::write` to avoid `Drop`ping the uninitialized memory in
// case `T` implements the `Drop` trait.
write(ptr.as_ptr(), T::default());

Ok(ptr)
}

/// Allocates uninitialized memory appropriate to store a `T`, and returns a
/// pointer to said memory. The caller is responsible for both initializing the
/// returned memory, and dropping it properly when finished.
unsafe fn alloc_raw<T>(&mut self) -> Result<NonNull<T>, Error> {
self.alloc_n_raw::<T>(1)
}

/// Allocates space for a dynamic number of items. The caller is responsible
/// for initializing and freeing returned memory. Returns memory appropriate
/// for storing `num_items` contiguous instances of `T`.
unsafe fn alloc_n_raw<T>(&mut self, num_items: usize) -> Result<NonNull<T>, Error> {
let alloc_size = size_of::<T>()
.checked_mul(num_items)
.ok_or(Error::OutOfMemory)?;
self.appid
.kernel
.process_map_or(Err(Error::NoSuchApp), self.appid, |process| {
process.alloc(size_of::<T>(), align_of::<T>()).map_or(
Err(Error::OutOfMemory),
|buf| {
process
.alloc(alloc_size, align_of::<T>())
.map_or(Err(Error::OutOfMemory), |buf| {
// Convert untyped `*mut u8` allocation to allocated type
let ptr = NonNull::cast::<T>(buf);

// We use `ptr::write` to avoid `Drop`ping the uninitialized memory in
// case `T` implements the `Drop` trait.
write(ptr.as_ptr(), T::default());

Ok(ptr)
},
)
})
})
}
}
Expand Down Expand Up @@ -212,7 +288,7 @@ impl<T: Default> Grant<T> {
// Note: This allocation is intentionally never
// freed. A grant region is valid once allocated
// for the lifetime of the process.
let new_region = allocator.alloc_unowned()?;
let new_region = allocator.alloc_default_unowned()?;

// Update the grant pointer in the process. Again,
// since the process struct does not know about the
Expand Down

0 comments on commit 3bfb711

Please sign in to comment.