[PATCH 0/6] MR55: ntdll: Factor out heap status handling and locking.

[PATCH 1/6] ntdll: Lock heap critical section outside of heap_validate(_ptr).

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/ntdll/heap.c | 84 +++++++++++++++++++++++------------------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index d7ac44a4247..10140fed339 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -181,7 +181,7 @@ typedef struct tagHEAP

static HEAP *processHeap;  /* main process heap */

-static BOOL HEAP_IsRealArena( HEAP *heapPtr, DWORD flags, LPCVOID block, BOOL quiet );
+static BOOL HEAP_IsRealArena( HEAP *heapPtr, LPCVOID block, BOOL quiet );

/* mark a block of memory as free for debugging purposes */
 static inline void mark_block_free( void *ptr, SIZE_T size, DWORD flags )
@@ -470,21 +470,27 @@ static HEAP *HEAP_GetPtr(
              HANDLE heap /* [in] Handle to the heap */
 ) {
     HEAP *heapPtr = heap;
+    BOOL valid = TRUE;
+
     if (!heapPtr || (heapPtr->magic != HEAP_MAGIC))
     {
         ERR("Invalid heap %p!\n", heap );
         return NULL;
     }
-    if ((heapPtr->flags & HEAP_VALIDATE_ALL) && !HEAP_IsRealArena( heapPtr, 0, NULL, NOISY ))
+    if (heapPtr->flags & HEAP_VALIDATE_ALL)
     {
-        if (TRACE_ON(heap))
+        heap_lock( heapPtr, 0 );
+        valid = HEAP_IsRealArena( heapPtr, NULL, NOISY );
+        heap_unlock( heapPtr, 0 );
+
+        if (!valid && TRACE_ON(heap))
         {
             HEAP_Dump( heapPtr );
             assert( FALSE );
         }
-        return NULL;
     }
-    return heapPtr;
+
+    return valid ? heapPtr : NULL;
 }

@@ -1346,64 +1352,58 @@ static BOOL HEAP_ValidateInUseArena( const SUBHEAP *subheap, const ARENA_INUSE *
  *	TRUE: Success
  *	FALSE: Failure
  */
-static BOOL HEAP_IsRealArena( HEAP *heapPtr,   /* [in] ptr to the heap */
-              DWORD flags,   /* [in] Bit flags that control access during operation */
+static BOOL HEAP_IsRealArena( HEAP *heap,   /* [in] ptr to the heap */
               LPCVOID block, /* [in] Optional pointer to memory block to validate */
               BOOL quiet )   /* [in] Flag - if true, HEAP_ValidateInUseArena
                               *             does not complain    */
 {
     SUBHEAP *subheap;
-    BOOL ret = FALSE;
     const ARENA_LARGE *large_arena;

-    heap_lock( heapPtr, flags );
-
     if (block)  /* only check this single memory block */
     {
         const ARENA_INUSE *arena = (const ARENA_INUSE *)block - 1;

-        if (!(subheap = HEAP_FindSubHeap( heapPtr, arena )) ||
+        if (!(subheap = HEAP_FindSubHeap( heap, arena )) ||
             ((const char *)arena < (char *)subheap->base + subheap->headerSize))
         {
-            if (!(large_arena = find_large_block( heapPtr, block )))
+            if (!(large_arena = find_large_block( heap, block )))
             {
                 if (quiet == NOISY)
-                    ERR("Heap %p: block %p is not inside heap\n", heapPtr, block );
+                    ERR("Heap %p: block %p is not inside heap\n", heap, block );
                 else if (WARN_ON(heap))
-                    WARN("Heap %p: block %p is not inside heap\n", heapPtr, block );
+                    WARN("Heap %p: block %p is not inside heap\n", heap, block );
+                return FALSE;
             }
-            else ret = validate_large_arena( heapPtr, large_arena, quiet );
+
+            return validate_large_arena( heap, large_arena, quiet );
         }
-        else ret = HEAP_ValidateInUseArena( subheap, arena, quiet );
-        goto done;
+
+        return HEAP_ValidateInUseArena( subheap, arena, quiet );
     }

-    LIST_FOR_EACH_ENTRY( subheap, &heapPtr->subheap_list, SUBHEAP, entry )
+    LIST_FOR_EACH_ENTRY( subheap, &heap->subheap_list, SUBHEAP, entry )
     {
         char *ptr = (char *)subheap->base + subheap->headerSize;
         while (ptr < (char *)subheap->base + subheap->size)
         {
             if (*(DWORD *)ptr & ARENA_FLAG_FREE)
             {
-                if (!HEAP_ValidateFreeArena( subheap, (ARENA_FREE *)ptr )) goto done;
+                if (!HEAP_ValidateFreeArena( subheap, (ARENA_FREE *)ptr )) return FALSE;
                 ptr += sizeof(ARENA_FREE) + (*(DWORD *)ptr & ARENA_SIZE_MASK);
             }
             else
             {
-                if (!HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)ptr, NOISY )) goto done;
+                if (!HEAP_ValidateInUseArena( subheap, (ARENA_INUSE *)ptr, NOISY )) return FALSE;
                 ptr += sizeof(ARENA_INUSE) + (*(DWORD *)ptr & ARENA_SIZE_MASK);
             }
         }
     }

-    LIST_FOR_EACH_ENTRY( large_arena, &heapPtr->large_list, ARENA_LARGE, entry )
-        if (!validate_large_arena( heapPtr, large_arena, quiet )) goto done;
+    LIST_FOR_EACH_ENTRY( large_arena, &heap->large_list, ARENA_LARGE, entry )
+        if (!validate_large_arena( heap, large_arena, quiet )) return FALSE;

-    ret = TRUE;
-
-done:
-    heap_unlock( heapPtr, flags );
-    return ret;
+    return TRUE;
 }

@@ -2076,23 +2076,23 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr )

