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

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

Open
Rosdf opened this issue Oct 8, 2024 · 4 comments
Open

Comments

@Rosdf
Copy link

Rosdf commented Oct 8, 2024

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)
@Dirbaio
Copy link
Member

Dirbaio commented Oct 8, 2024

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 rust-lang/rust#85697 and particularly this comment rust-lang/rust#85697 (comment) 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?

@Rosdf
Copy link
Author

Rosdf commented Oct 9, 2024

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

@Dirbaio
Copy link
Member

Dirbaio commented Oct 9, 2024

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.

@Rosdf
Copy link
Author

Rosdf commented Oct 11, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants