* [patch 1/2] Fix watchpoints across fork
@ 2012-01-02 16:46 Jan Kratochvil
2012-01-02 19:04 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2012-01-02 16:46 UTC (permalink / raw)
To: gdb-patches
Hi,
this is a new post of:
[patch 2/4] hw watchpoints across fork()
http://sourceware.org/ml/gdb-patches/2010-12/msg00043.html
This patch has been reimplemented by Pedro in the meantime so I have included
only the former comments so that the same problem does not regress:
[patch 3/4] hw watchpoints kernel workaround
http://sourceware.org/ml/gdb-patches/2010-12/msg00044.html
This patch needs to be tested with both Linux kernel 2.6.x and 3.1.x due to
its different behavior as described in the patch.
Former patch was implementing support also for ppc* and s390*. I will port
them later if this way gets accepted, this post affects x86* only.
The new LWP being detached has different PID than the current inferior.
This may suggest creating temporary new inferior to handle the detach.
I did not go that way, with default "set detach-on-fork on" there has been
always only a single inferior present, which should be IMO kept that way.
There is an issue that formerly linux-nat.c arch backends had ptid_t as their
LWP parameter. It has been changed by Pedro to `struct lwp_info *' recently
but there does not exist any struct lwp_info for the LWPs being detached. The
patch creates struct lwp_info temporarily for the detach while keeping the
current inferior intact (that lwp_info has then PID non-matching current
inferior).
I do not think it is related to the gdbserver protocol in any way, with
"set detach-on-fork on" the "set follow-fork-mode" setting is solely target
specific.
There is in the patch (twice):
inferior_ptid = ptid_build (child_pid, child_pid, 0);
+ reinit_frame_cache ();
+ registers_changed ();
which is required on ppc*-linux-nat (using my patch not posted yet) with the
new testcase, otherwise there is internal_error on stale frame cache.
The problem is not visible on x86*.
No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.
The new testcase has been also tested with RHEL-4.8 (legacy ptrace) and
RHEL-6.2 (utrace).
Thanks,
Jan
gdb/
2012-01-02 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix watchpoints across inferior fork.
* amd64-linux-nat.c (update_debug_registers_callback): Update the
comment for linux_nat_iterate_watchpoint_lwps.
(amd64_linux_dr_set_control, amd64_linux_dr_set_addr): Use
linux_nat_iterate_watchpoint_lwps.
(amd64_linux_prepare_to_resume): New comment on Linux kernel.
* i386-linux-nat.c (update_debug_registers_callback): Update the
comment for linux_nat_iterate_watchpoint_lwps.
(i386_linux_dr_set_control, i386_linux_dr_set_addr): Use
linux_nat_iterate_watchpoint_lwps.
(i386_linux_prepare_to_resume): New comment on Linux kernel.
* i386-nat.c: Include inferior.h.
(dr_mirror): Remove.
(i386_inferior_data, struct i386_inferior_data)
(i386_inferior_data_get): New.
(i386_debug_reg_state): Use i386_inferior_data_get.
(i386_cleanup_dregs, i386_update_inferior_debug_regs)
(i386_insert_watchpoint, i386_remove_watchpoint)
(i386_stopped_data_address, i386_insert_hw_breakpoint)
(i386_remove_hw_breakpoint): New variable state, use
i386_debug_reg_state instead of DR_MIRROR.
* linux-nat.c (delete_lwp): New declaration.
(num_lwps): Move here from downwards.
(delete_lwp_cleanup): New.
(linux_child_follow_fork): Create new child_lp, call
linux_nat_new_thread and linux_nat_prepare_to_resume before calling
PTRACE_DETACH. Call reinit_frame_cache and registers_changed after
changing inferior_ptid.
(num_lwps): Move upwards.
(linux_nat_iterate_watchpoint_lwps): New.
* linux-nat.h (linux_nat_iterate_watchpoint_lwps_ftype): New.
(linux_nat_iterate_watchpoint_lwps_ftype): New declaration.
gdb/testsuite/
2012-01-02 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix watchpoints across inferior fork.
* gdb.threads/watchpoint-fork-child.c: New file.
* gdb.threads/watchpoint-fork-mt.c: New file.
* gdb.threads/watchpoint-fork-parent.c: New file.
* gdb.threads/watchpoint-fork-st.c: New file.
* gdb.threads/watchpoint-fork.exp: New file.
* gdb.threads/watchpoint-fork.h: New file.
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -337,8 +337,8 @@ amd64_linux_dr_get_status (void)
return amd64_linux_dr_get (inferior_ptid, DR_STATUS);
}
-/* Callback for iterate_over_lwps. Update the debug registers of
- LWP. */
+/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
+ of LWP. */
static int
update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -364,9 +364,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
static void
amd64_linux_dr_set_control (unsigned long control)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -379,7 +377,7 @@ amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Called when resuming a thread.
@@ -401,6 +399,13 @@ amd64_linux_prepare_to_resume (struct lwp_info *lwp)
struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
+ /* On Linux kernel before 2.6.33 commit
+ 72f674d203cd230426437cdcf7dd6f681dad8b0d
+ if you enable a breakpoint by the DR_CONTROL bits you need to have
+ already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
+
+ Ensure DR_CONTROL gets written as the very last register here. */
+
for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
if (state->dr_ref_count[i] > 0)
{
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -709,8 +709,8 @@ i386_linux_dr_get_status (void)
return i386_linux_dr_get (inferior_ptid, DR_STATUS);
}
-/* Callback for iterate_over_lwps. Update the debug registers of
- LWP. */
+/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
+ of LWP. */
static int
update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -736,9 +736,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
static void
i386_linux_dr_set_control (unsigned long control)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -751,7 +749,7 @@ i386_linux_dr_set_addr (int regnum, CORE_ADDR addr)
gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Called when resuming a thread.
@@ -773,6 +771,9 @@ i386_linux_prepare_to_resume (struct lwp_info *lwp)
struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
+ /* See amd64_linux_prepare_to_resume for Linux kernel note on
+ i386_linux_dr_set calls ordering. */
+
for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
if (state->dr_ref_count[i] > 0)
{
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -25,6 +25,7 @@
#include "gdbcmd.h"
#include "target.h"
#include "gdb_assert.h"
+#include "inferior.h"
/* Support for hardware watchpoints and breakpoints using the i386
debug registers.
@@ -170,14 +171,73 @@ i386_init_dregs (struct i386_debug_reg_state *state)
state->dr_status_mirror = 0;
}
-/* The local mirror of the inferior's debug registers. Currently this
- is a global, but it should really be per-inferior. */
-static struct i386_debug_reg_state dr_mirror;
+/* Per-inferior data key. */
+static const struct inferior_data *i386_inferior_data;
+
+/* Per-inferior data. */
+struct i386_inferior_data
+ {
+ /* Copy of i386 hardware debug registers for performance reasons. */
+ struct i386_debug_reg_state state;
+ };
+
+/* Get data specific for INFERIOR_PTID LWP. Return special data area
+ for processes being detached. */
+
+static struct i386_inferior_data *
+i386_inferior_data_get (void)
+{
+ /* Intermediate patch stub. */
+ static struct i386_inferior_data inf_data_local;
+ struct inferior *inf = current_inferior ();
+ struct i386_inferior_data *inf_data = &inf_data_local;
+
+ if (inf->pid != ptid_get_pid (inferior_ptid))
+ {
+ /* INFERIOR_PTID is being detached from the inferior INF.
+ Provide local cache specific for the detached LWP. */
+
+ static struct i386_inferior_data detached_inf_data_local;
+ static int detached_inf_pid = -1;
+
+ if (detached_inf_pid != ptid_get_pid (inferior_ptid))
+ {
+ /* Reinitialize the local cache if INFERIOR_PTID is
+ different from the LWP last detached.
+
+ Linux kernel before 2.6.33 commit
+ 72f674d203cd230426437cdcf7dd6f681dad8b0d
+ will inherit hardware debug registers from parent
+ on fork/vfork/clone. Newer Linux kernels create such tasks with
+ zeroed debug registers.
+
+ GDB will remove all breakpoints (and watchpoints) from the forked
+ off process. We also need to reset the debug registers in that
+ process to be compatible with the older Linux kernels.
+
+ Copy the debug registers mirrors into the new process so that all
+ breakpoints and watchpoints can be removed together. The debug
+ registers mirror will become zeroed in the end before detaching
+ the forked off process. */
+
+ detached_inf_pid = ptid_get_pid (inferior_ptid);
+ memcpy (&detached_inf_data_local, inf_data,
+ sizeof (detached_inf_data_local));
+ }
+
+ return &detached_inf_data_local;
+ }
+
+ return inf_data;
+}
+
+/* Get debug registers state for INFERIOR_PTID, see
+ i386_inferior_data_get. */
struct i386_debug_reg_state *
i386_debug_reg_state (void)
{
- return &dr_mirror;
+ return &i386_inferior_data_get ()->state;
}
/* Whether or not to print the mirrored debug registers. */
@@ -230,7 +290,9 @@ static int i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state
void
i386_cleanup_dregs (void)
{
- i386_init_dregs (&dr_mirror);
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
+
+ i386_init_dregs (state);
}
/* Print the values of the mirrored debug registers. This is called
@@ -494,20 +556,21 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
static void
i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
ALL_DEBUG_REGISTERS (i)
{
- if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (&dr_mirror, i))
+ if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
i386_dr_low.set_addr (i, new_state->dr_mirror[i]);
else
- gdb_assert (new_state->dr_mirror[i] == dr_mirror.dr_mirror[i]);
+ gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
}
- if (new_state->dr_control_mirror != dr_mirror.dr_control_mirror)
+ if (new_state->dr_control_mirror != state->dr_control_mirror)
i386_dr_low.set_control (new_state->dr_control_mirror);
- dr_mirror = *new_state;
+ *state = *new_state;
}
/* Insert a watchpoint to watch a memory region which starts at
@@ -518,10 +581,11 @@ static int
i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
struct expression *cond)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int retval;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
if (type == hw_read)
return 1; /* unsupported */
@@ -542,7 +606,7 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "insert_watchpoint", addr, len, type);
+ i386_show_dr (state, "insert_watchpoint", addr, len, type);
return retval;
}
@@ -554,10 +618,11 @@ static int
i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
struct expression *cond)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int retval;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
|| addr % len != 0)
@@ -575,7 +640,7 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "remove_watchpoint", addr, len, type);
+ i386_show_dr (state, "remove_watchpoint", addr, len, type);
return retval;
}
@@ -586,11 +651,12 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
static int
i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int nregs;
/* Compute how many aligned watchpoints we would need to cover this
region. */
- nregs = i386_handle_nonaligned_watchpoint (&dr_mirror,
+ nregs = i386_handle_nonaligned_watchpoint (state,
WP_COUNT, addr, len, hw_write);
return nregs <= DR_NADDR ? 1 : 0;
}
@@ -602,6 +668,7 @@ i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
static int
i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
CORE_ADDR addr = 0;
int i;
int rc = 0;
@@ -615,25 +682,24 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
unsigned control = 0;
/* 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.
+ STATE (and friends). Say, we set a watchpoint, and let threads
+ resume. Now, say you delete the watchpoint, or add/remove
+ watchpoints such that STATE changes while threads are running.
+ On targets that support non-stop, inserting/deleting watchpoints
+ updates the STATE 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
+ STATE contents, and we haven't yet handled the corresponding
+ SIGTRAP. If we trusted STATE 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 STATE.
+ So to fix this, STATE 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
@@ -663,11 +729,11 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
addr = i386_dr_low.get_addr (i);
rc = 1;
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "watchpoint_hit", addr, -1, hw_write);
+ i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
}
}
if (maint_show_dr && addr == 0)
- i386_show_dr (&dr_mirror, "stopped_data_addr", 0, 0, hw_write);
+ i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
if (rc)
*addr_p = addr;
@@ -687,11 +753,12 @@ static int
i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
int retval = i386_insert_aligned_watchpoint (&local_state,
addr, len_rw) ? EBUSY : 0;
@@ -699,7 +766,7 @@ i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute);
+ i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
return retval;
}
@@ -711,11 +778,12 @@ static int
i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
int retval = i386_remove_aligned_watchpoint (&local_state,
addr, len_rw);
@@ -723,7 +791,7 @@ i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute);
+ i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
return retval;
}
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -289,6 +289,7 @@ static void restore_child_signals_mask (sigset_t *prev_mask);
struct lwp_info;
static struct lwp_info *add_lwp (ptid_t ptid);
static void purge_lwp_list (int pid);
+static void delete_lwp (ptid_t ptid);
static struct lwp_info *find_lwp_pid (ptid_t ptid);
\f
@@ -585,6 +586,31 @@ linux_child_post_startup_inferior (ptid_t ptid)
linux_enable_tracesysgood (ptid);
}
+/* Return the number of known LWPs in the tgid given by PID. */
+
+static int
+num_lwps (int pid)
+{
+ int count = 0;
+ struct lwp_info *lp;
+
+ for (lp = lwp_list; lp; lp = lp->next)
+ if (ptid_get_pid (lp->ptid) == pid)
+ count++;
+
+ return count;
+}
+
+/* Call delete_lwp with prototype compatible for make_cleanup. */
+
+static void
+delete_lwp_cleanup (void *lp_voidp)
+{
+ struct lwp_info *lp = lp_voidp;
+
+ delete_lwp (lp->ptid);
+}
+
static int
linux_child_follow_fork (struct target_ops *ops, int follow_child)
{
@@ -631,6 +657,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
/* Detach new forked process? */
if (detach_fork)
{
+ struct cleanup *old_chain;
+
/* Before detaching from the child, remove all breakpoints
from it. If we forked, then this has already been taken
care of by infrun.c. If we vforked however, any
@@ -653,7 +681,28 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_pid);
}
+ old_chain = save_inferior_ptid ();
+ inferior_ptid = ptid_build (child_pid, child_pid, 0);
+
+ child_lp = add_lwp (inferior_ptid);
+ child_lp->stopped = 1;
+ child_lp->last_resume_kind = resume_stop;
+ make_cleanup (delete_lwp_cleanup, child_lp);
+
+ /* CHILD_LP has new PID, therefore linux_nat_new_thread is not called for it.
+ See i386_inferior_data_get for the Linux kernel specifics.
+ Ensure linux_nat_prepare_to_resume will reset the hardware debug
+ registers. It is done by the linux_nat_new_thread call, which is
+ being skipped in add_lwp above for the first lwp of a pid. */
+ gdb_assert (num_lwps (GET_PID (child_lp->ptid)) == 1);
+ if (linux_nat_new_thread != NULL)
+ linux_nat_new_thread (child_lp);
+
+ if (linux_nat_prepare_to_resume != NULL)
+ linux_nat_prepare_to_resume (child_lp);
ptrace (PTRACE_DETACH, child_pid, 0, 0);
+
+ do_cleanups (old_chain);
}
else
{
@@ -671,6 +720,9 @@ holding the child stopped. Try \"set detach-on-fork\" or \
save_current_program_space ();
inferior_ptid = ptid_build (child_pid, child_pid, 0);
+ reinit_frame_cache ();
+ registers_changed ();
+
add_thread (inferior_ptid);
child_lp = add_lwp (inferior_ptid);
child_lp->stopped = 1;
@@ -862,6 +914,9 @@ holding the child stopped. Try \"set detach-on-fork\" or \
informing the solib layer about this new process. */
inferior_ptid = ptid_build (child_pid, child_pid, 0);
+ reinit_frame_cache ();
+ registers_changed ();
+
add_thread (inferior_ptid);
child_lp = add_lwp (inferior_ptid);
child_lp->stopped = 1;
@@ -1112,21 +1167,6 @@ purge_lwp_list (int pid)
}
}
-/* Return the number of known LWPs in the tgid given by PID. */
-
-static int
-num_lwps (int pid)
-{
- int count = 0;
- struct lwp_info *lp;
-
- for (lp = lwp_list; lp; lp = lp->next)
- if (ptid_get_pid (lp->ptid) == pid)
- count++;
-
- return count;
-}
-
/* Add the LWP specified by PID to the list. Return a pointer to the
structure describing the new LWP. The LWP should already be stopped
(with an exception for the very first LWP). */
@@ -1236,6 +1276,42 @@ iterate_over_lwps (ptid_t filter,
return NULL;
}
+/* Iterate like iterate_over_lwps does except when forking-off a child call
+ CALLBACK with CALLBACK_DATA specifically only for that new child PID. */
+
+void
+linux_nat_iterate_watchpoint_lwps
+ (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data)
+{
+ int inferior_pid = ptid_get_pid (inferior_ptid);
+ struct inferior *inf = current_inferior ();
+
+ if (inf->pid == inferior_pid)
+ {
+ iterate_over_lwps (minus_one_ptid, callback, callback_data);
+ }
+ else
+ {
+ /* Detaching a new child PID temporarily present in INFERIOR_PID. */
+
+ struct lwp_info *child_lp;
+ struct cleanup *old_chain;
+ pid_t child_pid = GET_PID (inferior_ptid);
+ ptid_t child_ptid = ptid_build (child_pid, child_pid, 0);
+
+ gdb_assert (!is_lwp (inferior_ptid));
+ gdb_assert (find_lwp_pid (child_ptid) == NULL);
+ child_lp = add_lwp (child_ptid);
+ child_lp->stopped = 1;
+ child_lp->last_resume_kind = resume_stop;
+ old_chain = make_cleanup (delete_lwp_cleanup, child_lp);
+
+ callback (child_lp, callback_data);
+
+ do_cleanups (old_chain);
+ }
+}
+
/* Update our internal state when changing from one checkpoint to
another indicated by NEW_PTID. We can only switch single-threaded
applications, so we only create one new LWP, and the previous list
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -159,6 +159,12 @@ struct lwp_info *iterate_over_lwps (ptid_t filter,
void *),
void *data);
+typedef int (*linux_nat_iterate_watchpoint_lwps_ftype) (struct lwp_info *lwp,
+ void *arg);
+
+extern void linux_nat_iterate_watchpoint_lwps
+ (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data);
+
/* Create a prototype generic GNU/Linux target. The client can
override it with local methods. */
struct target_ops * linux_target (void);
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
@@ -0,0 +1,129 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+
+#include "watchpoint-fork.h"
+
+/* `pid_t' may not be available. */
+
+static volatile int usr1_got;
+
+static void
+handler_usr1 (int signo)
+{
+ usr1_got++;
+}
+
+void
+forkoff (int nr)
+{
+ int child, save_parent = getpid ();
+ int i;
+ struct sigaction act, oldact;
+#ifdef THREAD
+ void *thread_result;
+#endif
+
+ memset (&act, 0, sizeof act);
+ act.sa_flags = SA_RESTART;
+ act.sa_handler = handler_usr1;
+ sigemptyset (&act.sa_mask);
+ i = sigaction (SIGUSR1, &act, &oldact);
+ assert (i == 0);
+
+ child = fork ();
+ switch (child)
+ {
+ case -1:
+ assert (0);
+ default:
+ printf ("parent%d: %d\n", nr, (int) child);
+
+ /* Sleep for a while to possibly get incorrectly ATTACH_THREADed by GDB
+ tracing the child fork with no longer valid thread/lwp entries of the
+ parent. */
+
+ i = sleep (2);
+ assert (i == 0);
+
+ /* We must not get caught here (against a forgotten breakpoint). */
+
+ var++;
+ marker ();
+
+#ifdef THREAD
+ /* And neither got caught our thread. */
+
+ step = 99;
+ i = pthread_join (thread, &thread_result);
+ assert (i == 0);
+ assert (thread_result == (void *) 99UL);
+#endif
+
+ /* Be sure our child knows we did not get caught above. */
+
+ i = kill (child, SIGUSR1);
+ assert (i == 0);
+
+ /* Sleep for a while to check GDB's `info threads' no longer tracks us in
+ the child fork. */
+
+ i = sleep (2);
+ assert (i == 0);
+
+ _exit (0);
+ case 0:
+ printf ("child%d: %d\n", nr, (int) getpid ());
+
+ /* Let the parent signal us about its success. Be careful of races. */
+
+ for (;;)
+ {
+ /* Parent either died (and USR1_GOT is zero) or it succeeded. */
+ if (getppid () != save_parent)
+ break;
+ if (kill (getppid (), 0) != 0)
+ break;
+ /* Parent succeeded? */
+ if (usr1_got)
+ break;
+
+#ifdef THREAD
+ i = pthread_yield ();
+ assert (i == 0);
+#endif
+ }
+ assert (usr1_got);
+
+ /* We must get caught here (against a false watchpoint removal). */
+
+ marker ();
+ }
+
+ i = sigaction (SIGUSR1, &oldact, NULL);
+ assert (i == 0);
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-mt.c
@@ -0,0 +1,174 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+#include <asm/unistd.h>
+#include <unistd.h>
+#define gettid() syscall (__NR_gettid)
+
+#include "watchpoint-fork.h"
+
+/* Non-atomic `var++' should not hurt as we synchronize the threads by the STEP
+ variable. Hit-comments need to be duplicite there to catch both at-stops
+ and behind-stops, depending on the target. */
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+empty (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+pthread_t thread;
+volatile int step;
+
+static void *
+start (void *arg)
+{
+ int i;
+
+ if (step >= 3)
+ goto step_3;
+
+ while (step != 1)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ var++; /* validity-thread-B */
+ empty (); /* validity-thread-B */
+ step = 2;
+ while (step != 3)
+ {
+ if (step == 99)
+ goto step_99;
+
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+step_3:
+ if (step >= 5)
+ goto step_5;
+
+ var++; /* after-fork1-B */
+ empty (); /* after-fork1-B */
+ step = 4;
+ while (step != 5)
+ {
+ if (step == 99)
+ goto step_99;
+
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+step_5:
+ var++; /* after-fork2-B */
+ empty (); /* after-fork2-B */
+ return (void *) 5UL;
+
+step_99:
+ /* We must not get caught here (against a forgotten breakpoint). */
+ var++;
+ marker ();
+ return (void *) 99UL;
+}
+
+int
+main (void)
+{
+ int i;
+ void *thread_result;
+
+ setbuf (stdout, NULL);
+ printf ("main: %d\n", (int) gettid ());
+
+ /* General hardware breakpoints and watchpoints validity. */
+ marker ();
+ var++; /* validity-first */
+ empty (); /* validity-first */
+
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+
+ var++; /* validity-thread-A */
+ empty (); /* validity-thread-A */
+ step = 1;
+ while (step != 2)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ /* Hardware watchpoints got disarmed here. */
+ forkoff (1);
+
+ var++; /* after-fork1-A */
+ empty (); /* after-fork1-A */
+ step = 3;
+#ifdef FOLLOW_CHILD
+ /* Spawn new thread as it was deleted in the child of FORK. */
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+#endif
+ while (step != 4)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ /* A sanity check for double hardware watchpoints removal. */
+ forkoff (2);
+
+ var++; /* after-fork2-A */
+ empty (); /* after-fork2-A */
+ step = 5;
+#ifdef FOLLOW_CHILD
+ /* Spawn new thread as it was deleted in the child of FORK. */
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+#endif
+
+ i = pthread_join (thread, &thread_result);
+ assert (i == 0);
+ assert (thread_result == (void *) 5UL);
+
+ mark_exit ();
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-parent.c
@@ -0,0 +1,74 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <string.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <assert.h>
+#include <stdio.h>
+#include <sys/wait.h>
+
+#include "watchpoint-fork.h"
+
+void
+forkoff (int nr)
+{
+ pid_t child, pid_got;
+ int exit_code = 42 + nr;
+ int status, i;
+
+ child = fork ();
+ switch (child)
+ {
+ case -1:
+ assert (0);
+ case 0:
+ printf ("child%d: %d\n", nr, (int) getpid ());
+ /* Delay to get both the "child%d" and "parent%d" message printed without
+ a race breaking expect by its endless wait on `$gdb_prompt$':
+ Breakpoint 3, marker () at ../../../gdb/testsuite/gdb.threads/watchpoint-fork.c:33
+ 33 }
+ (gdb) parent2: 14223 */
+ i = sleep (1);
+ assert (i == 0);
+
+ /* We must not get caught here (against a forgotten breakpoint). */
+ var++;
+ marker ();
+
+ _exit (exit_code);
+ default:
+ printf ("parent%d: %d\n", nr, (int) child);
+ /* Delay to get both the "child%d" and "parent%d" message printed, see
+ above. */
+ i = sleep (1);
+ assert (i == 0);
+
+ pid_got = wait (&status);
+ assert (pid_got == child);
+ assert (WIFEXITED (status));
+ assert (WEXITSTATUS (status) == exit_code);
+
+ /* We must get caught here (against a false watchpoint removal). */
+ marker ();
+ }
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-st.c
@@ -0,0 +1,61 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "watchpoint-fork.h"
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+int
+main (void)
+{
+ setbuf (stdout, NULL);
+ printf ("main: %d\n", (int) getpid ());
+
+ /* General hardware breakpoints and watchpoints validity. */
+ marker ();
+ var++;
+ /* Hardware watchpoints got disarmed here. */
+ forkoff (1);
+ /* This watchpoint got lost before. */
+ var++;
+ /* A sanity check for double hardware watchpoints removal. */
+ forkoff (2);
+ var++;
+
+ mark_exit ();
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.exp
@@ -0,0 +1,149 @@
+# Copyright 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+proc test {type symbol} {
+ global objdir subdir srcdir gdb_prompt
+
+ set testfile watchpoint-fork
+
+ global pf_prefix
+ set prefix_test $pf_prefix
+ lappend pf_prefix "$type:"
+ set prefix_mt $pf_prefix
+
+ set srcfile_type ${srcdir}/${subdir}/${testfile}-${type}.c
+
+
+ # no threads
+
+ set pf_prefix $prefix_mt
+ lappend pf_prefix "singlethreaded:"
+
+ set executable ${testfile}-${type}-st
+ set srcfile_main ${srcdir}/${subdir}/${testfile}-st.c
+ if { [gdb_compile "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug additional_flags=-D$symbol]] != "" } {
+ untested ${testfile}.exp
+ return
+ }
+ clean_restart $executable
+
+ gdb_test "show detach-on-fork" "Whether gdb will detach the child of a fork is on\\."
+ gdb_test_no_output "set follow-fork-mode $type"
+ gdb_test "show follow-fork-mode" "Debugger response to a program call of fork or vfork is \"$type\"\\."
+ # Testcase uses it for the `follow-fork-mode child' type.
+ gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+ if ![runto_main] {
+ return
+ }
+
+ gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+ # It is never hit but it should not be left over in the fork()ed-off child.
+ set hbreak "hbreak"
+ set test "hbreak marker"
+ gdb_test_multiple $test $test {
+ -re "Hardware assisted breakpoint \[0-9\]+ at .*\r\n$gdb_prompt $" {
+ pass $test
+ }
+ -re "(No hardware breakpoint support in the target\\.|Hardware breakpoints used exceeds limit\\.)\r\n$gdb_prompt $" {
+ pass $test
+ set hbreak "break"
+ gdb_test "break marker"
+ }
+ }
+
+ gdb_breakpoint "mark_exit"
+
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*forkoff *\\(1\\).*" "watchpoints work"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*forkoff *\\(2\\).*" "watchpoint after the first fork"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*mark_exit \\(\\);" "watchpoint after the second fork"
+ gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+ # threads
+
+ set pf_prefix $prefix_mt
+ lappend pf_prefix "multithreaded:"
+
+ set executable ${testfile}-${type}-mt
+ set srcfile_main ${srcdir}/${subdir}/${testfile}-mt.c
+ if { [gdb_compile_pthreads "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug "additional_flags=-D$symbol -DTHREAD"]] != "" } {
+ untested ${testfile}.exp
+ return
+ }
+ clean_restart $executable
+
+ gdb_test_no_output "set follow-fork-mode $type"
+ # Testcase uses it for the `follow-fork-mode child' type.
+ gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+ if ![runto_main] {
+ return
+ }
+
+ gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+ # It should not be left over in the fork()ed-off child.
+ gdb_test "$hbreak marker" {reakpoint [0-9]+.*}
+
+ gdb_breakpoint "mark_exit"
+
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*validity-first.*" "singlethread watchpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*validity-thread-A.*" "multithreaded watchpoints work at A"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*validity-thread-B.*" "multithreaded watchpoints work at B"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 3.*New value = 4.*after-fork1-A.*" "watchpoint A after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 4.*New value = 5.*after-fork1-B.*" "watchpoint B after the first fork"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 5.*New value = 6.*after-fork2-A.*" "watchpoint A after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 6.*New value = 7.*after-fork2-B.*" "watchpoint B after the second fork"
+ gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+ # cleanup
+ set pf_prefix $prefix_test
+}
+
+test parent FOLLOW_PARENT
+
+# Only GNU/Linux is known to support `set follow-fork-mode child'.
+if {[istarget "*-*-linux*"] && ![is_remote target]} {
+ test child FOLLOW_CHILD
+} else {
+ untested "child"
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.h
@@ -0,0 +1,32 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#ifdef THREAD
+#include <pthread.h>
+
+extern volatile int step;
+extern pthread_t thread;
+#endif /* THREAD */
+
+extern volatile int var;
+
+extern void marker (void);
+extern void forkoff (int nr);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 1/2] Fix watchpoints across fork
2012-01-02 16:46 [patch 1/2] Fix watchpoints across fork Jan Kratochvil
@ 2012-01-02 19:04 ` Pedro Alves
2012-01-20 21:31 ` [patch 1/2] Fix watchpoints across fork #2 Jan Kratochvil
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2012-01-02 19:04 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 01/02/2012 04:45 PM, Jan Kratochvil wrote:
> There is an issue that formerly linux-nat.c arch backends had ptid_t as their
> LWP parameter. It has been changed by Pedro to `struct lwp_info *' recently
> but there does not exist any struct lwp_info for the LWPs being detached. The
> patch creates struct lwp_info temporarily for the detach while keeping the
> current inferior intact (that lwp_info has then PID non-matching current
> inferior).
>
> I do not think it is related to the gdbserver protocol in any way, with
> "set detach-on-fork on" the "set follow-fork-mode" setting is solely target
> specific.
At some point, the remote protocol will need to learn about
following forks and maybe the follow-fork modes, but we don't have
to worry about it for this patch.
> -/* Callback for iterate_over_lwps. Update the debug registers of
> - LWP. */
> +/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
> + of LWP. */
>
> static int
> update_debug_registers_callback (struct lwp_info *lwp, void *arg)
> @@ -364,9 +364,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
> static void
> amd64_linux_dr_set_control (unsigned long control)
> {
> - ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
> -
> - iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
> + linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
This already iterated over threads of the current inferior only.
linux_nat_iterate_watchpoint_lwps does not do that, but leaves it to the
next patch instead. It'd be better to move that hunk into this patch
already.
> }
>
> /* Set address REGNUM (zero based) to ADDR in all LWPs of the current
> @@ -379,7 +377,7 @@ amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
>
> gdb_assert (regnum>= 0&& regnum<= DR_LASTADDR - DR_FIRSTADDR);
>
> - iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
> + linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
I guess this leaves a stale `pid_ptid' local behind.
> }
> else
> {
> @@ -671,6 +720,9 @@ holding the child stopped. Try \"set detach-on-fork\" or \
> save_current_program_space ();
>
> inferior_ptid = ptid_build (child_pid, child_pid, 0);
> + reinit_frame_cache ();
> + registers_changed ();
Only registers_changed is necessary. That always invalidates the
frame cache of inferior_ptid.
> +
> add_thread (inferior_ptid);
> child_lp = add_lwp (inferior_ptid);
> child_lp->stopped = 1;
> @@ -862,6 +914,9 @@ holding the child stopped. Try \"set detach-on-fork\" or \
> informing the solib layer about this new process. */
>
> inferior_ptid = ptid_build (child_pid, child_pid, 0);
> + reinit_frame_cache ();
> + registers_changed ();
Likewise.
But, I'm not sure I understand why are these calls are necessary. We
keep a register cache list, indexed by ptid nowadays. Could this
be a latent bug elsewhere?
> +
> add_thread (inferior_ptid);
> child_lp = add_lwp (inferior_ptid);
> child_lp->stopped = 1;
> @@ -1112,21 +1167,6 @@ purge_lwp_list (int pid)
> }
> }
> +test parent FOLLOW_PARENT
> +
> +# Only GNU/Linux is known to support `set follow-fork-mode child'.
A [target_info exists gdb,no_hardware_watchpoints] check appears
missing, either to skip to test, or call "set can-use-hw-watchpoints
0". Similarly, [skip_hw_breakpoint_tests] for the hardware breakpoints
bits.
Otherwise looks okay.
Took me a bit to convince myself i386_inferior_data_get was okay.
I wish it was cleaner (could be if we had a separate struct
process_info, which will end up with, if/when we merge
gdb+gdbserver), but this will do for now.
--
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch 1/2] Fix watchpoints across fork #2
2012-01-02 19:04 ` Pedro Alves
@ 2012-01-20 21:31 ` Jan Kratochvil
2012-01-24 13:20 ` Pedro Alves
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2012-01-20 21:31 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Mon, 02 Jan 2012 20:04:12 +0100, Pedro Alves wrote:
> This already iterated over threads of the current inferior only.
> linux_nat_iterate_watchpoint_lwps does not do that, but leaves it to the
> next patch instead. It'd be better to move that hunk into this patch
> already.
Changed, it was a patch rebase mistake.
> I guess this leaves a stale `pid_ptid' local behind.
Fixed.
> > inferior_ptid = ptid_build (child_pid, child_pid, 0);
> >+ reinit_frame_cache ();
> >+ registers_changed ();
>
> Only registers_changed is necessary. That always invalidates the
> frame cache of inferior_ptid.
+
> But, I'm not sure I understand why are these calls are necessary. We
> keep a register cache list, indexed by ptid nowadays. Could this
> be a latent bug elsewhere?
As this post does not include changes for ppc* I have dropped this part now.
Thanks for noticing me it is really not obvious, I agree it should work.
I will review it with the ppc*/s390* part to be ported later in a second patch
series.
> A [target_info exists gdb,no_hardware_watchpoints] check appears
> missing, either to skip to test, or call "set can-use-hw-watchpoints
> 0". Similarly, [skip_hw_breakpoint_tests] for the hardware breakpoints
> bits.
Fixed.
> Took me a bit to convince myself i386_inferior_data_get was okay.
> I wish it was cleaner (could be if we had a separate struct
> process_info, which will end up with, if/when we merge
> gdb+gdbserver), but this will do for now.
I tried it various ways, I found this one the most feasible, this is
variant "f" - the 6th variant.
No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu and in
gdbserver mode and on ppc64-rhel62-linux-gnu.
Thanks,
Jan
gdb/
2012-01-20 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix watchpoints across inferior fork.
* amd64-linux-nat.c (update_debug_registers_callback): Update the
comment for linux_nat_iterate_watchpoint_lwps.
(amd64_linux_dr_set_control, amd64_linux_dr_set_addr): Use
linux_nat_iterate_watchpoint_lwps.
(amd64_linux_prepare_to_resume): New comment on Linux kernel.
* i386-linux-nat.c (update_debug_registers_callback): Update the
comment for linux_nat_iterate_watchpoint_lwps.
(i386_linux_dr_set_control, i386_linux_dr_set_addr): Use
linux_nat_iterate_watchpoint_lwps.
(i386_linux_prepare_to_resume): New comment on Linux kernel.
* i386-nat.c: Include inferior.h.
(dr_mirror): Remove.
(i386_inferior_data, struct i386_inferior_data)
(i386_inferior_data_get): New.
(i386_debug_reg_state): Use i386_inferior_data_get.
(i386_cleanup_dregs, i386_update_inferior_debug_regs)
(i386_insert_watchpoint, i386_remove_watchpoint)
(i386_stopped_data_address, i386_insert_hw_breakpoint)
(i386_remove_hw_breakpoint): New variable state, use
i386_debug_reg_state instead of DR_MIRROR.
* linux-nat.c (delete_lwp): New declaration.
(num_lwps): Move here from downwards.
(delete_lwp_cleanup): New.
(linux_child_follow_fork): Create new child_lp, call
linux_nat_new_thread and linux_nat_prepare_to_resume before calling
PTRACE_DETACH.
(num_lwps): Move upwards.
(linux_nat_iterate_watchpoint_lwps): New.
* linux-nat.h (linux_nat_iterate_watchpoint_lwps_ftype): New.
(linux_nat_iterate_watchpoint_lwps_ftype): New declaration.
gdb/testsuite/
2012-01-20 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix watchpoints across inferior fork.
* gdb.threads/watchpoint-fork-child.c: New file.
* gdb.threads/watchpoint-fork-mt.c: New file.
* gdb.threads/watchpoint-fork-parent.c: New file.
* gdb.threads/watchpoint-fork-st.c: New file.
* gdb.threads/watchpoint-fork.exp: New file.
* gdb.threads/watchpoint-fork.h: New file.
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -336,8 +336,8 @@ amd64_linux_dr_get_status (void)
return amd64_linux_dr_get (inferior_ptid, DR_STATUS);
}
-/* Callback for iterate_over_lwps. Update the debug registers of
- LWP. */
+/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
+ of LWP. */
static int
update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -363,9 +363,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
static void
amd64_linux_dr_set_control (unsigned long control)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -374,11 +372,9 @@ amd64_linux_dr_set_control (unsigned long control)
static void
amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Called when resuming a thread.
@@ -400,6 +396,13 @@ amd64_linux_prepare_to_resume (struct lwp_info *lwp)
struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
+ /* On Linux kernel before 2.6.33 commit
+ 72f674d203cd230426437cdcf7dd6f681dad8b0d
+ if you enable a breakpoint by the DR_CONTROL bits you need to have
+ already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
+
+ Ensure DR_CONTROL gets written as the very last register here. */
+
for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
if (state->dr_ref_count[i] > 0)
{
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -708,8 +708,8 @@ i386_linux_dr_get_status (void)
return i386_linux_dr_get (inferior_ptid, DR_STATUS);
}
-/* Callback for iterate_over_lwps. Update the debug registers of
- LWP. */
+/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
+ of LWP. */
static int
update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -735,9 +735,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
static void
i386_linux_dr_set_control (unsigned long control)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -750,7 +748,7 @@ i386_linux_dr_set_addr (int regnum, CORE_ADDR addr)
gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Called when resuming a thread.
@@ -772,6 +770,9 @@ i386_linux_prepare_to_resume (struct lwp_info *lwp)
struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
+ /* See amd64_linux_prepare_to_resume for Linux kernel note on
+ i386_linux_dr_set calls ordering. */
+
for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
if (state->dr_ref_count[i] > 0)
{
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -25,6 +25,7 @@
#include "gdbcmd.h"
#include "target.h"
#include "gdb_assert.h"
+#include "inferior.h"
/* Support for hardware watchpoints and breakpoints using the i386
debug registers.
@@ -170,14 +171,73 @@ i386_init_dregs (struct i386_debug_reg_state *state)
state->dr_status_mirror = 0;
}
-/* The local mirror of the inferior's debug registers. Currently this
- is a global, but it should really be per-inferior. */
-static struct i386_debug_reg_state dr_mirror;
+/* Per-inferior data key. */
+static const struct inferior_data *i386_inferior_data;
+
+/* Per-inferior data. */
+struct i386_inferior_data
+ {
+ /* Copy of i386 hardware debug registers for performance reasons. */
+ struct i386_debug_reg_state state;
+ };
+
+/* Get data specific for INFERIOR_PTID LWP. Return special data area
+ for processes being detached. */
+
+static struct i386_inferior_data *
+i386_inferior_data_get (void)
+{
+ /* Intermediate patch stub. */
+ static struct i386_inferior_data inf_data_local;
+ struct inferior *inf = current_inferior ();
+ struct i386_inferior_data *inf_data = &inf_data_local;
+
+ if (inf->pid != ptid_get_pid (inferior_ptid))
+ {
+ /* INFERIOR_PTID is being detached from the inferior INF.
+ Provide local cache specific for the detached LWP. */
+
+ static struct i386_inferior_data detached_inf_data_local;
+ static int detached_inf_pid = -1;
+
+ if (detached_inf_pid != ptid_get_pid (inferior_ptid))
+ {
+ /* Reinitialize the local cache if INFERIOR_PTID is
+ different from the LWP last detached.
+
+ Linux kernel before 2.6.33 commit
+ 72f674d203cd230426437cdcf7dd6f681dad8b0d
+ will inherit hardware debug registers from parent
+ on fork/vfork/clone. Newer Linux kernels create such tasks with
+ zeroed debug registers.
+
+ GDB will remove all breakpoints (and watchpoints) from the forked
+ off process. We also need to reset the debug registers in that
+ process to be compatible with the older Linux kernels.
+
+ Copy the debug registers mirrors into the new process so that all
+ breakpoints and watchpoints can be removed together. The debug
+ registers mirror will become zeroed in the end before detaching
+ the forked off process. */
+
+ detached_inf_pid = ptid_get_pid (inferior_ptid);
+ memcpy (&detached_inf_data_local, inf_data,
+ sizeof (detached_inf_data_local));
+ }
+
+ return &detached_inf_data_local;
+ }
+
+ return inf_data;
+}
+
+/* Get debug registers state for INFERIOR_PTID, see
+ i386_inferior_data_get. */
struct i386_debug_reg_state *
i386_debug_reg_state (void)
{
- return &dr_mirror;
+ return &i386_inferior_data_get ()->state;
}
/* Whether or not to print the mirrored debug registers. */
@@ -230,7 +290,9 @@ static int i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state
void
i386_cleanup_dregs (void)
{
- i386_init_dregs (&dr_mirror);
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
+
+ i386_init_dregs (state);
}
/* Print the values of the mirrored debug registers. This is called
@@ -494,20 +556,21 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
static void
i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
ALL_DEBUG_REGISTERS (i)
{
- if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (&dr_mirror, i))
+ if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
i386_dr_low.set_addr (i, new_state->dr_mirror[i]);
else
- gdb_assert (new_state->dr_mirror[i] == dr_mirror.dr_mirror[i]);
+ gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
}
- if (new_state->dr_control_mirror != dr_mirror.dr_control_mirror)
+ if (new_state->dr_control_mirror != state->dr_control_mirror)
i386_dr_low.set_control (new_state->dr_control_mirror);
- dr_mirror = *new_state;
+ *state = *new_state;
}
/* Insert a watchpoint to watch a memory region which starts at
@@ -518,10 +581,11 @@ static int
i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
struct expression *cond)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int retval;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
if (type == hw_read)
return 1; /* unsupported */
@@ -542,7 +606,7 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "insert_watchpoint", addr, len, type);
+ i386_show_dr (state, "insert_watchpoint", addr, len, type);
return retval;
}
@@ -554,10 +618,11 @@ static int
i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
struct expression *cond)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int retval;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
|| addr % len != 0)
@@ -575,7 +640,7 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "remove_watchpoint", addr, len, type);
+ i386_show_dr (state, "remove_watchpoint", addr, len, type);
return retval;
}
@@ -586,11 +651,12 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
static int
i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int nregs;
/* Compute how many aligned watchpoints we would need to cover this
region. */
- nregs = i386_handle_nonaligned_watchpoint (&dr_mirror,
+ nregs = i386_handle_nonaligned_watchpoint (state,
WP_COUNT, addr, len, hw_write);
return nregs <= DR_NADDR ? 1 : 0;
}
@@ -602,6 +668,7 @@ i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
static int
i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
CORE_ADDR addr = 0;
int i;
int rc = 0;
@@ -615,25 +682,24 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
unsigned control = 0;
/* 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.
+ STATE (and friends). Say, we set a watchpoint, and let threads
+ resume. Now, say you delete the watchpoint, or add/remove
+ watchpoints such that STATE changes while threads are running.
+ On targets that support non-stop, inserting/deleting watchpoints
+ updates the STATE 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
+ STATE contents, and we haven't yet handled the corresponding
+ SIGTRAP. If we trusted STATE 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 STATE.
+ So to fix this, STATE 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
@@ -663,11 +729,11 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
addr = i386_dr_low.get_addr (i);
rc = 1;
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "watchpoint_hit", addr, -1, hw_write);
+ i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
}
}
if (maint_show_dr && addr == 0)
- i386_show_dr (&dr_mirror, "stopped_data_addr", 0, 0, hw_write);
+ i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
if (rc)
*addr_p = addr;
@@ -687,11 +753,12 @@ static int
i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
int retval = i386_insert_aligned_watchpoint (&local_state,
addr, len_rw) ? EBUSY : 0;
@@ -699,7 +766,7 @@ i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute);
+ i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
return retval;
}
@@ -711,11 +778,12 @@ static int
i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
int retval = i386_remove_aligned_watchpoint (&local_state,
addr, len_rw);
@@ -723,7 +791,7 @@ i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute);
+ i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
return retval;
}
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -288,6 +288,7 @@ static void restore_child_signals_mask (sigset_t *prev_mask);
struct lwp_info;
static struct lwp_info *add_lwp (ptid_t ptid);
static void purge_lwp_list (int pid);
+static void delete_lwp (ptid_t ptid);
static struct lwp_info *find_lwp_pid (ptid_t ptid);
\f
@@ -584,6 +585,31 @@ linux_child_post_startup_inferior (ptid_t ptid)
linux_enable_tracesysgood (ptid);
}
+/* Return the number of known LWPs in the tgid given by PID. */
+
+static int
+num_lwps (int pid)
+{
+ int count = 0;
+ struct lwp_info *lp;
+
+ for (lp = lwp_list; lp; lp = lp->next)
+ if (ptid_get_pid (lp->ptid) == pid)
+ count++;
+
+ return count;
+}
+
+/* Call delete_lwp with prototype compatible for make_cleanup. */
+
+static void
+delete_lwp_cleanup (void *lp_voidp)
+{
+ struct lwp_info *lp = lp_voidp;
+
+ delete_lwp (lp->ptid);
+}
+
static int
linux_child_follow_fork (struct target_ops *ops, int follow_child)
{
@@ -630,6 +656,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \
/* Detach new forked process? */
if (detach_fork)
{
+ struct cleanup *old_chain;
+
/* Before detaching from the child, remove all breakpoints
from it. If we forked, then this has already been taken
care of by infrun.c. If we vforked however, any
@@ -652,7 +680,28 @@ holding the child stopped. Try \"set detach-on-fork\" or \
child_pid);
}
+ old_chain = save_inferior_ptid ();
+ inferior_ptid = ptid_build (child_pid, child_pid, 0);
+
+ child_lp = add_lwp (inferior_ptid);
+ child_lp->stopped = 1;
+ child_lp->last_resume_kind = resume_stop;
+ make_cleanup (delete_lwp_cleanup, child_lp);
+
+ /* CHILD_LP has new PID, therefore linux_nat_new_thread is not called for it.
+ See i386_inferior_data_get for the Linux kernel specifics.
+ Ensure linux_nat_prepare_to_resume will reset the hardware debug
+ registers. It is done by the linux_nat_new_thread call, which is
+ being skipped in add_lwp above for the first lwp of a pid. */
+ gdb_assert (num_lwps (GET_PID (child_lp->ptid)) == 1);
+ if (linux_nat_new_thread != NULL)
+ linux_nat_new_thread (child_lp);
+
+ if (linux_nat_prepare_to_resume != NULL)
+ linux_nat_prepare_to_resume (child_lp);
ptrace (PTRACE_DETACH, child_pid, 0, 0);
+
+ do_cleanups (old_chain);
}
else
{
@@ -1111,21 +1160,6 @@ purge_lwp_list (int pid)
}
}
-/* Return the number of known LWPs in the tgid given by PID. */
-
-static int
-num_lwps (int pid)
-{
- int count = 0;
- struct lwp_info *lp;
-
- for (lp = lwp_list; lp; lp = lp->next)
- if (ptid_get_pid (lp->ptid) == pid)
- count++;
-
- return count;
-}
-
/* Add the LWP specified by PID to the list. Return a pointer to the
structure describing the new LWP. The LWP should already be stopped
(with an exception for the very first LWP). */
@@ -1235,6 +1269,46 @@ iterate_over_lwps (ptid_t filter,
return NULL;
}
+/* Iterate like iterate_over_lwps does except when forking-off a child call
+ CALLBACK with CALLBACK_DATA specifically only for that new child PID. */
+
+void
+linux_nat_iterate_watchpoint_lwps
+ (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data)
+{
+ int inferior_pid = ptid_get_pid (inferior_ptid);
+ struct inferior *inf = current_inferior ();
+
+ if (inf->pid == inferior_pid)
+ {
+ /* Iterate all the threads of the current inferior. Without specifying
+ INFERIOR_PID it would iterate all threads of all inferiors, which is
+ inappropriate for watchpoints. */
+
+ iterate_over_lwps (pid_to_ptid (inferior_pid), callback, callback_data);
+ }
+ else
+ {
+ /* Detaching a new child PID temporarily present in INFERIOR_PID. */
+
+ struct lwp_info *child_lp;
+ struct cleanup *old_chain;
+ pid_t child_pid = GET_PID (inferior_ptid);
+ ptid_t child_ptid = ptid_build (child_pid, child_pid, 0);
+
+ gdb_assert (!is_lwp (inferior_ptid));
+ gdb_assert (find_lwp_pid (child_ptid) == NULL);
+ child_lp = add_lwp (child_ptid);
+ child_lp->stopped = 1;
+ child_lp->last_resume_kind = resume_stop;
+ old_chain = make_cleanup (delete_lwp_cleanup, child_lp);
+
+ callback (child_lp, callback_data);
+
+ do_cleanups (old_chain);
+ }
+}
+
/* Update our internal state when changing from one checkpoint to
another indicated by NEW_PTID. We can only switch single-threaded
applications, so we only create one new LWP, and the previous list
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -158,6 +158,12 @@ struct lwp_info *iterate_over_lwps (ptid_t filter,
void *),
void *data);
+typedef int (*linux_nat_iterate_watchpoint_lwps_ftype) (struct lwp_info *lwp,
+ void *arg);
+
+extern void linux_nat_iterate_watchpoint_lwps
+ (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data);
+
/* Create a prototype generic GNU/Linux target. The client can
override it with local methods. */
struct target_ops * linux_target (void);
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
@@ -0,0 +1,129 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+
+#include "watchpoint-fork.h"
+
+/* `pid_t' may not be available. */
+
+static volatile int usr1_got;
+
+static void
+handler_usr1 (int signo)
+{
+ usr1_got++;
+}
+
+void
+forkoff (int nr)
+{
+ int child, save_parent = getpid ();
+ int i;
+ struct sigaction act, oldact;
+#ifdef THREAD
+ void *thread_result;
+#endif
+
+ memset (&act, 0, sizeof act);
+ act.sa_flags = SA_RESTART;
+ act.sa_handler = handler_usr1;
+ sigemptyset (&act.sa_mask);
+ i = sigaction (SIGUSR1, &act, &oldact);
+ assert (i == 0);
+
+ child = fork ();
+ switch (child)
+ {
+ case -1:
+ assert (0);
+ default:
+ printf ("parent%d: %d\n", nr, (int) child);
+
+ /* Sleep for a while to possibly get incorrectly ATTACH_THREADed by GDB
+ tracing the child fork with no longer valid thread/lwp entries of the
+ parent. */
+
+ i = sleep (2);
+ assert (i == 0);
+
+ /* We must not get caught here (against a forgotten breakpoint). */
+
+ var++;
+ marker ();
+
+#ifdef THREAD
+ /* And neither got caught our thread. */
+
+ step = 99;
+ i = pthread_join (thread, &thread_result);
+ assert (i == 0);
+ assert (thread_result == (void *) 99UL);
+#endif
+
+ /* Be sure our child knows we did not get caught above. */
+
+ i = kill (child, SIGUSR1);
+ assert (i == 0);
+
+ /* Sleep for a while to check GDB's `info threads' no longer tracks us in
+ the child fork. */
+
+ i = sleep (2);
+ assert (i == 0);
+
+ _exit (0);
+ case 0:
+ printf ("child%d: %d\n", nr, (int) getpid ());
+
+ /* Let the parent signal us about its success. Be careful of races. */
+
+ for (;;)
+ {
+ /* Parent either died (and USR1_GOT is zero) or it succeeded. */
+ if (getppid () != save_parent)
+ break;
+ if (kill (getppid (), 0) != 0)
+ break;
+ /* Parent succeeded? */
+ if (usr1_got)
+ break;
+
+#ifdef THREAD
+ i = pthread_yield ();
+ assert (i == 0);
+#endif
+ }
+ assert (usr1_got);
+
+ /* We must get caught here (against a false watchpoint removal). */
+
+ marker ();
+ }
+
+ i = sigaction (SIGUSR1, &oldact, NULL);
+ assert (i == 0);
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-mt.c
@@ -0,0 +1,174 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+#include <asm/unistd.h>
+#include <unistd.h>
+#define gettid() syscall (__NR_gettid)
+
+#include "watchpoint-fork.h"
+
+/* Non-atomic `var++' should not hurt as we synchronize the threads by the STEP
+ variable. Hit-comments need to be duplicite there to catch both at-stops
+ and behind-stops, depending on the target. */
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+empty (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+pthread_t thread;
+volatile int step;
+
+static void *
+start (void *arg)
+{
+ int i;
+
+ if (step >= 3)
+ goto step_3;
+
+ while (step != 1)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ var++; /* validity-thread-B */
+ empty (); /* validity-thread-B */
+ step = 2;
+ while (step != 3)
+ {
+ if (step == 99)
+ goto step_99;
+
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+step_3:
+ if (step >= 5)
+ goto step_5;
+
+ var++; /* after-fork1-B */
+ empty (); /* after-fork1-B */
+ step = 4;
+ while (step != 5)
+ {
+ if (step == 99)
+ goto step_99;
+
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+step_5:
+ var++; /* after-fork2-B */
+ empty (); /* after-fork2-B */
+ return (void *) 5UL;
+
+step_99:
+ /* We must not get caught here (against a forgotten breakpoint). */
+ var++;
+ marker ();
+ return (void *) 99UL;
+}
+
+int
+main (void)
+{
+ int i;
+ void *thread_result;
+
+ setbuf (stdout, NULL);
+ printf ("main: %d\n", (int) gettid ());
+
+ /* General hardware breakpoints and watchpoints validity. */
+ marker ();
+ var++; /* validity-first */
+ empty (); /* validity-first */
+
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+
+ var++; /* validity-thread-A */
+ empty (); /* validity-thread-A */
+ step = 1;
+ while (step != 2)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ /* Hardware watchpoints got disarmed here. */
+ forkoff (1);
+
+ var++; /* after-fork1-A */
+ empty (); /* after-fork1-A */
+ step = 3;
+#ifdef FOLLOW_CHILD
+ /* Spawn new thread as it was deleted in the child of FORK. */
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+#endif
+ while (step != 4)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ /* A sanity check for double hardware watchpoints removal. */
+ forkoff (2);
+
+ var++; /* after-fork2-A */
+ empty (); /* after-fork2-A */
+ step = 5;
+#ifdef FOLLOW_CHILD
+ /* Spawn new thread as it was deleted in the child of FORK. */
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+#endif
+
+ i = pthread_join (thread, &thread_result);
+ assert (i == 0);
+ assert (thread_result == (void *) 5UL);
+
+ mark_exit ();
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-parent.c
@@ -0,0 +1,74 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <string.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <assert.h>
+#include <stdio.h>
+#include <sys/wait.h>
+
+#include "watchpoint-fork.h"
+
+void
+forkoff (int nr)
+{
+ pid_t child, pid_got;
+ int exit_code = 42 + nr;
+ int status, i;
+
+ child = fork ();
+ switch (child)
+ {
+ case -1:
+ assert (0);
+ case 0:
+ printf ("child%d: %d\n", nr, (int) getpid ());
+ /* Delay to get both the "child%d" and "parent%d" message printed without
+ a race breaking expect by its endless wait on `$gdb_prompt$':
+ Breakpoint 3, marker () at ../../../gdb/testsuite/gdb.threads/watchpoint-fork.c:33
+ 33 }
+ (gdb) parent2: 14223 */
+ i = sleep (1);
+ assert (i == 0);
+
+ /* We must not get caught here (against a forgotten breakpoint). */
+ var++;
+ marker ();
+
+ _exit (exit_code);
+ default:
+ printf ("parent%d: %d\n", nr, (int) child);
+ /* Delay to get both the "child%d" and "parent%d" message printed, see
+ above. */
+ i = sleep (1);
+ assert (i == 0);
+
+ pid_got = wait (&status);
+ assert (pid_got == child);
+ assert (WIFEXITED (status));
+ assert (WEXITSTATUS (status) == exit_code);
+
+ /* We must get caught here (against a false watchpoint removal). */
+ marker ();
+ }
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-st.c
@@ -0,0 +1,61 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "watchpoint-fork.h"
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+int
+main (void)
+{
+ setbuf (stdout, NULL);
+ printf ("main: %d\n", (int) getpid ());
+
+ /* General hardware breakpoints and watchpoints validity. */
+ marker ();
+ var++;
+ /* Hardware watchpoints got disarmed here. */
+ forkoff (1);
+ /* This watchpoint got lost before. */
+ var++;
+ /* A sanity check for double hardware watchpoints removal. */
+ forkoff (2);
+ var++;
+
+ mark_exit ();
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.exp
@@ -0,0 +1,158 @@
+# Copyright 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+set testfile watchpoint-fork
+
+if [is_remote target] {
+ kfail "remote/13584" "gdbserver does not support debugging across fork"
+ return
+}
+
+proc test {type symbol} {
+ global testfile objdir subdir srcdir gdb_prompt
+
+ global pf_prefix
+ set prefix_test $pf_prefix
+ lappend pf_prefix "$type:"
+ set prefix_mt $pf_prefix
+
+ set srcfile_type ${srcdir}/${subdir}/${testfile}-${type}.c
+
+
+ # no threads
+
+ set pf_prefix $prefix_mt
+ lappend pf_prefix "singlethreaded:"
+
+ set executable ${testfile}-${type}-st
+ set srcfile_main ${srcdir}/${subdir}/${testfile}-st.c
+ if { [gdb_compile "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug additional_flags=-D$symbol]] != "" } {
+ untested ${testfile}.exp
+ return
+ }
+ clean_restart $executable
+
+ if [target_info exists gdb,no_hardware_watchpoints] {
+ # The software watchpoint functionality is in GDB an unrelated test.
+ gdb_test_no_output "set can-use-hw-watchpoints 0"
+ }
+
+ gdb_test "show detach-on-fork" "Whether gdb will detach the child of a fork is on\\."
+ gdb_test_no_output "set follow-fork-mode $type"
+ gdb_test "show follow-fork-mode" "Debugger response to a program call of fork or vfork is \"$type\"\\."
+ # Testcase uses it for the `follow-fork-mode child' type.
+ gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+ if ![runto_main] {
+ return
+ }
+
+ gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+ # It is never hit but it should not be left over in the fork()ed-off child.
+ if [skip_hw_breakpoint_tests] {
+ set hbreak "break"
+ } else {
+ set hbreak "hbreak"
+ }
+ gdb_test "$hbreak marker"
+
+ gdb_breakpoint "mark_exit"
+
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*forkoff *\\(1\\).*" "watchpoints work"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*forkoff *\\(2\\).*" "watchpoint after the first fork"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*mark_exit \\(\\);" "watchpoint after the second fork"
+ gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+ # threads
+
+ if [target_info exists gdb,no_hardware_watchpoints] {
+ # Watchpoint hits would get detected in unexpected threads.
+ return
+ }
+
+ set pf_prefix $prefix_mt
+ lappend pf_prefix "multithreaded:"
+
+ set executable ${testfile}-${type}-mt
+ set srcfile_main ${srcdir}/${subdir}/${testfile}-mt.c
+ if { [gdb_compile_pthreads "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug "additional_flags=-D$symbol -DTHREAD"]] != "" } {
+ untested ${testfile}.exp
+ return
+ }
+ clean_restart $executable
+
+ gdb_test_no_output "set follow-fork-mode $type"
+ # Testcase uses it for the `follow-fork-mode child' type.
+ gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+ if ![runto_main] {
+ return
+ }
+
+ gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+ # It should not be left over in the fork()ed-off child.
+ gdb_test "$hbreak marker" {reakpoint [0-9]+.*}
+
+ gdb_breakpoint "mark_exit"
+
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*validity-first.*" "singlethread watchpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*validity-thread-A.*" "multithreaded watchpoints work at A"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*validity-thread-B.*" "multithreaded watchpoints work at B"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 3.*New value = 4.*after-fork1-A.*" "watchpoint A after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 4.*New value = 5.*after-fork1-B.*" "watchpoint B after the first fork"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 5.*New value = 6.*after-fork2-A.*" "watchpoint A after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 6.*New value = 7.*after-fork2-B.*" "watchpoint B after the second fork"
+ gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+ # cleanup
+ set pf_prefix $prefix_test
+}
+
+test parent FOLLOW_PARENT
+
+# Only GNU/Linux is known to support `set follow-fork-mode child'.
+if [istarget "*-*-linux*"] {
+ test child FOLLOW_CHILD
+} else {
+ untested "${testfile}: child"
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.h
@@ -0,0 +1,32 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#ifdef THREAD
+#include <pthread.h>
+
+extern volatile int step;
+extern pthread_t thread;
+#endif /* THREAD */
+
+extern volatile int var;
+
+extern void marker (void);
+extern void forkoff (int nr);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch 1/2] Fix watchpoints across fork #2
2012-01-20 21:31 ` [patch 1/2] Fix watchpoints across fork #2 Jan Kratochvil
@ 2012-01-24 13:20 ` Pedro Alves
2012-01-24 13:56 ` [commit] " Jan Kratochvil
0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2012-01-24 13:20 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 01/20/2012 09:30 PM, Jan Kratochvil wrote:
> +/* Per-inferior data. */
> +struct i386_inferior_data
> + {
> + /* Copy of i386 hardware debug registers for performance reasons. */
> + struct i386_debug_reg_state state;
> + };
Indenting of {}'s should be on column 0.
> + memcpy (&detached_inf_data_local, inf_data,
> + sizeof (detached_inf_data_local));
Small nit, but
detached_inf_data_local = *inf_data;
should work just as well, no need for memcpy, afaics.
> + variable. Hit-comments need to be duplicite there to catch both at-stops
"duplicated", I think.
> + set pf_prefix $prefix_mt
> + lappend pf_prefix "singlethreaded:"
> +
> + set executable ${testfile}-${type}-st
> + set srcfile_main ${srcdir}/${subdir}/${testfile}-st.c
> + if { [gdb_compile "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug additional_flags=-D$symbol]] != "" } {
> + untested ${testfile}.exp
pf_prefix should be restored on all returns. Several instances of this issue.
> + return
> + }
Otherwise looks good to me. Thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* [commit] [patch 1/2] Fix watchpoints across fork #2
2012-01-24 13:20 ` Pedro Alves
@ 2012-01-24 13:56 ` Jan Kratochvil
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kratochvil @ 2012-01-24 13:56 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, 24 Jan 2012 13:54:20 +0100, Pedro Alves wrote:
> Otherwise looks good to me. Thanks.
Implemented all the suggested fixes. Checked in.
ppc* and s390* arch parts were fixed in the former patch variant so I had to
update/rework those ports. I will also check how it works on arm.
Thanks,
Jan
http://sourceware.org/ml/gdb-cvs/2012-01/msg00196.html
--- src/gdb/ChangeLog 2012/01/24 11:35:39 1.13766
+++ src/gdb/ChangeLog 2012/01/24 13:46:50 1.13767
@@ -1,3 +1,37 @@
+2012-01-24 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ Fix watchpoints across inferior fork.
+ * amd64-linux-nat.c (update_debug_registers_callback): Update the
+ comment for linux_nat_iterate_watchpoint_lwps.
+ (amd64_linux_dr_set_control, amd64_linux_dr_set_addr): Use
+ linux_nat_iterate_watchpoint_lwps.
+ (amd64_linux_prepare_to_resume): New comment on Linux kernel.
+ * i386-linux-nat.c (update_debug_registers_callback): Update the
+ comment for linux_nat_iterate_watchpoint_lwps.
+ (i386_linux_dr_set_control, i386_linux_dr_set_addr): Use
+ linux_nat_iterate_watchpoint_lwps.
+ (i386_linux_prepare_to_resume): New comment on Linux kernel.
+ * i386-nat.c: Include inferior.h.
+ (dr_mirror): Remove.
+ (i386_inferior_data, struct i386_inferior_data)
+ (i386_inferior_data_get): New.
+ (i386_debug_reg_state): Use i386_inferior_data_get.
+ (i386_cleanup_dregs, i386_update_inferior_debug_regs)
+ (i386_insert_watchpoint, i386_remove_watchpoint)
+ (i386_stopped_data_address, i386_insert_hw_breakpoint)
+ (i386_remove_hw_breakpoint): New variable state, use
+ i386_debug_reg_state instead of DR_MIRROR.
+ * linux-nat.c (delete_lwp): New declaration.
+ (num_lwps): Move here from downwards.
+ (delete_lwp_cleanup): New.
+ (linux_child_follow_fork): Create new child_lp, call
+ linux_nat_new_thread and linux_nat_prepare_to_resume before calling
+ PTRACE_DETACH.
+ (num_lwps): Move upwards.
+ (linux_nat_iterate_watchpoint_lwps): New.
+ * linux-nat.h (linux_nat_iterate_watchpoint_lwps_ftype): New.
+ (linux_nat_iterate_watchpoint_lwps_ftype): New declaration.
+
2012-01-24 Joel Brobecker <brobecker@adacore.com>
GDB 7.4 released.
--- src/gdb/testsuite/ChangeLog 2012/01/23 17:11:49 1.3032
+++ src/gdb/testsuite/ChangeLog 2012/01/24 13:46:54 1.3033
@@ -1,3 +1,13 @@
+2012-01-24 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ Fix watchpoints across inferior fork.
+ * gdb.threads/watchpoint-fork-child.c: New file.
+ * gdb.threads/watchpoint-fork-mt.c: New file.
+ * gdb.threads/watchpoint-fork-parent.c: New file.
+ * gdb.threads/watchpoint-fork-st.c: New file.
+ * gdb.threads/watchpoint-fork.exp: New file.
+ * gdb.threads/watchpoint-fork.h: New file.
+
2012-01-23 Pedro Alves <palves@redhat.com>
* gdb.base/call-signal-resume.exp: Allow output after "return".
--- src/gdb/amd64-linux-nat.c 2012/01/04 08:16:56 1.41
+++ src/gdb/amd64-linux-nat.c 2012/01/24 13:46:54 1.42
@@ -336,8 +336,8 @@
return amd64_linux_dr_get (inferior_ptid, DR_STATUS);
}
-/* Callback for iterate_over_lwps. Update the debug registers of
- LWP. */
+/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
+ of LWP. */
static int
update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -363,9 +363,7 @@
static void
amd64_linux_dr_set_control (unsigned long control)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -374,11 +372,9 @@
static void
amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Called when resuming a thread.
@@ -400,6 +396,13 @@
struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
+ /* On Linux kernel before 2.6.33 commit
+ 72f674d203cd230426437cdcf7dd6f681dad8b0d
+ if you enable a breakpoint by the DR_CONTROL bits you need to have
+ already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
+
+ Ensure DR_CONTROL gets written as the very last register here. */
+
for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
if (state->dr_ref_count[i] > 0)
{
--- src/gdb/i386-linux-nat.c 2012/01/04 08:17:04 1.110
+++ src/gdb/i386-linux-nat.c 2012/01/24 13:46:54 1.111
@@ -708,8 +708,8 @@
return i386_linux_dr_get (inferior_ptid, DR_STATUS);
}
-/* Callback for iterate_over_lwps. Update the debug registers of
- LWP. */
+/* Callback for linux_nat_iterate_watchpoint_lwps. Update the debug registers
+ of LWP. */
static int
update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -735,9 +735,7 @@
static void
i386_linux_dr_set_control (unsigned long control)
{
- ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -750,7 +748,7 @@
gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
- iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+ linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
}
/* Called when resuming a thread.
@@ -772,6 +770,9 @@
struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
+ /* See amd64_linux_prepare_to_resume for Linux kernel note on
+ i386_linux_dr_set calls ordering. */
+
for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
if (state->dr_ref_count[i] > 0)
{
--- src/gdb/i386-nat.c 2012/01/04 08:17:04 1.38
+++ src/gdb/i386-nat.c 2012/01/24 13:46:54 1.39
@@ -25,6 +25,7 @@
#include "gdbcmd.h"
#include "target.h"
#include "gdb_assert.h"
+#include "inferior.h"
/* Support for hardware watchpoints and breakpoints using the i386
debug registers.
@@ -170,14 +171,72 @@
state->dr_status_mirror = 0;
}
-/* The local mirror of the inferior's debug registers. Currently this
- is a global, but it should really be per-inferior. */
-static struct i386_debug_reg_state dr_mirror;
+/* Per-inferior data key. */
+static const struct inferior_data *i386_inferior_data;
+
+/* Per-inferior data. */
+struct i386_inferior_data
+{
+ /* Copy of i386 hardware debug registers for performance reasons. */
+ struct i386_debug_reg_state state;
+};
+
+/* Get data specific for INFERIOR_PTID LWP. Return special data area
+ for processes being detached. */
+
+static struct i386_inferior_data *
+i386_inferior_data_get (void)
+{
+ /* Intermediate patch stub. */
+ static struct i386_inferior_data inf_data_local;
+ struct inferior *inf = current_inferior ();
+ struct i386_inferior_data *inf_data = &inf_data_local;
+
+ if (inf->pid != ptid_get_pid (inferior_ptid))
+ {
+ /* INFERIOR_PTID is being detached from the inferior INF.
+ Provide local cache specific for the detached LWP. */
+
+ static struct i386_inferior_data detached_inf_data_local;
+ static int detached_inf_pid = -1;
+
+ if (detached_inf_pid != ptid_get_pid (inferior_ptid))
+ {
+ /* Reinitialize the local cache if INFERIOR_PTID is
+ different from the LWP last detached.
+
+ Linux kernel before 2.6.33 commit
+ 72f674d203cd230426437cdcf7dd6f681dad8b0d
+ will inherit hardware debug registers from parent
+ on fork/vfork/clone. Newer Linux kernels create such tasks with
+ zeroed debug registers.
+
+ GDB will remove all breakpoints (and watchpoints) from the forked
+ off process. We also need to reset the debug registers in that
+ process to be compatible with the older Linux kernels.
+
+ Copy the debug registers mirrors into the new process so that all
+ breakpoints and watchpoints can be removed together. The debug
+ registers mirror will become zeroed in the end before detaching
+ the forked off process. */
+
+ detached_inf_pid = ptid_get_pid (inferior_ptid);
+ detached_inf_data_local = *inf_data;
+ }
+
+ return &detached_inf_data_local;
+ }
+
+ return inf_data;
+}
+
+/* Get debug registers state for INFERIOR_PTID, see
+ i386_inferior_data_get. */
struct i386_debug_reg_state *
i386_debug_reg_state (void)
{
- return &dr_mirror;
+ return &i386_inferior_data_get ()->state;
}
/* Whether or not to print the mirrored debug registers. */
@@ -230,7 +289,9 @@
void
i386_cleanup_dregs (void)
{
- i386_init_dregs (&dr_mirror);
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
+
+ i386_init_dregs (state);
}
/* Print the values of the mirrored debug registers. This is called
@@ -494,20 +555,21 @@
static void
i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int i;
ALL_DEBUG_REGISTERS (i)
{
- if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (&dr_mirror, i))
+ if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
i386_dr_low.set_addr (i, new_state->dr_mirror[i]);
else
- gdb_assert (new_state->dr_mirror[i] == dr_mirror.dr_mirror[i]);
+ gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
}
- if (new_state->dr_control_mirror != dr_mirror.dr_control_mirror)
+ if (new_state->dr_control_mirror != state->dr_control_mirror)
i386_dr_low.set_control (new_state->dr_control_mirror);
- dr_mirror = *new_state;
+ *state = *new_state;
}
/* Insert a watchpoint to watch a memory region which starts at
@@ -518,10 +580,11 @@
i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
struct expression *cond)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int retval;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
if (type == hw_read)
return 1; /* unsupported */
@@ -542,7 +605,7 @@
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "insert_watchpoint", addr, len, type);
+ i386_show_dr (state, "insert_watchpoint", addr, len, type);
return retval;
}
@@ -554,10 +617,11 @@
i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
struct expression *cond)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int retval;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
|| addr % len != 0)
@@ -575,7 +639,7 @@
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "remove_watchpoint", addr, len, type);
+ i386_show_dr (state, "remove_watchpoint", addr, len, type);
return retval;
}
@@ -586,11 +650,12 @@
static int
i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
int nregs;
/* Compute how many aligned watchpoints we would need to cover this
region. */
- nregs = i386_handle_nonaligned_watchpoint (&dr_mirror,
+ nregs = i386_handle_nonaligned_watchpoint (state,
WP_COUNT, addr, len, hw_write);
return nregs <= DR_NADDR ? 1 : 0;
}
@@ -602,6 +667,7 @@
static int
i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
CORE_ADDR addr = 0;
int i;
int rc = 0;
@@ -615,25 +681,24 @@
unsigned control = 0;
/* 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.
+ STATE (and friends). Say, we set a watchpoint, and let threads
+ resume. Now, say you delete the watchpoint, or add/remove
+ watchpoints such that STATE changes while threads are running.
+ On targets that support non-stop, inserting/deleting watchpoints
+ updates the STATE 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
+ STATE contents, and we haven't yet handled the corresponding
+ SIGTRAP. If we trusted STATE 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 STATE.
+ So to fix this, STATE 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
@@ -663,11 +728,11 @@
addr = i386_dr_low.get_addr (i);
rc = 1;
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "watchpoint_hit", addr, -1, hw_write);
+ i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
}
}
if (maint_show_dr && addr == 0)
- i386_show_dr (&dr_mirror, "stopped_data_addr", 0, 0, hw_write);
+ i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
if (rc)
*addr_p = addr;
@@ -687,11 +752,12 @@
i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
int retval = i386_insert_aligned_watchpoint (&local_state,
addr, len_rw) ? EBUSY : 0;
@@ -699,7 +765,7 @@
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute);
+ i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
return retval;
}
@@ -711,11 +777,12 @@
i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
+ struct i386_debug_reg_state *state = i386_debug_reg_state ();
unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
CORE_ADDR addr = bp_tgt->placed_address;
/* Work on a local copy of the debug registers, and on success,
commit the change back to the inferior. */
- struct i386_debug_reg_state local_state = dr_mirror;
+ struct i386_debug_reg_state local_state = *state;
int retval = i386_remove_aligned_watchpoint (&local_state,
addr, len_rw);
@@ -723,7 +790,7 @@
i386_update_inferior_debug_regs (&local_state);
if (maint_show_dr)
- i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute);
+ i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
return retval;
}
--- src/gdb/linux-nat.c 2012/01/20 09:59:15 1.237
+++ src/gdb/linux-nat.c 2012/01/24 13:46:54 1.238
@@ -288,6 +288,7 @@
struct lwp_info;
static struct lwp_info *add_lwp (ptid_t ptid);
static void purge_lwp_list (int pid);
+static void delete_lwp (ptid_t ptid);
static struct lwp_info *find_lwp_pid (ptid_t ptid);
\f
@@ -584,6 +585,31 @@
linux_enable_tracesysgood (ptid);
}
+/* Return the number of known LWPs in the tgid given by PID. */
+
+static int
+num_lwps (int pid)
+{
+ int count = 0;
+ struct lwp_info *lp;
+
+ for (lp = lwp_list; lp; lp = lp->next)
+ if (ptid_get_pid (lp->ptid) == pid)
+ count++;
+
+ return count;
+}
+
+/* Call delete_lwp with prototype compatible for make_cleanup. */
+
+static void
+delete_lwp_cleanup (void *lp_voidp)
+{
+ struct lwp_info *lp = lp_voidp;
+
+ delete_lwp (lp->ptid);
+}
+
static int
linux_child_follow_fork (struct target_ops *ops, int follow_child)
{
@@ -630,6 +656,8 @@
/* Detach new forked process? */
if (detach_fork)
{
+ struct cleanup *old_chain;
+
/* Before detaching from the child, remove all breakpoints
from it. If we forked, then this has already been taken
care of by infrun.c. If we vforked however, any
@@ -652,7 +680,28 @@
child_pid);
}
+ old_chain = save_inferior_ptid ();
+ inferior_ptid = ptid_build (child_pid, child_pid, 0);
+
+ child_lp = add_lwp (inferior_ptid);
+ child_lp->stopped = 1;
+ child_lp->last_resume_kind = resume_stop;
+ make_cleanup (delete_lwp_cleanup, child_lp);
+
+ /* CHILD_LP has new PID, therefore linux_nat_new_thread is not called for it.
+ See i386_inferior_data_get for the Linux kernel specifics.
+ Ensure linux_nat_prepare_to_resume will reset the hardware debug
+ registers. It is done by the linux_nat_new_thread call, which is
+ being skipped in add_lwp above for the first lwp of a pid. */
+ gdb_assert (num_lwps (GET_PID (child_lp->ptid)) == 1);
+ if (linux_nat_new_thread != NULL)
+ linux_nat_new_thread (child_lp);
+
+ if (linux_nat_prepare_to_resume != NULL)
+ linux_nat_prepare_to_resume (child_lp);
ptrace (PTRACE_DETACH, child_pid, 0, 0);
+
+ do_cleanups (old_chain);
}
else
{
@@ -1111,21 +1160,6 @@
}
}
-/* Return the number of known LWPs in the tgid given by PID. */
-
-static int
-num_lwps (int pid)
-{
- int count = 0;
- struct lwp_info *lp;
-
- for (lp = lwp_list; lp; lp = lp->next)
- if (ptid_get_pid (lp->ptid) == pid)
- count++;
-
- return count;
-}
-
/* Add the LWP specified by PID to the list. Return a pointer to the
structure describing the new LWP. The LWP should already be stopped
(with an exception for the very first LWP). */
@@ -1235,6 +1269,46 @@
return NULL;
}
+/* Iterate like iterate_over_lwps does except when forking-off a child call
+ CALLBACK with CALLBACK_DATA specifically only for that new child PID. */
+
+void
+linux_nat_iterate_watchpoint_lwps
+ (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data)
+{
+ int inferior_pid = ptid_get_pid (inferior_ptid);
+ struct inferior *inf = current_inferior ();
+
+ if (inf->pid == inferior_pid)
+ {
+ /* Iterate all the threads of the current inferior. Without specifying
+ INFERIOR_PID it would iterate all threads of all inferiors, which is
+ inappropriate for watchpoints. */
+
+ iterate_over_lwps (pid_to_ptid (inferior_pid), callback, callback_data);
+ }
+ else
+ {
+ /* Detaching a new child PID temporarily present in INFERIOR_PID. */
+
+ struct lwp_info *child_lp;
+ struct cleanup *old_chain;
+ pid_t child_pid = GET_PID (inferior_ptid);
+ ptid_t child_ptid = ptid_build (child_pid, child_pid, 0);
+
+ gdb_assert (!is_lwp (inferior_ptid));
+ gdb_assert (find_lwp_pid (child_ptid) == NULL);
+ child_lp = add_lwp (child_ptid);
+ child_lp->stopped = 1;
+ child_lp->last_resume_kind = resume_stop;
+ old_chain = make_cleanup (delete_lwp_cleanup, child_lp);
+
+ callback (child_lp, callback_data);
+
+ do_cleanups (old_chain);
+ }
+}
+
/* Update our internal state when changing from one checkpoint to
another indicated by NEW_PTID. We can only switch single-threaded
applications, so we only create one new LWP, and the previous list
--- src/gdb/linux-nat.h 2012/01/04 08:17:05 1.43
+++ src/gdb/linux-nat.h 2012/01/24 13:46:54 1.44
@@ -158,6 +158,12 @@
void *),
void *data);
+typedef int (*linux_nat_iterate_watchpoint_lwps_ftype) (struct lwp_info *lwp,
+ void *arg);
+
+extern void linux_nat_iterate_watchpoint_lwps
+ (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data);
+
/* Create a prototype generic GNU/Linux target. The client can
override it with local methods. */
struct target_ops * linux_target (void);
--- src/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
+++ src/gdb/testsuite/gdb.threads/watchpoint-fork-child.c 2012-01-24 13:50:57.660589000 +0000
@@ -0,0 +1,129 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+
+#include "watchpoint-fork.h"
+
+/* `pid_t' may not be available. */
+
+static volatile int usr1_got;
+
+static void
+handler_usr1 (int signo)
+{
+ usr1_got++;
+}
+
+void
+forkoff (int nr)
+{
+ int child, save_parent = getpid ();
+ int i;
+ struct sigaction act, oldact;
+#ifdef THREAD
+ void *thread_result;
+#endif
+
+ memset (&act, 0, sizeof act);
+ act.sa_flags = SA_RESTART;
+ act.sa_handler = handler_usr1;
+ sigemptyset (&act.sa_mask);
+ i = sigaction (SIGUSR1, &act, &oldact);
+ assert (i == 0);
+
+ child = fork ();
+ switch (child)
+ {
+ case -1:
+ assert (0);
+ default:
+ printf ("parent%d: %d\n", nr, (int) child);
+
+ /* Sleep for a while to possibly get incorrectly ATTACH_THREADed by GDB
+ tracing the child fork with no longer valid thread/lwp entries of the
+ parent. */
+
+ i = sleep (2);
+ assert (i == 0);
+
+ /* We must not get caught here (against a forgotten breakpoint). */
+
+ var++;
+ marker ();
+
+#ifdef THREAD
+ /* And neither got caught our thread. */
+
+ step = 99;
+ i = pthread_join (thread, &thread_result);
+ assert (i == 0);
+ assert (thread_result == (void *) 99UL);
+#endif
+
+ /* Be sure our child knows we did not get caught above. */
+
+ i = kill (child, SIGUSR1);
+ assert (i == 0);
+
+ /* Sleep for a while to check GDB's `info threads' no longer tracks us in
+ the child fork. */
+
+ i = sleep (2);
+ assert (i == 0);
+
+ _exit (0);
+ case 0:
+ printf ("child%d: %d\n", nr, (int) getpid ());
+
+ /* Let the parent signal us about its success. Be careful of races. */
+
+ for (;;)
+ {
+ /* Parent either died (and USR1_GOT is zero) or it succeeded. */
+ if (getppid () != save_parent)
+ break;
+ if (kill (getppid (), 0) != 0)
+ break;
+ /* Parent succeeded? */
+ if (usr1_got)
+ break;
+
+#ifdef THREAD
+ i = pthread_yield ();
+ assert (i == 0);
+#endif
+ }
+ assert (usr1_got);
+
+ /* We must get caught here (against a false watchpoint removal). */
+
+ marker ();
+ }
+
+ i = sigaction (SIGUSR1, &oldact, NULL);
+ assert (i == 0);
+}
--- src/gdb/testsuite/gdb.threads/watchpoint-fork-mt.c
+++ src/gdb/testsuite/gdb.threads/watchpoint-fork-mt.c 2012-01-24 13:50:58.131220000 +0000
@@ -0,0 +1,174 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+#include <asm/unistd.h>
+#include <unistd.h>
+#define gettid() syscall (__NR_gettid)
+
+#include "watchpoint-fork.h"
+
+/* Non-atomic `var++' should not hurt as we synchronize the threads by the STEP
+ variable. Hit-comments need to be duplicated there to catch both at-stops
+ and behind-stops, depending on the target. */
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+empty (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+pthread_t thread;
+volatile int step;
+
+static void *
+start (void *arg)
+{
+ int i;
+
+ if (step >= 3)
+ goto step_3;
+
+ while (step != 1)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ var++; /* validity-thread-B */
+ empty (); /* validity-thread-B */
+ step = 2;
+ while (step != 3)
+ {
+ if (step == 99)
+ goto step_99;
+
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+step_3:
+ if (step >= 5)
+ goto step_5;
+
+ var++; /* after-fork1-B */
+ empty (); /* after-fork1-B */
+ step = 4;
+ while (step != 5)
+ {
+ if (step == 99)
+ goto step_99;
+
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+step_5:
+ var++; /* after-fork2-B */
+ empty (); /* after-fork2-B */
+ return (void *) 5UL;
+
+step_99:
+ /* We must not get caught here (against a forgotten breakpoint). */
+ var++;
+ marker ();
+ return (void *) 99UL;
+}
+
+int
+main (void)
+{
+ int i;
+ void *thread_result;
+
+ setbuf (stdout, NULL);
+ printf ("main: %d\n", (int) gettid ());
+
+ /* General hardware breakpoints and watchpoints validity. */
+ marker ();
+ var++; /* validity-first */
+ empty (); /* validity-first */
+
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+
+ var++; /* validity-thread-A */
+ empty (); /* validity-thread-A */
+ step = 1;
+ while (step != 2)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ /* Hardware watchpoints got disarmed here. */
+ forkoff (1);
+
+ var++; /* after-fork1-A */
+ empty (); /* after-fork1-A */
+ step = 3;
+#ifdef FOLLOW_CHILD
+ /* Spawn new thread as it was deleted in the child of FORK. */
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+#endif
+ while (step != 4)
+ {
+ i = pthread_yield ();
+ assert (i == 0);
+ }
+
+ /* A sanity check for double hardware watchpoints removal. */
+ forkoff (2);
+
+ var++; /* after-fork2-A */
+ empty (); /* after-fork2-A */
+ step = 5;
+#ifdef FOLLOW_CHILD
+ /* Spawn new thread as it was deleted in the child of FORK. */
+ i = pthread_create (&thread, NULL, start, NULL);
+ assert (i == 0);
+#endif
+
+ i = pthread_join (thread, &thread_result);
+ assert (i == 0);
+ assert (thread_result == (void *) 5UL);
+
+ mark_exit ();
+ return 0;
+}
--- src/gdb/testsuite/gdb.threads/watchpoint-fork-parent.c
+++ src/gdb/testsuite/gdb.threads/watchpoint-fork-parent.c 2012-01-24 13:50:58.598586000 +0000
@@ -0,0 +1,74 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <string.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <assert.h>
+#include <stdio.h>
+#include <sys/wait.h>
+
+#include "watchpoint-fork.h"
+
+void
+forkoff (int nr)
+{
+ pid_t child, pid_got;
+ int exit_code = 42 + nr;
+ int status, i;
+
+ child = fork ();
+ switch (child)
+ {
+ case -1:
+ assert (0);
+ case 0:
+ printf ("child%d: %d\n", nr, (int) getpid ());
+ /* Delay to get both the "child%d" and "parent%d" message printed without
+ a race breaking expect by its endless wait on `$gdb_prompt$':
+ Breakpoint 3, marker () at ../../../gdb/testsuite/gdb.threads/watchpoint-fork.c:33
+ 33 }
+ (gdb) parent2: 14223 */
+ i = sleep (1);
+ assert (i == 0);
+
+ /* We must not get caught here (against a forgotten breakpoint). */
+ var++;
+ marker ();
+
+ _exit (exit_code);
+ default:
+ printf ("parent%d: %d\n", nr, (int) child);
+ /* Delay to get both the "child%d" and "parent%d" message printed, see
+ above. */
+ i = sleep (1);
+ assert (i == 0);
+
+ pid_got = wait (&status);
+ assert (pid_got == child);
+ assert (WIFEXITED (status));
+ assert (WEXITSTATUS (status) == exit_code);
+
+ /* We must get caught here (against a false watchpoint removal). */
+ marker ();
+ }
+}
--- src/gdb/testsuite/gdb.threads/watchpoint-fork-st.c
+++ src/gdb/testsuite/gdb.threads/watchpoint-fork-st.c 2012-01-24 13:50:58.881130000 +0000
@@ -0,0 +1,61 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "watchpoint-fork.h"
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+int
+main (void)
+{
+ setbuf (stdout, NULL);
+ printf ("main: %d\n", (int) getpid ());
+
+ /* General hardware breakpoints and watchpoints validity. */
+ marker ();
+ var++;
+ /* Hardware watchpoints got disarmed here. */
+ forkoff (1);
+ /* This watchpoint got lost before. */
+ var++;
+ /* A sanity check for double hardware watchpoints removal. */
+ forkoff (2);
+ var++;
+
+ mark_exit ();
+ return 0;
+}
--- src/gdb/testsuite/gdb.threads/watchpoint-fork.exp
+++ src/gdb/testsuite/gdb.threads/watchpoint-fork.exp 2012-01-24 13:50:59.162234000 +0000
@@ -0,0 +1,163 @@
+# Copyright 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+set testfile watchpoint-fork
+
+if [is_remote target] {
+ kfail "remote/13584" "gdbserver does not support debugging across fork"
+ return
+}
+
+proc test {type symbol} {
+ global testfile objdir subdir srcdir gdb_prompt
+
+ global pf_prefix
+ set prefix_test $pf_prefix
+ lappend pf_prefix "$type:"
+ set prefix_mt $pf_prefix
+
+ set srcfile_type ${srcdir}/${subdir}/${testfile}-${type}.c
+
+
+ # no threads
+
+ set pf_prefix $prefix_mt
+ lappend pf_prefix "singlethreaded:"
+
+ set executable ${testfile}-${type}-st
+ set srcfile_main ${srcdir}/${subdir}/${testfile}-st.c
+ if { [gdb_compile "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug additional_flags=-D$symbol]] != "" } {
+ untested ${testfile}.exp
+ set pf_prefix $prefix_test
+ return
+ }
+ clean_restart $executable
+
+ if [target_info exists gdb,no_hardware_watchpoints] {
+ # The software watchpoint functionality is in GDB an unrelated test.
+ gdb_test_no_output "set can-use-hw-watchpoints 0"
+ }
+
+ gdb_test "show detach-on-fork" "Whether gdb will detach the child of a fork is on\\."
+ gdb_test_no_output "set follow-fork-mode $type"
+ gdb_test "show follow-fork-mode" "Debugger response to a program call of fork or vfork is \"$type\"\\."
+ # Testcase uses it for the `follow-fork-mode child' type.
+ gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+ if ![runto_main] {
+ set pf_prefix $prefix_test
+ return
+ }
+
+ gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+ # It is never hit but it should not be left over in the fork()ed-off child.
+ if [skip_hw_breakpoint_tests] {
+ set hbreak "break"
+ } else {
+ set hbreak "hbreak"
+ }
+ gdb_test "$hbreak marker"
+
+ gdb_breakpoint "mark_exit"
+
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*forkoff *\\(1\\).*" "watchpoints work"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*forkoff *\\(2\\).*" "watchpoint after the first fork"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*mark_exit \\(\\);" "watchpoint after the second fork"
+ gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+ # threads
+
+ if [target_info exists gdb,no_hardware_watchpoints] {
+ # Watchpoint hits would get detected in unexpected threads.
+ set pf_prefix $prefix_test
+ return
+ }
+
+ set pf_prefix $prefix_mt
+ lappend pf_prefix "multithreaded:"
+
+ set executable ${testfile}-${type}-mt
+ set srcfile_main ${srcdir}/${subdir}/${testfile}-mt.c
+ if { [gdb_compile_pthreads "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug "additional_flags=-D$symbol -DTHREAD"]] != "" } {
+ untested ${testfile}.exp
+ set pf_prefix $prefix_test
+ return
+ }
+ clean_restart $executable
+
+ gdb_test_no_output "set follow-fork-mode $type"
+ # Testcase uses it for the `follow-fork-mode child' type.
+ gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+ if ![runto_main] {
+ set pf_prefix $prefix_test
+ return
+ }
+
+ gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+ # It should not be left over in the fork()ed-off child.
+ gdb_test "$hbreak marker" {reakpoint [0-9]+.*}
+
+ gdb_breakpoint "mark_exit"
+
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*validity-first.*" "singlethread watchpoints work"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*validity-thread-A.*" "multithreaded watchpoints work at A"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*validity-thread-B.*" "multithreaded watchpoints work at B"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 3.*New value = 4.*after-fork1-A.*" "watchpoint A after the first fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 4.*New value = 5.*after-fork1-B.*" "watchpoint B after the first fork"
+ gdb_test "continue" \
+ "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 5.*New value = 6.*after-fork2-A.*" "watchpoint A after the second fork"
+ gdb_test "continue" \
+ "atchpoint \[0-9\]+: var.*Old value = 6.*New value = 7.*after-fork2-B.*" "watchpoint B after the second fork"
+ gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+ # cleanup
+ set pf_prefix $prefix_test
+}
+
+test parent FOLLOW_PARENT
+
+# Only GNU/Linux is known to support `set follow-fork-mode child'.
+if [istarget "*-*-linux*"] {
+ test child FOLLOW_CHILD
+} else {
+ untested "${testfile}: child"
+}
--- src/gdb/testsuite/gdb.threads/watchpoint-fork.h
+++ src/gdb/testsuite/gdb.threads/watchpoint-fork.h 2012-01-24 13:50:59.626014000 +0000
@@ -0,0 +1,32 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330,
+ Boston, MA 02111-1307, USA. */
+
+#ifdef THREAD
+#include <pthread.h>
+
+extern volatile int step;
+extern pthread_t thread;
+#endif /* THREAD */
+
+extern volatile int var;
+
+extern void marker (void);
+extern void forkoff (int nr);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-24 13:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-02 16:46 [patch 1/2] Fix watchpoints across fork Jan Kratochvil
2012-01-02 19:04 ` Pedro Alves
2012-01-20 21:31 ` [patch 1/2] Fix watchpoints across fork #2 Jan Kratochvil
2012-01-24 13:20 ` Pedro Alves
2012-01-24 13:56 ` [commit] " Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).