/***********************************************************************
  *           RtlValidateHeap   (NTDLL.@)
- *
- * Determine if a block is a valid allocation from a heap.
- *
- * PARAMS
- *  heap  [I] Heap that block was allocated from
- *  flags [I] HEAP_ flags from "winnt.h"
- *  ptr   [I] Block to check
- *
- * RETURNS
- *  Success: TRUE. The block was allocated from heap.
- *  Failure: FALSE, if heap is invalid or ptr was not allocated from it.
  */
-BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, LPCVOID ptr )
+BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr )
 {
-    HEAP *heapPtr = HEAP_GetPtr( heap );
-    if (!heapPtr) return FALSE;
-    return HEAP_IsRealArena( heapPtr, flags, ptr, QUIET );
+    HEAP *heapPtr;
+    BOOLEAN ret;
+
+    if (!(heapPtr = HEAP_GetPtr( heap )))
+        ret = FALSE;
+    else
+    {
+        heap_lock( heapPtr, flags );
+        ret = HEAP_IsRealArena( heapPtr, ptr, QUIET );
+        heap_unlock( heapPtr, flags );
+    }
+
+    TRACE( "heap %p, flags %#x, ptr %p, return %u.\n", heap, flags, ptr, !!ret );
+    return ret;
 }

[PATCH 2/6] ntdll: Split heap_validate to heap_validate and heap_validate_ptr.

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/ntdll/heap.c | 53 +++++++++++++++++++----------------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index 10140fed339..cd7e44cc717 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -181,7 +181,7 @@ typedef struct tagHEAP

static HEAP *processHeap;  /* main process heap */

-static BOOL HEAP_IsRealArena( HEAP *heapPtr, LPCVOID block, BOOL quiet );
+static BOOL heap_validate( HEAP *heap, BOOL quiet );

/* mark a block of memory as free for debugging purposes */
 static inline void mark_block_free( void *ptr, SIZE_T size, DWORD flags )
