|
|
From 70f17ca715d3d7e2fd79cc909b95fd3a6357c13e Mon Sep 17 00:00:00 2001
|
|
|
From: Yechan Bae <yechan@gatech.edu>
|
|
|
Date: Wed, 3 Feb 2021 16:36:33 -0500
|
|
|
Subject: [PATCH 1/2] Fixes #80335
|
|
|
|
|
|
---
|
|
|
library/alloc/src/str.rs | 42 ++++++++++++++++++++++----------------
|
|
|
library/alloc/tests/str.rs | 30 +++++++++++++++++++++++++++
|
|
|
2 files changed, 54 insertions(+), 18 deletions(-)
|
|
|
|
|
|
diff --git a/library/alloc/src/str.rs b/library/alloc/src/str.rs
|
|
|
index 70e0c7dba5ea..a7584c6b6510 100644
|
|
|
--- a/library/alloc/src/str.rs
|
|
|
+++ b/library/alloc/src/str.rs
|
|
|
@@ -90,8 +90,8 @@ fn join(slice: &Self, sep: &str) -> String {
|
|
|
}
|
|
|
}
|
|
|
|
|
|
-macro_rules! spezialize_for_lengths {
|
|
|
- ($separator:expr, $target:expr, $iter:expr; $($num:expr),*) => {
|
|
|
+macro_rules! specialize_for_lengths {
|
|
|
+ ($separator:expr, $target:expr, $iter:expr; $($num:expr),*) => {{
|
|
|
let mut target = $target;
|
|
|
let iter = $iter;
|
|
|
let sep_bytes = $separator;
|
|
|
@@ -102,7 +102,8 @@ macro_rules! spezialize_for_lengths {
|
|
|
$num => {
|
|
|
for s in iter {
|
|
|
copy_slice_and_advance!(target, sep_bytes);
|
|
|
- copy_slice_and_advance!(target, s.borrow().as_ref());
|
|
|
+ let content_bytes = s.borrow().as_ref();
|
|
|
+ copy_slice_and_advance!(target, content_bytes);
|
|
|
}
|
|
|
},
|
|
|
)*
|
|
|
@@ -110,11 +111,13 @@ macro_rules! spezialize_for_lengths {
|
|
|
// arbitrary non-zero size fallback
|
|
|
for s in iter {
|
|
|
copy_slice_and_advance!(target, sep_bytes);
|
|
|
- copy_slice_and_advance!(target, s.borrow().as_ref());
|
|
|
+ let content_bytes = s.borrow().as_ref();
|
|
|
+ copy_slice_and_advance!(target, content_bytes);
|
|
|
}
|
|
|
}
|
|
|
}
|
|
|
- };
|
|
|
+ target
|
|
|
+ }}
|
|
|
}
|
|
|
|
|
|
macro_rules! copy_slice_and_advance {
|
|
|
@@ -153,7 +156,7 @@ fn join_generic_copy<B, T, S>(slice: &[S], sep: &[T]) -> Vec<T>
|
|
|
// if the `len` calculation overflows, we'll panic
|
|
|
// we would have run out of memory anyway and the rest of the function requires
|
|
|
// the entire Vec pre-allocated for safety
|
|
|
- let len = sep_len
|
|
|
+ let reserved_len = sep_len
|
|
|
.checked_mul(iter.len())
|
|
|
.and_then(|n| {
|
|
|
slice.iter().map(|s| s.borrow().as_ref().len()).try_fold(n, usize::checked_add)
|
|
|
@@ -161,22 +164,25 @@ fn join_generic_copy<B, T, S>(slice: &[S], sep: &[T]) -> Vec<T>
|
|
|
.expect("attempt to join into collection with len > usize::MAX");
|
|
|
|
|
|
// crucial for safety
|
|
|
- let mut result = Vec::with_capacity(len);
|
|
|
- assert!(result.capacity() >= len);
|
|
|
+ let mut result = Vec::with_capacity(reserved_len);
|
|
|
+ debug_assert!(result.capacity() >= reserved_len);
|
|
|
|
|
|
result.extend_from_slice(first.borrow().as_ref());
|
|
|
|
|
|
unsafe {
|
|
|
- {
|
|
|
- let pos = result.len();
|
|
|
- let target = result.get_unchecked_mut(pos..len);
|
|
|
-
|
|
|
- // copy separator and slices over without bounds checks
|
|
|
- // generate loops with hardcoded offsets for small separators
|
|
|
- // massive improvements possible (~ x2)
|
|
|
- spezialize_for_lengths!(sep, target, iter; 0, 1, 2, 3, 4);
|
|
|
- }
|
|
|
- result.set_len(len);
|
|
|
+ let pos = result.len();
|
|
|
+ let target = result.get_unchecked_mut(pos..reserved_len);
|
|
|
+
|
|
|
+ // copy separator and slices over without bounds checks
|
|
|
+ // generate loops with hardcoded offsets for small separators
|
|
|
+ // massive improvements possible (~ x2)
|
|
|
+ let remain = specialize_for_lengths!(sep, target, iter; 0, 1, 2, 3, 4);
|
|
|
+
|
|
|
+ // issue #80335: A weird borrow implementation can return different
|
|
|
+ // slices for the length calculation and the actual copy, so
|
|
|
+ // `remain.len()` might be non-zero.
|
|
|
+ let result_len = reserved_len - remain.len();
|
|
|
+ result.set_len(result_len);
|
|
|
}
|
|
|
result
|
|
|
}
|
|
|
diff --git a/library/alloc/tests/str.rs b/library/alloc/tests/str.rs
|
|
|
index 604835e6cc4a..6df8d8c2f354 100644
|
|
|
--- a/library/alloc/tests/str.rs
|
|
|
+++ b/library/alloc/tests/str.rs
|
|
|
@@ -160,6 +160,36 @@ fn test_join_for_different_lengths_with_long_separator() {
|
|
|
test_join!("～～～～～a～～～～～bc", ["", "a", "bc"], "～～～～～");
|
|
|
}
|
|
|
|
|
|
+#[test]
|
|
|
+fn test_join_isue_80335() {
|
|
|
+ use core::{borrow::Borrow, cell::Cell};
|
|
|
+
|
|
|
+ struct WeirdBorrow {
|
|
|
+ state: Cell<bool>,
|
|
|
+ }
|
|
|
+
|
|
|
+ impl Default for WeirdBorrow {
|
|
|
+ fn default() -> Self {
|
|
|
+ WeirdBorrow { state: Cell::new(false) }
|
|
|
+ }
|
|
|
+ }
|
|
|
+
|
|
|
+ impl Borrow<str> for WeirdBorrow {
|
|
|
+ fn borrow(&self) -> &str {
|
|
|
+ let state = self.state.get();
|
|
|
+ if state {
|
|
|
+ "0"
|
|
|
+ } else {
|
|
|
+ self.state.set(true);
|
|
|
+ "123456"
|
|
|
+ }
|
|
|
+ }
|
|
|
+ }
|
|
|
+
|
|
|
+ let arr: [WeirdBorrow; 3] = Default::default();
|
|
|
+ test_join!("0-0-0", arr, "-");
|
|
|
+}
|
|
|
+
|
|
|
#[test]
|
|
|
#[cfg_attr(miri, ignore)] // Miri is too slow
|
|
|
fn test_unsafe_slice() {
|
|
|
--
|
|
|
2.31.1
|
|
|
|
|
|
|
|
|
From 10020817d2e6756be1ff2ac3c182af97cf7fe904 Mon Sep 17 00:00:00 2001
|
|
|
From: Yechan Bae <yechan@gatech.edu>
|
|
|
Date: Sat, 20 Mar 2021 13:42:54 -0400
|
|
|
Subject: [PATCH 2/2] Update the comment
|
|
|
|
|
|
---
|
|
|
library/alloc/src/str.rs | 8 ++++----
|
|
|
1 file changed, 4 insertions(+), 4 deletions(-)
|
|
|
|
|
|
diff --git a/library/alloc/src/str.rs b/library/alloc/src/str.rs
|
|
|
index a7584c6b6510..4d1e876457b8 100644
|
|
|
--- a/library/alloc/src/str.rs
|
|
|
+++ b/library/alloc/src/str.rs
|
|
|
@@ -163,7 +163,7 @@ fn join_generic_copy<B, T, S>(slice: &[S], sep: &[T]) -> Vec<T>
|
|
|
})
|
|
|
.expect("attempt to join into collection with len > usize::MAX");
|
|
|
|
|
|
- // crucial for safety
|
|
|
+ // prepare an uninitialized buffer
|
|
|
let mut result = Vec::with_capacity(reserved_len);
|
|
|
debug_assert!(result.capacity() >= reserved_len);
|
|
|
|
|
|
@@ -178,9 +178,9 @@ fn join_generic_copy<B, T, S>(slice: &[S], sep: &[T]) -> Vec<T>
|
|
|
// massive improvements possible (~ x2)
|
|
|
let remain = specialize_for_lengths!(sep, target, iter; 0, 1, 2, 3, 4);
|
|
|
|
|
|
- // issue #80335: A weird borrow implementation can return different
|
|
|
- // slices for the length calculation and the actual copy, so
|
|
|
- // `remain.len()` might be non-zero.
|
|
|
+ // A weird borrow implementation may return different
|
|
|
+ // slices for the length calculation and the actual copy.
|
|
|
+ // Make sure we don't expose uninitialized bytes to the caller.
|
|
|
let result_len = reserved_len - remain.len();
|
|
|
result.set_len(result_len);
|
|
|
}
|
|
|
--
|
|
|
2.31.1
|
|
|
|
