[PATCH 0/5] MR201: ntdll: Implement RtlSetUserValueHeap.

[Rémi Bernon (＠rbernon)]

And prepare HLOCAL allocation to use it.

[PATCH 1/5] ole32/tests: Update IStream_SetSize with large size test result.

[Rémi Bernon]

From: Rémi Bernon

The high part is ignored, and OOM error is only returned if there's not
enough memory available, not because of GlobalReAlloc specific behavior.

Signed-off-by: Rémi Bernon 
---
 dlls/ole32/tests/hglobalstream.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/dlls/ole32/tests/hglobalstream.c b/dlls/ole32/tests/hglobalstream.c
index 237bcca9251..8310800a5c7 100644
--- a/dlls/ole32/tests/hglobalstream.c
+++ b/dlls/ole32/tests/hglobalstream.c
@@ -294,10 +294,9 @@ static void test_streamonhglobal(void)

/* test OOM condition */
     ull.u.HighPart = -1;
-    ull.u.LowPart = -1;
+    ull.u.LowPart = 0;
     hr = IStream_SetSize(pStream, ull);
-    ok(hr == E_OUTOFMEMORY || broken(hr == S_OK), /* win9x */
-       "IStream_SetSize with large size should have returned E_OUTOFMEMORY instead of 0x%08lx\n", hr);
+    ok(hr == S_OK, "IStream_SetSize with large size should have returned S_OK instead of 0x%08lx\n", hr);

IStream_Release(pStream);
 }

[PATCH 2/5] ntdll: Add block padding when HEAP_ADD_USER_INFO flag is set.

[Rémi Bernon]

From: Rémi Bernon

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

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index 91f582f36de..97f4d9c9fe3 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -2277,14 +2277,13 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags
         expect_size = max( alloc_size, 2 * sizeof(void *) );
         expect_size = ALIGN_BLOCK_SIZE( expect_size + extra_size + 2 * sizeof(void *) );
         diff = min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) );
-        todo_wine
         ok( diff == expect_size, "got diff %#Ix\n", diff );

ok( !memcmp( ptr0 + alloc_size, tail_buf, tail_size ), "missing block tail\n" );
         ok( !memcmp( ptr1 + alloc_size, tail_buf, tail_size ), "missing block tail\n" );
         ok( !memcmp( ptr2 + alloc_size, tail_buf, tail_size ), "missing block tail\n" );

-        todo_wine
+        todo_wine_if( global_flags & FLG_HEAP_ENABLE_FREE_CHECK )
         ok( !memcmp( ptr0 + alloc_size + tail_size, padd_buf, 2 * sizeof(void *) ), "unexpected padding\n" );

tmp_ptr = (void *)0xdeadbeef;
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index 0ecff6f6cba..6d9f949b6a9 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -212,6 +212,7 @@ C_ASSERT( offsetof(HEAP, subheap) <= COMMIT_MASK );

/* some undocumented flags (names are made up) */
 #define HEAP_PRIVATE          0x00001000
+#define HEAP_ADD_USER_INFO    0x00000100
 #define HEAP_PAGE_ALLOCS      0x01000000
 #define HEAP_VALIDATE         0x10000000
 #define HEAP_VALIDATE_ALL     0x20000000
@@ -451,7 +452,7 @@ static RTL_CRITICAL_SECTION_DEBUG process_heap_cs_debug =
 static inline ULONG heap_get_flags( const HEAP *heap, ULONG flags )
 {
     if (flags & (HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED)) flags |= HEAP_CHECKING_ENABLED;
-    flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY | HEAP_REALLOC_IN_PLACE_ONLY | HEAP_CHECKING_ENABLED;
+    flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY | HEAP_REALLOC_IN_PLACE_ONLY | HEAP_CHECKING_ENABLED | HEAP_ADD_USER_INFO;
     return heap->flags | flags;
 }

@@ -1476,7 +1477,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE heap )

