Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework std::sys::windows::alloc #83065

Merged
merged 5 commits into from
Apr 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
237 changes: 211 additions & 26 deletions library/std/src/sys/windows/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,61 +1,246 @@
#![deny(unsafe_op_in_unsafe_fn)]

use crate::alloc::{GlobalAlloc, Layout, System};
use crate::ffi::c_void;
use crate::ptr;
use crate::sync::atomic::{AtomicPtr, Ordering};
use crate::sys::c;
use crate::sys_common::alloc::{realloc_fallback, MIN_ALIGN};

#[repr(C)]
struct Header(*mut u8);
#[cfg(test)]
mod tests;

// Heap memory management on Windows is done by using the system Heap API (heapapi.h)
// See https://docs.microsoft.com/windows/win32/api/heapapi/

// Flag to indicate that the memory returned by `HeapAlloc` should be zeroed.
const HEAP_ZERO_MEMORY: c::DWORD = 0x00000008;

unsafe fn get_header<'a>(ptr: *mut u8) -> &'a mut Header {
&mut *(ptr as *mut Header).offset(-1)
extern "system" {
// Get a handle to the default heap of the current process, or null if the operation fails.
//
// SAFETY: Successful calls to this function within the same process are assumed to
// always return the same handle, which remains valid for the entire lifetime of the process.
//
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-getprocessheap
fn GetProcessHeap() -> c::HANDLE;

// Allocate a block of `dwBytes` bytes of memory from a given heap `hHeap`.
// The allocated memory may be uninitialized, or zeroed if `dwFlags` is
// set to `HEAP_ZERO_MEMORY`.
//
// Returns a pointer to the newly-allocated memory or null if the operation fails.
// The returned pointer will be aligned to at least `MIN_ALIGN`.
//
// SAFETY:
// - `hHeap` must be a non-null handle returned by `GetProcessHeap`.
// - `dwFlags` must be set to either zero or `HEAP_ZERO_MEMORY`.
//
// Note that `dwBytes` is allowed to be zero, contrary to some other allocators.
//
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapalloc
fn HeapAlloc(hHeap: c::HANDLE, dwFlags: c::DWORD, dwBytes: c::SIZE_T) -> c::LPVOID;

// Reallocate a block of memory behind a given pointer `lpMem` from a given heap `hHeap`,
// to a block of at least `dwBytes` bytes, either shrinking the block in place,
// or allocating at a new location, copying memory, and freeing the original location.
//
// Returns a pointer to the reallocated memory or null if the operation fails.
// The returned pointer will be aligned to at least `MIN_ALIGN`.
// If the operation fails the given block will never have been freed.
//
// SAFETY:
// - `hHeap` must be a non-null handle returned by `GetProcessHeap`.
// - `dwFlags` must be set to zero.
// - `lpMem` must be a non-null pointer to an allocated block returned by `HeapAlloc` or
// `HeapReAlloc`, that has not already been freed.
// If the block was successfully reallocated at a new location, pointers pointing to
// the freed memory, such as `lpMem`, must not be dereferenced ever again.
//
// Note that `dwBytes` is allowed to be zero, contrary to some other allocators.
//
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heaprealloc
fn HeapReAlloc(
hHeap: c::HANDLE,
dwFlags: c::DWORD,
lpMem: c::LPVOID,
dwBytes: c::SIZE_T,
) -> c::LPVOID;

// Free a block of memory behind a given pointer `lpMem` from a given heap `hHeap`.
// Returns a nonzero value if the operation is successful, and zero if the operation fails.
//
// SAFETY:
// - `hHeap` must be a non-null handle returned by `GetProcessHeap`.
// - `dwFlags` must be set to zero.
// - `lpMem` must be a pointer to an allocated block returned by `HeapAlloc` or `HeapReAlloc`,
// that has not already been freed.
// If the block was successfully freed, pointers pointing to the freed memory, such as `lpMem`,
// must not be dereferenced ever again.
//
// Note that `lpMem` is allowed to be null, which will not cause the operation to fail.
//
// See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapfree
fn HeapFree(hHeap: c::HANDLE, dwFlags: c::DWORD, lpMem: c::LPVOID) -> c::BOOL;
}

unsafe fn align_ptr(ptr: *mut u8, align: usize) -> *mut u8 {
let aligned = ptr.add(align - (ptr as usize & (align - 1)));
*get_header(aligned) = Header(ptr);
aligned
// Cached handle to the default heap of the current process.
// Either a non-null handle returned by `GetProcessHeap`, or null when not yet initialized or `GetProcessHeap` failed.
static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe std is already guaranteed to have AtomicPtr available.


// Get a handle to the default heap of the current process, or null if the operation fails.
// If this operation is successful, `HEAP` will be successfully initialized and contain
// a non-null handle returned by `GetProcessHeap`.
#[inline]
fn init_or_get_process_heap() -> c::HANDLE {
let heap = HEAP.load(Ordering::Relaxed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the ordering here be Acquire?

if heap.is_null() {
// `HEAP` has not yet been successfully initialized
let heap = unsafe { GetProcessHeap() };
if !heap.is_null() {
// SAFETY: No locking is needed because within the same process,
// successful calls to `GetProcessHeap` will always return the same value, even on different threads.
HEAP.store(heap, Ordering::Release);

// SAFETY: `HEAP` contains a non-null handle returned by `GetProcessHeap`
heap
} else {
// Could not get the current process heap.
ptr::null_mut()
}
} else {
// SAFETY: `HEAP` contains a non-null handle returned by `GetProcessHeap`
heap
}
}

// Get a non-null handle to the default heap of the current process.
// SAFETY: `HEAP` must have been successfully initialized.
#[inline]
unsafe fn allocate_with_flags(layout: Layout, flags: c::DWORD) -> *mut u8 {
if layout.align() <= MIN_ALIGN {
return c::HeapAlloc(c::GetProcessHeap(), flags, layout.size()) as *mut u8;
unsafe fn get_process_heap() -> c::HANDLE {
HEAP.load(Ordering::Acquire)
}

// Header containing a pointer to the start of an allocated block.
// SAFETY: Size and alignment must be <= `MIN_ALIGN`.
#[repr(C)]
struct Header(*mut u8);

// Allocate a block of optionally zeroed memory for a given `layout`.
// SAFETY: Returns a pointer satisfying the guarantees of `System` about allocated pointers,
// or null if the operation fails. If this returns non-null `HEAP` will have been successfully
// initialized.
#[inline]
unsafe fn allocate(layout: Layout, zeroed: bool) -> *mut u8 {
let heap = init_or_get_process_heap();
if heap.is_null() {
// Allocation has failed, could not get the current process heap.
return ptr::null_mut();
}

let size = layout.size() + layout.align();
let ptr = c::HeapAlloc(c::GetProcessHeap(), flags, size);
if ptr.is_null() { ptr as *mut u8 } else { align_ptr(ptr as *mut u8, layout.align()) }
// Allocated memory will be either zeroed or uninitialized.
let flags = if zeroed { HEAP_ZERO_MEMORY } else { 0 };

if layout.align() <= MIN_ALIGN {
// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`.
// The returned pointer points to the start of an allocated block.
unsafe { HeapAlloc(heap, flags, layout.size()) as *mut u8 }
} else {
// Allocate extra padding in order to be able to satisfy the alignment.
let total = layout.align() + layout.size();

// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`.
let ptr = unsafe { HeapAlloc(heap, flags, total) as *mut u8 };
if ptr.is_null() {
// Allocation has failed.
return ptr::null_mut();
}

// Create a correctly aligned pointer offset from the start of the allocated block,
// and write a header before it.

let offset = layout.align() - (ptr as usize & (layout.align() - 1));
// SAFETY: `MIN_ALIGN` <= `offset` <= `layout.align()` and the size of the allocated
// block is `layout.align() + layout.size()`. `aligned` will thus be a correctly aligned
// pointer inside the allocated block with at least `layout.size()` bytes after it and at
// least `MIN_ALIGN` bytes of padding before it.
let aligned = unsafe { ptr.add(offset) };
// SAFETY: Because the size and alignment of a header is <= `MIN_ALIGN` and `aligned`
// is aligned to at least `MIN_ALIGN` and has at least `MIN_ALIGN` bytes of padding before
// it, it is safe to write a header directly before it.
unsafe { ptr::write((aligned as *mut Header).offset(-1), Header(ptr)) };

// SAFETY: The returned pointer does not point to the to the start of an allocated block,
// but there is a header readable directly before it containing the location of the start
// of the block.
aligned
}
}

// All pointers returned by this allocator have, in addition to the guarantees of `GlobalAlloc`, the
// following properties:
//
// If the pointer was allocated or reallocated with a `layout` specifying an alignment <= `MIN_ALIGN`
// the pointer will be aligned to at least `MIN_ALIGN` and point to the start of the allocated block.
//
// If the pointer was allocated or reallocated with a `layout` specifying an alignment > `MIN_ALIGN`
// the pointer will be aligned to the specified alignment and not point to the start of the allocated block.
// Instead there will be a header readable directly before the returned pointer, containing the actual
// location of the start of the block.
#[stable(feature = "alloc_system_type", since = "1.28.0")]
unsafe impl GlobalAlloc for System {
#[inline]
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
allocate_with_flags(layout, 0)
// SAFETY: Pointers returned by `allocate` satisfy the guarantees of `System`
let zeroed = false;
unsafe { allocate(layout, zeroed) }
}

#[inline]
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
allocate_with_flags(layout, c::HEAP_ZERO_MEMORY)
// SAFETY: Pointers returned by `allocate` satisfy the guarantees of `System`
let zeroed = true;
unsafe { allocate(layout, zeroed) }
}

#[inline]
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
if layout.align() <= MIN_ALIGN {
let err = c::HeapFree(c::GetProcessHeap(), 0, ptr as c::LPVOID);
debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
} else {
let header = get_header(ptr);
let err = c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID);
debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
Copy link
Contributor Author

@CDirkx CDirkx Apr 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation of GlobalAlloc has the point:

  • It's undefined behavior if global allocators unwind. This restriction may be lifted in the future, but currently a panic from any of these functions may lead to memory unsafety.

So I removed the debug_assert

}
let block = {
if layout.align() <= MIN_ALIGN {
ptr
} else {
// The location of the start of the block is stored in the padding before `ptr`.

// SAFETY: Because of the contract of `System`, `ptr` is guaranteed to be non-null
// and have a header readable directly before it.
unsafe { ptr::read((ptr as *mut Header).offset(-1)).0 }
}
};

// SAFETY: because `ptr` has been successfully allocated with this allocator,
// `HEAP` must have been successfully initialized.
let heap = unsafe { get_process_heap() };

// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`,
// `block` is a pointer to the start of an allocated block.
unsafe { HeapFree(heap, 0, block as c::LPVOID) };
}

#[inline]
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
if layout.align() <= MIN_ALIGN {
c::HeapReAlloc(c::GetProcessHeap(), 0, ptr as c::LPVOID, new_size) as *mut u8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not documented but I could believe this gets to assume GetProcessHeap returns nonnull without a check, because ptr is guaranteed to have been previously allocated from the default process heap, so the process has a default heap. Do you believe we need to add a null check? Is it possible for a process to go from having a heap to not having a heap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried looking into the conditions under which GetProcessHeap would fail, but couldn't find any extra information. It does sound like a reasonable assumption, and even if it is wrong it is legal for implemenations of GlobalAlloc to abort (not unwind), which is what HeapRealloc does in practice when given an invalid hHeap.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the return value need a null check. As we don't specify HEAP_GENERATE_EXCEPTIONS flag, the functions will fail when specified heap is null, and return a null pointer, which is just what we want.

Copy link
Contributor Author

@CDirkx CDirkx Mar 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HEAP_GENERATE_EXCEPTIONS may be enabled for the default process heap, at least on my machine the following code produces an exception, even with flags set to 0:

unsafe {
    let ptr = HeapAlloc(GetProcessHeap(), 0, 64);
    println!("{:?}", HeapReAlloc(std::ptr::null_mut(), 0, ptr, 128)) // exit code: 0xc0000005, STATUS_ACCESS_VIOLATION
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the following to the GetProcessHeap documentation:

// SAFETY: Successful calls to this function within the same process are assumed to
// always return the same handle, which remains valid for the entire lifetime of the process.

Which means we can cache the value, and assume we have a valid handle in dealloc and realloc.

// SAFETY: because `ptr` has been successfully allocated with this allocator,
// `HEAP` must have been successfully initialized.
let heap = unsafe { get_process_heap() };

// SAFETY: `heap` is a non-null handle returned by `GetProcessHeap`,
// `ptr` is a pointer to the start of an allocated block.
// The returned pointer points to the start of an allocated block.
unsafe { HeapReAlloc(heap, 0, ptr as c::LPVOID, new_size) as *mut u8 }
} else {
realloc_fallback(self, ptr, layout, new_size)
// SAFETY: `realloc_fallback` is implemented using `dealloc` and `alloc`, which will
// correctly handle `ptr` and return a pointer satisfying the guarantees of `System`
unsafe { realloc_fallback(self, ptr, layout, new_size) }
}
}
}
9 changes: 9 additions & 0 deletions library/std/src/sys/windows/alloc/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use super::{Header, MIN_ALIGN};
use crate::mem;

#[test]
fn alloc_header() {
// Header must fit in the padding before an aligned pointer
assert!(mem::size_of::<Header>() <= MIN_ALIGN);
assert!(mem::align_of::<Header>() <= MIN_ALIGN);
}
7 changes: 0 additions & 7 deletions library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,6 @@ pub const FD_SETSIZE: usize = 64;

pub const STACK_SIZE_PARAM_IS_A_RESERVATION: DWORD = 0x00010000;

pub const HEAP_ZERO_MEMORY: DWORD = 0x00000008;

pub const STATUS_SUCCESS: NTSTATUS = 0x00000000;

#[repr(C)]
Expand Down Expand Up @@ -1017,11 +1015,6 @@ extern "system" {
timeout: *const timeval,
) -> c_int;

pub fn GetProcessHeap() -> HANDLE;
pub fn HeapAlloc(hHeap: HANDLE, dwFlags: DWORD, dwBytes: SIZE_T) -> LPVOID;
pub fn HeapReAlloc(hHeap: HANDLE, dwFlags: DWORD, lpMem: LPVOID, dwBytes: SIZE_T) -> LPVOID;
pub fn HeapFree(hHeap: HANDLE, dwFlags: DWORD, lpMem: LPVOID) -> BOOL;

// >= Vista / Server 2008
// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createsymboliclinkw
pub fn CreateSymbolicLinkW(
Expand Down