allocator::win32::{Global, Local}: fix ZST (re)allocs by banning them

relates to #32

Author: MaulingMonkey

src/allocator/win32/_win32.rs

@@ -67,6 +67,7 @@ mod virtual_;           pub use virtual_::*;
 impl core::fmt::Debug   for Error { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { core::fmt::Debug::fmt(&self.0, f) } }
 //impl core::fmt::Display for Error { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { core::fmt::Display::fmt(&self.0, f) } }
 impl Error { pub(crate) fn get_last() -> Self { Self(winresult::ErrorHResultOrCode::from(get_last_error())) } }
+impl From<crate::error::BannedZeroSizedAllocationsError > for Error { fn from(_: crate::error::BannedZeroSizedAllocationsError  ) -> Self { Self(winresult::ERROR::BAD_LENGTH.into()) } } // XXX
 impl From<crate::error::ExcessiveAlignmentRequestedError> for Error { fn from(_: crate::error::ExcessiveAlignmentRequestedError ) -> Self { Self(winresult::ERROR::MAPPED_ALIGNMENT.into()) } } // XXX
 impl From<crate::error::ExcessiveSliceRequestedError    > for Error { fn from(_: crate::error::ExcessiveSliceRequestedError     ) -> Self { Self(winresult::ERROR::BUFFER_OVERFLOW.into()) } } // XXX
 

src/allocator/win32/global.rs

@@ -48,7 +48,15 @@ impl Meta for Global {
     const MIN_ALIGN : Alignment = super::MEMORY_ALLOCATION_ALIGNMENT; // Verified through testing
     const MAX_ALIGN : Alignment = super::MEMORY_ALLOCATION_ALIGNMENT; // Verified through testing
     const MAX_SIZE  : usize     = usize::MAX;
-    const ZST_SUPPORTED : bool  = true;
+
+    /// ZST support in `Global*` is incredibly cursed:
+    /// -   `GlobalAlloc(..., 0)` "allocates" 0 bytes.  This can be confirmed with `GlobalSize(zst)` (doesn't set `GetLastError()`)
+    /// -   `GlobalReAlloc(zst, 1, ...)` fails with <code>GetLastError() == ERROR_NOT_ENOUGH_MEMORY</code>.
+    /// -   `GlobalReAlloc(zst, 0, ...)` frees?  Oops.
+    ///
+    /// As such, this wrapper straight up bans such allocations.
+    ///
+    const ZST_SUPPORTED : bool  = false;
 }
 
 impl ZstSupported for Global {}
