Signed-off-by: Jacek Caban jacek@codeweavers.com ---
This patch demonstrates a problem that I noticed while debugging VS remote debugger on Wine. When you use 'break' command of the debugger, VS does following:
1. suspend all debuggee threads (with SuspendThread(), not any dedicated debug API) 2. for each thread, modify process code so that memory of its current instruction is 0xcc (int $3) 3. resume all threads 4. expect breakpoint exception 5. restore process memory
When process is restored in (3), all threads race to execute the breakpoint. The first exception will suspend the whole process until debugger processes the exception. However, multiple threads may have already executed the breakpoint before OS has a chance to suspend them. This is the case on Windows as well (although it seems more likely on Wine, since its supervention requires server round trip and sending signals).
Due to the way this currently works on Wine, it's problematic for two reasons:
1. the thread that loses the race may have already set its suspension context by queue_exception_event and receive SIGUSR1 later. Suspending inside signal handler will fail because the context is already set and the whole thread enters an invalid state. We could fix it by blocking signals inside send_debug_event(), but:
2. suspended threads may already be in exception handling code. This is not really expected. On Windows (except for a few anomalies I've seen; I don't think they are expected behaviour), other threads that manage to execute the breakpoint are reported to be executing it when the process is suspended on the first exception. On Wine, however, they are more likely to be suspended inside exception handling code at this point. This is somewhat weird from debugger point of view: they are not reported as executing the breakpoint before it's removed, but they will report that they hit it anyway.
My solution, implemented for x86_64 by this patch series, is to report the exception and do suspension inside signal handler. Currently we don't do any blocking in signal handlers other than SIGUSR1, but as far as I can see we should be able to do that.
dlls/kernel32/tests/debugger.c | 83 ++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
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=55821
Your paranoid android.
=== debian10 (32 bit report) ===
kernel32: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x00439c82).
=== debian10 (32 bit French report) ===
kernel32: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x00439c82).
=== debian10 (32 bit Japanese:Japan report) ===
kernel32: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x00439c82).
=== debian10 (32 bit Chinese:China report) ===
kernel32: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x00439c82).
=== debian10 (32 bit WoW report) ===
kernel32: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x00439c82).
=== debian10 (64 bit WoW report) ===
kernel32: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x00439c82).