@@ -480,7 +480,7 @@ static HEAP *HEAP_GetPtr(
     if (heapPtr->flags & HEAP_VALIDATE_ALL)
     {
         heap_lock( heapPtr, 0 );
-        valid = HEAP_IsRealArena( heapPtr, NULL, NOISY );
+        valid = heap_validate( heapPtr, NOISY );
         heap_unlock( heapPtr, 0 );

if (!valid && TRACE_ON(heap))
@@ -1344,44 +1344,32 @@ static BOOL HEAP_ValidateInUseArena( const SUBHEAP *subheap, const ARENA_INUSE *
 }

-/***********************************************************************
- *           HEAP_IsRealArena  [Internal]
- * Validates a block is a valid arena.
- *
- * RETURNS
- *	TRUE: Success
- *	FALSE: Failure
- */
-static BOOL HEAP_IsRealArena( HEAP *heap,   /* [in] ptr to the heap */
-              LPCVOID block, /* [in] Optional pointer to memory block to validate */
-              BOOL quiet )   /* [in] Flag - if true, HEAP_ValidateInUseArena
-                              *             does not complain    */
+static BOOL heap_validate_ptr( HEAP *heap, const void *ptr )
 {
-    SUBHEAP *subheap;
+    const ARENA_INUSE *arena = (const ARENA_INUSE *)ptr - 1;
     const ARENA_LARGE *large_arena;
+    SUBHEAP *subheap;

-    if (block)  /* only check this single memory block */
+    if (!(subheap = HEAP_FindSubHeap( heap, arena )) ||
+        ((const char *)arena < (char *)subheap->base + subheap->headerSize))
     {
-        const ARENA_INUSE *arena = (const ARENA_INUSE *)block - 1;
-
-        if (!(subheap = HEAP_FindSubHeap( heap, arena )) ||
-            ((const char *)arena < (char *)subheap->base + subheap->headerSize))
+        if (!(large_arena = find_large_block( heap, ptr )))
         {
-            if (!(large_arena = find_large_block( heap, block )))
-            {
-                if (quiet == NOISY)
-                    ERR("Heap %p: block %p is not inside heap\n", heap, block );
-                else if (WARN_ON(heap))
-                    WARN("Heap %p: block %p is not inside heap\n", heap, block );
-                return FALSE;
-            }
-
-            return validate_large_arena( heap, large_arena, quiet );
+            if (WARN_ON(heap)) WARN("heap %p, ptr %p is not inside heap\n", heap, ptr );
+            return FALSE;
         }

-        return HEAP_ValidateInUseArena( subheap, arena, quiet );
+        return validate_large_arena( heap, large_arena, QUIET );
     }

+    return HEAP_ValidateInUseArena( subheap, arena, QUIET );
+}
+
+static BOOL heap_validate( HEAP *heap, BOOL quiet )
+{
+    const ARENA_LARGE *large_arena;
+    SUBHEAP *subheap;
+
     LIST_FOR_EACH_ENTRY( subheap, &heap->subheap_list, SUBHEAP, entry )
     {
         char *ptr = (char *)subheap->base + subheap->headerSize;
@@ -2087,7 +2075,8 @@ BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr )
     else
     {
         heap_lock( heapPtr, flags );
-        ret = HEAP_IsRealArena( heapPtr, ptr, QUIET );
+        if (ptr) ret = heap_validate_ptr( heapPtr, ptr );
+        else ret = heap_validate( heapPtr, QUIET );
         heap_unlock( heapPtr, flags );
     }

[PATCH 3/6] ntdll: Split RtlAllocateHeap to a separate heap_allocate helper.

[Rémi Bernon]

From: Rémi Bernon

Factoring out locking and status handling.

Signed-off-by: Rémi Bernon 
---
 dlls/kernel32/tests/heap.c |  1 -
 dlls/ntdll/heap.c          | 84 ++++++++++++++++----------------------
 2 files changed, 36 insertions(+), 49 deletions(-)

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index cc02140f453..2628da9cec3 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -482,7 +482,6 @@ static void test_HeapCreate(void)
     SetLastError( 0xdeadbeef );
     ptr1 = HeapAlloc( heap, 0, 4 * alloc_size );
     ok( !ptr1, "HeapAlloc succeeded\n" );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_ENOUGH_MEMORY, "got error %lu\n", GetLastError() );
     ret = HeapFree( heap, 0, ptr1 );
     ok( ret, "HeapFree failed, error %lu\n", GetLastError() );
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index cd7e44cc717..f3dd6ab6c67 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -349,6 +349,12 @@ static void heap_unlock( HEAP *heap, ULONG flags )
     RtlLeaveCriticalSection( &heap->cs );
 }

+static void heap_set_status( const HEAP *heap, ULONG flags, NTSTATUS status )
+{
+    if (status == STATUS_NO_MEMORY && (flags & HEAP_GENERATE_EXCEPTIONS)) RtlRaiseStatus( status );
+    if (status) RtlSetLastWin32ErrorAndNtStatusFromNtStatus( status );
+}
+
 /***********************************************************************
  *           HEAP_Dump
  */
@@ -1648,66 +1654,27 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE heap )
     return 0;
 }