static SIZE_T heap_get_block_size( HEAP *heap, ULONG flags, SIZE_T size )
 {
-    static const ULONG padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS;
+    static const ULONG padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS | HEAP_ADD_USER_INFO;
     static const ULONG check_flags = HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED | HEAP_CHECKING_ENABLED;
     SIZE_T overhead;

[PATCH 3/5] kernelbase: Use HEAP_ADD_USER_INFO for HLOCAL allocations.

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/kernelbase/memory.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index 736d3642995..776f0aafa18 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -713,6 +713,9 @@ BOOL WINAPI DECLSPEC_HOTPATCH HeapWalk( HANDLE heap, PROCESS_HEAP_ENTRY *entry )
  * Global/local heap functions
  ***********************************************************************/

+/* some undocumented flags (names are made up) */
+#define HEAP_ADD_USER_INFO    0x00000100
+
 /* not compatible with windows */
 struct kernelbase_global_data
 {
@@ -826,15 +829,15 @@ HGLOBAL WINAPI DECLSPEC_HOTPATCH GlobalFree( HLOCAL handle )
  */
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size )
 {
+    DWORD heap_flags = HEAP_ADD_USER_INFO;
     HANDLE heap = GetProcessHeap();
     struct mem_entry *mem;
-    DWORD heap_flags = 0;
     HLOCAL handle;
     void *ptr;

TRACE_(globalmem)( "flags %#x, size %#Ix\n", flags, size );

-    if (flags & LMEM_ZEROINIT) heap_flags = HEAP_ZERO_MEMORY;
+    if (flags & LMEM_ZEROINIT) heap_flags |= HEAP_ZERO_MEMORY;

if (!(flags & LMEM_MOVEABLE)) /* pointer */
     {
@@ -964,13 +967,15 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
  */
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT flags )
 {
+    DWORD heap_flags = HEAP_ADD_USER_INFO;
     struct mem_entry *mem;
     HLOCAL ret = 0;
-    DWORD heap_flags = (flags & LMEM_ZEROINIT) ? HEAP_ZERO_MEMORY : 0;
     void *ptr;

TRACE_(globalmem)( "handle %p, size %#Ix, flags %#x\n", handle, size, flags );

+    if (flags & LMEM_ZEROINIT) heap_flags |= HEAP_ZERO_MEMORY;
+
     RtlLockHeap( GetProcessHeap() );
     if (flags & LMEM_MODIFY) /* modify flags */
     {

[PATCH 4/5] kernelbase: Cleanup and simplify LocalReAlloc.

[Rémi Bernon]

From: Rémi Bernon

Signed-off-by: Rémi Bernon 
---
 dlls/kernel32/heap.c       |   2 +
 dlls/kernel32/tests/heap.c |  11 ----
 dlls/kernelbase/memory.c   | 111 ++++++++++++++-----------------------
 3 files changed, 45 insertions(+), 79 deletions(-)

diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c
index df094c94257..6a1a02c814b 100644
--- a/dlls/kernel32/heap.c
+++ b/dlls/kernel32/heap.c
@@ -315,6 +315,8 @@ HGLOBAL WINAPI GlobalHandle( const void *ptr )
  */
 HGLOBAL WINAPI GlobalReAlloc( HGLOBAL handle, SIZE_T size, UINT flags )
 {
+    struct mem_entry *mem;
+    if ((mem = unsafe_mem_from_HLOCAL( handle )) && mem->lock) return 0;
     return LocalReAlloc( handle, size, flags );
 }

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index 97f4d9c9fe3..c237d515648 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -1635,9 +1635,7 @@ static void test_GlobalAlloc(void)
     mem = GlobalAlloc( GMEM_FIXED, 10 );
     ok( !!mem, "GlobalAlloc failed, error %lu\n", GetLastError() );
     tmp_mem = GlobalReAlloc( mem, 9, GMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "GlobalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = GlobalSize( mem );
     ok( size == 10, "GlobalSize returned %Iu\n", size );
@@ -1651,9 +1649,7 @@ static void test_GlobalAlloc(void)
         "got error %lu\n", GetLastError() );
     if (tmp_mem) mem = tmp_mem;
     tmp_mem = GlobalReAlloc( mem, 1024 * 1024, GMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "GlobalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = GlobalSize( mem );
     ok( size == 10, "GlobalSize returned %Iu\n", size );
@@ -1988,9 +1984,7 @@ static void test_LocalAlloc(void)
     mem = LocalAlloc( LMEM_FIXED, 10 );
     ok( !!mem, "LocalAlloc failed, error %lu\n", GetLastError() );
     tmp_mem = LocalReAlloc( mem, 9, LMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "LocalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = LocalSize( mem );
     ok( size == 10, "LocalSize returned %Iu\n", size );
@@ -2004,9 +1998,7 @@ static void test_LocalAlloc(void)
         "got error %lu\n", GetLastError() );
     if (tmp_mem) mem = tmp_mem;
     tmp_mem = LocalReAlloc( mem, 1024 * 1024, LMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "LocalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = LocalSize( mem );
     ok( size == 10, "LocalSize returned %Iu\n", size );
@@ -2105,13 +2097,10 @@ static void test_LocalAlloc(void)
     tmp_mem = LocalHandle( ptr );
     ok( tmp_mem == mem, "LocalHandle returned unexpected handle\n" );
     tmp_mem = LocalDiscard( mem );
-    todo_wine
     ok( !!tmp_mem, "LocalDiscard failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "LocalDiscard returned unexpected handle\n" );
     ret = LocalUnlock( mem );
     ok( !ret, "LocalUnlock succeeded, error %lu\n", GetLastError() );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_LOCKED, "got error %lu\n", GetLastError() );

tmp_mem = LocalDiscard( mem );
diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index 776f0aafa18..341b859b555 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -868,6 +868,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size )
     if (!size) mem->flags |= MEM_FLAG_DISCARDED;
     else
     {
+        if (size + HLOCAL_STORAGE < size) goto failed;
         if (!(ptr = HeapAlloc( heap, heap_flags, size + HLOCAL_STORAGE ))) goto failed;
         *(HLOCAL *)ptr = handle;
         mem->ptr = (char *)ptr + HLOCAL_STORAGE;
@@ -967,7 +968,8 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
  */
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT flags )
 {
-    DWORD heap_flags = HEAP_ADD_USER_INFO;
+    DWORD heap_flags = HEAP_ADD_USER_INFO | HEAP_NO_SERIALIZE;
+    HANDLE heap = GetProcessHeap();
     struct mem_entry *mem;
     HLOCAL ret = 0;
     void *ptr;
@@ -976,90 +978,63 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f

if (flags & LMEM_ZEROINIT) heap_flags |= HEAP_ZERO_MEMORY;

-    RtlLockHeap( GetProcessHeap() );
-    if (flags & LMEM_MODIFY) /* modify flags */
+    RtlLockHeap( heap );
+    if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
     {
-        if (unsafe_ptr_from_HLOCAL( handle ) && (flags & LMEM_MOVEABLE))
+        if (!(flags & LMEM_MOVEABLE))
         {
-            /* make a fixed block moveable
-             * actually only NT is able to do this. But it's soo simple
-             */
-            if (handle == 0)
-            {
-                WARN_(globalmem)( "null handle\n" );
-                SetLastError( ERROR_NOACCESS );
-            }
-            else
-            {
-                size = RtlSizeHeap( GetProcessHeap(), 0, handle );
-                ret = LocalAlloc( flags, size );
-                ptr = LocalLock( ret );
-                memcpy( ptr, handle, size );
-                LocalUnlock( ret );
-                LocalFree( handle );
-            }
+            heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY;
+            if (!(flags & LMEM_MODIFY) || !(flags & LMEM_MOVEABLE)) ret = handle;
+            else ret = HeapReAlloc( heap, heap_flags, ptr, size );
         }
-        else if ((mem = unsafe_mem_from_HLOCAL( handle )) && (flags & LMEM_DISCARDABLE))
+        else if (flags & LMEM_MODIFY)
         {
-            /* change the flags to make our block "discardable" */
-            mem->flags |= LMEM_DISCARDABLE >> 8;
-            ret = handle;
+            size = RtlSizeHeap( heap, 0, handle );
+            ret = LocalAlloc( flags, size );
+            ptr = LocalLock( ret );
+            memcpy( ptr, handle, size );
+            LocalUnlock( ret );
+            LocalFree( handle );
         }
-        else SetLastError( ERROR_INVALID_PARAMETER );
+        else ret = HeapReAlloc( heap, heap_flags, handle, size );
     }
-    else
+    else if ((mem = unsafe_mem_from_HLOCAL( handle )))
     {
-        if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
+        if (!(flags & LMEM_MODIFY))
         {
-            /* reallocate fixed memory */
-            if (!(flags & LMEM_MOVEABLE)) heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY;
-            ret = HeapReAlloc( GetProcessHeap(), heap_flags, ptr, size );
-        }
-        else if ((mem = unsafe_mem_from_HLOCAL( handle )))
-        {
-            /* reallocate a moveable block */
-            if (size != 0)
+            if (size)
             {
-                if (size <= INT_MAX - HLOCAL_STORAGE)
+                if (size + HLOCAL_STORAGE < size) ptr = NULL;
+                else if (!mem->ptr) ptr = HeapAlloc( heap, heap_flags, size + HLOCAL_STORAGE );
+                else ptr = HeapReAlloc( heap, heap_flags, (char *)mem->ptr - HLOCAL_STORAGE, size + HLOCAL_STORAGE );
+
+                if (!ptr) SetLastError( ERROR_NOT_ENOUGH_MEMORY );
+                else
                 {
-                    if (mem->ptr)
-                    {
-                        if ((ptr = HeapReAlloc( GetProcessHeap(), heap_flags, (char *)mem->ptr - HLOCAL_STORAGE,
-                                                size + HLOCAL_STORAGE )))
-                        {
-                            mem->ptr = (char *)ptr + HLOCAL_STORAGE;
-                            ret = handle;
-                        }
-                    }
-                    else
-                    {
-                        if ((ptr = HeapAlloc( GetProcessHeap(), heap_flags, size + HLOCAL_STORAGE )))
-                        {
-                            *(HLOCAL *)ptr = handle;
-                            mem->ptr = (char *)ptr + HLOCAL_STORAGE;
-                            ret = handle;
-                        }
-                    }
+                    mem->flags &= ~MEM_FLAG_DISCARDED;
+                    mem->ptr = (char *)ptr + HLOCAL_STORAGE;
+                    ret = handle;
                 }
-                else SetLastError( ERROR_OUTOFMEMORY );
             }
             else
             {
-                if (mem->lock == 0)
-                {
-                    if (mem->ptr)
-                    {
-                        HeapFree( GetProcessHeap(), 0, (char *)mem->ptr - HLOCAL_STORAGE );
-                        mem->ptr = NULL;
-                    }
-                    ret = handle;
-                }
-                else WARN_(globalmem)( "not freeing memory associated with locked handle\n" );
+                if (mem->ptr) HeapFree( heap, heap_flags, (char *)mem->ptr - HLOCAL_STORAGE );
+                mem->flags |= MEM_FLAG_DISCARDED;
+                mem->lock = 0;
+                mem->ptr = NULL;
+                ret = handle;
             }
         }
-        else SetLastError( ERROR_INVALID_HANDLE );
+        else if (flags & LMEM_DISCARDABLE)
+        {
+            mem->flags |= MEM_FLAG_DISCARDABLE;
+            ret = handle;
+        }
+        else SetLastError( ERROR_INVALID_PARAMETER );
     }
-    RtlUnlockHeap( GetProcessHeap() );
+    else SetLastError( ERROR_INVALID_HANDLE );
+    RtlUnlockHeap( heap );
+
     return ret;
 }

[PATCH 5/5] ntdll: Implement RtlSetUserValueHeap.

[Rémi Bernon]

From: Rémi Bernon

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

diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index c237d515648..d4022b0d949 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -354,9 +354,7 @@ static void test_HeapCreate(void)
         ok( !!ptrs[i], "HeapAlloc failed, error %lu\n", GetLastError() );
         align |= (UINT_PTR)ptrs[i];
     }
-    todo_wine_if( sizeof(void *) == 8 )
     ok( !(align & (8 * sizeof(void *) - 1)), "got wrong alignment\n" );
-    todo_wine_if( sizeof(void *) == 8 )
     ok( align & (8 * sizeof(void *)), "got wrong alignment\n" );
     for (i = 0; i < ARRAY_SIZE(ptrs); ++i)
     {
@@ -2217,7 +2215,7 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags
         ok( !!ptr2, "HeapAlloc failed, error %lu\n", GetLastError() );

align = (UINT_PTR)ptr0 | (UINT_PTR)ptr1 | (UINT_PTR)ptr2;
-        todo_wine_if( sizeof(void *) == 8 || alloc_size == 0x7efe9 )
+        todo_wine_if( alloc_size == 0x7efe9 )
         ok( !(align & (8 * sizeof(void *) - 1)), "wrong align\n" );

expect_size = max( alloc_size, 2 * sizeof(void *) );
@@ -2292,7 +2290,6 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags
         ok( tmp_flags == 0x200, "got flags %#lx\n", tmp_flags );

ret = pRtlSetUserValueHeap( heap, 0, ptr0, (void *)0xdeadbeef );
-        todo_wine
         ok( ret, "RtlSetUserValueHeap failed, error %lu\n", GetLastError() );
         SetLastError( 0xdeadbeef );
         ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0, 0x1000 );
@@ -2319,13 +2316,9 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags
             "got flags %#lx\n", tmp_flags );

user_ptr = (void **)(ptr0 + alloc_size + tail_size);
-        todo_wine
         ok( user_ptr[1] == (void *)0xdeadbeef, "unexpected user value\n" );
-        if (user_ptr[1] == (void *)0xdeadbeef)
-        {
-            user_ptr[0] = (void *)0xdeadbeef;
-            user_ptr[1] = (void *)0xdeadbee0;
-        }
+        user_ptr[0] = (void *)0xdeadbeef;
+        user_ptr[1] = (void *)0xdeadbee0;

tmp_ptr = NULL;
         tmp_flags = 0;
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index 6d9f949b6a9..64d4f6644af 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -105,10 +105,11 @@ C_ASSERT( sizeof(struct entry) == 2 * ALIGNMENT );

typedef struct
 {
+    SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)];
     struct list           entry;      /* entry in heap large blocks list */
     SIZE_T                data_size;  /* size of user data */
     SIZE_T                block_size; /* total size of virtual memory block */
-    DWORD                 pad[2];     /* padding to ensure 16-byte alignment of data */
+    void                 *user_value;
     DWORD                 size;       /* fields for compatibility with normal arenas */
     DWORD                 magic;      /* these must remain at the end of the structure */
 } ARENA_LARGE;
@@ -362,6 +363,12 @@ static inline void mark_block_tail( struct block *block, DWORD flags )
         memset( tail, ARENA_TAIL_FILLER, ALIGNMENT );
     }
     valgrind_make_noaccess( tail, ALIGNMENT );
