From 95f242c5255a6585a98504c7e1edaae02ceabb70 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 28 Apr 2022 20:32:01 -0400 Subject: [PATCH] Avoid Box invalidation, for tag-raw-pointers It looks like the only outstanding issue aliasing issue in this repo (with raw pointer tagging) was this one move of a Box after saving a pointer to the allocation that the Box guards. This is UB according to the Stacked Borrows with raw pointer tagging in combination with the way rustc applies noalias to Box. As is often the case, the resolution here is to convert the Box down to a raw pointer before a pointer into the allocation is created. --- .github/workflows/ci.yml | 2 ++ src/buffer.rs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4e36649b3d..124f7b1278 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,6 +113,8 @@ jobs: - uses: actions/checkout@v3 - uses: dtolnay/rust-toolchain@miri - run: cargo miri test --all-features + env: + MIRIFLAGS: '-Zmiri-tag-raw-pointers' clippy: name: Clippy diff --git a/src/buffer.rs b/src/buffer.rs index 43e77e97f7..28bf061ca5 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -88,25 +88,25 @@ impl TokenBuffer { // length of the backing buffer. The backing buffer must remain at a // constant address after this point, as we are going to store a raw // pointer into it. - let mut entries = entries.into_boxed_slice(); + let entries = entries.into_boxed_slice(); + let len = entries.len(); + // Convert our boxed slice into a pointer to the first element early, + // to avoid invalidating pointers into this slice when we move the Box + // see https://github.com/rust-lang/unsafe-code-guidelines/issues/326 + let entries = Box::into_raw(entries) as *mut Entry; for (idx, group) in groups { // We know that this index refers to one of the temporary // `End(null)` entries, and we know that the last entry is // `End(up)`, so the next index is also valid. - let group_up = unsafe { entries.as_ptr().add(idx + 1) }; + let group_up = unsafe { entries.add(idx + 1) }; // The end entry stored at the end of this Entry::Group should // point to the Entry which follows the Group in the list. let inner = Self::inner_new(group.stream(), group_up); - entries[idx] = Entry::Group(group, inner); + unsafe { *entries.add(idx) = Entry::Group(group, inner) }; } - let len = entries.len(); - let ptr = Box::into_raw(entries); - TokenBuffer { - ptr: ptr as *const Entry, - len, - } + TokenBuffer { ptr: entries, len } } /// Creates a `TokenBuffer` containing all the tokens from the input