Skip to content

Change to &raw #102

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Change to &raw #102

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8cf2418
Change to &raw
Riceman2000 Jan 28, 2025
20d9db5
Review changes
Riceman2000 Feb 3, 2025
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
3 changes: 1 addition & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Starting with Rust 1.68, this crate can be used as a global allocator on stable

extern crate alloc;

use core::ptr::addr_of_mut;
use cortex_m_rt::entry;
use embedded_alloc::LlffHeap as Heap;

Expand All @@ -36,7 +35,7 @@ fn main() -> ! {
use core::mem::MaybeUninit;
const HEAP_SIZE: usize = 1024;
static mut HEAP_MEM: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
unsafe { HEAP.init(addr_of_mut!(HEAP_MEM) as usize, HEAP_SIZE) }
unsafe { HEAP.init(&raw mut HEAP_MEM as usize, HEAP_SIZE) }
}

// now the allocator is ready types like Box, Vec can be used.
Expand Down
3 changes: 1 addition & 2 deletions examples/allocator_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ extern crate alloc;
use alloc::vec::Vec;
use core::mem::MaybeUninit;
use core::panic::PanicInfo;
use core::ptr::addr_of_mut;
use cortex_m_rt::entry;
use embedded_alloc::LlffHeap as Heap;

Expand All @@ -21,7 +20,7 @@ fn main() -> ! {
const HEAP_SIZE: usize = 16;
static mut HEAP_MEM: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
let heap: Heap = Heap::empty();
unsafe { heap.init(addr_of_mut!(HEAP_MEM) as usize, HEAP_SIZE) }
unsafe { heap.init(&raw mut HEAP_MEM as usize, HEAP_SIZE) }

let mut xs = Vec::new_in(heap);
xs.push(1);
Expand Down
3 changes: 1 addition & 2 deletions examples/global_alloc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ extern crate alloc;

use alloc::vec::Vec;
use core::panic::PanicInfo;
use core::ptr::addr_of_mut;
use cortex_m_rt::entry;
// Linked-List First Fit Heap allocator (feature = "llff")
use embedded_alloc::LlffHeap as Heap;
Expand All @@ -22,7 +21,7 @@ fn main() -> ! {
use core::mem::MaybeUninit;
const HEAP_SIZE: usize = 1024;
static mut HEAP_MEM: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
unsafe { HEAP.init(addr_of_mut!(HEAP_MEM) as usize, HEAP_SIZE) }
unsafe { HEAP.init(&raw mut HEAP_MEM as usize, HEAP_SIZE) }
}

let mut xs = Vec::new();
Expand Down
7 changes: 3 additions & 4 deletions examples/llff_integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ extern crate panic_semihosting;

use alloc::vec::Vec;
use core::mem::{size_of, MaybeUninit};
use core::ptr::addr_of_mut;
use cortex_m_rt::entry;
use cortex_m_semihosting::{debug, hprintln};
use embedded_alloc::LlffHeap as Heap;
Expand All @@ -46,9 +45,9 @@ fn test_global_heap() {
fn test_allocator_api() {
// small local heap
const HEAP_SIZE: usize = 16;
let heap_mem: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
let mut heap_mem: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
let local_heap: Heap = Heap::empty();
unsafe { local_heap.init(heap_mem.as_ptr() as usize, HEAP_SIZE) }
unsafe { local_heap.init(&raw mut heap_mem as usize, HEAP_SIZE) }

assert_eq!(local_heap.used(), 0);

Expand All @@ -67,7 +66,7 @@ fn main() -> ! {
{
const HEAP_SIZE: usize = 1024;
static mut HEAP_MEM: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
unsafe { HEAP.init(addr_of_mut!(HEAP_MEM) as usize, HEAP_SIZE) }
unsafe { HEAP.init(&raw mut HEAP_MEM as usize, HEAP_SIZE) }
}

#[allow(clippy::type_complexity)]
Expand Down
7 changes: 3 additions & 4 deletions examples/tlsf_integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ extern crate panic_semihosting;

use alloc::collections::LinkedList;
use core::mem::MaybeUninit;
use core::ptr::addr_of_mut;
use cortex_m_rt::entry;
use cortex_m_semihosting::{debug, hprintln};
use embedded_alloc::TlsfHeap as Heap;
Expand Down Expand Up @@ -55,9 +54,9 @@ fn test_global_heap() {
fn test_allocator_api() {
// small local heap
const HEAP_SIZE: usize = 256;
let heap_mem: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
let mut heap_mem: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
let local_heap: Heap = Heap::empty();
unsafe { local_heap.init(heap_mem.as_ptr() as usize, HEAP_SIZE) }
unsafe { local_heap.init(&raw mut heap_mem as usize, HEAP_SIZE) }
Copy link
Member

Choose a reason for hiding this comment

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

this one doesn't need &raw because it's not a static mut, it's just a plain old array.

Arguably the old code is wrong as well because it used .as_ptr() which gives you a pointer that you're not allowed to use for writing. Could you change this to .as_mut_ptr()?

(and why is heap init taking the address as usize 😭 it should take a pointer. maybe we should change it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this, I agree that this should take a pointer.

To give some background on how I came across this issue, I used .as_ptr() and the resulting code ended up being optimized in such a way that it was broken. When using .as_mut_ptr() it would compile and run no problem. I suspect this is to do with provenance and LLVM pulling some tricks because it thinks that memory will never be modified.

Requiring that the pointer be mutable using the type system would remove this kind of issue but it would obviously alter the API.

Docs for .as_ptr() and .as_mut_ptr(). Note the return types of both and that .as_ptr() explicitly returns a const.

Related RFC:
https://rust-lang.github.io/rfcs/3559-rust-has-provenance.html

Thanks for the review!


const ELEMS: usize = 2;

Expand All @@ -84,7 +83,7 @@ fn test_allocator_api() {
fn main() -> ! {
{
static mut HEAP_MEM: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE];
unsafe { HEAP.init(addr_of_mut!(HEAP_MEM) as usize, HEAP_SIZE) }
unsafe { HEAP.init(&raw mut HEAP_MEM as usize, HEAP_SIZE) }
}

#[allow(clippy::type_complexity)]
Expand Down
Loading