+    if (flags & HEAP_ADD_USER_INFO)
+    {
+        if (flags & HEAP_TAIL_CHECKING_ENABLED || RUNNING_ON_VALGRIND) tail += ALIGNMENT;
+        valgrind_make_writable( tail + sizeof(void *), sizeof(void *) );
+        memset( tail + sizeof(void *), 0, sizeof(void *) );
+    }
 }

/* initialize contents of a newly created block of memory */
@@ -2001,10 +2008,31 @@ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE heap, ULONG flags, void *ptr, void **u
 /***********************************************************************
  *           RtlSetUserValueHeap    (NTDLL.@)
  */
-BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE heap, ULONG flags, void *ptr, void *user_value )
+BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void *user_value )
 {
-    FIXME( "heap %p, flags %#x, ptr %p, user_value %p stub!\n", heap, flags, ptr, user_value );
-    return FALSE;
+    ARENA_LARGE *large = (ARENA_LARGE *)ptr - 1;
+    struct block *block;
+    BOOLEAN ret = TRUE;
+    SUBHEAP *subheap;
+    HEAP *heap;
+    char *tmp;
+
+    TRACE( "handle %p, flags %#x, ptr %p, user_value %p.\n", handle, flags, ptr, user_value );
+
+    if (!(heap = HEAP_GetPtr( handle ))) return TRUE;
+
+    heap_lock( heap, flags );
+    if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) ret = FALSE;
+    else if (!subheap) large->user_value = user_value;
+    else
+    {
+        tmp = (char *)block + block_get_size( block ) - block->tail_size + sizeof(void *);
+        if ((heap_get_flags( heap, flags ) & HEAP_TAIL_CHECKING_ENABLED) || RUNNING_ON_VALGRIND) tmp += ALIGNMENT;
+        *(void **)tmp = user_value;
+    }
+    heap_unlock( heap, flags );
+
+    return ret;
 }

