Skip to content

Commit da143d3

Browse files
authored
Rollup merge of #82554 - SkiFire13:fix-string-retain-unsoundness, r=m-ou-se
Fix invalid slice access in String::retain As noted in #78499, the previous fix was technically still unsound because it accessed elements of a slice outside its bounds (even though they were still inside the same allocation). This PR addresses that concern by switching to a dropguard approach.
2 parents 29a53e6 + c89e643 commit da143d3

File tree

1 file changed

+22
-15
lines changed

1 file changed

+22
-15
lines changed

library/alloc/src/string.rs

+22-15
Original file line numberDiff line numberDiff line change
@@ -1289,37 +1289,44 @@ impl String {
12891289
where
12901290
F: FnMut(char) -> bool,
12911291
{
1292-
let len = self.len();
1293-
let mut del_bytes = 0;
1294-
let mut idx = 0;
1292+
struct SetLenOnDrop<'a> {
1293+
s: &'a mut String,
1294+
idx: usize,
1295+
del_bytes: usize,
1296+
}
12951297

1296-
unsafe {
1297-
self.vec.set_len(0);
1298+
impl<'a> Drop for SetLenOnDrop<'a> {
1299+
fn drop(&mut self) {
1300+
let new_len = self.idx - self.del_bytes;
1301+
debug_assert!(new_len <= self.s.len());
1302+
unsafe { self.s.vec.set_len(new_len) };
1303+
}
12981304
}
12991305

1300-
while idx < len {
1301-
let ch = unsafe { self.get_unchecked(idx..len).chars().next().unwrap() };
1306+
let len = self.len();
1307+
let mut guard = SetLenOnDrop { s: self, idx: 0, del_bytes: 0 };
1308+
1309+
while guard.idx < len {
1310+
let ch = unsafe { guard.s.get_unchecked(guard.idx..len).chars().next().unwrap() };
13021311
let ch_len = ch.len_utf8();
13031312

13041313
if !f(ch) {
1305-
del_bytes += ch_len;
1306-
} else if del_bytes > 0 {
1314+
guard.del_bytes += ch_len;
1315+
} else if guard.del_bytes > 0 {
13071316
unsafe {
13081317
ptr::copy(
1309-
self.vec.as_ptr().add(idx),
1310-
self.vec.as_mut_ptr().add(idx - del_bytes),
1318+
guard.s.vec.as_ptr().add(guard.idx),
1319+
guard.s.vec.as_mut_ptr().add(guard.idx - guard.del_bytes),
13111320
ch_len,
13121321
);
13131322
}
13141323
}
13151324

13161325
// Point idx to the next char
1317-
idx += ch_len;
1326+
guard.idx += ch_len;
13181327
}
13191328

1320-
unsafe {
1321-
self.vec.set_len(len - del_bytes);
1322-
}
1329+
drop(guard);
13231330
}
13241331

13251332
/// Inserts a character into this `String` at a byte position.

0 commit comments

Comments
 (0)