-
-/***********************************************************************
- *           RtlAllocateHeap   (NTDLL.@)
- *
- * Allocate a memory block from a Heap.
- *
- * PARAMS
- *  heap  [I] Heap to allocate block from
- *  flags [I] HEAP_ flags from "winnt.h"
- *  size  [I] Size of the memory block to allocate
- *
- * RETURNS
- *  Success: A pointer to the newly allocated block
- *  Failure: NULL.
- *
- * NOTES
- *  This call does not SetLastError().
- */
-void * WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_T size )
+static NTSTATUS heap_allocate( HEAP *heap, ULONG flags, SIZE_T size, void **ret )
 {
     ARENA_FREE *pArena;
     ARENA_INUSE *pInUse;
     SUBHEAP *subheap;
-    HEAP *heapPtr = HEAP_GetPtr( heap );
     SIZE_T rounded_size;

-    /* Validate the parameters */
-
-    if (!heapPtr) return NULL;
-    flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY;
-    flags |= heapPtr->flags;
     rounded_size = ROUND_SIZE(size) + HEAP_TAIL_EXTRA_SIZE( flags );
-    if (rounded_size < size)  /* overflow */
-    {
-        if (flags & HEAP_GENERATE_EXCEPTIONS) RtlRaiseStatus( STATUS_NO_MEMORY );
-        return NULL;
-    }
-    if (rounded_size < HEAP_MIN_DATA_SIZE) rounded_size = HEAP_MIN_DATA_SIZE;

-    heap_lock( heapPtr, flags );
+    if (rounded_size < size) return STATUS_NO_MEMORY;  /* overflow */
+    if (rounded_size < HEAP_MIN_DATA_SIZE) rounded_size = HEAP_MIN_DATA_SIZE;

if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE && (flags & HEAP_GROWABLE))
     {
-        void *ret = allocate_large_block( heap, flags, size );
-        heap_unlock( heapPtr, flags );
-        if (!ret && (flags & HEAP_GENERATE_EXCEPTIONS)) RtlRaiseStatus( STATUS_NO_MEMORY );
-        TRACE("(%p,%08x,%08lx): returning %p\n", heap, flags, size, ret );
-        return ret;
+        if (!(*ret = allocate_large_block( heap, flags, size ))) return STATUS_NO_MEMORY;
+        return STATUS_SUCCESS;
     }

/* Locate a suitable free block */

-    if (!(pArena = HEAP_FindFreeBlock( heapPtr, rounded_size, &subheap )))
-    {
-        TRACE("(%p,%08x,%08lx): returning NULL\n",
-                  heap, flags, size  );
-        heap_unlock( heapPtr, flags );
-        if (flags & HEAP_GENERATE_EXCEPTIONS) RtlRaiseStatus( STATUS_NO_MEMORY );
-        return NULL;
-    }
+    if (!(pArena = HEAP_FindFreeBlock( heap, rounded_size, &subheap ))) return STATUS_NO_MEMORY;

/* Remove the arena from the free list */

@@ -1730,10 +1697,31 @@ void * WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_
     notify_alloc( pInUse + 1, size, flags & HEAP_ZERO_MEMORY );
     initialize_block( pInUse + 1, size, pInUse->unused_bytes, flags );

