Skip to content

Commit 6023ac2

Browse files
authored
Rollup merge of #86521 - the8472:add-footgun-comments, r=RalfJung
Add comments around code where ordering is important due for panic-safety Iterators contain arbitrary code which may panic. Unsafe code has to be careful to do its state updates at the right point between calls that may panic. As requested in #86452 (comment) r? `@RalfJung`
2 parents af9e5d1 + e0d7015 commit 6023ac2

File tree

4 files changed

+17
-0
lines changed

4 files changed

+17
-0
lines changed

library/alloc/src/vec/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2568,6 +2568,8 @@ impl<T, A: Allocator> Vec<T, A> {
25682568
}
25692569
unsafe {
25702570
ptr::write(self.as_mut_ptr().add(len), element);
2571+
// Since next() executes user code which can panic we have to bump the length
2572+
// after each step.
25712573
// NB can't overflow since we would have had to alloc the address space
25722574
self.set_len(len + 1);
25732575
}

library/alloc/src/vec/source_iter_marker.rs

+4
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ fn write_in_place_with_drop<T>(
8989
// all we can do is check if it's still in range
9090
debug_assert!(sink.dst as *const _ <= src_end, "InPlaceIterable contract violation");
9191
ptr::write(sink.dst, item);
92+
// Since this executes user code which can panic we have to bump the pointer
93+
// after each step.
9294
sink.dst = sink.dst.add(1);
9395
}
9496
Ok(sink)
@@ -136,6 +138,8 @@ where
136138
let dst = dst_buf.offset(i as isize);
137139
debug_assert!(dst as *const _ <= end, "InPlaceIterable contract violation");
138140
ptr::write(dst, self.__iterator_get_unchecked(i));
141+
// Since this executes user code which can panic we have to bump the pointer
142+
// after each step.
139143
drop_guard.dst = dst.add(1);
140144
}
141145
}

library/alloc/src/vec/spec_extend.rs

+2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ where
4040
iterator.for_each(move |element| {
4141
ptr::write(ptr, element);
4242
ptr = ptr.offset(1);
43+
// Since the loop executes user code which can panic we have to bump the pointer
44+
// after each step.
4345
// NB can't overflow since we would have had to alloc the address space
4446
local_len.increment_len(1);
4547
});

library/core/src/iter/adapters/zip.rs

+9
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,16 @@ where
227227
fn next(&mut self) -> Option<(A::Item, B::Item)> {
228228
if self.index < self.len {
229229
let i = self.index;
230+
// since get_unchecked executes code which can panic we increment the counters beforehand
231+
// so that the same index won't be accessed twice, as required by TrustedRandomAccess
230232
self.index += 1;
231233
// SAFETY: `i` is smaller than `self.len`, thus smaller than `self.a.len()` and `self.b.len()`
232234
unsafe {
233235
Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i)))
234236
}
235237
} else if A::MAY_HAVE_SIDE_EFFECT && self.index < self.a_len {
236238
let i = self.index;
239+
// as above, increment before executing code that may panic
237240
self.index += 1;
238241
self.len += 1;
239242
// match the base implementation's potential side effects
@@ -259,6 +262,8 @@ where
259262
let end = self.index + delta;
260263
while self.index < end {
261264
let i = self.index;
265+
// since get_unchecked executes code which can panic we increment the counters beforehand
266+
// so that the same index won't be accessed twice, as required by TrustedRandomAccess
262267
self.index += 1;
263268
if A::MAY_HAVE_SIDE_EFFECT {
264269
// SAFETY: the usage of `cmp::min` to calculate `delta`
@@ -295,6 +300,8 @@ where
295300
let sz_a = self.a.size();
296301
if A::MAY_HAVE_SIDE_EFFECT && sz_a > self.len {
297302
for _ in 0..sz_a - self.len {
303+
// since next_back() may panic we increment the counters beforehand
304+
// to keep Zip's state in sync with the underlying iterator source
298305
self.a_len -= 1;
299306
self.a.next_back();
300307
}
@@ -309,6 +316,8 @@ where
309316
}
310317
}
311318
if self.index < self.len {
319+
// since get_unchecked executes code which can panic we increment the counters beforehand
320+
// so that the same index won't be accessed twice, as required by TrustedRandomAccess
312321
self.len -= 1;
313322
self.a_len -= 1;
314323
let i = self.len;

0 commit comments

Comments
 (0)