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