RFC: gdbserver crash on win XP during watchpoint removal
Commit Message
> dr_control_mirror is never supposed to be updated from the target. It's a
> one way street - the state we _want_ dr7 to have on next resume. Where is this
> 0x400 coming from then?
>
> Also, what's different in native GDB? This debug registers code is
> shared between GDB and GDBserver now.
It is read back in win32-i386-low.c::i386_get_thread_context:
if (th->tid == current_event->dwThreadId)
{
/* Copy dr values from the current thread. */
struct x86_debug_reg_state *dr = &debug_reg_state;
dr->dr_mirror[0] = th->context.Dr0;
dr->dr_mirror[1] = th->context.Dr1;
dr->dr_mirror[2] = th->context.Dr2;
dr->dr_mirror[3] = th->context.Dr3;
dr->dr_status_mirror = th->context.Dr6;
dr->dr_control_mirror = th->context.Dr7;
}
debug_reg_state is a global, which is passed to x86_dr_insert_watchpoint
in win32-i386-low.c::i386_insert_point:
return x86_dr_insert_watchpoint (&debug_reg_state,
hw_type, addr, size);
i386_get_thread_context gets called after we receive the watchpoint
signal (TARGET_WAITKIND_STOPPED) in win32_wait:
case TARGET_WAITKIND_STOPPED:
case TARGET_WAITKIND_LOADED:
OUTMSG2 (("Child Stopped with signal = %d \n",
ourstatus->value.sig));
regcache = get_thread_regcache (current_thread, 1);
child_fetch_inferior_registers (regcache, -1);
return debug_event_ptid (¤t_event);
It looks like just removing the statement that sets dr_control_mirror
from the thread's DR7 is sufficient to allow hardware watchpoints
to work again.
gdb/gdbserver/ChangeLog:
* win32-i386-low.c (i386_get_thread_context): Do not set
dr->dr_control_mirror.
Tested on x86-windows using AdaCore's testsuite. No regression while
all watchpoint-related issues get resolved.
I admit I could have spent a little more time trying to understand
the bigger picture, but I'm a bit under time pressure, so I'm hoping
this is enough. Otherwise, I'll look at a better fix.
Once we have a fix, I think we'll want it for GDB 7.8.1. I haven't
completely checked eitherr, but I think it's there as well. I'll create
a PR as soon as I have a minute.
Thanks!
From 8f41c96eda3e46363d6e68e9fc0179e24d0c8922 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 14 Oct 2014 23:18:35 +0200
Subject: [PATCH] state->dr_control_mirror == 0 failed assertion in gdbserver
on Windows XP
When using GDBserver on Windows XP, GDBserver reports and assertion
failure after hitting a hardware watchpoint. The problem was reproduced
using the sources from gdb.ada/int_deref, but should probably reproduce
with any scenario involving hardware watchpoints.
In our scenario, we break on line 5, just before the increment, insert
a watchhpoint on it, and then continue:
(gdb) b foo.adb:5
Breakpoint 1 at 0x4017c2: file foo.adb, line 5.
(gdb) cont
Continuing.
Breakpoint 1, foo () at foo.adb:5
5 Pck.Watch := Pck.Watch + 1;
(gdb) watch watch
Hardware watchpoint 2: watch
(gdb) c
Continuing.
Remote communication error. Target disconnected.: Invalid argument.
The immediate cause for the communication error is easily explained,
gdbserver crashes due to a failed assertion:
x86_remove_aligned_watchpoint: Assertion `state->dr_control_mirror == 0' failed.
The assertion occurs because debug_reg_state.dr_control_mirror gets
overwritten by the value read from the inferior, when processing
the watchpoint event in win32_wait: win32_wait finds that we stopped,
calls get_thread_regcache which causes i386_get_thread_context to
get called, which then...
if (th->tid == current_event->dwThreadId)
{
/* Copy dr values from the current thread. */
struct x86_debug_reg_state *dr = &debug_reg_state;
[...]
dr->dr_control_mirror = th->context.Dr7;
}
Both should be identical, normally making this a no-op, but
it turns out that bits 12-11-10 are documented as being fixed
and equal to 001. Our handling of dr_control_mirror does not
manage those bits, and leaves them as zeros instead. So, when
we overwrite the value from the thread's DR7 register, we
accidentally set bit 10, causing state->dr_control_mirror
to be 0x400 after we've cleared everything internally.
This patch fixes the issue by removing the statement setting
state->dr_control_mirror to the thread's DR7 register value.
gdb/gdbserver/ChangeLog:
* win32-i386-low.c (i386_get_thread_context): Do not set
dr->dr_control_mirror.
---
gdb/gdbserver/win32-i386-low.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
@@ -222,7 +222,6 @@ i386_get_thread_context (win32_thread_info *th, DEBUG_EVENT* current_event)
dr->dr_mirror[2] = th->context.Dr2;
dr->dr_mirror[3] = th->context.Dr3;
dr->dr_status_mirror = th->context.Dr6;
- dr->dr_control_mirror = th->context.Dr7;
}
}
--
1.7.9