From: Pedro Alves Date: Tue, 13 Dec 2011 16:11:26 +0000 (+0000) Subject: 2011-12-13 Pedro Alves X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=6210a125f8517756c18bf50cd313c39d9487f184;p=binutils-gdb.git 2011-12-13 Pedro Alves PR remote/13492 * i386-low.c (i386_low_stopped_data_address): Avoid fetching DR_CONTROL unless necessary. Extend comments. * linux-x86-low.c (x86_linux_prepare_to_resume): Don't write to DR0-3 if not used. If any watchpoint was set, clear DR_STATUS. --- diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 4798660b184..c1142d302b3 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,12 @@ +2011-12-13 Pedro Alves + + PR remote/13492 + + * i386-low.c (i386_low_stopped_data_address): Avoid fetching + DR_CONTROL unless necessary. Extend comments. + * linux-x86-low.c (x86_linux_prepare_to_resume): Don't write to + DR0-3 if not used. If any watchpoint was set, clear DR_STATUS. + 2011-12-13 Yao Qi * tracepoint.c (trace_buffer_alloc): Replace magic numbers with diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c index a9591794680..0ac37c85664 100644 --- a/gdb/gdbserver/i386-low.c +++ b/gdb/gdbserver/i386-low.c @@ -559,24 +559,60 @@ i386_low_stopped_data_address (struct i386_debug_reg_state *state, CORE_ADDR addr = 0; int i; int rc = 0; + /* The current thread's DR_STATUS. We always need to read this to + check whether some watchpoint caused the trap. */ unsigned status; + /* We need DR_CONTROL as well, but only iff DR_STATUS indicates a + data breakpoint trap. Only fetch it when necessary, to avoid an + unnecessary extra syscall when no watchpoint triggered. */ + int control_p = 0; unsigned control; - /* Get the current values the inferior has. If the thread was - running when we last changed watchpoints, the mirror no longer - represents what was set in this LWP's debug registers. */ + /* In non-stop/async, threads can be running while we change the + global dr_mirror (and friends). Say, we set a watchpoint, and + let threads resume. Now, say you delete the watchpoint, or + add/remove watchpoints such that dr_mirror changes while threads + are running. On targets that support non-stop, + inserting/deleting watchpoints updates the global dr_mirror only. + It does not update the real thread's debug registers; that's only + done prior to resume. Instead, if threads are running when the + mirror changes, a temporary and transparent stop on all threads + is forced so they can get their copy of the debug registers + updated on re-resume. Now, say, a thread hit a watchpoint before + having been updated with the new dr_mirror contents, and we + haven't yet handled the corresponding SIGTRAP. If we trusted + dr_mirror below, we'd mistake the real trapped address (from the + last time we had updated debug registers in the thread) with + whatever was currently in dr_mirror. So to fix this, dr_mirror + always represents intention, what we _want_ threads to have in + debug registers. To get at the address and cause of the trap, we + need to read the state the thread still has in its debug + registers. + + In sum, always get the current debug register values the current + thread has, instead of trusting the global mirror. If the thread + was running when we last changed watchpoints, the mirror no + longer represents what was set in this thread's debug + registers. */ status = i386_dr_low_get_status (); - control = i386_dr_low_get_control (); ALL_DEBUG_REGISTERS (i) { - if (I386_DR_WATCH_HIT (status, i) - /* This second condition makes sure DRi is set up for a data - watchpoint, not a hardware breakpoint. The reason is - that GDB doesn't call the target_stopped_data_address - method except for data watchpoints. In other words, I'm - being paranoiac. */ - && I386_DR_GET_RW_LEN (control, i) != 0) + if (!I386_DR_WATCH_HIT (status, i)) + continue; + + if (!control_p) + { + control = i386_dr_low_get_control (); + control_p = 1; + } + + /* This second condition makes sure DRi is set up for a data + watchpoint, not a hardware breakpoint. The reason is that + GDB doesn't call the target_stopped_data_address method + except for data watchpoints. In other words, I'm being + paranoiac. */ + if (I386_DR_GET_RW_LEN (control, i) != 0) { addr = i386_dr_low_get_addr (i); rc = 1; diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c index d1c760e9e6c..db87ceec5bd 100644 --- a/gdb/gdbserver/linux-x86-low.c +++ b/gdb/gdbserver/linux-x86-low.c @@ -655,6 +655,7 @@ static void x86_linux_prepare_to_resume (struct lwp_info *lwp) { ptid_t ptid = ptid_of (lwp); + int clear_status = 0; if (lwp->arch_private->debug_registers_changed) { @@ -665,14 +666,23 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp) = &proc->private->arch_private->debug_reg_state; for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++) - x86_linux_dr_set (ptid, i, state->dr_mirror[i]); + if (state->dr_ref_count[i] > 0) + { + x86_linux_dr_set (ptid, i, state->dr_mirror[i]); + + /* If we're setting a watchpoint, any change the inferior + had done itself to the debug registers needs to be + discarded, otherwise, i386_low_stopped_data_address can + get confused. */ + clear_status = 1; + } x86_linux_dr_set (ptid, DR_CONTROL, state->dr_control_mirror); lwp->arch_private->debug_registers_changed = 0; } - if (lwp->stopped_by_watchpoint) + if (clear_status || lwp->stopped_by_watchpoint) x86_linux_dr_set (ptid, DR_STATUS, 0); }