August 2019 - Wine-Devel - list.winehq.org

[PATCH 5/8] ntdll/tests: Use MEM_TOP_DOWN parameter to consistently fail zero_bits tests

by Rémi Bernon

The todo_wine_if made the tests not really test anything, and one test was succeeding when it shouldn't. Now we can actually remove the todo when 1 zero_bits handling is implemented. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index d45df72a741..8a53562cc7f 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -101,12 +101,14 @@ static void test_NtAllocateVirtualMemory(void) addr2 = NULL; zero_bits = 1; status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr2, zero_bits, &size, - MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + MEM_RESERVE | MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE); ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY || broken(status == STATUS_INVALID_PARAMETER_3) /* winxp */, "NtAllocateVirtualMemory returned %08x\n", status); if (status == STATUS_SUCCESS) { + todo_wine_if(is_win64) ok(((UINT_PTR)addr2 >> (32 - zero_bits)) == 0, "NtAllocateVirtualMemory returned address: %p\n", addr2); @@ -120,13 +122,14 @@ static void test_NtAllocateVirtualMemory(void) size = 0x1000; addr2 = NULL; status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr2, zero_bits, &size, - MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + MEM_RESERVE | MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE); ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY || broken(zero_bits == 20 && status == STATUS_CONFLICTING_ADDRESSES) /* w1064v1809 */, "NtAllocateVirtualMemory with %d zero_bits returned %08x\n", (int)zero_bits, status); if (status == STATUS_SUCCESS) { - todo_wine_if((UINT_PTR)addr2 >> (32 - zero_bits)) + todo_wine ok(((UINT_PTR)addr2 >> (32 - zero_bits)) == 0, "NtAllocateVirtualMemory with %d zero_bits returned address %p\n", (int)zero_bits, addr2); @@ -164,7 +167,8 @@ static void test_NtAllocateVirtualMemory(void) addr2 = NULL; zero_bits = 0x1fffffff; status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr2, zero_bits, &size, - MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); + MEM_RESERVE | MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE); if (!is_win64 && !is_wow64) { @@ -176,7 +180,7 @@ static void test_NtAllocateVirtualMemory(void) "NtAllocateVirtualMemory returned %08x\n", status); if (status == STATUS_SUCCESS) { - todo_wine_if((UINT_PTR)addr2 & ~zero_bits) + todo_wine ok(((UINT_PTR)addr2 & ~zero_bits) == 0, "NtAllocateVirtualMemory returned address %p\n", addr2); @@ -308,12 +312,12 @@ static void test_NtMapViewOfSection(void) size = 0; zero_bits = 1; offset.QuadPart = 0; - status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, 0, PAGE_READWRITE); + status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, MEM_TOP_DOWN, PAGE_READWRITE); ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY, "NtMapViewOfSection returned %08x\n", status); if (status == STATUS_SUCCESS) { - todo_wine_if((UINT_PTR)ptr2 >> (32 - zero_bits)) + todo_wine_if(is_win64) ok(((UINT_PTR)ptr2 >> (32 - zero_bits)) == 0, "NtMapViewOfSection returned address: %p\n", ptr2); @@ -326,12 +330,12 @@ static void test_NtMapViewOfSection(void) ptr2 = NULL; size = 0; offset.QuadPart = 0; - status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, 0, PAGE_READWRITE); + status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, MEM_TOP_DOWN, PAGE_READWRITE); ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY, "NtMapViewOfSection with %d zero_bits returned %08x\n", (int)zero_bits, status); if (status == STATUS_SUCCESS) { - todo_wine_if((UINT_PTR)ptr2 >> (32 - zero_bits)) + todo_wine ok(((UINT_PTR)ptr2 >> (32 - zero_bits)) == 0, "NtMapViewOfSection with %d zero_bits returned address %p\n", (int)zero_bits, ptr2); @@ -362,7 +366,7 @@ static void test_NtMapViewOfSection(void) size = 0; zero_bits = 0x1fffffff; offset.QuadPart = 0; - status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, 0, PAGE_READWRITE); + status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, MEM_TOP_DOWN, PAGE_READWRITE); if (!is_win64 && !is_wow64) { @@ -374,7 +378,7 @@ static void test_NtMapViewOfSection(void) "NtMapViewOfSection returned %08x\n", status); if (status == STATUS_SUCCESS) { - todo_wine_if((UINT_PTR)ptr2 & ~zero_bits) + todo_wine ok(((UINT_PTR)ptr2 & ~zero_bits) == 0, "NtMapViewOfSection returned address %p\n", ptr2); -- 2.20.1

Aug. 1, 2019

[PATCH 4/8] ntdll/tests: Factor WoW64 checks and add is_win64 constant

by Rémi Bernon