/***********************************************************************

[PATCH 5/5] ntdll: Implement RtlSetUserValueHeap.

[Zebediah Figura]

On 6/7/22 13:33, Rémi Bernon wrote:
> typedef struct
>   {
> - SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)];
>  struct list           entry;      /* entry in heap large blocks list */
>  SIZE_T                data_size;  /* size of user data */
>  SIZE_T                block_size; /* total size of virtual memory block */
> - DWORD                 pad[2];     /* padding to ensure 16-byte alignment of data */
> - void                 *user_value;
>  DWORD                 size;       /* fields for compatibility with normal arenas */
>  DWORD                 magic;      /* these must remain at the end of the structure */
> } ARENA_LARGE;

Doesn't this end up wasting space on 64 bits? I.e. the structure is 
already aligned to a multiple of 16 bytes without any padding.

(Also, somewhat out of curiosity, why move the member?)

[Rémi Bernon (＠rbernon)]

On Tue Jun  7 19:33:23 2022 +0000, **** wrote:
> Zebediah Figura replied on the mailing list:
> On 6/7/22 13:33, Rémi Bernon wrote:
> >   typedef struct
> >   {
> > +    SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)];
> >       struct list           entry;      /* entry in heap large blocks
> list */
> >       SIZE_T                data_size;  /* size of user data */
> >       SIZE_T                block_size; /* total size of virtual
> memory block */
> > -    DWORD                 pad[2];     /* padding to ensure 16-byte
> alignment of data */
> > +    void                 *user_value;
> >       DWORD                 size;       /* fields for compatibility
> with normal arenas */
> >       DWORD                 magic;      /* these must remain at the
> end of the structure */
> >   } ARENA_LARGE;
> Doesn't this end up wasting space on 64 bits? I.e. the structure is 
> already aligned to a multiple of 16 bytes without any padding.
> (Also, somewhat out of curiosity, why move the member?)