-    heap_unlock( heapPtr, flags );
+    *ret = pInUse + 1;
+    return STATUS_SUCCESS;
+}
+
+/***********************************************************************
+ *           RtlAllocateHeap   (NTDLL.@)
+ */
+void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_T size )
+{
+    void *ptr = NULL;
+    NTSTATUS status;
+    HEAP *heapPtr;
+
+    if (!(heapPtr = HEAP_GetPtr( heap )))
+        status = STATUS_INVALID_HANDLE;
+    else
+    {
+        heap_lock( heapPtr, flags );
+        status = heap_allocate( heapPtr, heap_get_flags( heapPtr, flags ), size, &ptr );
+        heap_unlock( heapPtr, flags );
+    }

-    TRACE("(%p,%08x,%08lx): returning %p\n", heap, flags, size, pInUse + 1 );
-    return pInUse + 1;
+    TRACE( "heap %p, flags %#x, size %#Ix, return %p, status %#x.\n", heap, flags, size, ptr, status );
+    heap_set_status( heapPtr, flags, status );
+    return ptr;
 }

[PATCH 3/6] ntdll: Split RtlAllocateHeap to a separate heap_allocate helper.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=114429

Your paranoid android.

=== debian11 (32 bit report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Arabic:Morocco report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit German report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit French report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Hebrew:Israel report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Hindi:India report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Japanese:Japan report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Chinese:China report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

ntdll:
virtual: Timeout

=== debian11 (32 bit WoW report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

=== debian11 (64 bit WoW report) ===

kernel32:
heap.c:418: Test succeeded inside todo block: got error 8

[PATCH 4/6] ntdll: Split RtlFreeHeap to a separate heap_free helper.

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/kernel32/tests/heap.c |  2 ++
 dlls/ntdll/heap.c          | 71 +++++++++++++++-----------------------
 2 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index 2628da9cec3..8a7260303d3 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -229,6 +229,8 @@ static void test_HeapCreate(void)

/* test some border cases */

+    ret = HeapFree( NULL, 0, NULL );
+    ok( ret, "HeapFree failed, error %lu\n", GetLastError() );
     ret = HeapFree( heap, 0, NULL );
     ok( ret, "HeapFree failed, error %lu\n", GetLastError() );
 #if 0 /* crashes */
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index f3dd6ab6c67..1a4d5b1a2bb 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -1725,60 +1725,45 @@ void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_T
 }

-/***********************************************************************
- *           RtlFreeHeap   (NTDLL.@)
- *
- * Free a memory block allocated with RtlAllocateHeap().
- *
- * PARAMS
- *  heap  [I] Heap that block was allocated from
- *  flags [I] HEAP_ flags from "winnt.h"
- *  ptr   [I] Block to free
- *
- * RETURNS
- *  Success: TRUE, if ptr is NULL or was freed successfully.
- *  Failure: FALSE.
- */
-BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *ptr )
+static NTSTATUS heap_free( HEAP *heap, void *ptr )
 {
-    ARENA_INUSE *pInUse;
+    ARENA_INUSE *block;
     SUBHEAP *subheap;
-    HEAP *heapPtr;

-    /* Validate the parameters */
+    /* Inform valgrind we are trying to free memory, so it can throw up an error message */
+    notify_free( ptr );

-    if (!ptr) return TRUE;  /* freeing a NULL ptr isn't an error in Win2k */
+    block = (ARENA_INUSE *)ptr - 1;
+    if (!validate_block_pointer( heap, &subheap, block )) return STATUS_INVALID_PARAMETER;

-    heapPtr = HEAP_GetPtr( heap );
-    if (!heapPtr)
-    {
-        RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_HANDLE );
-        return FALSE;
-    }
+    if (!subheap) free_large_block( heap, ptr );
+    else HEAP_MakeInUseBlockFree( subheap, block );

-    heap_lock( heapPtr, flags );
+    return STATUS_SUCCESS;
+}

