From 11f1165e8a2f5840467e748c8108dc53c948ee9a Mon Sep 17 00:00:00 2001
From: Aria Beingessner <a.beingessner@gmail.com>
Date: Sun, 30 Jan 2022 14:30:35 -0500
Subject: [PATCH] Make the Vec impl be slightly more careful with ZSTs and
 alignment.

This also incidentally makes the ZST code and final code have the same formatting for the divide.
---
 src/vec/vec-final.md | 28 +++++++++++----------
 src/vec/vec-zsts.md  | 60 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 63 insertions(+), 25 deletions(-)

diff --git a/src/vec/vec-final.md b/src/vec/vec-final.md
index f4cd597..1f8af37 100644
--- a/src/vec/vec-final.md
+++ b/src/vec/vec-final.md
@@ -239,21 +239,22 @@ impl<T> Iterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                let result = ptr::read(self.start);
-                self.start = if mem::size_of::<T>() == 0 {
-                    (self.start as usize + 1) as *const _
+                if mem::size_of::<T>() == 0 {
+                    self.start = (self.start as usize + 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.start.offset(1)
-                };
-                Some(result)
+                    let old_ptr = self.start;
+                    self.start = self.start.offset(1);
+                    Some(ptr::read(old_ptr))
+                }
             }
         }
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
         let elem_size = mem::size_of::<T>();
-        let len = (self.end as usize - self.start as usize) /
-                  if elem_size == 0 { 1 } else { elem_size };
+        let len = (self.end as usize - self.start as usize)
+                  / if elem_size == 0 { 1 } else { elem_size };
         (len, Some(len))
     }
 }
@@ -264,12 +265,13 @@ impl<T> DoubleEndedIterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                self.end = if mem::size_of::<T>() == 0 {
-                    (self.end as usize - 1) as *const _
+                if mem::size_of::<T>() == 0 {
+                    self.end = (self.end as usize - 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.end.offset(-1)
-                };
-                Some(ptr::read(self.end))
+                    self.end = self.end.offset(-1);
+                    Some(ptr::read(self.end))
+                }
             }
         }
     }
diff --git a/src/vec/vec-zsts.md b/src/vec/vec-zsts.md
index 3469f0c..73a97ba 100644
--- a/src/vec/vec-zsts.md
+++ b/src/vec/vec-zsts.md
@@ -130,7 +130,41 @@ Now we have a different bug. Instead of our iterators not running at all, our
 iterators now run *forever*. We need to do the same trick in our iterator impls.
 Also, our size_hint computation code will divide by 0 for ZSTs. Since we'll
 basically be treating the two pointers as if they point to bytes, we'll just
-map size 0 to divide by 1.
+map size 0 to divide by 1. Here's what `next` will be:
+
+<!-- ignore: simplified code -->
+```rust,ignore
+fn next(&mut self) -> Option<T> {
+    if self.start == self.end {
+        None
+    } else {
+        unsafe {
+            let result = ptr::read(self.start);
+            self.start = if mem::size_of::<T>() == 0 {
+                (self.start as usize + 1) as *const _
+            } else {
+                self.start.offset(1)
+            };
+            Some(result)
+        }
+    }
+}
+```
+
+Do you see the "bug"? No one else did! The original author only noticed the
+problem when linking to this page years later. This code is kind of dubious
+because abusing the iterator pointers to be *counters* makes them unaligned!
+Our *one job* when using ZSTs is to keep pointers aligned! *forehead slap*
+
+Raw pointers don't need to be aligned at all times, so the basic trick of
+using pointers as counters is *fine*, but they *should* definitely be aligned
+when passed to `ptr::read`! This is *possibly* needless pedantry
+because `ptr::read` is a noop for a ZST, but let's be a *little* more
+responsible and read from `NonNull::dangling` on the ZST path.
+
+(Alternatively you could call `read_unaligned` on the ZST path. Either is fine,
+because either way we're making up a value from nothing and it all compiles
+to doing nothing.)
 
 <!-- ignore: simplified code -->
 ```rust,ignore
@@ -141,13 +175,14 @@ impl<T> Iterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                let result = ptr::read(self.start);
-                self.start = if mem::size_of::<T>() == 0 {
-                    (self.start as usize + 1) as *const _
+                if mem::size_of::<T>() == 0 {
+                    self.start = (self.start as usize + 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.start.offset(1)
-                };
-                Some(result)
+                    let old_ptr = self.start;
+                    self.start = self.start.offset(1);
+                    Some(ptr::read(old_ptr))
+                }
             }
         }
     }
@@ -166,12 +201,13 @@ impl<T> DoubleEndedIterator for RawValIter<T> {
             None
         } else {
             unsafe {
-                self.end = if mem::size_of::<T>() == 0 {
-                    (self.end as usize - 1) as *const _
+                if mem::size_of::<T>() == 0 {
+                    self.end = (self.end as usize - 1) as *const _;
+                    Some(ptr::read(NonNull::<T>::dangling().as_ptr()))
                 } else {
-                    self.end.offset(-1)
-                };
-                Some(ptr::read(self.end))
+                    self.end = self.end.offset(-1);
+                    Some(ptr::read(self.end))
+                }
             }
         }
     }