It makes the large block header size match native size, of `8 * sizeof(void *)`.

Moving the member makes the struct layout closer to SUBHEAP, and I think the same struct could be use for both later. Native has a concept of heap region, which seems to correspond to both large blocks and SUBHEAP at the same time. For instance only growable heap let you allocate large blocks.

[Zebediah Figura]

On 6/7/22 14:46, Rémi Bernon (@rbernon) wrote:
> On Tue Jun  7 19:33:23 2022 +0000, **** wrote:
> > Zebediah Figura replied on the mailing list:
> > On 6/7/22 13:33, Rémi Bernon wrote:
> > >    typedef struct
> > >    {
> > > +    SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)];
> > >        struct list           entry;      /* entry in heap large blocks
> > list */
> > >        SIZE_T                data_size;  /* size of user data */
> > >        SIZE_T                block_size; /* total size of virtual
> > memory block */
> > > -    DWORD                 pad[2];     /* padding to ensure 16-byte
> > alignment of data */
> > > +    void                 *user_value;
> > >        DWORD                 size;       /* fields for compatibility
> > with normal arenas */
> > >        DWORD                 magic;      /* these must remain at the
> > end of the structure */
> > >    } ARENA_LARGE;
> > Doesn't this end up wasting space on 64 bits? I.e. the structure is
> > already aligned to a multiple of 16 bytes without any padding.
> > (Also, somewhat out of curiosity, why move the member?)
>
> It makes the large block header size match native size, of `8 * sizeof(void *)`.
>
> Moving the member makes the struct layout closer to SUBHEAP, and I think the same struct could be use for both later. Native has a concept of heap region, which seems to correspond to both large blocks and SUBHEAP at the same time. For instance only growable heap let you allocate large blocks.