-    /* Inform valgrind we are trying to free memory, so it can throw up an error message */
-    notify_free( ptr );
+/***********************************************************************
+ *           RtlFreeHeap   (NTDLL.@)
+ */
+BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *ptr )
+{
+    NTSTATUS status;
+    HEAP *heapPtr;

-    /* Some sanity checks */
-    pInUse  = (ARENA_INUSE *)ptr - 1;
-    if (!validate_block_pointer( heapPtr, &subheap, pInUse )) goto error;
+    if (!ptr) return TRUE;

-    if (!subheap)
-        free_large_block( heapPtr, ptr );
+    if (!(heapPtr = HEAP_GetPtr( heap )))
+        status = STATUS_INVALID_PARAMETER;
     else
-        HEAP_MakeInUseBlockFree( subheap, pInUse );
-
-    heap_unlock( heapPtr, flags );
-    TRACE("(%p,%08x,%p): returning TRUE\n", heap, flags, ptr );
-    return TRUE;
+    {
+        heap_lock( heapPtr, flags );
+        status = heap_free( heapPtr, ptr );
+        heap_unlock( heapPtr, flags );
+    }

-error:
-    heap_unlock( heapPtr, flags );
-    RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_PARAMETER );
-    TRACE("(%p,%08x,%p): returning FALSE\n", heap, flags, ptr );
-    return FALSE;
+    TRACE( "heap %p, flags %#x, ptr %p, return %u, status %#x.\n", heap, flags, ptr, !status, status );
+    heap_set_status( heapPtr, flags, status );
+    return !status;
 }

[PATCH 4/6] ntdll: Split RtlFreeHeap to a separate heap_free helper.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=114430

Your paranoid android.

=== debian11 (32 bit report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Arabic:Morocco report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit German report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit French report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Hebrew:Israel report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Hindi:India report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Japanese:Japan report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit Chinese:China report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (32 bit WoW report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

=== debian11 (64 bit WoW report) ===

kernel32:
heap.c:420: Test succeeded inside todo block: got error 8

[PATCH 5/6] ntdll: Split RtlReAllocateHeap to a separate heap_reallocate helper.

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/kernel32/tests/heap.c |   1 +
 dlls/ntdll/heap.c          | 100 +++++++++++++++----------------------
 2 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index 8a7260303d3..45a22adcf2e 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -413,6 +413,7 @@ static void test_HeapCreate(void)
     /* shrinking a very large block decommits pages and fail to grow in place */
     ptr1 = HeapReAlloc( heap, HEAP_REALLOC_IN_PLACE_ONLY, ptr, alloc_size * 3 / 2 );
     ok( ptr1 == ptr, "HeapReAlloc HEAP_REALLOC_IN_PLACE_ONLY failed, error %lu\n", GetLastError() );
+    SetLastError( 0xdeadbeef );
     ptr1 = HeapReAlloc( heap, HEAP_REALLOC_IN_PLACE_ONLY, ptr, 2 * alloc_size );
     todo_wine
     ok( ptr1 != ptr, "HeapReAlloc HEAP_REALLOC_IN_PLACE_ONLY succeeded\n" );
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index 1a4d5b1a2bb..2d03cd59729 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -333,7 +333,7 @@ static RTL_CRITICAL_SECTION_DEBUG process_heap_cs_debug =

static inline ULONG heap_get_flags( const HEAP *heap, ULONG flags )
 {
-    flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY;
+    flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY | HEAP_REALLOC_IN_PLACE_ONLY;
     return heap->flags | flags;
 }

@@ -1767,53 +1767,23 @@ BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *pt
 }

-/***********************************************************************
- *           RtlReAllocateHeap   (NTDLL.@)
- *
- * Change the size of a memory block allocated with RtlAllocateHeap().
- *
- * PARAMS
- *  heap  [I] Heap that block was allocated from
- *  flags [I] HEAP_ flags from "winnt.h"
- *  ptr   [I] Block to resize
- *  size  [I] Size of the memory block to allocate
- *
- * RETURNS
- *  Success: A pointer to the resized block (which may be different).
- *  Failure: NULL.
- */
-PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size )
+static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size, void **ret )
 {
     ARENA_INUSE *pArena;
-    HEAP *heapPtr;
     SUBHEAP *subheap;
     SIZE_T oldBlockSize, oldActualSize, rounded_size;
-    void *ret;
-
-    if (!ptr) return NULL;
-    if (!(heapPtr = HEAP_GetPtr( heap )))
-    {
-        RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_HANDLE );
-        return NULL;
-    }
-
-    /* Validate the parameters */
-
-    flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY |
-             HEAP_REALLOC_IN_PLACE_ONLY;
-    flags |= heapPtr->flags;
-    heap_lock( heapPtr, flags );

