Correctly fixing bug 48665 requires keeping track of which pages of a PAGE_WRITECOPY mapping have been modified. Thankfully, there is already a patchset in wine-staging that adds support for PAGE_WRITECOPY, but unfortunately it is disabled by default because it has some bugs.
This series fixes the bugs I observed and adds an additional bit to keep track of whether the WRITECOPY pages have been modified. This can be used by QueryWorkingSetEx to properly report a page as shared or not.
I suggest that these patches are first applied to wine-staging to shake out any other bugs in the patchset, since it had been disabled by default and these patches will unconditionally enable it for x86 and x64. Provided that no regressions are observed, we can consider whether it should go into upstream. I suspect performance regressions are also possible.
Andrew Wesie (4): ntdll: Track if a WRITECOPY page has been modified. ntdll: Support WRITECOPY on x64. ntdll: Always enable WRITECOPY support. ntdll: Report unmodified WRITECOPY pages as shared.
dlls/ntdll/signal_x86_64.c | 40 ++++++++++++++++++++++++++++++ dlls/ntdll/virtual.c | 51 +++++++++++++++++++------------------- 2 files changed, 65 insertions(+), 26 deletions(-)
Once a WRITECOPY page is modified, it should be mapped as if it is a normal read-write page.
Signed-off-by: Andrew Wesie awesie@gmail.com --- dlls/ntdll/virtual.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 0d943de437..2c8d612f8a 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -84,6 +84,7 @@ struct file_view #define VPROT_GUARD 0x10 #define VPROT_COMMITTED 0x20 #define VPROT_WRITEWATCH 0x40 +#define VPROT_WRITTEN 0x80 /* per-mapping protection flags */ #define VPROT_SYSTEM 0x0200 /* system view (underlying mmap not under our control) */
@@ -342,7 +343,7 @@ static int VIRTUAL_GetUnixProt( BYTE vprot ) #if defined(__i386__) if (vprot & VPROT_WRITECOPY) { - if (experimental_WRITECOPY()) + if (experimental_WRITECOPY() && !(vprot & VPROT_WRITTEN)) prot = (prot & ~PROT_WRITE) | PROT_READ; else prot |= PROT_WRITE | PROT_READ; @@ -927,7 +928,11 @@ static NTSTATUS create_view( struct file_view **view_ret, void *base, size_t siz */ static DWORD VIRTUAL_GetWin32Prot( BYTE vprot, unsigned int map_prot ) { - DWORD ret = VIRTUAL_Win32Flags[vprot & 0x0f]; + DWORD ret; + + if ((vprot & VPROT_WRITECOPY) && (vprot & VPROT_WRITTEN)) + vprot = (vprot & ~VPROT_WRITECOPY) | VPROT_WRITE; + ret = VIRTUAL_Win32Flags[vprot & 0x0f]; if (vprot & VPROT_GUARD) ret |= PAGE_GUARD; if (map_prot & SEC_NOCACHE) ret |= PAGE_NOCACHE; return ret; @@ -1051,7 +1056,7 @@ static BOOL VIRTUAL_SetProt( struct file_view *view, /* [in] Pointer to view */ if (view->protect & VPROT_WRITEWATCH) { /* each page may need different protections depending on write watch flag */ - set_page_vprot_bits( base, size, vprot & ~VPROT_WRITEWATCH, ~vprot & ~VPROT_WRITEWATCH ); + set_page_vprot_bits( base, size, vprot & ~VPROT_WRITEWATCH, ~vprot & ~(VPROT_WRITEWATCH|VPROT_WRITTEN) ); mprotect_range( base, size, 0, 0 ); return TRUE; } @@ -1067,10 +1072,18 @@ static BOOL VIRTUAL_SetProt( struct file_view *view, /* [in] Pointer to view */ return TRUE; }
+ /* check that we can map this memory with PROT_WRITE since we cannot fail later */ + if (vprot & VPROT_WRITECOPY) + unix_prot |= PROT_WRITE; + if (mprotect_exec( base, size, unix_prot )) /* FIXME: last error */ return FALSE;
- set_page_vprot( base, size, vprot ); + /* each page may need different protections depending on writecopy */ + set_page_vprot_bits( base, size, vprot, ~vprot & ~VPROT_WRITTEN ); + if (vprot & VPROT_WRITECOPY) + mprotect_range( base, size, 0, 0 ); + return TRUE; }
@@ -2253,7 +2266,7 @@ NTSTATUS virtual_handle_fault( LPCVOID addr, DWORD err, BOOL on_signal_stack ) } if (vprot & VPROT_WRITECOPY) { - set_page_vprot_bits( page, page_size, VPROT_WRITE, VPROT_WRITECOPY ); + set_page_vprot_bits( page, page_size, VPROT_WRITE | VPROT_WRITTEN, VPROT_WRITECOPY ); mprotect_range( page, page_size, 0, 0 ); } /* ignore fault if page is writable now */ @@ -3157,7 +3170,7 @@ static NTSTATUS get_basic_memory_info( HANDLE process, LPCVOID addr, else if (view->protect & (SEC_FILE | SEC_RESERVE | SEC_COMMIT)) info->Type = MEM_MAPPED; else info->Type = MEM_PRIVATE; for (ptr = base; ptr < base + range_size; ptr += page_size) - if ((get_page_vprot( ptr ) ^ vprot) & ~VPROT_WRITEWATCH) break; + if ((get_page_vprot( ptr ) ^ vprot) & ~(VPROT_WRITEWATCH|VPROT_WRITTEN)) break; info->RegionSize = ptr - base; } server_leave_uninterrupted_section( &csVirtual, &sigset );
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=70460
Your paranoid android.
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 Task: Patch failed to apply
Signed-off-by: Andrew Wesie awesie@gmail.com --- dlls/ntdll/signal_x86_64.c | 40 ++++++++++++++++++++++++++++++++++++++ dlls/ntdll/virtual.c | 2 +- 2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index be23c50898..e8620bff3b 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -2871,6 +2871,29 @@ static inline BOOL handle_interrupt( ucontext_t *sigcontext, struct stack_layout }
+/********************************************************************** + * segv_handler_early + * + * Handler for SIGSEGV and related errors. Used only during the initialization + * of the process to handle virtual faults. + */ +static void segv_handler_early( int signal, siginfo_t *siginfo, void *sigcontext ) +{ + ucontext_t *ucontext = sigcontext; + + switch(TRAP_sig(ucontext)) + { + case TRAP_x86_PAGEFLT: /* Page fault */ + if (!virtual_handle_fault( siginfo->si_addr, (ERROR_sig(ucontext) >> 1) & 0x09, TRUE )) + return; + /* fall-through */ + default: + WINE_ERR( "Got unexpected trap %lld during process initialization\n", TRAP_sig(ucontext) ); + abort_thread(1); + break; + } +} + /********************************************************************** * segv_handler * @@ -3307,6 +3330,23 @@ void signal_init_process(void) */ void signal_init_early(void) { + struct sigaction sig_act; + + sig_act.sa_mask = server_block_set; + sig_act.sa_flags = SA_RESTART | SA_SIGINFO | SA_ONSTACK; + + sig_act.sa_sigaction = segv_handler_early; + if (sigaction( SIGSEGV, &sig_act, NULL ) == -1) goto error; + if (sigaction( SIGILL, &sig_act, NULL ) == -1) goto error; +#ifdef SIGBUS + if (sigaction( SIGBUS, &sig_act, NULL ) == -1) goto error; +#endif + + return; + + error: + perror("sigaction"); + exit(1); }
static ULONG64 get_int_reg( CONTEXT *context, int reg ) diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 2c8d612f8a..6d57e50c7b 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -340,7 +340,7 @@ static int VIRTUAL_GetUnixProt( BYTE vprot ) if (vprot & VPROT_READ) prot |= PROT_READ; if (vprot & VPROT_WRITE) prot |= PROT_WRITE | PROT_READ; if (vprot & VPROT_EXEC) prot |= PROT_EXEC | PROT_READ; -#if defined(__i386__) +#if defined(__i386__) || defined(__x86_64__) if (vprot & VPROT_WRITECOPY) { if (experimental_WRITECOPY() && !(vprot & VPROT_WRITTEN))
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=70461
Your paranoid android.
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 error: patch failed: dlls/ntdll/signal_x86_64.c:3307 error: patch failed: dlls/ntdll/virtual.c:340 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 error: patch failed: dlls/ntdll/signal_x86_64.c:3307 error: patch failed: dlls/ntdll/virtual.c:340 Task: Patch failed to apply
Signed-off-by: Andrew Wesie awesie@gmail.com --- dlls/ntdll/virtual.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 6d57e50c7b..dba2e3380c 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -311,22 +311,6 @@ static const char *VIRTUAL_GetProtStr( BYTE prot ) return buffer; }
-/* This might look like a hack, but it actually isn't - the 'experimental' version - * is correct, but it already has revealed a couple of additional Wine bugs, which - * were not triggered before, and there are probably some more. - * To avoid breaking Wine for everyone, the new correct implementation has to be - * manually enabled, until it is tested a bit more. */ -static inline BOOL experimental_WRITECOPY( void ) -{ - static int enabled = -1; - if (enabled == -1) - { - const char *str = getenv("STAGING_WRITECOPY"); - enabled = str && (atoi(str) != 0); - } - return enabled; -} - /*********************************************************************** * VIRTUAL_GetUnixProt * @@ -343,10 +327,10 @@ static int VIRTUAL_GetUnixProt( BYTE vprot ) #if defined(__i386__) || defined(__x86_64__) if (vprot & VPROT_WRITECOPY) { - if (experimental_WRITECOPY() && !(vprot & VPROT_WRITTEN)) - prot = (prot & ~PROT_WRITE) | PROT_READ; - else + if (vprot & VPROT_WRITTEN) prot |= PROT_WRITE | PROT_READ; + else + prot = (prot & ~PROT_WRITE) | PROT_READ; } #else /* FIXME: Architecture needs implementation of signal_init_early. */
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=70462
Your paranoid android.
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 error: patch failed: dlls/ntdll/signal_x86_64.c:3307 error: patch failed: dlls/ntdll/virtual.c:340 error: patch failed: dlls/ntdll/virtual.c:311 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 error: patch failed: dlls/ntdll/signal_x86_64.c:3307 error: patch failed: dlls/ntdll/virtual.c:340 error: patch failed: dlls/ntdll/virtual.c:311 Task: Patch failed to apply
We also need to clear the modified bit after we clear memory in map_image to match the behavior of Windows.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48665 Signed-off-by: Andrew Wesie awesie@gmail.com --- dlls/ntdll/virtual.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index dba2e3380c..70eaf8d43f 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -1721,6 +1721,8 @@ static NTSTATUS map_image( HANDLE hmapping, ACCESS_MASK access, int fd, int top_ ptr + sec->VirtualAddress + file_size, ptr + sec->VirtualAddress + end ); memset( ptr + sec->VirtualAddress + file_size, 0, end - file_size ); + /* clear WRITTEN mark so QueryVirtualMemory returns correct values */ + set_page_vprot_bits( ptr + sec->VirtualAddress + file_size, 1, 0, VPROT_WRITTEN ); } }
@@ -3205,7 +3207,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, (vprot & VPROT_COMMITTED)) { p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); - p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); + p->VirtualAttributes.Shared = (!is_view_valloc( view ) && ((pagemap >> 61) & 1)) || ((view->protect & VPROT_WRITECOPY) && !(vprot & VPROT_WRITTEN)); if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) p->VirtualAttributes.ShareCount = 1; /* FIXME */ if (p->VirtualAttributes.Valid)
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=70463
Your paranoid android.
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 error: patch failed: dlls/ntdll/signal_x86_64.c:3307 error: patch failed: dlls/ntdll/virtual.c:340 error: patch failed: dlls/ntdll/virtual.c:311 Task: Patch failed to apply
=== debiant (build log) ===
error: patch failed: dlls/ntdll/virtual.c:342 error: patch failed: dlls/ntdll/signal_x86_64.c:3307 error: patch failed: dlls/ntdll/virtual.c:340 error: patch failed: dlls/ntdll/virtual.c:311 Task: Patch failed to apply
-
Andrew Wesie -
Marvin