From 2ffc56f5c2e2366728c06c69d8ebd92bf036ed59 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Wed, 8 Oct 2025 18:11:37 -0600 Subject: [PATCH] Improve heapapi implementation --- CMakeLists.txt | 1 + dll/kernel32/heapapi.cpp | 103 +++++++++++++-------------------------- dll/kernel32/internal.h | 7 ++- 3 files changed, 40 insertions(+), 71 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d00d8e7..e8ca69b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,6 +82,7 @@ FetchContent_MakeAvailable(mimalloc) # Disable `note: the alignment of '_Atomic long long int' fields changed in GCC 11.1` target_compile_options(mimalloc-obj PRIVATE -Wno-psabi) +target_compile_definitions(mimalloc-obj PRIVATE MI_USE_BUILTIN_THREAD_POINTER=1) if (WIBO_ENABLE_LIBURING) FetchContent_Declare( diff --git a/dll/kernel32/heapapi.cpp b/dll/kernel32/heapapi.cpp index 7717217..6a4f73b 100644 --- a/dll/kernel32/heapapi.cpp +++ b/dll/kernel32/heapapi.cpp @@ -26,7 +26,6 @@ void ensureProcessHeapInitialized() { if (!record) { return; } - record->heap = heap; record->isProcessHeap = true; g_processHeapRecord = record.get(); g_processHeapHandle = wibo::handles().alloc(std::move(record), 0, 0); @@ -91,31 +90,23 @@ HANDLE WIN_FUNC HeapCreate(DWORD flOptions, SIZE_T dwInitialSize, SIZE_T dwMaxim } auto record = make_pin(heap); - record->heap = heap; record->createFlags = flOptions; record->initialSize = dwInitialSize; record->maximumSize = dwMaximumSize; - record->isProcessHeap = false; - - HANDLE handle = wibo::handles().alloc(std::move(record), 0, 0); - return handle; + return wibo::handles().alloc(std::move(record), 0, 0); } BOOL WIN_FUNC HeapDestroy(HANDLE hHeap) { HOST_CONTEXT_GUARD(); DEBUG_LOG("HeapDestroy(%p)\n", hHeap); auto record = wibo::handles().getAs(hHeap); - if (!record) { - wibo::lastError = ERROR_INVALID_HANDLE; - return FALSE; - } - std::lock_guard lk(record->m); - if (record->isProcessHeap || record->heap == nullptr) { + if (!record || !record->isOwner() || record->isProcessHeap) { wibo::lastError = ERROR_INVALID_HANDLE; return FALSE; } mi_heap_destroy(record->heap); record->heap = nullptr; + wibo::handles().release(hHeap); return TRUE; } @@ -132,15 +123,10 @@ BOOL WIN_FUNC HeapSetInformation(HANDLE HeapHandle, HEAP_INFORMATION_CLASS HeapI DEBUG_LOG("HeapSetInformation(%p, %d, %p, %zu)\n", HeapHandle, static_cast(HeapInformationClass), HeapInformation, HeapInformationLength); auto record = wibo::handles().getAs(HeapHandle); - if (!record) { + if (!record || !record->canAccess()) { wibo::lastError = ERROR_INVALID_HANDLE; return FALSE; } - std::lock_guard lk(record->m); - if (!record->heap) { - wibo::lastError = ERROR_INVALID_PARAMETER; - return FALSE; - } switch (HeapInformationClass) { case HeapCompatibilityInformation: { if (!HeapInformation || HeapInformationLength < sizeof(ULONG)) { @@ -163,51 +149,39 @@ BOOL WIN_FUNC HeapSetInformation(HANDLE HeapHandle, HEAP_INFORMATION_CLASS HeapI LPVOID WIN_FUNC HeapAlloc(HANDLE hHeap, DWORD dwFlags, SIZE_T dwBytes) { HOST_CONTEXT_GUARD(); - DEBUG_LOG("HeapAlloc(%p, 0x%x, %zu) ", hHeap, dwFlags, dwBytes); + VERBOSE_LOG("HeapAlloc(%p, 0x%x, %zu) ", hHeap, dwFlags, dwBytes); auto record = wibo::handles().getAs(hHeap); - if (!record) { - DEBUG_LOG("-> NULL\n"); + if (!record || !record->canAccess()) { + VERBOSE_LOG("-> NULL\n"); wibo::lastError = ERROR_INVALID_HANDLE; return nullptr; } - std::lock_guard lk(record->m); - if (!record->heap) { - DEBUG_LOG("-> NULL\n"); - wibo::lastError = ERROR_INVALID_PARAMETER; - return nullptr; - } void *mem = heapAllocFromRecord(record.get(), dwFlags, dwBytes); - DEBUG_LOG("-> %p\n", mem); + VERBOSE_LOG("-> %p\n", mem); return mem; } LPVOID WIN_FUNC HeapReAlloc(HANDLE hHeap, DWORD dwFlags, LPVOID lpMem, SIZE_T dwBytes) { HOST_CONTEXT_GUARD(); - DEBUG_LOG("HeapReAlloc(%p, 0x%x, %p, %zu) ", hHeap, dwFlags, lpMem, dwBytes); + VERBOSE_LOG("HeapReAlloc(%p, 0x%x, %p, %zu) ", hHeap, dwFlags, lpMem, dwBytes); auto record = wibo::handles().getAs(hHeap); - if (!record) { - DEBUG_LOG("-> NULL\n"); + if (!record || !record->canAccess()) { + VERBOSE_LOG("-> NULL\n"); wibo::lastError = ERROR_INVALID_HANDLE; return nullptr; } - std::lock_guard lk(record->m); - if (!record->heap) { - DEBUG_LOG("-> NULL\n"); - wibo::lastError = ERROR_INVALID_PARAMETER; - return nullptr; - } if (lpMem == nullptr) { void *alloc = heapAllocFromRecord(record.get(), dwFlags, dwBytes); - DEBUG_LOG("-> %p (alloc)\n", alloc); + VERBOSE_LOG("-> %p (alloc)\n", alloc); return alloc; } - // if (!mi_heap_check_owned(record->heap, lpMem)) { - // DEBUG_LOG("-> NULL (not owned)\n"); - // wibo::lastError = ERROR_INVALID_PARAMETER; - // return nullptr; - // } + if (!mi_heap_check_owned(record->heap, lpMem)) { + VERBOSE_LOG("-> NULL (not owned)\n"); + wibo::lastError = ERROR_INVALID_PARAMETER; + return nullptr; + } if ((record->createFlags | dwFlags) & HEAP_GENERATE_EXCEPTIONS) { - DEBUG_LOG("-> NULL (exceptions unsupported)\n"); + VERBOSE_LOG("-> NULL (exceptions unsupported)\n"); wibo::lastError = ERROR_NOT_SUPPORTED; return nullptr; } @@ -216,10 +190,10 @@ LPVOID WIN_FUNC HeapReAlloc(HANDLE hHeap, DWORD dwFlags, LPVOID lpMem, SIZE_T dw if (dwBytes == 0) { if (!inplaceOnly) { mi_free(lpMem); - DEBUG_LOG("-> NULL (freed)\n"); + VERBOSE_LOG("-> NULL (freed)\n"); return nullptr; } - DEBUG_LOG("-> NULL (zero size with in-place flag)\n"); + VERBOSE_LOG("-> NULL (zero size with in-place flag)\n"); wibo::lastError = ERROR_NOT_ENOUGH_MEMORY; return nullptr; } @@ -228,11 +202,11 @@ LPVOID WIN_FUNC HeapReAlloc(HANDLE hHeap, DWORD dwFlags, LPVOID lpMem, SIZE_T dw const SIZE_T oldSize = mi_usable_size(lpMem); if (inplaceOnly || requestSize <= oldSize) { if (requestSize > oldSize) { - DEBUG_LOG("-> NULL (cannot grow in place)\n"); + VERBOSE_LOG("-> NULL (cannot grow in place)\n"); wibo::lastError = ERROR_NOT_ENOUGH_MEMORY; return nullptr; } - DEBUG_LOG("-> %p (in-place)\n", lpMem); + VERBOSE_LOG("-> %p (in-place)\n", lpMem); return lpMem; } @@ -251,30 +225,27 @@ LPVOID WIN_FUNC HeapReAlloc(HANDLE hHeap, DWORD dwFlags, LPVOID lpMem, SIZE_T dw if (isExecutableHeap(record.get())) { tryMarkExecutable(ret); } - DEBUG_LOG("-> %p\n", ret); + VERBOSE_LOG("-> %p\n", ret); return ret; } SIZE_T WIN_FUNC HeapSize(HANDLE hHeap, DWORD dwFlags, LPCVOID lpMem) { HOST_CONTEXT_GUARD(); - DEBUG_LOG("HeapSize(%p, 0x%x, %p)\n", hHeap, dwFlags, lpMem); + VERBOSE_LOG("HeapSize(%p, 0x%x, %p)\n", hHeap, dwFlags, lpMem); (void)dwFlags; auto record = wibo::handles().getAs(hHeap); - if (!record) { + if (!record || !record->canAccess()) { + VERBOSE_LOG("-> ERROR_INVALID_HANDLE\n"); wibo::lastError = ERROR_INVALID_HANDLE; return static_cast(-1); } - std::lock_guard lk(record->m); - if (!record->heap) { - wibo::lastError = ERROR_INVALID_PARAMETER; - return static_cast(-1); - } if (!lpMem) { + VERBOSE_LOG("-> ERROR_INVALID_PARAMETER\n"); wibo::lastError = ERROR_INVALID_PARAMETER; return static_cast(-1); } - if (!mi_heap_check_owned(record->heap, const_cast(lpMem))) { - DEBUG_LOG("HeapSize: block %p not owned by heap %p\n", lpMem, record->heap); + if (!mi_heap_check_owned(record->heap, lpMem)) { + VERBOSE_LOG("-> ERROR_INVALID_PARAMETER (not owned)\n"); wibo::lastError = ERROR_INVALID_PARAMETER; return static_cast(-1); } @@ -284,30 +255,24 @@ SIZE_T WIN_FUNC HeapSize(HANDLE hHeap, DWORD dwFlags, LPCVOID lpMem) { BOOL WIN_FUNC HeapFree(HANDLE hHeap, DWORD dwFlags, LPVOID lpMem) { HOST_CONTEXT_GUARD(); - DEBUG_LOG("HeapFree(%p, 0x%x, %p)\n", hHeap, dwFlags, lpMem); + VERBOSE_LOG("HeapFree(%p, 0x%x, %p)\n", hHeap, dwFlags, lpMem); (void)dwFlags; if (lpMem == nullptr) { return TRUE; } auto record = wibo::handles().getAs(hHeap); - if (!record) { - DEBUG_LOG("-> INVALID_HANDLE\n"); + if (!record || !record->canAccess()) { + VERBOSE_LOG("-> ERROR_INVALID_HANDLE\n"); wibo::lastError = ERROR_INVALID_HANDLE; return FALSE; } - std::lock_guard lk(record->m); - if (!record->heap) { - DEBUG_LOG("-> INVALID_PARAMETER\n"); - wibo::lastError = ERROR_INVALID_PARAMETER; - return FALSE; - } if (!mi_heap_check_owned(record->heap, lpMem)) { - DEBUG_LOG("-> INVALID_PARAMETER (not owned)\n"); + VERBOSE_LOG("-> ERROR_INVALID_PARAMETER (not owned)\n"); wibo::lastError = ERROR_INVALID_PARAMETER; return FALSE; } mi_free(lpMem); - DEBUG_LOG("-> SUCCESS\n"); + VERBOSE_LOG("-> SUCCESS\n"); return TRUE; } diff --git a/dll/kernel32/internal.h b/dll/kernel32/internal.h index 79b2330..87139f1 100644 --- a/dll/kernel32/internal.h +++ b/dll/kernel32/internal.h @@ -153,16 +153,19 @@ struct SemaphoreObject final : WaitableObject { struct HeapObject : public ObjectBase { static constexpr ObjectType kType = ObjectType::Heap; - std::mutex m; mi_heap_t *heap; + const pthread_t owner; DWORD createFlags = 0; SIZE_T initialSize = 0; SIZE_T maximumSize = 0; DWORD compatibility = 0; bool isProcessHeap = false; - explicit HeapObject(mi_heap_t *heap) : ObjectBase(kType), heap(heap) {} + explicit HeapObject(mi_heap_t *heap) : ObjectBase(kType), heap(heap), owner(pthread_self()) {} ~HeapObject() override; + + [[nodiscard]] inline bool isOwner() const { return pthread_equal(owner, pthread_self()); } + [[nodiscard]] inline bool canAccess() const { return (isProcessHeap || isOwner()) && heap != nullptr; } }; inline constexpr uintptr_t kPseudoCurrentProcessHandleValue = static_cast(-1);