This is in order to use todo_wine_if(is_win64) in next patch. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 5c3a0b341ff..d45df72a741 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -31,6 +31,7 @@ static unsigned int page_size; static NTSTATUS (WINAPI *pRtlCreateUserStack)(SIZE_T, SIZE_T, ULONG, SIZE_T, SIZE_T, INITIAL_TEB *); static NTSTATUS (WINAPI *pRtlFreeUserStack)(void *); static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL); +static const BOOL is_win64 = sizeof(void*) != sizeof(int); static HANDLE create_target_process(const char *arg) { @@ -72,6 +73,8 @@ static void test_NtAllocateVirtualMemory(void) ULONG_PTR zero_bits; BOOL is_wow64; + if (!pIsWow64Process || !pIsWow64Process(NtCurrentProcess(), &is_wow64)) is_wow64 = FALSE; + /* simple allocation should success */ size = 0x1000; addr1 = NULL; @@ -163,8 +166,7 @@ static void test_NtAllocateVirtualMemory(void) status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr2, zero_bits, &size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); - if (sizeof(void *) == sizeof(int) && (!pIsWow64Process || - !pIsWow64Process(NtCurrentProcess(), &is_wow64) || !is_wow64)) + if (!is_win64 && !is_wow64) { ok(status == STATUS_INVALID_PARAMETER_3, "NtAllocateVirtualMemory returned %08x\n", status); } @@ -273,6 +275,8 @@ static void test_NtMapViewOfSection(void) LARGE_INTEGER offset; ULONG_PTR zero_bits; + if (!pIsWow64Process || !pIsWow64Process(NtCurrentProcess(), &is_wow64)) is_wow64 = FALSE; + file = CreateFileA(testfile, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); ok(file != INVALID_HANDLE_VALUE, "Failed to create test file\n"); WriteFile(file, data, sizeof(data), &written, NULL); @@ -360,8 +364,7 @@ static void test_NtMapViewOfSection(void) offset.QuadPart = 0; status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, 0, PAGE_READWRITE); - if (sizeof(void *) == sizeof(int) && (!pIsWow64Process || - !pIsWow64Process(NtCurrentProcess(), &is_wow64) || !is_wow64)) + if (!is_win64 && !is_wow64) { ok(status == STATUS_INVALID_PARAMETER_4, "NtMapViewOfSection returned %08x\n", status); } @@ -424,8 +427,7 @@ static void test_NtMapViewOfSection(void) status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, 0, PAGE_READWRITE); ok(status == STATUS_MAPPED_ALIGNMENT, "NtMapViewOfSection returned %08x\n", status); - if (sizeof(void *) == sizeof(int) && (!pIsWow64Process || - !pIsWow64Process(NtCurrentProcess(), &is_wow64) || !is_wow64)) + if (!is_win64 && !is_wow64) { /* new memory region conflicts with previous mapping */ ptr2 = ptr; -- 2.20.1

Aug. 1, 2019

[PATCH 3/8] ntdll: Change zero_bits parameter type to ULONG_PTR for NtMapViewOfSection and NtAllocateVirtualMemory

by Rémi Bernon

It can be a pointer mask, eventually 64bit, and ULONG_PTR is what can be found in several documentation of these functions. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernel32/tests/loader.c | 4 ++-- dlls/kernel32/tests/sync.c | 2 +- dlls/kernel32/tests/virtual.c | 2 +- dlls/ntdll/tests/info.c | 2 +- dlls/ntdll/tests/virtual.c | 12 ++++++------ dlls/ntdll/virtual.c | 4 ++-- include/winternl.h | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index eb02e95efca..65184887b8e 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -57,14 +57,14 @@ static BOOL is_wow64; static NTSTATUS (WINAPI *pNtCreateSection)(HANDLE *, ACCESS_MASK, const OBJECT_ATTRIBUTES *, const LARGE_INTEGER *, ULONG, ULONG, HANDLE ); static NTSTATUS (WINAPI *pNtQuerySection)(HANDLE, SECTION_INFORMATION_CLASS, void *, SIZE_T, SIZE_T *); -static NTSTATUS (WINAPI *pNtMapViewOfSection)(HANDLE, HANDLE, PVOID *, ULONG, SIZE_T, const LARGE_INTEGER *, SIZE_T *, ULONG, ULONG, ULONG); +static NTSTATUS (WINAPI *pNtMapViewOfSection)(HANDLE, HANDLE, PVOID *, ULONG_PTR, SIZE_T, const LARGE_INTEGER *, SIZE_T *, ULONG, ULONG, ULONG); static NTSTATUS (WINAPI *pNtUnmapViewOfSection)(HANDLE, PVOID); static NTSTATUS (WINAPI *pNtQueryInformationProcess)(HANDLE, PROCESSINFOCLASS, PVOID, ULONG, PULONG); static NTSTATUS (WINAPI *pNtSetInformationProcess)(HANDLE, PROCESSINFOCLASS, PVOID, ULONG); static NTSTATUS (WINAPI *pNtTerminateProcess)(HANDLE, DWORD); static void (WINAPI *pLdrShutdownProcess)(void); static BOOLEAN (WINAPI *pRtlDllShutdownInProgress)(void); -static NTSTATUS (WINAPI *pNtAllocateVirtualMemory)(HANDLE, PVOID *, ULONG, SIZE_T *, ULONG, ULONG); +static NTSTATUS (WINAPI *pNtAllocateVirtualMemory)(HANDLE, PVOID *, ULONG_PTR, SIZE_T *, ULONG, ULONG); static NTSTATUS (WINAPI *pNtFreeVirtualMemory)(HANDLE, PVOID *, SIZE_T *, ULONG); static NTSTATUS (WINAPI *pLdrLockLoaderLock)(ULONG, ULONG *, ULONG_PTR *); static NTSTATUS (WINAPI *pLdrUnlockLoaderLock)(ULONG, ULONG_PTR); diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c index 97566f1d84b..40cad1c4c50 100644 --- a/dlls/kernel32/tests/sync.c +++ b/dlls/kernel32/tests/sync.c @@ -53,7 +53,7 @@ static VOID (WINAPI *pReleaseSRWLockShared)(PSRWLOCK); static BOOLEAN (WINAPI *pTryAcquireSRWLockExclusive)(PSRWLOCK); static BOOLEAN (WINAPI *pTryAcquireSRWLockShared)(PSRWLOCK); -static NTSTATUS (WINAPI *pNtAllocateVirtualMemory)(HANDLE, PVOID *, ULONG, SIZE_T *, ULONG, ULONG); +static NTSTATUS (WINAPI *pNtAllocateVirtualMemory)(HANDLE, PVOID *, ULONG_PTR, SIZE_T *, ULONG, ULONG); static NTSTATUS (WINAPI *pNtFreeVirtualMemory)(HANDLE, PVOID *, SIZE_T *, ULONG); static NTSTATUS (WINAPI *pNtWaitForSingleObject)(HANDLE, BOOLEAN, const LARGE_INTEGER *); static NTSTATUS (WINAPI *pNtWaitForMultipleObjects)(ULONG,const HANDLE*,BOOLEAN,BOOLEAN,const LARGE_INTEGER*); diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index 402ab6b0b58..bde71b9c648 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -44,7 +44,7 @@ static UINT (WINAPI *pResetWriteWatch)(LPVOID,SIZE_T); static NTSTATUS (WINAPI *pNtAreMappedFilesTheSame)(PVOID,PVOID); static NTSTATUS (WINAPI *pNtCreateSection)(HANDLE *, ACCESS_MASK, const OBJECT_ATTRIBUTES *, const LARGE_INTEGER *, ULONG, ULONG, HANDLE ); -static NTSTATUS (WINAPI *pNtMapViewOfSection)(HANDLE, HANDLE, PVOID *, ULONG, SIZE_T, const LARGE_INTEGER *, SIZE_T *, ULONG, ULONG, ULONG); +static NTSTATUS (WINAPI *pNtMapViewOfSection)(HANDLE, HANDLE, PVOID *, ULONG_PTR, SIZE_T, const LARGE_INTEGER *, SIZE_T *, ULONG, ULONG, ULONG); static DWORD (WINAPI *pNtUnmapViewOfSection)(HANDLE, PVOID); static NTSTATUS (WINAPI *pNtQuerySection)(HANDLE, SECTION_INFORMATION_CLASS, void *, SIZE_T, SIZE_T *); static PVOID (WINAPI *pRtlAddVectoredExceptionHandler)(ULONG, PVECTORED_EXCEPTION_HANDLER); diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index 262d2f421fc..25c379929b0 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -32,7 +32,7 @@ static NTSTATUS (WINAPI * pNtSetInformationThread)(HANDLE, THREADINFOCLASS, PVOI static NTSTATUS (WINAPI * pNtReadVirtualMemory)(HANDLE, const void*, void*, SIZE_T, SIZE_T*); static NTSTATUS (WINAPI * pNtQueryVirtualMemory)(HANDLE, LPCVOID, MEMORY_INFORMATION_CLASS , PVOID , SIZE_T , SIZE_T *); static NTSTATUS (WINAPI * pNtCreateSection)(HANDLE*,ACCESS_MASK,const OBJECT_ATTRIBUTES*,const LARGE_INTEGER*,ULONG,ULONG,HANDLE); -static NTSTATUS (WINAPI * pNtMapViewOfSection)(HANDLE,HANDLE,PVOID*,ULONG,SIZE_T,const LARGE_INTEGER*,SIZE_T*,SECTION_INHERIT,ULONG,ULONG); +static NTSTATUS (WINAPI * pNtMapViewOfSection)(HANDLE,HANDLE,PVOID*,ULONG_PTR,SIZE_T,const LARGE_INTEGER*,SIZE_T*,SECTION_INHERIT,ULONG,ULONG); static NTSTATUS (WINAPI * pNtUnmapViewOfSection)(HANDLE,PVOID); static NTSTATUS (WINAPI * pNtClose)(HANDLE); static ULONG (WINAPI * pNtGetCurrentProcessorNumber)(void); diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index bb110f15681..5c3a0b341ff 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -69,7 +69,7 @@ static void test_NtAllocateVirtualMemory(void) void *addr1, *addr2; NTSTATUS status; SIZE_T size; - ULONG zero_bits; + ULONG_PTR zero_bits; BOOL is_wow64; /* simple allocation should success */ @@ -120,12 +120,12 @@ static void test_NtAllocateVirtualMemory(void) MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY || broken(zero_bits == 20 && status == STATUS_CONFLICTING_ADDRESSES) /* w1064v1809 */, - "NtAllocateVirtualMemory with %d zero_bits returned %08x\n", zero_bits, status); + "NtAllocateVirtualMemory with %d zero_bits returned %08x\n", (int)zero_bits, status); if (status == STATUS_SUCCESS) { todo_wine_if((UINT_PTR)addr2 >> (32 - zero_bits)) ok(((UINT_PTR)addr2 >> (32 - zero_bits)) == 0, - "NtAllocateVirtualMemory with %d zero_bits returned address %p\n", zero_bits, addr2); + "NtAllocateVirtualMemory with %d zero_bits returned address %p\n", (int)zero_bits, addr2); size = 0; status = NtFreeVirtualMemory(NtCurrentProcess(), &addr2, &size, MEM_RELEASE); @@ -271,7 +271,7 @@ static void test_NtMapViewOfSection(void) DWORD status, written; SIZE_T size, result; LARGE_INTEGER offset; - ULONG zero_bits; + ULONG_PTR zero_bits; file = CreateFileA(testfile, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); ok(file != INVALID_HANDLE_VALUE, "Failed to create test file\n"); @@ -324,12 +324,12 @@ static void test_NtMapViewOfSection(void) offset.QuadPart = 0; status = NtMapViewOfSection(mapping, process, &ptr2, zero_bits, 0, &offset, &size, 1, 0, PAGE_READWRITE); ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY, - "NtMapViewOfSection with %d zero_bits returned %08x\n", zero_bits, status); + "NtMapViewOfSection with %d zero_bits returned %08x\n", (int)zero_bits, status); if (status == STATUS_SUCCESS) { todo_wine_if((UINT_PTR)ptr2 >> (32 - zero_bits)) ok(((UINT_PTR)ptr2 >> (32 - zero_bits)) == 0, - "NtMapViewOfSection with %d zero_bits returned address %p\n", zero_bits, ptr2); + "NtMapViewOfSection with %d zero_bits returned address %p\n", (int)zero_bits, ptr2); status = NtUnmapViewOfSection(process, ptr2); ok(status == STATUS_SUCCESS, "NtUnmapViewOfSection returned %08x\n", status); diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 292d4b97258..57536eb6295 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -2501,7 +2501,7 @@ void virtual_set_large_address_space(void) * NtAllocateVirtualMemory (NTDLL.@) * ZwAllocateVirtualMemory (NTDLL.@) */ -NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_bits, +NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR zero_bits, SIZE_T *size_ptr, ULONG type, ULONG protect ) { SIZE_T size = *size_ptr; @@ -3132,7 +3132,7 @@ NTSTATUS WINAPI NtOpenSection( HANDLE *handle, ACCESS_MASK access, const OBJECT_ * NtMapViewOfSection (NTDLL.@) * ZwMapViewOfSection (NTDLL.@) */ -NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_ptr, ULONG zero_bits, +NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_ptr, ULONG_PTR zero_bits, SIZE_T commit_size, const LARGE_INTEGER *offset_ptr, SIZE_T *size_ptr, SECTION_INHERIT inherit, ULONG alloc_type, ULONG protect ) { diff --git a/include/winternl.h b/include/winternl.h index 49eedaf1f98..3d31f8b462a 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -2318,7 +2318,7 @@ NTSYSAPI NTSTATUS WINAPI NtAlertResumeThread(HANDLE,PULONG); NTSYSAPI NTSTATUS WINAPI NtAlertThread(HANDLE ThreadHandle); NTSYSAPI NTSTATUS WINAPI NtAllocateLocallyUniqueId(PLUID lpLuid); NTSYSAPI NTSTATUS WINAPI NtAllocateUuids(PULARGE_INTEGER,PULONG,PULONG,PUCHAR); -NTSYSAPI NTSTATUS WINAPI NtAllocateVirtualMemory(HANDLE,PVOID*,ULONG,SIZE_T*,ULONG,ULONG); +NTSYSAPI NTSTATUS WINAPI NtAllocateVirtualMemory(HANDLE,PVOID*,ULONG_PTR,SIZE_T*,ULONG,ULONG); NTSYSAPI NTSTATUS WINAPI NtAreMappedFilesTheSame(PVOID,PVOID); NTSYSAPI NTSTATUS WINAPI NtAssignProcessToJobObject(HANDLE,HANDLE); NTSYSAPI NTSTATUS WINAPI NtCallbackReturn(PVOID,ULONG,NTSTATUS); @@ -2390,7 +2390,7 @@ NTSYSAPI NTSTATUS WINAPI NtLoadKey(const OBJECT_ATTRIBUTES *,OBJECT_ATTRIBUTES NTSYSAPI NTSTATUS WINAPI NtLockFile(HANDLE,HANDLE,PIO_APC_ROUTINE,void*,PIO_STATUS_BLOCK,PLARGE_INTEGER,PLARGE_INTEGER,ULONG*,BOOLEAN,BOOLEAN); NTSYSAPI NTSTATUS WINAPI NtLockVirtualMemory(HANDLE,PVOID*,SIZE_T*,ULONG); NTSYSAPI NTSTATUS WINAPI NtMakeTemporaryObject(HANDLE); -NTSYSAPI NTSTATUS WINAPI NtMapViewOfSection(HANDLE,HANDLE,PVOID*,ULONG,SIZE_T,const LARGE_INTEGER*,SIZE_T*,SECTION_INHERIT,ULONG,ULONG); +NTSYSAPI NTSTATUS WINAPI NtMapViewOfSection(HANDLE,HANDLE,PVOID*,ULONG_PTR,SIZE_T,const LARGE_INTEGER*,SIZE_T*,SECTION_INHERIT,ULONG,ULONG); NTSYSAPI NTSTATUS WINAPI NtNotifyChangeDirectoryFile(HANDLE,HANDLE,PIO_APC_ROUTINE,PVOID,PIO_STATUS_BLOCK,PVOID,ULONG,ULONG,BOOLEAN); NTSYSAPI NTSTATUS WINAPI NtNotifyChangeKey(HANDLE,HANDLE,PIO_APC_ROUTINE,PVOID,PIO_STATUS_BLOCK,ULONG,BOOLEAN,PVOID,ULONG,BOOLEAN); NTSYSAPI NTSTATUS WINAPI NtNotifyChangeMultipleKeys(HANDLE,ULONG,OBJECT_ATTRIBUTES*,HANDLE,PIO_APC_ROUTINE,PVOID,PIO_STATUS_BLOCK,ULONG,BOOLEAN,PVOID,ULONG,BOOLEAN); -- 2.20.1

Aug. 1, 2019

[PATCH 2/8] ntdll: Directly call internal functions from APC handler

by Rémi Bernon

This is in preparation for the following patches that are changing the internal zero_bits format. Also, parameter validation has already been done before queueing the APC. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/server.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c index b82fbc56584..16e73d497c4 100644 --- a/dlls/ntdll/server.c +++ b/dlls/ntdll/server.c @@ -391,6 +391,7 @@ BOOL invoke_apc( const apc_call_t *call, apc_result_t *result ) BOOL user_apc = FALSE; SIZE_T size; void *addr; + pe_image_info_t image_info; memset( result, 0, sizeof(*result) ); @@ -429,10 +430,11 @@ BOOL invoke_apc( const apc_call_t *call, apc_result_t *result ) size = call->virtual_alloc.size; if ((ULONG_PTR)addr == call->virtual_alloc.addr && size == call->virtual_alloc.size) { - result->virtual_alloc.status = NtAllocateVirtualMemory( NtCurrentProcess(), &addr, - call->virtual_alloc.zero_bits, &size, - call->virtual_alloc.op_type, - call->virtual_alloc.prot ); + result->virtual_alloc.status = virtual_alloc_aligned( &addr, + call->virtual_alloc.zero_bits, &size, + call->virtual_alloc.op_type, + call->virtual_alloc.prot, + 0 ); result->virtual_alloc.addr = wine_server_client_ptr( addr ); result->virtual_alloc.size = size; } @@ -534,11 +536,12 @@ BOOL invoke_apc( const apc_call_t *call, apc_result_t *result ) { LARGE_INTEGER offset; offset.QuadPart = call->map_view.offset; - result->map_view.status = NtMapViewOfSection( wine_server_ptr_handle(call->map_view.handle), - NtCurrentProcess(), &addr, - call->map_view.zero_bits, 0, - &offset, &size, ViewShare, - call->map_view.alloc_type, call->map_view.prot ); + result->map_view.status = virtual_map_section( wine_server_ptr_handle(call->map_view.handle), + &addr, + call->map_view.zero_bits, 0, + &offset, &size, + call->map_view.alloc_type, call->map_view.prot, + &image_info ); result->map_view.addr = wine_server_client_ptr( addr ); result->map_view.size = size; } -- 2.20.1

Aug. 1, 2019

[PATCH 1/8] ntdll: Add alloc_type parameter to virtual_map_section and top_down to map_image

by Rémi Bernon

NtMapViewOfSection also accepts the MEM_TOP_DOWN parameter that can be used in combination with zero_bits to map memory on the highest possible address. This is useful for the next patches so we can consistently fail zero_bits tests. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/loader.c | 2 +- dlls/ntdll/ntdll_misc.h | 4 ++-- dlls/ntdll/virtual.c | 19 ++++++++++--------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 67efcafdaec..c1358c67cdd 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2135,7 +2135,7 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, *module = NULL; } status = virtual_map_section( mapping, module, 0, 0, NULL, &len, - PAGE_EXECUTE_READ, image_info ); + 0, PAGE_EXECUTE_READ, image_info ); if (status == STATUS_IMAGE_NOT_AT_BASE) status = STATUS_SUCCESS; NtClose( mapping ); } diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index e8db1ca07d2..1950d8c1c7e 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -166,8 +166,8 @@ extern NTSTATUS nt_to_unix_file_name_attr( const OBJECT_ATTRIBUTES *attr, ANSI_S extern NTSTATUS virtual_alloc_aligned( PVOID *ret, ULONG zero_bits, SIZE_T *size_ptr, ULONG type, ULONG protect, ULONG alignment ) DECLSPEC_HIDDEN; extern NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG zero_bits, SIZE_T commit_size, - const LARGE_INTEGER *offset_ptr, SIZE_T *size_ptr, ULONG protect, - pe_image_info_t *image_info ) DECLSPEC_HIDDEN; + const LARGE_INTEGER *offset_ptr, SIZE_T *size_ptr, ULONG alloc_type, + ULONG protect, pe_image_info_t *image_info ) DECLSPEC_HIDDEN; extern void virtual_get_system_info( SYSTEM_BASIC_INFORMATION *info ) DECLSPEC_HIDDEN; extern NTSTATUS virtual_create_builtin_view( void *base ) DECLSPEC_HIDDEN; extern NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 6196a4e3ee1..292d4b97258 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -1361,7 +1361,7 @@ static NTSTATUS map_pe_header( void *ptr, size_t size, int fd, BOOL *removable ) * * Map an executable (PE format) image into memory. */ -static NTSTATUS map_image( HANDLE hmapping, ACCESS_MASK access, int fd, SIZE_T zero_bits, +static NTSTATUS map_image( HANDLE hmapping, ACCESS_MASK access, int fd, int top_down, SIZE_T zero_bits, pe_image_info_t *image_info, int shared_fd, BOOL removable, PVOID *addr_ptr ) { IMAGE_DOS_HEADER *dos; @@ -1391,11 +1391,11 @@ static NTSTATUS map_image( HANDLE hmapping, ACCESS_MASK access, int fd, SIZE_T z server_enter_uninterrupted_section( &csVirtual, &sigset ); if (base >= (char *)address_space_start) /* make sure the DOS area remains free */ - status = map_view( &view, base, total_size, 0, FALSE, SEC_IMAGE | SEC_FILE | + status = map_view( &view, base, total_size, 0, top_down, SEC_IMAGE | SEC_FILE | VPROT_COMMITTED | VPROT_READ | VPROT_EXEC | VPROT_WRITECOPY, zero_bits ); if (status != STATUS_SUCCESS) - status = map_view( &view, NULL, total_size, 0, FALSE, SEC_IMAGE | SEC_FILE | + status = map_view( &view, NULL, total_size, 0, top_down, SEC_IMAGE | SEC_FILE | VPROT_COMMITTED | VPROT_READ | VPROT_EXEC | VPROT_WRITECOPY, zero_bits ); if (status != STATUS_SUCCESS) goto error; @@ -1612,8 +1612,8 @@ static NTSTATUS map_image( HANDLE hmapping, ACCESS_MASK access, int fd, SIZE_T z * Map a file section into memory. */ NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG zero_bits, SIZE_T commit_size, - const LARGE_INTEGER *offset_ptr, SIZE_T *size_ptr, ULONG protect, - pe_image_info_t *image_info ) + const LARGE_INTEGER *offset_ptr, SIZE_T *size_ptr, ULONG alloc_type, + ULONG protect, pe_image_info_t *image_info ) { NTSTATUS res; mem_size_t full_size; @@ -1673,14 +1673,14 @@ NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG zero_bits, S if ((res = server_get_unix_fd( shared_file, FILE_READ_DATA|FILE_WRITE_DATA, &shared_fd, &shared_needs_close, NULL, NULL ))) goto done; - res = map_image( handle, access, unix_handle, zero_bits, image_info, + res = map_image( handle, access, unix_handle, alloc_type & MEM_TOP_DOWN, zero_bits, image_info, shared_fd, needs_close, addr_ptr ); if (shared_needs_close) close( shared_fd ); close_handle( shared_file ); } else { - res = map_image( handle, access, unix_handle, zero_bits, image_info, + res = map_image( handle, access, unix_handle, alloc_type & MEM_TOP_DOWN, zero_bits, image_info, -1, needs_close, addr_ptr ); } if (needs_close) close( unix_handle ); @@ -1718,7 +1718,7 @@ NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG zero_bits, S get_vprot_flags( protect, &vprot, sec_flags & SEC_IMAGE ); vprot |= sec_flags; if (!(sec_flags & SEC_RESERVE)) vprot |= VPROT_COMMITTED; - res = map_view( &view, *addr_ptr, size, 0, FALSE, vprot, zero_bits ); + res = map_view( &view, *addr_ptr, size, 0, alloc_type & MEM_TOP_DOWN, vprot, zero_bits ); if (res) { server_leave_uninterrupted_section( &csVirtual, &sigset ); @@ -3198,7 +3198,8 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p } return virtual_map_section( handle, addr_ptr, zero_bits, commit_size, - offset_ptr, size_ptr, protect, &image_info ); + offset_ptr, size_ptr, alloc_type, protect, + &image_info ); } -- 2.20.1

Aug. 1, 2019

Re: [PATCH vkd3d] vkd3d: Add vkd3d_get_program_name() implementation for macOS.

by Józef Kucia

On Thu, Aug 1, 2019 at 9:43 AM Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: > It doesn not in case of insufficient target buffer size. That's probably > not the common case though, however strncpy() always fills up the buffer > with zeros which is worst behaviour ever, so it's probably better to just > use plain strcpy(). Right, we even place '\0' ourselves in vkd3d_get_program_name(). Sorry for the misleading comment.

Aug. 1, 2019

Re: [PATCH vkd3d] vkd3d: Add vkd3d_get_program_name() implementation for macOS.

by Dmitry Timoshkov

Józef Kucia <joseph.kucia(a)gmail.com> wrote: > > +bool vkd3d_get_program_name(char program_name[PATH_MAX]) > > +{ > > + strncpy(program_name, getprogname(), PATH_MAX); > > + program_name[PATH_MAX - 1] = '\0'; > > strncpy() always adds a terminating null byte. It doesn not in case of insufficient target buffer size. That's probably not the common case though, however strncpy() always fills up the buffer with zeros which is worst behaviour ever, so it's probably better to just use plain strcpy(). -- Dmitry.

Aug. 1, 2019

Re: [PATCH 2/2] vkd3d/tests: Enable ReadFromSubresource() read-back test and remove todo.

by Józef Kucia

On Wed, Jul 31, 2019 at 1:24 PM Conor McCarthy <cmccarthy(a)codeweavers.com> wrote: > > Signed-off-by: Conor McCarthy <cmccarthy(a)codeweavers.com> > --- > tests/d3d12.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/tests/d3d12.c b/tests/d3d12.c > index 6cee005..7308525 100644 > --- a/tests/d3d12.c > +++ b/tests/d3d12.c > @@ -30119,7 +30119,7 @@ static void test_read_write_subresource(void) > todo ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); > > hr = ID3D12Resource_ReadFromSubresource(rb_buffer, dst_buffer, row_pitch, slice_pitch, 0, &box); > - todo ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); > + ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); todo() changes need to be made together with the implementation. The previous patch introduces test failures. > > ID3D12Resource_Release(rb_buffer); > > @@ -30133,7 +30133,7 @@ static void test_read_write_subresource(void) > resource_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM; > resource_desc.SampleDesc.Count = 1; > resource_desc.SampleDesc.Quality = 0; > - resource_desc.Layout = D3D12_TEXTURE_LAYOUT_UNKNOWN; > + resource_desc.Layout = D3D12_TEXTURE_LAYOUT_ROW_MAJOR; > resource_desc.Flags = 0; I don't like this change. The main point of ReadFromSubresource() is to read textures with undefined layout on CPU. We should add additional test cases instead. Also, it should be possible to make ReadFromSubresource() work with D3D12_TEXTURE_LAYOUT_UNKNOWN in our implementations for some cases. We can try to use VK_IMAGE_TILING_LINEAR for textures with custom heaps. This isn't very nice but Vulkan doesn't allow us to read textures with optimal tiling. See also VK_IMAGE_TILING_LINEAR restrictions in the Vulkan spec. > + /* WriteToSubresource() is not implemented so upload test data */ > + transition_resource_state(command_list, src_texture, > + D3D12_RESOURCE_STATE_COMMON, D3D12_RESOURCE_STATE_COPY_DEST); > + texture_data.pData = dst_buffer; > + texture_data.RowPitch = row_pitch; > + texture_data.SlicePitch = slice_pitch; > + upload_texture_data(src_texture, &texture_data, 1, queue, command_list); > + reset_command_list(command_list, context.allocator); > + transition_resource_state(command_list, src_texture, > + D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_COMMON); This makes a bunch of WriteToSubresource() and ID3D12Resource_ReadFromSubresource() tests much less interesting. Please try to extends tests instead and add this as a separate test case as suggested above.

Aug. 1, 2019

Re: [PATCH 1/2] vkd3d: Implement ID3D12Resource::ReadFromSubresource().

by Józef Kucia

On Wed, Jul 31, 2019 at 1:24 PM Conor McCarthy <cmccarthy(a)codeweavers.com> wrote: > diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c > index e0e1aad..35290f3 100644 > --- a/libs/vkd3d/resource.c > +++ b/libs/vkd3d/resource.c > @@ -1205,12 +1205,99 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour > void *dst_data, UINT dst_row_pitch, UINT dst_slice_pitch, > UINT src_sub_resource, const D3D12_BOX *src_box) > { > - FIXME("iface %p, dst_data %p, dst_row_pitch %u, dst_slice_pitch %u, " > - "src_sub_resource %u, src_box %p stub!\n", > + struct d3d12_resource *resource = impl_from_ID3D12Resource(iface); > + const struct vkd3d_vk_device_procs *vk_procs; > + VkImageSubresourceLayers vk_sub_layers; > + VkImageSubresource vk_sub_resource; > + const struct vkd3d_format *format; > + VkSubresourceLayout vk_layout; > + struct d3d12_device *device; > + void *src_map_ptr; > + D3D12_BOX box; > + HRESULT hr; > + BYTE *dst; > + UINT z; Please use "unsigned int" instead of "INT". We stay away from UINT, VOID and similar defines in d3d code. Prototypes and definitions of D3D API functions are an exception. > + > + TRACE("iface %p, dst_data %p, dst_row_pitch %u, dst_slice_pitch %u, " > + "src_sub_resource %u, src_box %p\n", Missing dot ad the end of TRACE() message. > + if (src_box) > + { > + box = *src_box; > + } > + else > + { > + box.left = 0; > + box.top = 0; > + box.front = 0; > + box.right = resource->desc.Width; > + box.bottom = resource->desc.Height; > + box.back = d3d12_resource_desc_get_depth(&resource->desc, 0); We need to call d3d12_resource_desc_get_depth() with the correct mip-level index. > + if (!resource->heap) > + { > + FIXME("Not implemented for this resource type.\n"); > + return E_NOTIMPL; > + } We should probably use d3d12_resource_is_cpu_accessible() instead. See d3d12_resource_is_cpu_accessible() for reference. > + if (resource->desc.Layout != D3D12_TEXTURE_LAYOUT_ROW_MAJOR) > + FIXME_ONCE("Layouts other than D3D12_TEXTURE_LAYOUT_ROW_MAJOR are not supported and results are implementation-dependent.\n"); The function needs to exit at this point. It is invalid usage to call vkGetImageSubresourceLayout() for images with VK_IMAGE_TILING_OPTIMAL. > + vk_image_subresource_layers_from_d3d12(&vk_sub_layers, format, src_sub_resource, resource->desc.MipLevels); > + vk_sub_resource.arrayLayer = vk_sub_layers.baseArrayLayer; > + vk_sub_resource.mipLevel = vk_sub_layers.mipLevel; > + vk_sub_resource.aspectMask = vk_sub_layers.aspectMask; It doesn't seem beneficial to reuse vk_image_subresource_layers_from_d3d12(). Please simply fill members of vk_sub_resource(): it's less code, it doesn't need a new prototype in the header, it's a bit awkward to fill other structure with vk_image_subresource_layers_from_d3d12() and then copy some fields to vk_sub_resource(). vk_sub_resource.aspectMask = format->vk_aspect_mask; vk_sub_resource.mipLevel = src_sub_resource % resource->desc.MipLevels; vk_sub_resource.baseArrayLayer = src_sub_resource / resource->desc.MipLevels; > + > + VK_CALL(vkGetImageSubresourceLayout(device->vk_device, resource->u.vk_image, &vk_sub_resource, &vk_layout)); > + TRACE("offset %#"PRIx64", size %#"PRIx64", rowPitch %#"PRIx64", arrayPitch %#"PRIx64", depthPitch %#"PRIx64".\n", > + vk_layout.offset, vk_layout.size, vk_layout.rowPitch, vk_layout.arrayPitch, vk_layout.depthPitch); Please avoid camelCase in TRACE() messages: "Offset ..., size, row pitch ..., array pitch ..., depth pitch ..." > + > + src_map_ptr = (BYTE*)src_map_ptr + vk_layout.offset; > + for (z = box.front; z < box.back; ++z) > + { > + UINT y; unsigned int. It could also be moved to other declarations at the top of the function. > + dst = dst_data + (z - box.front) * dst_slice_pitch; Are you sure that src_box should be used to offset dst? > + for (y = box.top; y < box.bottom; y += format->block_height) > + { > + SIZE_T size = (box.right - box.left) / format->block_width > + * format->byte_count * format->block_byte_count; size_t

Aug. 1, 2019

Re: [PATCH vkd3d] vkd3d: Add vkd3d_get_program_name() implementation for macOS.

by Józef Kucia

On Thu, Aug 1, 2019 at 4:46 AM Jactry Zeng <jzeng(a)codeweavers.com> wrote: > -#ifdef _GNU_SOURCE > +#ifdef __APPLE__ It's probably better to test for getprogname() instead of __APPLE__. > +bool vkd3d_get_program_name(char program_name[PATH_MAX]) > +{ > + strncpy(program_name, getprogname(), PATH_MAX); > + program_name[PATH_MAX - 1] = '\0'; strncpy() always adds a terminating null byte. > + return true; > +} vkd3d_get_program_name() is supposed to get just the name of the executable, e.g. it needs to extract it from the full path. Our other implementation also extracts the executable name when a program is run under Wine. I'm afraid it might not be as simple as calling getprogrname(). Moreover, in the long term it might not be necessary to have an implementation of vkd3d_get_program_name() for programs running under Wine. We'll probably start filling vkd3d_application_info in Wine after the next vkd3d release.

Aug. 1, 2019