rounded_size = ROUND_SIZE(size) + HEAP_TAIL_EXTRA_SIZE(flags);
-    if (rounded_size < size) goto oom;  /* overflow */
+    if (rounded_size < size) return STATUS_NO_MEMORY;  /* overflow */
     if (rounded_size < HEAP_MIN_DATA_SIZE) rounded_size = HEAP_MIN_DATA_SIZE;

pArena = (ARENA_INUSE *)ptr - 1;
-    if (!validate_block_pointer( heapPtr, &subheap, pArena )) goto error;
+    if (!validate_block_pointer( heap, &subheap, pArena )) return STATUS_INVALID_PARAMETER;
+
     if (!subheap)
     {
-        if (!(ret = realloc_large_block( heapPtr, flags, ptr, size ))) goto oom;
-        goto done;
+        if (!(*ret = realloc_large_block( heap, flags, ptr, size ))) return STATUS_NO_MEMORY;
+        return STATUS_SUCCESS;
     }

/* Check if we need to grow the block */
@@ -1826,12 +1796,12 @@ PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size

if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE && (flags & HEAP_GROWABLE))
         {
-            if (flags & HEAP_REALLOC_IN_PLACE_ONLY) goto oom;
-            if (!(ret = allocate_large_block( heapPtr, flags, size ))) goto oom;
-            memcpy( ret, pArena + 1, oldActualSize );
+            if (flags & HEAP_REALLOC_IN_PLACE_ONLY) return STATUS_NO_MEMORY;
+            if (!(*ret = allocate_large_block( heap, flags, size ))) return STATUS_NO_MEMORY;
+            memcpy( *ret, pArena + 1, oldActualSize );
             notify_free( pArena + 1 );
             HEAP_MakeInUseBlockFree( subheap, pArena );
-            goto done;
+            return STATUS_SUCCESS;
         }
         if ((pNext < (char *)subheap->base + subheap->size) &&
             (*(DWORD *)pNext & ARENA_FLAG_FREE) &&
@@ -1841,7 +1811,7 @@ PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size
             ARENA_FREE *pFree = (ARENA_FREE *)pNext;
             list_remove( &pFree->entry );
             pArena->size += (pFree->size & ARENA_SIZE_MASK) + sizeof(*pFree);
-            if (!HEAP_Commit( subheap, pArena, rounded_size )) goto oom;
+            if (!HEAP_Commit( subheap, pArena, rounded_size )) return STATUS_NO_MEMORY;
             notify_realloc( pArena + 1, oldActualSize, size );
             HEAP_ShrinkBlock( subheap, pArena, rounded_size );
         }
@@ -1851,9 +1821,8 @@ PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size
             ARENA_INUSE *pInUse;
             SUBHEAP *newsubheap;

-            if ((flags & HEAP_REALLOC_IN_PLACE_ONLY) ||
-                !(pNew = HEAP_FindFreeBlock( heapPtr, rounded_size, &newsubheap )))
-                goto oom;
+            if (flags & HEAP_REALLOC_IN_PLACE_ONLY) return STATUS_NO_MEMORY;
+            if (!(pNew = HEAP_FindFreeBlock( heap, rounded_size, &newsubheap ))) return STATUS_NO_MEMORY;

/* Build the in-use arena */

@@ -1894,24 +1863,33 @@ PVOID WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, PVOID ptr, SIZE_T size

/* Return the new arena */

-    ret = pArena + 1;
-done:
-    heap_unlock( heapPtr, flags );
-    TRACE("(%p,%08x,%p,%08lx): returning %p\n", heap, flags, ptr, size, ret );
-    return ret;
+    *ret = pArena + 1;
+    return STATUS_SUCCESS;
+}

