Skip to content

Commit 8363ea8

Browse files
authored
Auto merge of #277 - saethlin:fix-aliasing, r=mbrubeck
Fix all problems encounted with Miri -Ztag-raw-pointers I poked at this crate with `-Zmiri-tag-raw-pointers` before, and I was unable to fix what I found (I just added a test case that ruled out one of my wrong ideas #271). I tried again just now and I guess I just understand better this time. This PR fixes 3 separate pointer invalidation problems, which are detected by running `MIRIFLAGS=-Zmiri-tag-raw-pointers cargo miri test`. Depending on how you squint, 2 or 3 of these are rust-lang/unsafe-code-guidelines#133. The last one is _probably_ still present even with late invalidation, because `set_len` does a write through a `&mut`. It's unclear to me if any of these things that Miri complains about are potentially a miscompilation in rustc due to the use of LLVM `noalias`. But perhaps given how subtle this codebase is overall, it would be best to run the tools on their pickiest settings, even if there are a few things more like a false positive than a real problem.
2 parents 0a4fdff + ecd69b9 commit 8363ea8

File tree

2 files changed

+14
-4
lines changed

2 files changed

+14
-4
lines changed

.github/workflows/main.yml

+2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ jobs:
6161
- name: miri
6262
if: matrix.toolchain == 'nightly'
6363
run: bash ./scripts/run_miri.sh
64+
env:
65+
MIRIFLAGS: '-Zmiri-tag-raw-pointers'
6466

6567
- name: fuzz
6668
if: env.DO_FUZZ == '1'

src/lib.rs

+12-4
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,11 @@ impl<'a, T: 'a + Array> Drop for Drain<'a, T> {
392392
let start = source_vec.len();
393393
let tail = self.tail_start;
394394
if tail != start {
395-
let src = source_vec.as_ptr().add(tail);
396-
let dst = source_vec.as_mut_ptr().add(start);
395+
// as_mut_ptr creates a &mut, invalidating other pointers.
396+
// This pattern avoids calling it with a pointer already present.
397+
let ptr = source_vec.as_mut_ptr();
398+
let src = ptr.add(tail);
399+
let dst = ptr.add(start);
397400
ptr::copy(src, dst, self.tail_len);
398401
}
399402
source_vec.set_len(start + self.tail_len);
@@ -813,13 +816,14 @@ impl<A: Array> SmallVec<A> {
813816
unsafe {
814817
self.set_len(start);
815818

816-
let range_slice = slice::from_raw_parts_mut(self.as_mut_ptr().add(start), end - start);
819+
let range_slice = slice::from_raw_parts(self.as_ptr().add(start), end - start);
817820

818821
Drain {
819822
tail_start: end,
820823
tail_len: len - end,
821824
iter: range_slice.iter(),
822-
vec: NonNull::from(self),
825+
// Since self is a &mut, passing it to a function would invalidate the slice iterator.
826+
vec: NonNull::new_unchecked(self as *mut _),
823827
}
824828
}
825829
}
@@ -1112,6 +1116,10 @@ impl<A: Array> SmallVec<A> {
11121116
len: old_len + lower_size_bound,
11131117
};
11141118

1119+
// The set_len above invalidates the previous pointers, so we must re-create them.
1120+
let start = self.as_mut_ptr();
1121+
let ptr = start.add(index);
1122+
11151123
while num_added < lower_size_bound {
11161124
let element = match iter.next() {
11171125
Some(x) => x,

0 commit comments

Comments
 (0)