[PATCH v3 1/5] kernel32/tests: Add NtAllocateVirtualMemory tests for ZeroBits behavior

[Huw Davies]

On Tue, May 28, 2019 at 09:39:47AM +0200, Rémi Bernon wrote:
> The ZeroBits parameter doesn't behave as expected, and some 64bit code
> use it to allocate memory in the lower 32bit address space.
> 
> The expected full behaviour is:
> 
> * ZeroBits == 0: no constraint on address range
> * 0 < ZeroBits <= 15: returned address should have as many upper bits
>                       set to 0, starting at bit 31. In 64bit mode,
>                       upper 64bits should all be 0 as well.

upper 32 bits, right?

> * 15 < ZeroBits <= 31: unsure, but probably same as ZeroBits == 15.
> * ZeroBits > 31: (64bit/WoW64 only) ZeroBits behaves as a bitmask, as
>                  if it was set to the number of leading 0 in the
>                  bitmask, works in the whole 64bit range.
> 
> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
> ---
>  dlls/kernel32/tests/virtual.c | 77 +++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c
> index 474955630fd..91dc654580e 100644
> --- a/dlls/kernel32/tests/virtual.c
> +++ b/dlls/kernel32/tests/virtual.c
> @@ -228,10 +228,12 @@ static void test_VirtualAllocEx(void)
>  static void test_VirtualAlloc(void)
>  {
>      void *addr1, *addr2;
> +    ULONG zero_bits;
>      DWORD old_prot;
>      MEMORY_BASIC_INFORMATION info;
>      NTSTATUS status;
>      SIZE_T size;
> +    BOOL is_wow64;
>  
>      SetLastError(0xdeadbeef);
>      addr1 = VirtualAlloc(0, 0, MEM_RESERVE, PAGE_NOACCESS);
> @@ -461,6 +463,7 @@ static void test_VirtualAlloc(void)
>      addr2 = NULL;
>      status = pNtAllocateVirtualMemory(GetCurrentProcess(), &addr2, 21, &size,
>                                        MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE);
> +    todo_wine
>      ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY,
>         "NtAllocateVirtualMemory returned %08x\n", status);
>      if (status == STATUS_SUCCESS) ok(VirtualFree(addr2, 0, MEM_RELEASE), "VirtualFree failed\n");
> @@ -473,6 +476,80 @@ static void test_VirtualAlloc(void)
>      ok(status == STATUS_INVALID_PARAMETER_3, "NtAllocateVirtualMemory returned %08x\n", status);
>      if (status == STATUS_SUCCESS) ok(VirtualFree(addr2, 0, MEM_RELEASE), "VirtualFree failed\n");
>  
> +    /* 1 zero bits should zero 63-31 upper bits */
> +    size = 0x1000;
> +    addr2 = NULL;
> +    zero_bits = 1;
> +    status = pNtAllocateVirtualMemory(GetCurrentProcess(), &addr2, 1, &size,
> +                                      MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
> +    todo_wine
> +    ok((status == STATUS_SUCCESS || broken(status == STATUS_INVALID_PARAMETER_3) /* winxp */) &&
> +       ((UINT_PTR)addr2 >> (32 - zero_bits)) == 0,
> +       "NtAllocateVirtualMemory returned %08x, addr2: %p\n", status, addr2);
> +    if (status == STATUS_SUCCESS)
> +    {
> +        size = 0;
> +        status = pNtFreeVirtualMemory(GetCurrentProcess(), &addr2, &size, MEM_RELEASE);
> +        ok(status == STATUS_SUCCESS, "pNtFreeVirtualMemory return %08x, addr2: %p\n", status, addr2);
> +    }
> +
> +    /* 2 zero bits should zero 63-31 upper bits */
> +    size = 0x1000;
> +    addr2 = NULL;
> +    zero_bits = 2;
> +    status = pNtAllocateVirtualMemory(GetCurrentProcess(), &addr2, zero_bits, &size,
> +                                      MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
> +    todo_wine
> +    ok(status == STATUS_SUCCESS && ((UINT_PTR)addr2 >> (32 - zero_bits)) == 0,
> +       "NtAllocateVirtualMemory returned %08x, addr2: %p\n", status, addr2);
> +    if (status == STATUS_SUCCESS)
> +    {
> +        size = 0;
> +        status = pNtFreeVirtualMemory(GetCurrentProcess(), &addr2, &size, MEM_RELEASE);
> +        ok(status == STATUS_SUCCESS, "pNtFreeVirtualMemory return %08x, addr2: %p\n", status, addr2);
> +    }
> +
> +    /* 15 zero bits should zero 63-17 upper bits */
> +    size = 0x1000;
> +    addr2 = NULL;
> +    zero_bits = 15;
> +    status = pNtAllocateVirtualMemory(GetCurrentProcess(), &addr2, zero_bits, &size,
> +                                      MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
> +    todo_wine
> +    ok((status == STATUS_SUCCESS || status == STATUS_NO_MEMORY) && ((UINT_PTR)addr2 >> (32 - zero_bits)) == 0,
> +       "NtAllocateVirtualMemory returned %08x, addr2: %p\n", status, addr2);
> +    if (status == STATUS_SUCCESS)
> +    {
> +        size = 0;
> +        status = pNtFreeVirtualMemory(GetCurrentProcess(), &addr2, &size, MEM_RELEASE);
> +        ok(status == STATUS_SUCCESS, "pNtFreeVirtualMemory return %08x, addr2: %p\n", status, addr2);
> +    }

You're essentially repeating the same tests here three times with
different values of zero_bits.  It would be neater (and cover more
values) to loop zero_bits from 1 to 15.  Sure, you'll need to special
case the broken(status == STATUS_INVALID_PARAMETER_3) for zero_bits ==
1, but that shouldn't look too ugly.

Huw.