-oom:
-    heap_unlock( heapPtr, flags );
-    if (flags & HEAP_GENERATE_EXCEPTIONS) RtlRaiseStatus( STATUS_NO_MEMORY );
-    RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_NO_MEMORY );
-    TRACE("(%p,%08x,%p,%08lx): returning NULL\n", heap, flags, ptr, size );
-    return NULL;
+/***********************************************************************
+ *           RtlReAllocateHeap   (NTDLL.@)
+ */
+void *WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, void *ptr, SIZE_T size )
+{
+    void *ret = NULL;
+    NTSTATUS status;
+    HEAP *heapPtr;

-error:
-    heap_unlock( heapPtr, flags );
-    RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_PARAMETER );
-    TRACE("(%p,%08x,%p,%08lx): returning NULL\n", heap, flags, ptr, size );
-    return NULL;
+    if (!ptr) return NULL;
+
+    if (!(heapPtr = HEAP_GetPtr( heap )))
+        status = STATUS_INVALID_HANDLE;
+    else
+    {
+        heap_lock( heapPtr, flags );
+        status = heap_reallocate( heapPtr, heap_get_flags( heapPtr, flags ), ptr, size, &ret );
+        heap_unlock( heapPtr, flags );
+    }
+
+    TRACE( "heap %p, flags %#x, ptr %p, size %#Ix, return %p, status %#x.\n", heap, flags, ptr, size, ret, status );
+    heap_set_status( heap, flags, status );
+    return ret;
 }

[PATCH 6/6] ntdll: Split RtlSizeHeap to a separate heap_size helper.

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/ntdll/heap.c | 71 ++++++++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 41 deletions(-)

diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index 2d03cd59729..51567d0552b 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -1958,58 +1958,47 @@ BOOLEAN WINAPI RtlUnlockHeap( HANDLE heap )
 }

-/***********************************************************************
- *           RtlSizeHeap   (NTDLL.@)
- *
- * Get the actual size of a memory block allocated from a Heap.
- *
- * PARAMS
- *  heap  [I] Heap that block was allocated from
- *  flags [I] HEAP_ flags from "winnt.h"
- *  ptr   [I] Block to get the size of
- *
- * RETURNS
- *  Success: The size of the block.
- *  Failure: -1, heap or ptr are invalid.
- *
- * NOTES
- *  The size may be bigger than what was passed to RtlAllocateHeap().
- */
-SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr )
+static NTSTATUS heap_size( HEAP *heap, const void *ptr, SIZE_T *size )
 {
-    SIZE_T ret;
-    const ARENA_INUSE *pArena;
+    const ARENA_INUSE *block;
     SUBHEAP *subheap;
-    HEAP *heapPtr = HEAP_GetPtr( heap );
-
-    if (!heapPtr)
-    {
-        RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_HANDLE );
-        return ~(SIZE_T)0;
-    }
-
-    heap_lock( heapPtr, flags );

-    pArena = (const ARENA_INUSE *)ptr - 1;
-    if (!validate_block_pointer( heapPtr, &subheap, pArena ))
-    {
-        RtlSetLastWin32ErrorAndNtStatusFromNtStatus( STATUS_INVALID_PARAMETER );
-        ret = ~(SIZE_T)0;
-    }
-    else if (!subheap)
+    block = (const ARENA_INUSE *)ptr - 1;
+    if (!validate_block_pointer( heap, &subheap, block )) return STATUS_INVALID_PARAMETER;
+    if (!subheap)
     {
         const ARENA_LARGE *large_arena = (const ARENA_LARGE *)ptr - 1;
-        ret = large_arena->data_size;
+        *size = large_arena->data_size;
     }
     else
     {
-        ret = (pArena->size & ARENA_SIZE_MASK) - pArena->unused_bytes;
+        *size = (block->size & ARENA_SIZE_MASK) - block->unused_bytes;
     }

-    heap_unlock( heapPtr, flags );
+    return STATUS_SUCCESS;
+}

-    TRACE("(%p,%08x,%p): returning %08lx\n", heap, flags, ptr, ret );
-    return ret;
+/***********************************************************************
+ *           RtlSizeHeap   (NTDLL.@)
+ */
+SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr )
+{
+    SIZE_T size = ~(SIZE_T)0;
+    NTSTATUS status;
+    HEAP *heapPtr;
+
+    if (!(heapPtr = HEAP_GetPtr( heap )))
+        status = STATUS_INVALID_PARAMETER;
+    else
+    {
+        heap_lock( heapPtr, flags );
+        status = heap_size( heapPtr, ptr, &size );
+        heap_unlock( heapPtr, flags );
+    }
+
+    TRACE( "heap %p, flags %#x, ptr %p, return %#Ix, status %#x.\n", heap, flags, ptr, size, status );
+    heap_set_status( heapPtr, flags, status );
+    return size;
 }