Skip to content

Commit ca75967

Browse files
committed
consolidate: fix a Miri error
Prior to this commit, Miri would produce the following error when executed on the code of `consolidate_updates_slice`: ``` error: Undefined Behavior: attempting a read access using <3403> at alloc1431[0x8], but that tag does not exist in the borrow stack for this location --> src/main.rs:12:16 | 12 | if (*ptr1).0 == (*ptr2).0 && (*ptr1).1 == (*ptr2).1 { | ^^^^^^^^^ | | | attempting a read access using <3403> at alloc1431[0x8], but that tag does not exist in the borrow stack for this location | this error occurs as part of an access at alloc1431[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: <3403> was created by a SharedReadWrite retag at offsets [0x0..0x48] --> src/main.rs:9:24 | 9 | let ptr1 = slice.as_mut_ptr().offset(offset as isize); | ^^^^^^^^^^^^^^^^^^ help: <3403> was later invalidated at offsets [0x0..0x48] by a Unique function-entry retag inside this call --> src/main.rs:10:24 | 10 | let ptr2 = slice.as_mut_ptr().offset(index as isize); | ^^^^^^^^^^^^^^^^^^ = note: BACKTRACE (of the first span): = note: inside `consolidate_updates_slice` at src/main.rs:12:16: 12:25 note: inside `main` --> src/main.rs:34:5 | 34 | consolidate_updates_slice(&mut v); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` The same is true for `consolidate_slice`. The warning is fixed by making sure that `slice.get_mut_ptr()` is only invoked a single time. It seems like calling `get_mut_ptr` on a slice invalidates all existing pointers to the slice. My guess is that this is because `get_mut_ptr` takes a `&mut self` and could therefore in principle swap/replace/truncate the slice buffer, which could make existing pointers dangle. `get_mut_ptr` doesn't do that but Rust cannot know based on the method signature only.
1 parent 8b3fea6 commit ca75967

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

src/consolidation.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ pub fn consolidate_slice<T: Ord, R: Semigroup>(slice: &mut [(T, R)]) -> usize {
3838
// In a world where there are not many results, we may never even need to call in to merge sort.
3939
slice.sort_by(|x,y| x.0.cmp(&y.0));
4040

41+
let slice_ptr = slice.as_mut_ptr();
42+
4143
// Counts the number of distinct known-non-zero accumulations. Indexes the write location.
4244
let mut offset = 0;
4345
for index in 1 .. slice.len() {
@@ -55,8 +57,8 @@ pub fn consolidate_slice<T: Ord, R: Semigroup>(slice: &mut [(T, R)]) -> usize {
5557
assert!(offset < index);
5658

5759
// LOOP INVARIANT: offset < index
58-
let ptr1 = slice.as_mut_ptr().offset(offset as isize);
59-
let ptr2 = slice.as_mut_ptr().offset(index as isize);
60+
let ptr1 = slice_ptr.add(offset);
61+
let ptr2 = slice_ptr.add(index);
6062

6163
if (*ptr1).0 == (*ptr2).0 {
6264
(*ptr1).1.plus_equals(&(*ptr2).1);
@@ -65,7 +67,7 @@ pub fn consolidate_slice<T: Ord, R: Semigroup>(slice: &mut [(T, R)]) -> usize {
6567
if !(*ptr1).1.is_zero() {
6668
offset += 1;
6769
}
68-
let ptr1 = slice.as_mut_ptr().offset(offset as isize);
70+
let ptr1 = slice_ptr.add(offset);
6971
std::ptr::swap(ptr1, ptr2);
7072
}
7173
}
@@ -103,6 +105,8 @@ pub fn consolidate_updates_slice<D: Ord, T: Ord, R: Semigroup>(slice: &mut [(D,
103105
// In a world where there are not many results, we may never even need to call in to merge sort.
104106
slice.sort_unstable_by(|x,y| (&x.0, &x.1).cmp(&(&y.0, &y.1)));
105107

108+
let slice_ptr = slice.as_mut_ptr();
109+
106110
// Counts the number of distinct known-non-zero accumulations. Indexes the write location.
107111
let mut offset = 0;
108112
for index in 1 .. slice.len() {
@@ -118,8 +122,8 @@ pub fn consolidate_updates_slice<D: Ord, T: Ord, R: Semigroup>(slice: &mut [(D,
118122
unsafe {
119123

120124
// LOOP INVARIANT: offset < index
121-
let ptr1 = slice.as_mut_ptr().offset(offset as isize);
122-
let ptr2 = slice.as_mut_ptr().offset(index as isize);
125+
let ptr1 = slice_ptr.add(offset);
126+
let ptr2 = slice_ptr.add(index);
123127

124128
if (*ptr1).0 == (*ptr2).0 && (*ptr1).1 == (*ptr2).1 {
125129
(*ptr1).2.plus_equals(&(*ptr2).2);
@@ -128,7 +132,7 @@ pub fn consolidate_updates_slice<D: Ord, T: Ord, R: Semigroup>(slice: &mut [(D,
128132
if !(*ptr1).2.is_zero() {
129133
offset += 1;
130134
}
131-
let ptr1 = slice.as_mut_ptr().offset(offset as isize);
135+
let ptr1 = slice_ptr.add(offset);
132136
std::ptr::swap(ptr1, ptr2);
133137
}
134138

0 commit comments

Comments
 (0)