Skip to content

Commit 2fefd9a

Browse files
Qwazcuviper
authored andcommitted
Fixes #80335
(cherry picked from commit 6d43225)
1 parent cc6d66f commit 2fefd9a

File tree

2 files changed

+54
-18
lines changed

2 files changed

+54
-18
lines changed

library/alloc/src/str.rs

+24-18
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ impl<S: Borrow<str>> Join<&str> for [S] {
9090
}
9191
}
9292

93-
macro_rules! spezialize_for_lengths {
94-
($separator:expr, $target:expr, $iter:expr; $($num:expr),*) => {
93+
macro_rules! specialize_for_lengths {
94+
($separator:expr, $target:expr, $iter:expr; $($num:expr),*) => {{
9595
let mut target = $target;
9696
let iter = $iter;
9797
let sep_bytes = $separator;
@@ -102,19 +102,22 @@ macro_rules! spezialize_for_lengths {
102102
$num => {
103103
for s in iter {
104104
copy_slice_and_advance!(target, sep_bytes);
105-
copy_slice_and_advance!(target, s.borrow().as_ref());
105+
let content_bytes = s.borrow().as_ref();
106+
copy_slice_and_advance!(target, content_bytes);
106107
}
107108
},
108109
)*
109110
_ => {
110111
// arbitrary non-zero size fallback
111112
for s in iter {
112113
copy_slice_and_advance!(target, sep_bytes);
113-
copy_slice_and_advance!(target, s.borrow().as_ref());
114+
let content_bytes = s.borrow().as_ref();
115+
copy_slice_and_advance!(target, content_bytes);
114116
}
115117
}
116118
}
117-
};
119+
target
120+
}}
118121
}
119122

120123
macro_rules! copy_slice_and_advance {
@@ -153,30 +156,33 @@ where
153156
// if the `len` calculation overflows, we'll panic
154157
// we would have run out of memory anyway and the rest of the function requires
155158
// the entire Vec pre-allocated for safety
156-
let len = sep_len
159+
let reserved_len = sep_len
157160
.checked_mul(iter.len())
158161
.and_then(|n| {
159162
slice.iter().map(|s| s.borrow().as_ref().len()).try_fold(n, usize::checked_add)
160163
})
161164
.expect("attempt to join into collection with len > usize::MAX");
162165

163166
// crucial for safety
164-
let mut result = Vec::with_capacity(len);
165-
assert!(result.capacity() >= len);
167+
let mut result = Vec::with_capacity(reserved_len);
168+
debug_assert!(result.capacity() >= reserved_len);
166169

167170
result.extend_from_slice(first.borrow().as_ref());
168171

169172
unsafe {
170-
{
171-
let pos = result.len();
172-
let target = result.get_unchecked_mut(pos..len);
173-
174-
// copy separator and slices over without bounds checks
175-
// generate loops with hardcoded offsets for small separators
176-
// massive improvements possible (~ x2)
177-
spezialize_for_lengths!(sep, target, iter; 0, 1, 2, 3, 4);
178-
}
179-
result.set_len(len);
173+
let pos = result.len();
174+
let target = result.get_unchecked_mut(pos..reserved_len);
175+
176+
// copy separator and slices over without bounds checks
177+
// generate loops with hardcoded offsets for small separators
178+
// massive improvements possible (~ x2)
179+
let remain = specialize_for_lengths!(sep, target, iter; 0, 1, 2, 3, 4);
180+
181+
// issue #80335: A weird borrow implementation can return different
182+
// slices for the length calculation and the actual copy, so
183+
// `remain.len()` might be non-zero.
184+
let result_len = reserved_len - remain.len();
185+
result.set_len(result_len);
180186
}
181187
result
182188
}

library/alloc/tests/str.rs

+30
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,36 @@ fn test_join_for_different_lengths_with_long_separator() {
160160
test_join!("~~~~~a~~~~~bc", ["", "a", "bc"], "~~~~~");
161161
}
162162

163+
#[test]
164+
fn test_join_isue_80335() {
165+
use core::{borrow::Borrow, cell::Cell};
166+
167+
struct WeirdBorrow {
168+
state: Cell<bool>,
169+
}
170+
171+
impl Default for WeirdBorrow {
172+
fn default() -> Self {
173+
WeirdBorrow { state: Cell::new(false) }
174+
}
175+
}
176+
177+
impl Borrow<str> for WeirdBorrow {
178+
fn borrow(&self) -> &str {
179+
let state = self.state.get();
180+
if state {
181+
"0"
182+
} else {
183+
self.state.set(true);
184+
"123456"
185+
}
186+
}
187+
}
188+
189+
let arr: [WeirdBorrow; 3] = Default::default();
190+
test_join!("0-0-0", arr, "-");
191+
}
192+
163193
#[test]
164194
#[cfg_attr(miri, ignore)] // Miri is too slow
165195
fn test_unsafe_slice() {

0 commit comments

Comments
 (0)