Okay, makes sense.

In the future I guess it'd be nice to see that spelled out in the patch, 
or perhaps ideally as separate patches; that'd help make things clearer.

[Alexandre Julliard (＠julliard)]

This is still causing test failures:

```
tools/runtest -q -P wine -T . -M user32.dll -p dlls/user32/tests/user32_test.exe clipboard && touch dlls/user32/tests/clipboard.ok
clipboard.c:853: Test failed: 0: 0: wrong len 1024
clipboard.c:844: Test failed: 0: 1: size 1024
clipboard.c:853: Test failed: 0: 2: wrong len 1024
clipboard.c:856: Test failed: 0: 3: wrong len 2048
clipboard.c:853: Test failed: 1: 0: wrong len 1024
clipboard.c:844: Test failed: 1: 1: size 1024
clipboard.c:853: Test failed: 1: 2: wrong len 1024
clipboard.c:856: Test failed: 1: 3: wrong len 2048
clipboard.c:856: Test failed: 2: 0: wrong len 1024
clipboard.c:844: Test failed: 2: 1: size 1024
clipboard.c:853: Test failed: 2: 2: wrong len 512
clipboard.c:853: Test failed: 2: 3: wrong len 512
clipboard.c:1644: Test failed: wrong size 1024
clipboard.c:1648: Test failed: wrong size 1024
clipboard.c:1652: Test failed: wrong size 1024
clipboard.c:1656: Test failed: wrong size 1024
clipboard.c:1644: Test failed: wrong size 1024
clipboard.c:1648: Test failed: wrong size 1024
clipboard.c:1652: Test failed: wrong size 1024
clipboard.c:1656: Test failed: wrong size 1024
clipboard.c:2293: Test failed: 0: wrong size 1024 / 3
clipboard.c:2296: Test failed: 0: wrong data fo
clipboard.c:2301: Test failed: 0: wrong size 2048 / 3
clipboard.c:2302: Test failed: 0: wrong data L"fo\0000"
clipboard.c:230: 4 failures in child process
clipboard.c:2293: Test failed: 1: wrong size 1024 / 4
clipboard.c:2296: Test failed: 1: wrong data foo
clipboard.c:2301: Test failed: 1: wrong size 2048 / 4
clipboard.c:2302: Test failed: 1: wrong data L"foo\0000"
clipboard.c:230: 4 failures in child process
clipboard.c:2293: Test failed: 2: wrong size 1024 / 7
clipboard.c:2296: Test failed: 2: wrong data foo
clipboard.c:2301: Test failed: 2: wrong size 2048 / 7
clipboard.c:2302: Test failed: 2: wrong data L"foo\0000ba\0000"
clipboard.c:230: 4 failures in child process
clipboard.c:2293: Test failed: 3: wrong size 1024 / 8
clipboard.c:2296: Test failed: 3: wrong data foo
clipboard.c:2301: Test failed: 3: wrong size 2048 / 8
clipboard.c:2302: Test failed: 3: wrong data L"foo\0000bar\0000"
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 4: wrong size 1024 / 6
clipboard.c:2312: Test failed: 4: wrong data L"fo\0000\0013\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 4: wrong size 512 / 3
clipboard.c:2323: Test failed: 4: wrong data fo
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 5: wrong size 1024 / 8
clipboard.c:2312: Test failed: 5: wrong data L"foo\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 5: wrong size 512 / 4
clipboard.c:2323: Test failed: 5: wrong data foo
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 6: wrong size 1024 / 14
clipboard.c:2312: Test failed: 6: wrong data L"foo\0000ba\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 6: wrong size 512 / 7
clipboard.c:2323: Test failed: 6: wrong data foo
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 7: wrong size 1024 / 16
clipboard.c:2312: Test failed: 7: wrong data L"foo\0000bar\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 7: wrong size 512 / 8
clipboard.c:2323: Test failed: 7: wrong data foo
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 8: wrong size 1024 / 1
clipboard.c:2312: Test failed: 8: wrong data L"\0000\0013\02a8\0013\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2327: Test failed: 8: got data for empty string
clipboard.c:230: 3 failures in child process
clipboard.c:2309: Test failed: 9: wrong size 1024 / 2
clipboard.c:2312: Test failed: 9: wrong data L"\0000\0013\02a8\0013\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 9: wrong size 512 / 1
clipboard.c:2323: Test failed: 9: wrong data 
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 10: wrong size 1024 / 5
clipboard.c:2312: Test failed: 10: wrong data L"fo\0200\0013\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 10: wrong size 512 / 2
clipboard.c:2323: Test failed: 10: wrong data fo?
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 11: wrong size 1024 / 7
clipboard.c:2312: Test failed: 11: wrong data L"foo\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 11: wrong size 512 / 3
clipboard.c:2323: Test failed: 11: wrong data foo
clipboard.c:230: 4 failures in child process
clipboard.c:2309: Test failed: 12: wrong size 1024 / 9
clipboard.c:2312: Test failed: 12: wrong data L"foo\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000\0000"...
clipboard.c:2322: Test failed: 12: wrong size 512 / 4
clipboard.c:2323: Test failed: 12: wrong data foo
clipboard.c:230: 4 failures in child process
make: *** [Makefile:120149: dlls/user32/tests/clipboard.ok] Error 71
```

[Rémi Bernon (＠rbernon)]

Crap, I tried to run the whole test suite but it didn't go too well. I skipped user32 as it crashes Xephyr, I need to find a better way. Thanks for the feedback, I'll have a look.

[PATCH 0/5] MR201: ntdll: Implement RtlSetUserValueHeap. - closed

[Rémi Bernon (＠rbernon)]

This merge request was closed by Rémi Bernon.