heapless icon indicating copy to clipboard operation
heapless copied to clipboard

heapless Vec does not work good with stacked borrows, while std Vec does

Open Rosdf opened this issue 1 year ago • 4 comments

Pushing to heapless:Vec invalidates all previously created pointers into Vector

small example

fn vector() {
    let mut a = Vec::with_capacity(10);
    a.push(1);

    let b = std::ptr::from_ref(a.get(0).unwrap());
    a.push(1);

    println!("{:?}", unsafe { *b });
}

fn heapless_vector() {
    let mut a = heapless::Vec::<i32, 10>::new();
    a.push(1).unwrap();

    let b = std::ptr::from_ref(a.get(0).unwrap());
    a.push(1).unwrap();

    println!("{:?}", unsafe { *b });
}

miri shows error

running 2 tests
test test::heapless_vector ... error: Undefined Behavior: attempting a read access using <261088> at alloc102617[0x8], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:25:35
   |
25 |         println!("{:?}", unsafe { *b });
   |                                   ^^
   |                                   |
   |                                   attempting a read access using <261088> at alloc102617[0x8], but that tag does not exist in the borrow stack for this location
   |                                   this error occurs as part of an access at alloc102617[0x8..0xc]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <261088> was created by a SharedReadOnly retag at offsets [0x8..0xc]
  --> src/main.rs:22:17
   |
22 |         let b = std::ptr::from_ref(a.get(0).unwrap());
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <261088> was later invalidated at offsets [0x0..0x30] by a Unique function-entry retag inside this call
  --> src/main.rs:23:9
   |
23 |         a.push(1).unwrap();
   |         ^^^^^^^^^
   = note: BACKTRACE (of the first span) on thread `test::heapless_`:
   = note: inside `test::heapless_vector` at src/main.rs:25:35: 25:37
note: inside closure
  --> src/main.rs:18:25
   |
17 |     #[test]
   |     ------- in this procedural macro expansion
18 |     fn heapless_vector() {
   |                         ^
   = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

right now i don't see any way to go around it, except for method like this

unsafe fn push_by_ptr(this: *mut Self, item: T)

Rosdf avatar Oct 08 '24 19:10 Rosdf

miri doesn't have false positives, but it can have false negatives, ie it's possible it reports no error on code that is unsound. So "there's no miri error" doesn't prove it's sound. And i'm actually not sure if it is. This issue https://github.com/rust-lang/rust/issues/85697 and particularly this comment https://github.com/rust-lang/rust/issues/85697#issuecomment-851655153 have interesting info. So it seems it's OK with today's implementation.

The reason it works for Vec is the current implementation contains a raw pointer to the data in the heap, and raw pointers allow aliasing.

Allowing this for heapless would require adding an UnsafeCell to all containers, which would complicate the code and inhibit some optimizations, so I'm not sure if it's worth it.

What are you doing that requires this? Why can't you use indexes instead of pointers?

Dirbaio avatar Oct 08 '24 20:10 Dirbaio

i have a structure like this

struct StableVec<T, const N: usize> {
    inner: Vec<Box<heapless::Vec<T, N>>>,
    size: usize,
}

it works as a vector with stable pointers, after pushing element to it, i am passing pointer to it to different parts of code, so they don't need to know about the whole vector, but recently i found a problem, that pushing a new element invalidates all previously created pointers

Rosdf avatar Oct 09 '24 04:10 Rosdf

Vec is not really designed for this. To make it sound you need some UnsafeCell somewhere, and IMO Vec is not the right place for it. I'd suggest you find an alternative. Maybe Box<[UnsafeCell<MaybeUninit<T>>; N]> could work.

Dirbaio avatar Oct 09 '24 07:10 Dirbaio

I've already done my workaround, mostly wanted to attract attention, to potential problem if someone wants to use heapless::Vec instead of regular Vec

Rosdf avatar Oct 11 '24 15:10 Rosdf