[PATCH v2 0/7] MR5907: ntdll: Optimize NtQueryVirtualMemory(MemoryWorkingSetExInformation).
-- v2: ntdll: Buffer pagemap reads in fill_working_set_info(). ntdll: Fill range of output in fill_working_set_info(). ntdll: Limit vprot scan range to the needed interval in get_working_set_ex(). ntdll: Iterate views instead of requested addresses in get_working_set_ex(). https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/psapi/tests/psapi_main.c | 85 +++++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 24 deletions(-) diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c index e7e5e5f04e6..515364f156d 100644 --- a/dlls/psapi/tests/psapi_main.c +++ b/dlls/psapi/tests/psapi_main.c @@ -1150,6 +1150,32 @@ free_page: VirtualFree(addr, 0, MEM_RELEASE); } +static void check_working_set_info(PSAPI_WORKING_SET_EX_INFORMATION *info, const char *desc, DWORD expected_valid, + DWORD expected_protection, DWORD expected_shared, BOOL todo) +{ + todo_wine_if(todo) + ok(info->VirtualAttributes.Valid == expected_valid, "%s expected Valid=%lu but got %u\n", + desc, expected_valid, info->VirtualAttributes.Valid); + + todo_wine_if(todo) + ok(info->VirtualAttributes.Win32Protection == expected_protection, "%s expected Win32Protection=%lu but got %u\n", + desc, expected_protection, info->VirtualAttributes.Win32Protection); + + ok(info->VirtualAttributes.Node == 0, "%s expected Node=0 but got %u\n", + desc, info->VirtualAttributes.Node); + ok(info->VirtualAttributes.LargePage == 0, "%s expected LargePage=0 but got %u\n", + desc, info->VirtualAttributes.LargePage); + + ok(info->VirtualAttributes.Shared == expected_shared || broken(!info->VirtualAttributes.Valid) /* w2003 */, + "%s expected Shared=%lu but got %u\n", desc, expected_shared, info->VirtualAttributes.Shared); + if (info->VirtualAttributes.Valid && info->VirtualAttributes.Shared) + ok(info->VirtualAttributes.ShareCount > 0, "%s expected ShareCount > 0 but got %u\n", + desc, info->VirtualAttributes.ShareCount); + else + ok(info->VirtualAttributes.ShareCount == 0, "%s expected ShareCount == 0 but got %u\n", + desc, info->VirtualAttributes.ShareCount); +} + static void check_QueryWorkingSetEx(PVOID addr, const char *desc, DWORD expected_valid, DWORD expected_protection, DWORD expected_shared, BOOL todo) { @@ -1161,32 +1187,13 @@ static void check_QueryWorkingSetEx(PVOID addr, const char *desc, DWORD expected ret = pQueryWorkingSetEx(GetCurrentProcess(), &info, sizeof(info)); ok(ret, "QueryWorkingSetEx failed with %ld\n", GetLastError()); - todo_wine_if(todo) - ok(info.VirtualAttributes.Valid == expected_valid, "%s expected Valid=%lu but got %u\n", - desc, expected_valid, info.VirtualAttributes.Valid); - - todo_wine_if(todo) - ok(info.VirtualAttributes.Win32Protection == expected_protection, "%s expected Win32Protection=%lu but got %u\n", - desc, expected_protection, info.VirtualAttributes.Win32Protection); - - ok(info.VirtualAttributes.Node == 0, "%s expected Node=0 but got %u\n", - desc, info.VirtualAttributes.Node); - ok(info.VirtualAttributes.LargePage == 0, "%s expected LargePage=0 but got %u\n", - desc, info.VirtualAttributes.LargePage); - - ok(info.VirtualAttributes.Shared == expected_shared || broken(!info.VirtualAttributes.Valid) /* w2003 */, - "%s expected Shared=%lu but got %u\n", desc, expected_shared, info.VirtualAttributes.Shared); - if (info.VirtualAttributes.Valid && info.VirtualAttributes.Shared) - ok(info.VirtualAttributes.ShareCount > 0, "%s expected ShareCount > 0 but got %u\n", - desc, info.VirtualAttributes.ShareCount); - else - ok(info.VirtualAttributes.ShareCount == 0, "%s expected ShareCount == 0 but got %u\n", - desc, info.VirtualAttributes.ShareCount); + check_working_set_info(&info, desc, expected_valid, expected_protection, expected_shared, todo); } static void test_QueryWorkingSetEx(void) { - PVOID addr; + PSAPI_WORKING_SET_EX_INFORMATION info[4]; + char *addr, *addr2; DWORD prot; BOOL ret; @@ -1196,7 +1203,7 @@ static void test_QueryWorkingSetEx(void) return; } - addr = GetModuleHandleA(NULL); + addr = (void *)GetModuleHandleA(NULL); check_QueryWorkingSetEx(addr, "exe", 1, PAGE_READONLY, 1, FALSE); ret = VirtualProtect(addr, 0x1000, PAGE_NOACCESS, &prot); @@ -1211,7 +1218,7 @@ static void test_QueryWorkingSetEx(void) ok(ret, "VirtualProtect failed with %ld\n", GetLastError()); check_QueryWorkingSetEx(addr, "exe,readonly2", 1, PAGE_READONLY, 1, FALSE); - addr = VirtualAlloc(NULL, 0x1000, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + addr = VirtualAlloc(NULL, 0x2000, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); ok(addr != NULL, "VirtualAlloc failed with %ld\n", GetLastError()); check_QueryWorkingSetEx(addr, "valloc", 0, 0, 0, FALSE); @@ -1232,9 +1239,39 @@ static void test_QueryWorkingSetEx(void) *(volatile char *)addr; check_QueryWorkingSetEx(addr, "valloc,readwrite2", 1, PAGE_READWRITE, 0, FALSE); + addr2 = VirtualAlloc(NULL, 0x2000, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + ok(!!addr2, "VirtualAlloc failed with %ld\n", GetLastError()); + *(addr2 + 0x1000) = 1; + + info[1].VirtualAddress = addr; + info[0].VirtualAddress = addr + 0x1000; + info[3].VirtualAddress = addr2; + info[2].VirtualAddress = addr2 + 0x1000; + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, sizeof(info)); + ok(ret, "got error %lu\n", GetLastError()); + check_working_set_info(&info[1], "[1] range[1] valid", 1, PAGE_READWRITE, 0, FALSE); + check_working_set_info(&info[0], "[1] range[0] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[3], "[1] range[3] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[2], "[1] range[2] valid", 1, PAGE_READWRITE, 0, FALSE); + ret = VirtualFree(addr, 0, MEM_RELEASE); ok(ret, "VirtualFree failed with %ld\n", GetLastError()); check_QueryWorkingSetEx(addr, "valloc,free", FALSE, 0, 0, FALSE); + + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, sizeof(info)); + ok(ret, "got error %lu\n", GetLastError()); + check_working_set_info(&info[1], "[2] range[1] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[0], "[2] range[0] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[3], "[2] range[3] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[2], "[2] range[2] valid", 1, PAGE_READWRITE, 0, FALSE); + + VirtualFree(addr2, 0, MEM_RELEASE); + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, sizeof(info)); + ok(ret, "got error %lu\n", GetLastError()); + check_working_set_info(&info[1], "[3] range[1] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[0], "[3] range[0] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[3], "[3] range[3] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[2], "[3] range[2] invalid", 0, 0, 0, FALSE); } START_TEST(psapi_main) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 4 +++- dlls/psapi/tests/psapi_main.c | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 3981905bcd3..077b68f59f7 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5145,6 +5145,8 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, return STATUS_INVALID_INFO_CLASS; } + if (len < sizeof(*info)) return STATUS_INFO_LENGTH_MISMATCH; + #if defined(HAVE_LIBPROCSTAT) { struct procstat *pstat; @@ -5241,7 +5243,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, #endif if (res_len) - *res_len = (UINT_PTR)p - (UINT_PTR)info; + *res_len = len; return STATUS_SUCCESS; } diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c index 515364f156d..92529447afa 100644 --- a/dlls/psapi/tests/psapi_main.c +++ b/dlls/psapi/tests/psapi_main.c @@ -1194,6 +1194,8 @@ static void test_QueryWorkingSetEx(void) { PSAPI_WORKING_SET_EX_INFORMATION info[4]; char *addr, *addr2; + NTSTATUS status; + SIZE_T size; DWORD prot; BOOL ret; @@ -1203,6 +1205,25 @@ static void test_QueryWorkingSetEx(void) return; } + size = 0xdeadbeef; + memset(info, 0, sizeof(info)); + status = pNtQueryVirtualMemory(GetCurrentProcess(), NULL, MemoryWorkingSetExInformation, info, 0, &size); + ok(status == STATUS_INFO_LENGTH_MISMATCH, "got %#lx.\n", status); + ok(size == 0xdeadbeef, "got %Iu.\n", size); + + memset(&info, 0, sizeof(info)); + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, 0); + ok(!ret && GetLastError() == ERROR_BAD_LENGTH, "got ret %d, err %lu.\n", ret, GetLastError()); + + size = 0xdeadbeef; + memset(info, 0, sizeof(info)); + status = pNtQueryVirtualMemory(GetCurrentProcess(), NULL, MemoryWorkingSetExInformation, info, + sizeof(*info) + sizeof(*info) / 2, &size); + ok(!status, "got %#lx.\n", status); + ok(!info->VirtualAttributes.Valid, "got %d.\n", info->VirtualAttributes.Valid); + ok(size == sizeof(*info) /* wow64 */ || size == sizeof(*info) + sizeof(*info) / 2 /* win64 */, + "got %Iu, sizeof(info) %Iu.\n", size, sizeof(info)); + addr = (void *)GetModuleHandleA(NULL); check_QueryWorkingSetEx(addr, "exe", 1, PAGE_READONLY, 1, FALSE); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 202 +++++++++++++++++++++----------------- 1 file changed, 111 insertions(+), 91 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 077b68f59f7..1372eea78ae 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5129,15 +5129,117 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY return STATUS_SUCCESS; } +#if defined(HAVE_LIBPROCSTAT) +struct fill_working_set_info_data +{ + struct procstat *pstat; + struct kinfo_proc *kip; + unsigned int vmentry_count; + struct kinfo_vmentry *vmentries; +}; + +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ + unsigned int proc_count; + + d->kip = NULL; + d->vmentry_count = 0; + d->vmentries = NULL; + + if ((d->pstat = procstat_open_sysctl())) + d->kip = procstat_getprocs( d->pstat, KERN_PROC_PID, getpid(), &proc_count ); + if (d->kip) + d->vmentries = procstat_getvmmap( d->pstat, d->kip, &d->vmentry_count ); + if (!d->vmentries) + WARN( "couldn't get process vmmap, errno %d\n", errno ); +} + +static void free_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ + if (d->vmentries) + procstat_freevmmap( d->pstat, d->vmentries ); + if (d->kip) + procstat_freeprocs( d->pstat, d->kip ); + if (d->pstat) + procstat_close( d->pstat ); +} + +static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, + MEMORY_WORKING_SET_EX_INFORMATION *p ) +{ + struct kinfo_vmentry *entry = NULL; + int i; + + for (i = 0; i < d->vmentry_count; i++) + { + if (d->vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= d->vmentries[i].kve_end) + { + entry = &d->vmentries[i]; + break; + } + } + + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; + p->VirtualAttributes.Shared = !is_view_valloc( view ); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); +} +#else +static int pagemap_fd = -2; + +struct fill_working_set_info_data +{ +}; + +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ + if (pagemap_fd != -2) return; + +#ifdef O_CLOEXEC + if ((pagemap_fd = open( "/proc/self/pagemap", O_RDONLY | O_CLOEXEC, 0 )) == -1 && errno == EINVAL) +#endif + pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 ); + + if (pagemap_fd == -1) WARN( "unable to open /proc/self/pagemap\n" ); + else fcntl(pagemap_fd, F_SETFD, FD_CLOEXEC); /* in case O_CLOEXEC isn't supported */ +} + +static void free_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ +} + +static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, + MEMORY_WORKING_SET_EX_INFORMATION *p ) +{ + UINT64 pagemap; + + if (pagemap_fd == -1 || + pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + { + /* If we don't have pagemap information, default to invalid. */ + pagemap = 0; + } + + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); + p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); +} +#endif + static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, MEMORY_WORKING_SET_EX_INFORMATION *info, SIZE_T len, SIZE_T *res_len ) { -#if !defined(HAVE_LIBPROCSTAT) - static int pagemap_fd = -2; -#endif + struct fill_working_set_info_data data; MEMORY_WORKING_SET_EX_INFORMATION *p; + struct file_view *view; sigset_t sigset; + BYTE vprot; if (process != NtCurrentProcess()) { @@ -5147,100 +5249,18 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, if (len < sizeof(*info)) return STATUS_INFO_LENGTH_MISMATCH; -#if defined(HAVE_LIBPROCSTAT) - { - struct procstat *pstat; - unsigned int proc_count; - struct kinfo_proc *kip = NULL; - unsigned int vmentry_count = 0; - struct kinfo_vmentry *vmentries = NULL; - - pstat = procstat_open_sysctl(); - if (pstat) - kip = procstat_getprocs( pstat, KERN_PROC_PID, getpid(), &proc_count ); - if (kip) - vmentries = procstat_getvmmap( pstat, kip, &vmentry_count ); - if (vmentries == NULL) - WARN( "couldn't get process vmmap, errno %d\n", errno ); - - server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) - { - int i; - struct kinfo_vmentry *entry = NULL; - BYTE vprot; - struct file_view *view; - - memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) ); - if ((view = find_view( p->VirtualAddress, 0 )) && - get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) && - (vprot & VPROT_COMMITTED)) - { - for (i = 0; i < vmentry_count && entry == NULL; i++) - { - if (vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= vmentries[i].kve_end) - entry = &vmentries[i]; - } - - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; - p->VirtualAttributes.Shared = !is_view_valloc( view ); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); - } - } - server_leave_uninterrupted_section( &virtual_mutex, &sigset ); - - if (vmentries) - procstat_freevmmap( pstat, vmentries ); - if (kip) - procstat_freeprocs( pstat, kip ); - if (pstat) - procstat_close( pstat ); - } -#else server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - if (pagemap_fd == -2) - { -#ifdef O_CLOEXEC - if ((pagemap_fd = open( "/proc/self/pagemap", O_RDONLY | O_CLOEXEC, 0 )) == -1 && errno == EINVAL) -#endif - pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 ); - - if (pagemap_fd == -1) WARN( "unable to open /proc/self/pagemap\n" ); - else fcntl(pagemap_fd, F_SETFD, FD_CLOEXEC); /* in case O_CLOEXEC isn't supported */ - } - + init_fill_working_set_info_data( &data ); for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) { - BYTE vprot; - UINT64 pagemap; - struct file_view *view; - memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) ); - - if ((view = find_view( p->VirtualAddress, 0 )) && - get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) && - (vprot & VPROT_COMMITTED)) - { - if (pagemap_fd == -1 || - pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) - { - /* If we don't have pagemap information, default to invalid. */ - pagemap = 0; - } - - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); - p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); - } + if ((view = find_view( p->VirtualAddress, 0 )) + && get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) + && (vprot & VPROT_COMMITTED)) + fill_working_set_info( &data, view, vprot, p ); } + free_fill_working_set_info_data( &data ); server_leave_uninterrupted_section( &virtual_mutex, &sigset ); -#endif if (res_len) *res_len = len; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 68 ++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 1372eea78ae..14bc510bcc4 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5129,6 +5129,12 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY return STATUS_SUCCESS; } +struct working_set_info_ref +{ + char *addr; + SIZE_T orig_index; +}; + #if defined(HAVE_LIBPROCSTAT) struct fill_working_set_info_data { @@ -5231,13 +5237,24 @@ static void fill_working_set_info( struct fill_working_set_info_data *d, struct } #endif +static int compare_working_set_info_ref( const void *a, const void *b ) +{ + const struct working_set_info_ref *r1 = a, *r2 = b; + + if (r1->addr < r2->addr) return -1; + return r1->addr > r2->addr; +} + static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, MEMORY_WORKING_SET_EX_INFORMATION *info, SIZE_T len, SIZE_T *res_len ) { + struct working_set_info_ref ref_buffer[256], *ref = ref_buffer, *r; struct fill_working_set_info_data data; - MEMORY_WORKING_SET_EX_INFORMATION *p; - struct file_view *view; + char *start, *end; + SIZE_T i, count; + size_t size; + struct file_view *view, *prev_view; sigset_t sigset; BYTE vprot; @@ -5249,17 +5266,52 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, if (len < sizeof(*info)) return STATUS_INFO_LENGTH_MISMATCH; + count = len / sizeof(*info); + + if (count > ARRAY_SIZE(ref_buffer)) ref = malloc( count * sizeof(*ref) ); + for (i = 0; i < count; ++i) + { + ref[i].orig_index = i; + ref[i].addr = ROUND_ADDR( info[i].VirtualAddress, page_mask ); + info[i].VirtualAttributes.Flags = 0; + } + qsort( ref, count, sizeof(*ref), compare_working_set_info_ref ); + start = ref[0].addr; + end = ref[count - 1].addr + page_size; + server_enter_uninterrupted_section( &virtual_mutex, &sigset ); init_fill_working_set_info_data( &data ); - for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) + + view = find_view_range( start, end - start ); + while (view && (char *)view->base > start) { - memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) ); - if ((view = find_view( p->VirtualAddress, 0 )) - && get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) - && (vprot & VPROT_COMMITTED)) - fill_working_set_info( &data, view, vprot, p ); + prev_view = RB_ENTRY_VALUE( rb_prev( &view->entry ), struct file_view, entry ); + if (!prev_view || (char *)prev_view->base + prev_view->size <= start) break; + view = prev_view; } + + r = ref; + while (view && (char *)view->base < end) + { + if (start < (char *)view->base) start = view->base; + while (r != ref + count && r->addr < start) ++r; + while (start != (char *)view->base + view->size && r != ref + count + && r->addr < (char *)view->base + view->size) + { + size = get_committed_size( view, start, &vprot, ~VPROT_WRITEWATCH ); + while (r != ref + count && r->addr < start + size) + { + if (vprot & VPROT_COMMITTED) fill_working_set_info( &data, view, vprot, &info[r->orig_index] ); + ++r; + } + start += size; + } + if (r == ref + count) break; + view = RB_ENTRY_VALUE( rb_next( &view->entry ), struct file_view, entry ); + } + free_fill_working_set_info_data( &data ); + if (ref != ref_buffer) free( ref ); server_leave_uninterrupted_section( &virtual_mutex, &sigset ); if (res_len) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 14bc510bcc4..7e61806b259 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -2140,7 +2140,7 @@ done: * Get the size of the committed range with equal masked vprot bytes starting at base. * Also return the protections for the first page. */ -static SIZE_T get_committed_size( struct file_view *view, void *base, BYTE *vprot, BYTE vprot_mask ) +static SIZE_T get_committed_size( struct file_view *view, void *base, size_t max_size, BYTE *vprot, BYTE vprot_mask ) { SIZE_T offset, size; @@ -2159,7 +2159,7 @@ static SIZE_T get_committed_size( struct file_view *view, void *base, BYTE *vpro req->offset = offset; if (!wine_server_call( req )) { - size = reply->size; + size = min( reply->size, max_size ); if (reply->committed) { *vprot |= VPROT_COMMITTED; @@ -2171,7 +2171,7 @@ static SIZE_T get_committed_size( struct file_view *view, void *base, BYTE *vpro if (!size || !(vprot_mask & ~VPROT_COMMITTED)) return size; } - else size = view->size - offset; + else size = min( view->size - offset, max_size ); return get_vprot_range_size( base, size, vprot_mask, vprot ); } @@ -4921,7 +4921,7 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T if ((view = find_view( base, size ))) { /* Make sure all the pages are committed */ - if (get_committed_size( view, base, &vprot, VPROT_COMMITTED ) >= size && (vprot & VPROT_COMMITTED)) + if (get_committed_size( view, base, ~(size_t)0, &vprot, VPROT_COMMITTED ) >= size && (vprot & VPROT_COMMITTED)) { old = get_win32_prot( vprot, view->protect ); status = set_protection( view, base, size, new_prot ); @@ -5043,7 +5043,7 @@ static unsigned int fill_basic_memory_info( const void *addr, MEMORY_BASIC_INFOR BYTE vprot; info->AllocationBase = alloc_base; - info->RegionSize = get_committed_size( view, base, &vprot, ~VPROT_WRITEWATCH ); + info->RegionSize = get_committed_size( view, base, ~(size_t)0, &vprot, ~VPROT_WRITEWATCH ); info->State = (vprot & VPROT_COMMITTED) ? MEM_COMMIT : MEM_RESERVE; info->Protect = (vprot & VPROT_COMMITTED) ? get_win32_prot( vprot, view->protect ) : 0; info->AllocationProtect = get_win32_prot( view->protect, view->protect ); @@ -5298,7 +5298,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, while (start != (char *)view->base + view->size && r != ref + count && r->addr < (char *)view->base + view->size) { - size = get_committed_size( view, start, &vprot, ~VPROT_WRITEWATCH ); + size = get_committed_size( view, start, end - start, &vprot, ~VPROT_WRITEWATCH ); while (r != ref + count && r->addr < start + size) { if (vprot & VPROT_COMMITTED) fill_working_set_info( &data, view, vprot, &info[r->orig_index] ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 75 ++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 7e61806b259..406e5cefe54 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5171,26 +5171,33 @@ static void free_fill_working_set_info_data( struct fill_working_set_info_data * } static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, - MEMORY_WORKING_SET_EX_INFORMATION *p ) + struct working_set_info_ref *ref, SIZE_T count, + MEMORY_WORKING_SET_EX_INFORMATION *info ) { - struct kinfo_vmentry *entry = NULL; - int i; + SIZE_T i; + int j; - for (i = 0; i < d->vmentry_count; i++) + for (i = 0; i < count; ++i) { - if (d->vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= d->vmentries[i].kve_end) + MEMORY_WORKING_SET_EX_INFORMATION *p = &info[ref[i].orig_index]; + struct kinfo_vmentry *entry = NULL; + + for (j = 0; j < d->vmentry_count; j++) { - entry = &d->vmentries[i]; - break; + if (d->vmentries[j].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= d->vmentries[j].kve_end) + { + entry = &d->vmentries[j]; + break; + } } - } - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; - p->VirtualAttributes.Shared = !is_view_valloc( view ); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; + p->VirtualAttributes.Shared = !is_view_valloc( view ); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + } } #else static int pagemap_fd = -2; @@ -5217,23 +5224,31 @@ static void free_fill_working_set_info_data( struct fill_working_set_info_data * } static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, - MEMORY_WORKING_SET_EX_INFORMATION *p ) + struct working_set_info_ref *ref, SIZE_T count, + MEMORY_WORKING_SET_EX_INFORMATION *info ) { + MEMORY_WORKING_SET_EX_INFORMATION *p; UINT64 pagemap; + SIZE_T i; - if (pagemap_fd == -1 || - pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + for (i = 0; i < count; ++i) { - /* If we don't have pagemap information, default to invalid. */ - pagemap = 0; - } + p = &info[ref[i].orig_index]; + + if (pagemap_fd == -1 || + pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + { + /* If we don't have pagemap information, default to invalid. */ + pagemap = 0; + } - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); - p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); + p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + } } #endif @@ -5299,12 +5314,14 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, && r->addr < (char *)view->base + view->size) { size = get_committed_size( view, start, end - start, &vprot, ~VPROT_WRITEWATCH ); + start += size; while (r != ref + count && r->addr < start + size) { - if (vprot & VPROT_COMMITTED) fill_working_set_info( &data, view, vprot, &info[r->orig_index] ); - ++r; + i = 0; + while (r + i != ref + count && r[i].addr < start) ++i; + if (vprot & VPROT_COMMITTED) fill_working_set_info( &data, view, vprot, r, i, info ); + r += i; } - start += size; } if (r == ref + count) break; view = RB_ENTRY_VALUE( rb_next( &view->entry ), struct file_view, entry ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 406e5cefe54..ef7868b4b4a 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5144,7 +5144,7 @@ struct fill_working_set_info_data struct kinfo_vmentry *vmentries; }; -static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d, char *end ) { unsigned int proc_count; @@ -5204,10 +5204,19 @@ static int pagemap_fd = -2; struct fill_working_set_info_data { + UINT64 pm_buffer[256]; + SIZE_T buffer_start; + ssize_t buffer_len; + SIZE_T end_page; }; -static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d, char *end ) { + d->buffer_start = 0; + d->buffer_len = 0; + d->end_page = (UINT_PTR)end >> page_shift; + memset( d->pm_buffer, 0, sizeof(d->pm_buffer) ); + if (pagemap_fd != -2) return; #ifdef O_CLOEXEC @@ -5229,18 +5238,31 @@ static void fill_working_set_info( struct fill_working_set_info_data *d, struct { MEMORY_WORKING_SET_EX_INFORMATION *p; UINT64 pagemap; - SIZE_T i; + SIZE_T i, page; + ssize_t len; for (i = 0; i < count; ++i) { + page = (UINT_PTR)ref[i].addr >> page_shift; p = &info[ref[i].orig_index]; - if (pagemap_fd == -1 || - pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + assert(page >= d->buffer_start); + if (page >= d->buffer_start + d->buffer_len) { - /* If we don't have pagemap information, default to invalid. */ - pagemap = 0; + d->buffer_start = page; + len = min( sizeof(d->pm_buffer), (d->end_page - page) * sizeof(pagemap) ); + if (pagemap_fd != -1) + { + d->buffer_len = pread( pagemap_fd, d->pm_buffer, len, page * sizeof(pagemap) ); + if (d->buffer_len != len) + { + d->buffer_len = max( d->buffer_len, 0 ); + memset( d->pm_buffer + d->buffer_len / sizeof(pagemap), 0, len - d->buffer_len ); + } + } + d->buffer_len = len / sizeof(pagemap); } + pagemap = d->pm_buffer[page - d->buffer_start]; p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); @@ -5295,7 +5317,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, end = ref[count - 1].addr + page_size; server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - init_fill_working_set_info_data( &data ); + init_fill_working_set_info_data( &data, end ); view = find_view_range( start, end - start ); while (view && (char *)view->base > start) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146518 Your paranoid android. === debian11b (64 bit WoW report) === Report validation errors: mshtml:misc crashed (c0000005)
v2: - properly skip uncommitted ranges without calling fill_working_set_info() for those. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5907#note_74231
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)