@@ -75,12 +83,14 @@ unsafe impl Stateless for Global {}
 // SAFETY: per above
 unsafe impl thin::Alloc for Global {
     fn alloc_uninit(&self, size: usize) -> Result<AllocNN, Self::Error> {
+        if size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant above
         // SAFETY: per above
         let alloc = unsafe { GlobalAlloc(0, size) };
         NonNull::new(alloc.cast()).ok_or_else(Error::get_last)
     }
 
     fn alloc_zeroed(&self, size: usize) -> Result<AllocNN0, Self::Error> {
+        if size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant above
         // SAFETY: ✔️ `GMEM_ZEROINIT` should ensure the newly allocated memory is zeroed.
         let alloc = unsafe { GlobalAlloc(GMEM_ZEROINIT, size) };
         NonNull::new(alloc.cast()).ok_or_else(Error::get_last)
@@ -106,12 +116,14 @@ unsafe impl thin::Realloc for Global {
     const CAN_REALLOC_ZEROED : bool = true;
 
     unsafe fn realloc_uninit(&self, ptr: AllocNN, new_size: usize) -> Result<AllocNN, Self::Error> {
+        if new_size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant above
         // SAFETY: ✔️ `ptr` belongs to `self` per thin::Realloc's documented safety preconditions - and thus was allocated with `Global{,Re}Alloc` - which should be safe to `GlobalReAlloc`.
         let alloc = unsafe { GlobalReAlloc(ptr.as_ptr().cast(), new_size, 0) };
         NonNull::new(alloc.cast()).ok_or_else(Error::get_last)
     }
 
     unsafe fn realloc_zeroed(&self, ptr: AllocNN, new_size: usize) -> Result<AllocNN, Self::Error> {
+        if new_size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant above
         // SAFETY: ✔️ `GMEM_ZEROINIT` should ensure the newly reallocated memory is zeroed.
         // SAFETY: ✔️ `ptr` belongs to `self` per thin::Realloc's documented safety preconditions - and thus was allocated with `Global{,Re}Alloc` - which should be safe to `GlobalReAlloc`.
         let alloc = unsafe { GlobalReAlloc(ptr.as_ptr().cast(), new_size, GMEM_ZEROINIT) };
@@ -178,13 +190,17 @@ unsafe impl thin::SizeOfDebug for Global {
 #[test] fn thin_alignment()         { thin::test::alignment(Global) }
 #[test] fn thin_edge_case_sizes()   { thin::test::edge_case_sizes(Global) }
 #[test] fn thin_nullable()          { thin::test::nullable(Global) }
-#[test] fn thin_size()              { thin::test::size_over_alloc(Global) }
+#[test] fn thin_size()              { thin::test::size_exact_alloc(Global) }
 #[test] fn thin_uninit()            { unsafe { thin::test::uninit_alloc_unsound(Global) } }
+#[test] fn thin_uninit_realloc()    { thin::test::uninit_realloc(Global) }
 #[test] fn thin_zeroed()            { thin::test::zeroed_alloc(Global) }
+#[test] fn thin_zeroed_realloc()    { thin::test::zeroed_realloc(Global) }
 #[test] fn thin_zst_support()       { thin::test::zst_supported_accurate(Global) }
 
 #[test] fn fat_alignment()          { fat::test::alignment(Global) }
 #[test] fn fat_edge_case_sizes()    { fat::test::edge_case_sizes(Global) }
 #[test] fn fat_uninit()             { unsafe { fat::test::uninit_alloc_unsound(Global) } }
+#[test] fn fat_uninit_realloc()     { fat::test::uninit_realloc(Global) }
 #[test] fn fat_zeroed()             { fat::test::zeroed_alloc(Global) }
+#[test] fn fat_zeroed_realloc()     { fat::test::zeroed_realloc(Global) }
 #[test] fn fat_zst_support()        { fat::test::zst_supported_accurate(Global) }

src/allocator/win32/local.rs

@@ -48,7 +48,15 @@ impl Meta for Local {
     const MIN_ALIGN : Alignment = super::MEMORY_ALLOCATION_ALIGNMENT; // Verified through testing
     const MAX_ALIGN : Alignment = super::MEMORY_ALLOCATION_ALIGNMENT; // Verified through testing
     const MAX_SIZE  : usize     = usize::MAX;
-    const ZST_SUPPORTED : bool  = true;
+
+    /// ZST support in `Local*` is incredibly cursed:
+    /// -   `LocalAlloc(..., 0)` "allocates" 0 bytes.  This can be confirmed with `LocalSize(zst)` (doesn't set `GetLastError()`)
+    /// -   `LocalReAlloc(zst, 1, ...)` fails with <code>GetLastError() == ERROR_NOT_ENOUGH_MEMORY</code>.
+    /// -   `LocalReAlloc(zst, 0, ...)` frees?  Oops.
+    ///
+    /// As such, this wrapper straight up bans such allocations.
+    ///
+    const ZST_SUPPORTED : bool  = false;
 }
 
 impl ZstSupported for Local {}
@@ -75,12 +83,14 @@ unsafe impl Stateless for Local {}
 // SAFETY: per above
 unsafe impl thin::Alloc for Local {
     fn alloc_uninit(&self, size: usize) -> Result<AllocNN, Self::Error> {
+        if size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant
         // SAFETY: per above
         let alloc = unsafe { LocalAlloc(0, size) };
         NonNull::new(alloc.cast()).ok_or_else(Error::get_last)
     }
 
     fn alloc_zeroed(&self, size: usize) -> Result<AllocNN0, Self::Error> {
+        if size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant
         // SAFETY: ✔️ `LMEM_ZEROINIT` should ensure the newly allocated memory is zeroed.
         let alloc = unsafe { LocalAlloc(LMEM_ZEROINIT, size) };
         NonNull::new(alloc.cast()).ok_or_else(Error::get_last)
@@ -106,12 +116,14 @@ unsafe impl thin::Realloc for Local {
     const CAN_REALLOC_ZEROED : bool = true;
 
     unsafe fn realloc_uninit(&self, ptr: AllocNN, new_size: usize) -> Result<AllocNN, Self::Error> {
+        if new_size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant
         // SAFETY: ✔️ `ptr` belongs to `self` per thin::Realloc's documented safety preconditions - and thus was allocated with `Local{,Re}Alloc` - which should be safe to `LocalReAlloc`.
         let alloc = unsafe { LocalReAlloc(ptr.as_ptr().cast(), new_size, 0) };
         NonNull::new(alloc.cast()).ok_or_else(Error::get_last)
     }
 
     unsafe fn realloc_zeroed(&self, ptr: AllocNN, new_size: usize) -> Result<AllocNN, Self::Error> {
+        if new_size == 0 { Err(error::BannedZeroSizedAllocationsError)? } // see ZST_SUPPORTED rant
         // SAFETY: ✔️ `LMEM_ZEROINIT` should ensure the newly reallocated memory is zeroed.
         // SAFETY: ✔️ `ptr` belongs to `self` per thin::Realloc's documented safety preconditions - and thus was allocated with `Local{,Re}Alloc` - which should be safe to `LocalReAlloc`.
         let alloc = unsafe { LocalReAlloc(ptr.as_ptr().cast(), new_size, LMEM_ZEROINIT) };
@@ -180,11 +192,15 @@ unsafe impl thin::SizeOfDebug for Local {
 #[test] fn thin_nullable()          { thin::test::nullable(Local) }
 #[test] fn thin_size()              { thin::test::size_exact_alloc(Local) }
 #[test] fn thin_uninit()            { unsafe { thin::test::uninit_alloc_unsound(Local) } }
+#[test] fn thin_uninit_realloc()    { thin::test::uninit_realloc(Local) }
 #[test] fn thin_zeroed()            { thin::test::zeroed_alloc(Local) }
+#[test] fn thin_zeroed_realloc()    { thin::test::zeroed_realloc(Local) }
 #[test] fn thin_zst_support()       { thin::test::zst_supported_accurate(Local) }
 
 #[test] fn fat_alignment()          { fat::test::alignment(Local) }
 #[test] fn fat_edge_case_sizes()    { fat::test::edge_case_sizes(Local) }
 #[test] fn fat_uninit()             { unsafe { fat::test::uninit_alloc_unsound(Local) } }
+#[test] fn fat_uninit_realloc()     { fat::test::uninit_realloc(Local) }
 #[test] fn fat_zeroed()             { fat::test::zeroed_alloc(Local) }
+#[test] fn fat_zeroed_realloc()     { fat::test::zeroed_realloc(Local) }
 #[test] fn fat_zst_support()        { fat::test::zst_supported_accurate(Local) }

src/error.rs

@@ -7,6 +7,13 @@ use core::fmt::{self, Debug, Display, Formatter};
 
 
 
+/// The allocator explicitly rejected a zero-sized allocation, because they behave inconsistently.
+///
+/// This may be because:
+/// -   A platform specific allocator fails on some but not all platforms.
+/// -   Allocating 0 bytes succeeds, but reallocating to 0 bytes fails.
+#[derive(Clone, Copy, Debug)] pub struct BannedZeroSizedAllocationsError;
+
 /// More alignment was requested than the allocator could support.
 #[derive(Clone, Copy, Debug)] pub struct ExcessiveAlignmentRequestedError {
     pub requested: Alignment,
@@ -18,11 +25,15 @@ use core::fmt::{self, Debug, Display, Formatter};
     pub requested: usize,
 }
 
+impl Display for BannedZeroSizedAllocationsError  { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "requested a 0-sized allocation, but those have inconsistent behavior with this allocator") } }
 impl Display for ExcessiveAlignmentRequestedError { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "requested {:?} alignment, but a maximum of {:?} is supported", self.requested, self.supported) } }
 impl Display for ExcessiveSliceRequestedError     { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "requested {} elements, but that would result in a LayoutError", self.requested) } }
+impl From<BannedZeroSizedAllocationsError > for () { fn from(_: BannedZeroSizedAllocationsError) -> Self {} }
 impl From<ExcessiveAlignmentRequestedError> for () { fn from(_: ExcessiveAlignmentRequestedError) -> Self {} }
 impl From<ExcessiveSliceRequestedError    > for () { fn from(_: ExcessiveSliceRequestedError) -> Self {} }
+#[cfg(feature = "std")] impl std::error::Error for BannedZeroSizedAllocationsError  { fn description(&self) -> &str { "requested a 0-sized allocation, but those have inconsistent behavior with this allocator" } }
 #[cfg(feature = "std")] impl std::error::Error for ExcessiveAlignmentRequestedError { fn description(&self) -> &str { "requested more alignment than was supported" } }
 #[cfg(feature = "std")] impl std::error::Error for ExcessiveSliceRequestedError     { fn description(&self) -> &str { "requested too many elements" } }
+#[cfg(allocator_api = "*")] impl From<BannedZeroSizedAllocationsError > for core::alloc::AllocError { fn from(_: BannedZeroSizedAllocationsError ) -> Self { core::alloc::AllocError } }
 #[cfg(allocator_api = "*")] impl From<ExcessiveAlignmentRequestedError> for core::alloc::AllocError { fn from(_: ExcessiveAlignmentRequestedError) -> Self { core::alloc::AllocError } }
 #[cfg(allocator_api = "*")] impl From<ExcessiveSliceRequestedError    > for core::alloc::AllocError { fn from(_: ExcessiveSliceRequestedError    ) -> Self { core::alloc::AllocError } }