[PATCH] kernel32: implement Windows NT style GMEM_MOVEABLE/LMEM_MOVEABLE memory

[Damjan Jovanovic]

Tests were performed on many Windows versions. Wine's unit tests
cannot be used on Win9x any more; mingw's binaries require
symbols only found in newer versions of MSVCRT.DLL.
A GCC 2.95-based mingw had to be used, with a custom test
(available on request).

On Win9x, GlobalLock()/LocalLock() return pointers aligned
to 4 byte boundaries (never 8), as they call HeapAlloc()
which returns pointers aligned to 8 bytes, but then use the first
4 bytes to store the address of the HGLOBAL/HLOCAL.

On WinNT, GlobalLock()/LocalLock() return pointers aligned
to 8 byte boundaries. It is not known how
GlobalHandle()/LocalHandle() converts the pointer back to the
HGLOBAL/HLOCAL, but its address certainly isn't stored 4 bytes
before the pointer.

Wine initially followed the Win9x layout, but then as of Git
commit cd0219e941505ac7026c5412ea68b54722ab234a started using
8 bytes before the pointer returned from *Lock() to maintain
alignment on an 8 byte boundary, which some applications
(certainly developed and tested only on Windows NT derivatives)
wanted, and got memory corruption without. This however isn't
enough. wxWidgets expects to be able to pass the pointer returned
from GlobalLock() to HeapSize(), and applications crash if that
fails. Not only must we maintain 8 byte alignment when emulating
Windows NT, but the pointer returned from GlobalLock() must point
to the start of a valid heap block.

In this patch we support all Windows versions correctly:
* When emulating Win9x, use only 4 bytes behind the pointer
  to store the address of the HGLOBAL/HLOCAL, instead of
  the current 8.
* When emulating WinNT, allocate only the memory we need,
  storing the mapping from the pointer to
  its HGLOBAL/HLOCAL in a hash table, and implement
  GlobalHandle()/LocalHandle() by searching the hash table.

Apart from improving correctness, this design also decreases
memory usage by 4 bytes per allocation when emulating WinNT.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=38924

Signed-off-by: Damjan Jovanovic 
---
 dlls/kernel32/heap.c            |  89 ++++++++--------
 dlls/kernel32/tests/heap.c      |  10 ++
 dlls/kernelbase/kernelbase.spec |   1 +
 dlls/kernelbase/memory.c        | 175 +++++++++++++++++++++++++++-----
 include/winbase.h               |  16 +++
 5 files changed, 219 insertions(+), 72 deletions(-)

[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=60323

Your paranoid android.

=== debian10 (32 bit report) ===

kernel32:
debugger: Timeout

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

kernel32:
comm.c:919: Test failed: OutQueue should not be empty
debugger.c:320: Test failed: GetThreadContext failed: 5

=== debian10 (32 bit WoW report) ===

kernel32:
comm.c:919: Test failed: OutQueue should not be empty

=== debian10 (64 bit WoW report) ===

kernel32:
debugger.c:320: Test failed: GetThreadContext failed: 5

[Dmitry Timoshkov]

Damjan Jovanovic  wrote:
> diff --git a/dlls/kernelbase/kernelbase.spec b/dlls/kernelbase/kernelbase.spec
> index 7c5ae99a79..9546d011c7 100644
> --- a/dlls/kernelbase/kernelbase.spec
> +++ b/dlls/kernelbase/kernelbase.spec
> @@ -1769,3 +1769,4 @@
>  @ stdcall lstrlenW(wstr) KERNELBASE_lstrlenW
>  # @ stub time
>  # @ stub wprintf
> +@ cdecl wine_hlocal_hashtable_find_unlocked(ptr)

This is not acceptable.
> diff --git a/include/winbase.h b/include/winbase.h
> index 8b30c5a69a..0a1ad0b3c0 100644
> --- a/include/winbase.h
> +++ b/include/winbase.h
> @@ -2915,6 +2915,22 @@ WINBASEAPI UINT        WINAPI _lwrite(HFILE,LPCSTR,UINT);
>  extern char * CDECL wine_get_unix_file_name( LPCWSTR dos );
>  extern WCHAR * CDECL wine_get_dos_file_name( LPCSTR str );
>
> +#include "pshpack1.h"
> -
>
> +struct local_header
> +{
> - WORD  magic;
> - void *ptr;
> - BYTE flags;
> - BYTE lock;
> - struct local_header *next;
>
> +};
> -
>
> +#include "poppack.h"
> -
>
> +extern struct local_header *wine_hlocal_hashtable_find_unlocked(const void *ptr);

And this.