Re: [PATCH v4 2/6] kernel32/tests: Add NtAllocateVirtualMemory tests for zero_bits behavior

[Huw Davies]

On Tue, May 28, 2019 at 12:15:14PM +0200, Rémi Bernon wrote:
> The zero_bits 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:
> - zero_bits == 0: no constraint on address range
> - 0 < zero_bits <= 15: returned address should have as many upper bits
>                      set to 0, starting at bit 31. In 64bit mode,
>                      upper 32bits should all be 0 as well.
> - 15 < zero_bits <= 31: unsure, but probably same as zero_bits == 15.
> - zero_bits > 31: (64bit/WoW64 only) zero_bits 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
>
> dlls/kernel32/tests/virtual.c | 57 ++++++++++++++++++++++++++++++++---
>  1 file changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c
> index dcaeb147b04..1778509d2ac 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,13 +463,16 @@ static void test_VirtualAlloc(void)
>          ok(status == STATUS_SUCCESS, "pNtFreeVirtualMemory return %08x, addr2: %p\n", status, addr2);
>      }
> - /* 21 zero bits is valid */
> - /* 1 zero bits should zero 63-31 upper bits */
> size = 0x1000;
> addr2 = NULL;
> - status = pNtAllocateVirtualMemory(GetCurrentProcess(), &addr2, 21, &size,
> -
>                                  MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE);
> - ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY,
> -
>   "NtAllocateVirtualMemory returned %08x\n", status);
> - 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;
>
> @@ -475,6 +480,23 @@ static void test_VirtualAlloc(void)
>          ok(status == STATUS_SUCCESS, "pNtFreeVirtualMemory return %08x, addr2: %p\n", status, addr2);
>      }
> - for (zero_bits = 2; zero_bits <= 21; zero_bits++)
> - {
> -
>    size = 0x1000;
> -
>    addr2 = NULL;
> -
>    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 with %d zero_bits returned %08x, addr2: %p\n", zero_bits, 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);
> -
>    }
> - }
> -

Problem is sometimes these actually work (and so fail the todo_wine):

virtual.c:473: Test succeeded inside todo block: NtAllocateVirtualMemory returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 2 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 3 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 4 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 5 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 6 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 7 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 8 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 9 zero_bits returned 00000000, addr2: 00222000
virtual.c:490: Test succeeded inside todo block: NtAllocateVirtualMemory with 10 zero_bits returned 00000000, addr2: 00222000

Also having these tests in kernel32 seems wrong; though short of
adding a new virtual.c to the ntdll tests, I couldn't find a sensible
place to put them.

Huw.