Re: [win32] Fix watchpoint support

[Christopher Faylor <cgf-use-the-mailinglist-please at sourceware dot org>]

On Wed, Nov 21, 2007 at 12:35:21AM +0000, Pedro Alves wrote:
> Hi,
>
> The current watchpoint support for native debugging on win32 systems
> doesn't work reliably.  Quite often, we'd miss that the inferior
> stopped due to a watchpoint triggering, because Windows wouldn't
> report in DR6 which watchpoint triggered.  Take a look at this
> "set debug infrun 1; maint show-debug-regs 1" snippet [1]:
>
> infrun: TARGET_WAITKIND_STOPPED
> infrun: stop_pc = 0x40108e
> stopped_data_addr:
>            CONTROL (DR7): 000d0101          STATUS (DR6): 00000000
>                                                           ^^^^^^^^
>            DR0: addr=0x00403010, ref.count=1  DR1: addr=0x00000000, 
> ref.count=0
>            DR2: addr=0x00000000, ref.count=0  DR3: addr=0x00000000, 
> ref.count=0
> infrun: random signal 5
> ^^^^^^^^^^^^^^^^^^^^^^^
>
> Program received signal SIGTRAP, Trace/breakpoint trap.
> infrun: stop_stepping
> remove_watchpoint (addr=403010, len=4, type=data-write):
>            CONTROL (DR7): 000d0100          STATUS (DR6): 00000000
>            DR0: addr=0x00000000, ref.count=0  DR1: addr=0x00000000, 
> ref.count=0
>            DR2: addr=0x00000000, ref.count=0  DR3: addr=0x00000000, 
> ref.count=0
> main () at watch.c:11
> 11        printf ("count %d\n", count);
>
> 0x00403010 in that example is the address of a variable that
> was just written to.
>
> [1] Full example here:
> http://cygwin.com/ml/cygwin/2007-10/msg00057.html
>
> I ended up tracing the problem to win32_continue (gdb/win32-nat.c).
> Currently, it looks somewhat like this:
>
> 1 win32_continue(TID)
> 2       ContinueDebugEvent(current_TID)
> 3       foreach thread in threads do
> 4              if thread == TID
> 5                     ResumeThread(TID)
> 6                     SetThreadContext(TID, DEBUG_REGS)
> 7              fi
> 8       hcaerof
>
> The first problem is that we shoudn't be calling
> SetThreadContext after ResumeThread (5,6) -- it should
> be the other way around.

Yes, that's clearly a bug.  It seems to have been introduced by the
debug register handling.

>2007-11-21  Pedro Alves  <pedro_alves@portugalmail.pt>
>
>	* win32-nat.c (win32_add_thread): Set Dr6 to 0xffff0ff0.
>	(win32_continue): Resume threads and set the debug registers
>	before calling ContinueDebugEvent.

I'm not clear on how this differs from your other patch.  It seems to do
many of the same things.  The principles are ok but it doesn't seem like
it could be applied in addition to the suspend count handling patch.

cgf