Skip to content

Commit e0d7015

Browse files
committed
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.
1 parent 6a5b97a commit e0d7015

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)