[PATCH 1/8] kernel32/tests: Add exception handling race test.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)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(+)

[Marvin]

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).