* [PATCH v8 4/7] Arch-specific remote follow fork
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
@ 2015-04-17 20:59 ` Don Breazeal
2015-04-17 20:59 ` [PATCH v8 1/7] Identify remote fork event support Don Breazeal
` (5 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 20:59 UTC (permalink / raw)
To: gdb-patches, palves
Hi Pedro,
This version of this patch is unchanged except possibly for merges from
the mainline. It was previously approved here:
https://sourceware.org/ml/gdb-patches/2015-02/msg00262.html
Thanks,
--Don
This patch implements the architecture-specific pieces of follow-fork
for remote and extended-remote Linux targets, which in the current
implementation copyies the parent's debug register state into the new
child's data structures. This is required for x86, arm, aarch64, and
mips.
This follows the native implementation as closely as possible by
implementing a new linux_target_ops function 'new_fork', which is
analogous to 'linux_nat_new_fork' in linux-nat.c. In gdbserver, the debug
registers are stored in the process list, instead of an
architecture-specific list, so the function arguments are process_info
pointers instead of an lwp_info and a pid as in the native implementation.
In the MIPS implementation the debug register mirror is stored differently
from x86, ARM, and aarch64, so instead of doing a simple structure assignment
I had to clone the list of watchpoint structures.
Tested using gdb.threads/watchpoint-fork.exp on x86, and ran manual tests
on a MIPS board and an ARM board.
I don't currently have access to an aarch64 board, so if someone is able
to test this easily, please do.
gdb/gdbserver/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* linux-aarch64-low.c (aarch64_linux_new_fork): New function.
(the_low_target) <new_fork>: Initialize new member.
* linux-arm-low.c (arm_new_fork): New function.
(the_low_target) <new_fork>: Initialize new member.
* linux-low.c (handle_extended_wait): Call new target function
new_fork.
* linux-low.h (struct linux_target_ops) <new_fork>: New member.
* linux-mips-low.c (mips_add_watchpoint): New function
extracted from mips_insert_point.
(the_low_target) <new_fork>: Initialize new member.
(mips_linux_new_fork): New function.
(mips_insert_point): Call mips_add_watchpoint.
* linux-x86-low.c (x86_linux_new_fork): New function.
(the_low_target) <new_fork>: Initialize new member.
---
gdb/gdbserver/linux-aarch64-low.c | 28 +++++++++++++++
gdb/gdbserver/linux-arm-low.c | 42 ++++++++++++++++++++++
gdb/gdbserver/linux-low.c | 4 +++
gdb/gdbserver/linux-low.h | 3 ++
gdb/gdbserver/linux-mips-low.c | 76 ++++++++++++++++++++++++++++++++-------
gdb/gdbserver/linux-x86-low.c | 29 +++++++++++++++
6 files changed, 170 insertions(+), 12 deletions(-)
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index d44175e..7e6e9ac 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -1135,6 +1135,33 @@ aarch64_linux_new_thread (struct lwp_info *lwp)
lwp->arch_private = info;
}
+static void
+aarch64_linux_new_fork (struct process_info *parent,
+ struct process_info *child)
+{
+ /* These are allocated by linux_add_process. */
+ gdb_assert (parent->private != NULL
+ && parent->private->arch_private != NULL);
+ gdb_assert (child->private != NULL
+ && child->private->arch_private != NULL);
+
+ /* 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 core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. 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, thus making
+ this compatible with older Linux kernels too. */
+
+ *child->private->arch_private = *parent->private->arch_private;
+}
+
/* Called when resuming a thread.
If the debug regs have changed, update the thread's copies. */
@@ -1299,6 +1326,7 @@ struct linux_target_ops the_low_target =
NULL,
aarch64_linux_new_process,
aarch64_linux_new_thread,
+ aarch64_linux_new_fork,
aarch64_linux_prepare_to_resume,
};
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index d408dcd..3420dea 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -717,6 +717,47 @@ arm_new_thread (struct lwp_info *lwp)
lwp->arch_private = info;
}
+static void
+arm_new_fork (struct process_info *parent, struct process_info *child)
+{
+ struct arch_process_info *parent_proc_info = parent->private->arch_private;
+ struct arch_process_info *child_proc_info = child->private->arch_private;
+ struct lwp_info *child_lwp;
+ struct arch_lwp_info *child_lwp_info;
+ int i;
+
+ /* These are allocated by linux_add_process. */
+ gdb_assert (parent->private != NULL
+ && parent->private->arch_private != NULL);
+ gdb_assert (child->private != NULL
+ && child->private->arch_private != NULL);
+
+ /* 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 core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. 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, thus making
+ this compatible with older Linux kernels too. */
+
+ *child_proc_info = *parent_proc_info;
+
+ /* Mark all the hardware breakpoints and watchpoints as changed to
+ make sure that the registers will be updated. */
+ child_lwp = find_lwp_pid (ptid_of (child));
+ child_lwp_info = child_lwp->arch_private;
+ for (i = 0; i < MAX_BPTS; i++)
+ child_lwp_info->bpts_changed[i] = 1;
+ for (i = 0; i < MAX_WPTS; i++)
+ child_lwp_info->wpts_changed[i] = 1;
+}
+
/* Called when resuming a thread.
If the debug regs have changed, update the thread's copies. */
static void
@@ -920,6 +961,7 @@ struct linux_target_ops the_low_target = {
NULL, /* siginfo_fixup */
arm_new_process,
arm_new_thread,
+ arm_new_fork,
arm_prepare_to_resume,
};
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 7f61946..bf41eab 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -489,6 +489,10 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
child_proc->tdesc = tdesc;
child_lwp->must_set_ptrace_flags = 1;
+ /* Clone arch-specific process data. */
+ if (the_low_target.new_fork != NULL)
+ the_low_target.new_fork (parent_proc, child_proc);
+
/* Save fork info in the parent thread. */
event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
event_lwp->waitstatus.value.related_pid = ptid;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 41067d6..3300da9 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -187,6 +187,9 @@ struct linux_target_ops
allocate it here. */
void (*new_thread) (struct lwp_info *);
+ /* Hook to call, if any, when a new fork is attached. */
+ void (*new_fork) (struct process_info *parent, struct process_info *child);
+
/* Hook to call prior to resuming a thread. */
void (*prepare_to_resume) (struct lwp_info *);
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index 7988d07..4601ad0 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -344,6 +344,68 @@ mips_linux_new_thread (struct lwp_info *lwp)
lwp->arch_private = info;
}
+/* Create a new mips_watchpoint and add it to the list. */
+
+static void
+mips_add_watchpoint (struct arch_process_info *private, CORE_ADDR addr,
+ int len, int watch_type)
+{
+ struct mips_watchpoint *new_watch;
+ struct mips_watchpoint **pw;
+
+ new_watch = xmalloc (sizeof (struct mips_watchpoint));
+ new_watch->addr = addr;
+ new_watch->len = len;
+ new_watch->type = watch_type;
+ new_watch->next = NULL;
+
+ pw = &private->current_watches;
+ while (*pw != NULL)
+ pw = &(*pw)->next;
+ *pw = new_watch;
+}
+
+/* Hook to call when a new fork is attached. */
+
+static void
+mips_linux_new_fork (struct process_info *parent,
+ struct process_info *child)
+{
+ struct arch_process_info *parent_private;
+ struct arch_process_info *child_private;
+ struct mips_watchpoint *wp;
+
+ /* These are allocated by linux_add_process. */
+ gdb_assert (parent->private != NULL
+ && parent->private->arch_private != NULL);
+ gdb_assert (child->private != NULL
+ && child->private->arch_private != NULL);
+
+ /* 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 core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. 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, thus making
+ this compatible with older Linux kernels too. */
+
+ parent_private = parent->private->arch_private;
+ child_private = child->private->arch_private;
+
+ child_private->watch_readback_valid = parent_private->watch_readback_valid;
+ child_private->watch_readback = parent_private->watch_readback;
+
+ for (wp = parent_private->current_watches; wp != NULL; wp = wp->next)
+ mips_add_watchpoint (child_private, wp->addr, wp->len, wp->type);
+
+ child_private->watch_mirror = parent_private->watch_mirror;
+}
/* This is the implementation of linux_target_ops method
prepare_to_resume. If the watch regs have changed, update the
thread's copies. */
@@ -397,8 +459,6 @@ mips_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
struct process_info *proc = current_process ();
struct arch_process_info *priv = proc->priv->arch_private;
struct pt_watch_regs regs;
- struct mips_watchpoint *new_watch;
- struct mips_watchpoint **pw;
int pid;
long lwpid;
enum target_hw_bp_type watch_type;
@@ -425,16 +485,7 @@ mips_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
return -1;
/* It fit. Stick it on the end of the list. */
- new_watch = xmalloc (sizeof (struct mips_watchpoint));
- new_watch->addr = addr;
- new_watch->len = len;
- new_watch->type = watch_type;
- new_watch->next = NULL;
-
- pw = &priv->current_watches;
- while (*pw != NULL)
- pw = &(*pw)->next;
- *pw = new_watch;
+ mips_add_watchpoint (priv, addr, len, watch_type);
priv->watch_mirror = regs;
@@ -845,6 +896,7 @@ struct linux_target_ops the_low_target = {
NULL, /* siginfo_fixup */
mips_linux_new_process,
mips_linux_new_thread,
+ mips_linux_new_fork,
mips_linux_prepare_to_resume
};
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 763df08..4aef7b7 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -634,6 +634,34 @@ x86_linux_new_process (void)
return info;
}
+/* Target routine for linux_new_fork. */
+
+static void
+x86_linux_new_fork (struct process_info *parent, struct process_info *child)
+{
+ /* These are allocated by linux_add_process. */
+ gdb_assert (parent->priv != NULL
+ && parent->priv->arch_private != NULL);
+ gdb_assert (child->priv != NULL
+ && child->priv->arch_private != NULL);
+
+ /* 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 core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. 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, thus making
+ this compatible with older Linux kernels too. */
+
+ *child->priv->arch_private = *parent->priv->arch_private;
+}
+
/* See nat/x86-dregs.h. */
struct x86_debug_reg_state *
@@ -3263,6 +3291,7 @@ struct linux_target_ops the_low_target =
x86_siginfo_fixup,
x86_linux_new_process,
x86_linux_new_thread,
+ x86_linux_new_fork,
x86_linux_prepare_to_resume,
x86_linux_process_qsupported,
x86_supports_tracepoints,
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v8 1/7] Identify remote fork event support
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
2015-04-17 20:59 ` [PATCH v8 4/7] Arch-specific remote follow fork Don Breazeal
@ 2015-04-17 20:59 ` Don Breazeal
2015-04-23 14:17 ` Pedro Alves
2015-04-17 20:59 ` [PATCH v8 3/7] Extended-remote Linux follow fork Don Breazeal
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 20:59 UTC (permalink / raw)
To: gdb-patches, palves
Hi Pedro,
This patch is mostly unchanged from the previous version, except for
the comment described below. From
https://sourceware.org/ml/gdb-patches/2015-04/msg00572.html:
On 4/15/2015 8:37 AM, Pedro Alves wrote:> On 04/10/2015 06:09 PM, Don Breazeal wrote:
>> @@ -6469,4 +6487,7 @@ initialize_low (void)
>> sigaction (SIGCHLD, &sigchld_action, NULL);
>>
>> initialize_low_arch ();
>> +
>> + /* Enable extended ptrace events. */
>> + linux_check_ptrace_features ();
>
> I think the comment might have made sense possibly in a earlier revision
> that added to additional ptrace options, but it looks a bit strange
> now, as this isn't enabling the features yet, just checking what is
> supported. A comment like
>
> /* Check supported ptrace features. */
>
> would just repeat the function's signature, so I'd suggest just
> dropping the comment.
This is done.
>
> Otherwise OK.
>
> Thanks,
> Pedro Alves
>
Thanks!
--Don
This patch implements a mechanism for GDB to determine whether fork
events are supported in gdbserver. This is a preparatory patch for
remote fork and exec event support.
Two new RSP packets are defined to represent fork and vfork event
support. These packets are used just like PACKET_multiprocess_feature
to denote whether the corresponding event is supported. GDB sends
fork-events+ and vfork-events+ to gdbserver to inquire about fork
event support. If the response enables these packets, then GDB
knows that gdbserver supports the corresponding events and will
enable them.
Target functions used to query for support are included along with
each new packet.
In order for gdbserver to know whether the events are supported at the
point where the qSupported packet arrives, the code in nat/linux-ptrace.c
had to be reorganized. Previously it would test for fork/exec event
support, then enable the events using the pid of the inferior. When the
qSupported packet arrives there may not be an inferior. So the mechanism
was split into two parts: a function that checks whether the events are
supported, called when gdbserver starts up, and another that enables the
events when the inferior stops for the first time.
Another gdbserver change was to add some global variables similar to
multi_process, one per new packet. These are used to control whether
the corresponding fork events are enabled. If GDB does not inquire
about the event support in the qSupported packet, then gdbserver will
not set these "report the event" flags. If the flags are not set, the
events are ignored like they were in the past. Thus, gdbserver will
never send fork event notification to an older GDB that doesn't
recognize fork events.
Tested on Ubuntu x64, native/remote/extended-remote, and as part of
subsequent patches in the series.
gdb/gdbserver/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* linux-low.c (linux_supports_fork_events): New function.
(linux_supports_vfork_events): New function.
(linux_target_ops): Initialize new structure members.
(initialize_low): Call linux_check_ptrace_features.
* lynx-low.c (lynx_target_ops): Initialize new structure
members.
* server.c (report_fork_events, report_vfork_events):
New global flags.
(handle_query): Add new features to qSupported packet and
response.
(captured_main): Initialize new global variables.
* target.h (struct target_ops) <supports_fork_events>:
New member.
<supports_vfork_events>: New member.
(target_supports_fork_events): New macro.
(target_supports_vfork_events): New macro.
* win32-low.c (win32_target_ops): Initialize new structure
members.
gdb/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* nat/linux-ptrace.c (linux_check_ptrace_features): Change
from static to extern.
* nat/linux-ptrace.h (linux_check_ptrace_features): Declare.
* remote.c (anonymous enum): <PACKET_fork_event_feature,
* PACKET_vfork_event_feature>: New enumeration constants.
(remote_protocol_features): Add table entries for new packets.
(remote_query_supported): Add new feature queries to qSupported
packet.
(_initialize_remote): Exempt new packets from the requirement
to have 'set remote' commands.
---
gdb/gdbserver/linux-low.c | 20 ++++++++++++++++++++
gdb/gdbserver/lynx-low.c | 2 ++
gdb/gdbserver/server.c | 22 ++++++++++++++++++++++
gdb/gdbserver/target.h | 14 ++++++++++++++
gdb/gdbserver/win32-low.c | 2 ++
gdb/nat/linux-ptrace.c | 2 +-
gdb/nat/linux-ptrace.h | 1 +
gdb/remote.c | 26 ++++++++++++++++++++++++++
8 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6dd9224..ddd0c69 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5425,6 +5425,22 @@ linux_supports_multi_process (void)
return 1;
}
+/* Check if fork events are supported. */
+
+static int
+linux_supports_fork_events (void)
+{
+ return linux_supports_tracefork ();
+}
+
+/* Check if vfork events are supported. */
+
+static int
+linux_supports_vfork_events (void)
+{
+ return linux_supports_tracefork ();
+}
+
static int
linux_supports_disable_randomization (void)
{
@@ -6397,6 +6413,8 @@ static struct target_ops linux_target_ops = {
linux_async,
linux_start_non_stop,
linux_supports_multi_process,
+ linux_supports_fork_events,
+ linux_supports_vfork_events,
#ifdef USE_THREAD_DB
thread_db_handle_monitor_command,
#else
@@ -6473,4 +6491,6 @@ initialize_low (void)
sigaction (SIGCHLD, &sigchld_action, NULL);
initialize_low_arch ();
+
+ linux_check_ptrace_features ();
}
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 2f85829..687cce0 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -758,6 +758,8 @@ static struct target_ops lynx_target_ops = {
NULL, /* async */
NULL, /* start_non_stop */
NULL, /* supports_multi_process */
+ NULL, /* supports_fork_events */
+ NULL, /* supports_vfork_events */
NULL, /* handle_monitor_command */
};
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3408ef7..c609db9 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -57,6 +57,8 @@ static int exit_requested;
int run_once;
int multi_process;
+int report_fork_events;
+int report_vfork_events;
int non_stop;
int swbreak_feature;
int hwbreak_feature;
@@ -1992,6 +1994,18 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_stopped_by_hw_breakpoint ())
hwbreak_feature = 1;
}
+ else if (strcmp (p, "fork-events+") == 0)
+ {
+ /* GDB supports and wants fork events if possible. */
+ if (target_supports_fork_events ())
+ report_fork_events = 1;
+ }
+ else if (strcmp (p, "vfork-events+") == 0)
+ {
+ /* GDB supports and wants vfork events if possible. */
+ if (target_supports_vfork_events ())
+ report_vfork_events = 1;
+ }
else
target_process_qsupported (p);
@@ -2042,6 +2056,12 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_multi_process ())
strcat (own_buf, ";multiprocess+");
+ if (target_supports_fork_events ())
+ strcat (own_buf, ";fork-events+");
+
+ if (target_supports_vfork_events ())
+ strcat (own_buf, ";vfork-events+");
+
if (target_supports_non_stop ())
strcat (own_buf, ";QNonStop+");
@@ -3414,6 +3434,8 @@ captured_main (int argc, char *argv[])
noack_mode = 0;
multi_process = 0;
+ report_fork_events = 0;
+ report_vfork_events = 0;
/* Be sure we're out of tfind mode. */
current_traceframe = -1;
cont_thread = null_ptid;
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 126c861..dc51627 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -277,6 +277,12 @@ struct target_ops
/* Returns true if the target supports multi-process debugging. */
int (*supports_multi_process) (void);
+ /* Returns true if fork events are supported. */
+ int (*supports_fork_events) (void);
+
+ /* Returns true if vfork events are supported. */
+ int (*supports_vfork_events) (void);
+
/* If not NULL, target-specific routine to process monitor command.
Returns 1 if handled, or 0 to perform default processing. */
int (*handle_monitor_command) (char *);
@@ -408,6 +414,14 @@ void set_target_ops (struct target_ops *);
int kill_inferior (int);
+#define target_supports_fork_events() \
+ (the_target->supports_fork_events ? \
+ (*the_target->supports_fork_events) () : 0)
+
+#define target_supports_vfork_events() \
+ (the_target->supports_vfork_events ? \
+ (*the_target->supports_vfork_events) () : 0)
+
#define detach_inferior(pid) \
(*the_target->detach) (pid)
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 6c86765..5f50e46 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -1827,6 +1827,8 @@ static struct target_ops win32_target_ops = {
NULL, /* async */
NULL, /* start_non_stop */
NULL, /* supports_multi_process */
+ NULL, /* supports_fork_events */
+ NULL, /* supports_vfork_events */
NULL, /* handle_monitor_command */
NULL, /* core_of_thread */
NULL, /* read_loadmap */
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 2244d9d..aba3da8 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -337,7 +337,7 @@ static void linux_test_for_exitkill (int child_pid);
/* Determine ptrace features available on this target. */
-static void
+void
linux_check_ptrace_features (void)
{
int child_pid, ret, status;
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 8354a4d..03d98c9 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -149,6 +149,7 @@ extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer);
extern char *linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
extern void linux_ptrace_init_warnings (void);
+extern void linux_check_ptrace_features (void);
extern void linux_enable_event_reporting (pid_t pid, int attached);
extern void linux_disable_event_reporting (pid_t pid);
extern int linux_supports_tracefork (void);
diff --git a/gdb/remote.c b/gdb/remote.c
index dcd24c4..f939059 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1341,6 +1341,12 @@ enum {
/* Support for hwbreak+ feature. */
PACKET_hwbreak_feature,
+ /* Support for fork events. */
+ PACKET_fork_event_feature,
+
+ /* Support for vfork events. */
+ PACKET_vfork_event_feature,
+
PACKET_MAX
};
@@ -4040,6 +4046,10 @@ static const struct protocol_feature remote_protocol_features[] = {
PACKET_Qbtrace_conf_bts_size },
{ "swbreak", PACKET_DISABLE, remote_supported_packet, PACKET_swbreak_feature },
{ "hwbreak", PACKET_DISABLE, remote_supported_packet, PACKET_hwbreak_feature },
+ { "fork-events", PACKET_DISABLE, remote_supported_packet,
+ PACKET_fork_event_feature },
+ { "vfork-events", PACKET_DISABLE, remote_supported_packet,
+ PACKET_vfork_event_feature },
};
static char *remote_support_xml;
@@ -4118,6 +4128,16 @@ remote_query_supported (void)
q = remote_query_supported_append (q, "qRelocInsn+");
+ if (rs->extended)
+ {
+ if (packet_set_cmd_state (PACKET_fork_event_feature)
+ != AUTO_BOOLEAN_FALSE)
+ q = remote_query_supported_append (q, "fork-events+");
+ if (packet_set_cmd_state (PACKET_vfork_event_feature)
+ != AUTO_BOOLEAN_FALSE)
+ q = remote_query_supported_append (q, "vfork-events+");
+ }
+
q = reconcat (q, "qSupported:", q, (char *) NULL);
putpkt (q);
@@ -12378,6 +12398,12 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_hwbreak_feature],
"hwbreak-feature", "hwbreak-feature", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_fork_event_feature],
+ "fork-event-feature", "fork-event-feature", 0);
+
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_vfork_event_feature],
+ "vfork-event-feature", "vfork-event-feature", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 1/7] Identify remote fork event support
2015-04-17 20:59 ` [PATCH v8 1/7] Identify remote fork event support Don Breazeal
@ 2015-04-23 14:17 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2015-04-23 14:17 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 04/17/2015 09:58 PM, Don Breazeal wrote:
> gdb/
> 2015-04-17 Don Breazeal <donb@codesourcery.com>
>
> * nat/linux-ptrace.c (linux_check_ptrace_features): Change
> from static to extern.
> * nat/linux-ptrace.h (linux_check_ptrace_features): Declare.
> * remote.c (anonymous enum): <PACKET_fork_event_feature,
> * PACKET_vfork_event_feature>: New enumeration constants.
> (remote_protocol_features): Add table entries for new packets.
> (remote_query_supported): Add new feature queries to qSupported
> packet.
> (_initialize_remote): Exempt new packets from the requirement
> to have 'set remote' commands.
This bit is stale.
Otherwise OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 3/7] Extended-remote Linux follow fork
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
2015-04-17 20:59 ` [PATCH v8 4/7] Arch-specific remote follow fork Don Breazeal
2015-04-17 20:59 ` [PATCH v8 1/7] Identify remote fork event support Don Breazeal
@ 2015-04-17 20:59 ` Don Breazeal
2015-04-23 14:18 ` Pedro Alves
2015-04-17 20:59 ` [PATCH v8 2/7] Clone remote breakpoints Don Breazeal
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 20:59 UTC (permalink / raw)
To: gdb-patches, palves
Hi Pedro,
This version of the patch incorporates changes based on your comments on
the previous version here:
https://sourceware.org/ml/gdb-patches/2015-04/msg00573.html.
On 4/15/2015 8:38 AM, Pedro Alves wrote:
> On 04/10/2015 06:09 PM, Don Breazeal wrote:
>
>>>> /* Do low-level handling of the event, and check if we should go on
>>>> and pass it to caller code. Return the affected lwp if we are, or
>>>> NULL otherwise. */
>>>> @@ -1912,11 +1983,11 @@ linux_low_filter_event (int lwpid, int wstat)
>>>> }
>>>> }
>>>>
>>>> - if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
>>>> + if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags && gdb_connected ())
>>>> {
>>>
>>> I don't really understand this. If the flag is set, why would it matter
>>> whether gdb is connected?
>>
>> My thinking was that there was no point in setting the ptrace options
>> until GDB had connected, since the qSupported packet right after connection
>> would cause a reset of the ptrace options. However, your point is taken, it
>> doesn't really matter, so I have remove the call to gdb_connected.
>
> It's not just that it doesn't matter; it's really wrong. If we attach to
> a multi-threaded program in non-stop mode, and set some tracepoints and
> detach for disconnected tracing before all threads stop and set their
> ptrace flags, we'll miss setting the ptrace options on some threads.
> But we need to be able to at least follow clone events in that case.
OK, thanks.
---snip snip---
>
>>>
>>>> +static int
>>>> +remote_follow_fork (struct target_ops *ops, int follow_child,
>>>> + int detach_fork)
>>>> +{
>>>> + struct remote_state *rs = get_remote_state ();
>>>> +
>>>> + if (remote_fork_event_p (rs))
>>>> + {
>>>> + if (detach_fork && !follow_child)
>>>
>>> Aren't we missing the "detach_fork && follow_child" case?
>>
>> That case is handled in the target-independent code in infrun.c by
>> calling target_detach.
>
> Could you add a one-liner comment mentioning that?
Done, a bit more than one line.
>
>> @@ -449,6 +451,57 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
>> warning ("wait returned unexpected status 0x%x", status);
>> }
>>
>> + if (event == PTRACE_EVENT_FORK)
>> + {
>> + struct process_info *parent_proc;
>> + struct process_info *child_proc;
>> + struct lwp_info *child_lwp;
>> + struct target_desc *tdesc;
>> +
>> + ptid = ptid_build (new_pid, new_pid, 0);
>> +
>> + if (debug_threads)
>> + {
>> + debug_printf ("HEW: Got fork event from LWP %ld, "
>> + "new child is %d\n",
>> + ptid_get_lwp (ptid_of (event_thr)),
>> + ptid_get_pid (ptid));
>> + }
>> +
>> + /* Add the new process to the tables and clone the breakpoint
>> + lists of the parent. We need to do this even if the new process
>> + will be detached, since we will need the process object and the
>> + breakpoints to remove any breakpoints from memory when we
>> + detach, and the host side will access registers. */
>
> Say "client side" instead of "host side".
Done.
>
>
>> +/* Callback for 'find_inferior'. Set the (possibly changed) ptrace
>> + options for the specified lwp. */
>> +
>> +static int
>> +reset_lwp_ptrace_options_callback (struct inferior_list_entry *entry,
>> + void *args)
>> +{
>> + struct thread_info *thread = (struct thread_info *) entry;
>> + struct lwp_info *lwp = get_thread_lwp (thread);
>> + struct process_info *proc = find_process_pid (pid_of (thread));
>> + int options = linux_low_ptrace_options (proc->attached);
>> +
>> + if (!lwp->stopped)
>> + {
>> + /* Stop the lwp so we can modify its ptrace options. */
>> + linux_stop_lwp (lwp);
>> + }
>> +
>> + linux_enable_event_reporting (lwpid_of (thread), options);
>> + lwp->must_set_ptrace_flags = 0;
>
> This still has the same problem. linux_stop_lwp does not
> wait for the LWP to stop, it just sends it a SIGSTOP signal.
> We can only set the ptrace options when the LWP ptrace-stops,
> that is, after we see it stop with waitpid. That is the whole
> point of the must_set_ptrace_flags flag. Do it like this:
>
> if (!lwp->stopped)
> {
> /* Stop the lwp so we can modify its ptrace options. */
> lwp->must_set_ptrace_flags = 1;
> stop_lwp (lwp);
> }
> else
> {
> /* Already stopped; go ahead and set the ptrace options. */
> struct process_info *proc = find_process_pid (pid_of (thread));
> int options = linux_low_ptrace_options (proc->attached);
>
> linux_enable_event_reporting (lwpid_of (thread), options);
> }
>
Sheesh. Thanks.
>
>
>>
>> +static int
>> +linux_nat_ptrace_options (int attached)
>
> Missing intro comment.
Fixed.
---snip snip---
>> --- a/gdb/nat/linux-ptrace.c
>> +++ b/gdb/nat/linux-ptrace.c
>> @@ -25,14 +25,10 @@
>>
>> #include <stdint.h>
>>
>> -/* Stores the currently supported ptrace options. A value of
>> - -1 means we did not check for features yet. A value of 0 means
>> - there are no supported features. */
>> -static int current_ptrace_options = -1;
>> -
>> -/* Additional flags to test. */
>> -
>> -static int additional_flags;
>> +/* Stores the ptrace options supported by the target.
>
> s/by the target/by the running kernel/
Done.
>
>> + A value of -1 means we did not check for features yet. A value
>> + of 0 means there are no supported features. */
>> +static int supported_ptrace_options = -1;
>>
>> /* Find all possible reasons we could fail to attach PID and append
>> these as strings to the already initialized BUFFER. '\0'
>> @@ -343,7 +339,7 @@ linux_check_ptrace_features (void)
>> int child_pid, ret, status;
>>
>> /* Initialize the options. */
>> - current_ptrace_options = 0;
>> + supported_ptrace_options = 0;
>>
>> /* Fork a child so we can do some testing. The child will call
>> linux_child_function and will get traced. The child will
>> @@ -387,14 +383,11 @@ linux_test_for_tracesysgood (int child_pid)
>> {
>> int ret;
>>
>> - if ((additional_flags & PTRACE_O_TRACESYSGOOD) == 0)
>> - return;
>> -
>> ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
>> (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
>>
>> if (ret == 0)
>> - current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
>> + supported_ptrace_options |= PTRACE_O_TRACESYSGOOD;
>> }
>>
>> /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
>> @@ -414,15 +407,12 @@ linux_test_for_tracefork (int child_pid)
>> if (ret != 0)
>> return;
>>
>> - if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
>> - {
>> - /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
>> - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
>> - (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
>> - | PTRACE_O_TRACEVFORKDONE));
>> - if (ret == 0)
>> - current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
>> - }
>> + /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
>> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
>> + (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
>> + | PTRACE_O_TRACEVFORKDONE));
>> + if (ret == 0)
>> + supported_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
>>
>> /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
>> don't know for sure that the feature is available; old
>> @@ -458,10 +448,13 @@ linux_test_for_tracefork (int child_pid)
>>
>> /* We got the PID from the grandchild, which means fork
>> tracing is supported. */
>> - current_ptrace_options |= PTRACE_O_TRACECLONE;
>> - current_ptrace_options |= (additional_flags & (PTRACE_O_TRACEFORK
>> - | PTRACE_O_TRACEVFORK
>> - | PTRACE_O_TRACEEXEC));
>> + supported_ptrace_options |= PTRACE_O_TRACECLONE;
>> +
>> + /* Save the "extended" options in case we need to reset
>> + the options later for a connect from a different GDB. */
>
> Remove now stale comment.
Done.
>
>> + supported_ptrace_options |= (PTRACE_O_TRACEFORK
>> + | PTRACE_O_TRACEVFORK
>> + | PTRACE_O_TRACEEXEC);
>>
>> /* Do some cleanup and kill the grandchild. */
>> my_waitpid (second_pid, &second_status, 0);
>
>>
>> /* Enable reporting of all currently supported ptrace events.
>> ATTACHED should be nonzero if we have attached to the inferior. */
>
> Update comment. I had suggested:
>
> /* Enable reporting of all currently supported ptrace events.
> OPTIONS is a bit mask of extended features we want enabled,
> if supported by the kernel. PTRACE_O_TRACECLONE is always
> enabled, if supported. */
Ow, sorry for the repeat. There are several things that were somehow
dropped from the final version of my patch. Hopefully things are
better this time around.
---snip snip---
>> static void
>> -remote_detach_1 (const char *args, int from_tty, int extended)
>> +remote_detach_1 (const char *args, int from_tty, int is_fork_child)
>
> Do we still need this "is_fork_child" change? I don't see where
> any caller has been updated in this version of the patch. AFAICS
> current callers still pass "extended":
This is another item that I apparently dropped from the final version
of the last patch. :-/ We no longer need the is_fork_child change, nor
do we need the 'extended' argument since we can get that information
from struct remote_state.
>
> static void
> remote_detach (struct target_ops *ops, const char *args, int from_tty)
> {
> remote_detach_1 (args, from_tty, 0);
> }
>
> static void
> extended_remote_detach (struct target_ops *ops, const char *args, int from_tty)
> {
> remote_detach_1 (args, from_tty, 1);
> }
>
> So something doesn't feel right here.
Fixed in this version.
>
>> {
>> int pid = ptid_get_pid (inferior_ptid);
>> struct remote_state *rs = get_remote_state ();
>> + struct thread_info *tp = find_thread_ptid (inferior_ptid);
>> + int is_fork_parent;
>>
>> if (args)
>> error (_("Argument given to \"detach\" when remotely debugging."));
>> @@ -4447,25 +4483,25 @@ remote_detach_1 (const char *args, int from_tty, int extended)
>> }
>>
>> /* Tell the remote target to detach. */
>> - if (remote_multi_process_p (rs))
>> - xsnprintf (rs->buf, get_remote_packet_size (), "D;%x", pid);
>> - else
>> - strcpy (rs->buf, "D");
>> + remote_detach_pid (pid);
>>
>> - putpkt (rs->buf);
>> - getpkt (&rs->buf, &rs->buf_size, 0);
>> -
>> - if (rs->buf[0] == 'O' && rs->buf[1] == 'K')
>> - ;
>> - else if (rs->buf[0] == '\0')
>> - error (_("Remote doesn't know how to detach"));
>> - else
>> - error (_("Can't detach process."));
>> -
>> - if (from_tty && !extended)
>> + if (from_tty && !rs->extended)
>> puts_filtered (_("Ending remote debugging.\n"));
>>
>> - target_mourn_inferior ();
>> + /* Check to see if we are detaching a fork parent. Note that if we
>> + are detaching a fork child, tp == NULL. */
>> + if (tp != NULL)
>> + is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
>> +
>> + /* If doing detach-on-fork, we don't mourn, because that will delete
>> + breakpoints that should be available for the followed inferior. */
>
> How does this work in the native case then?
In this scenario we are following the fork child and detaching from
the parent, which is sending us through the target's detach code.
In the native Linux case the detach code is linux_nat_detach, which in
turn calls inf_ptrace_detach. There is no call to target_mourn_inferior
in this sequence of function calls, so the breakpoints aren't affected
at that point. The breakpoints are "fixed up" for the child by
follow_inferior_reset_breakpoints at the very end of follow_fork, at
which point the child inferior has been set up.
The is_fork_parent condition in remote_detach_1 preventing the call
to target_mourn_inferior essentially makes the procedure the same as
for the native case.
>
>> + if (!is_fork_child && !is_fork_parent)
>> + target_mourn_inferior ();
>> + else
>> + {
>> + inferior_ptid = null_ptid;
>> + detach_inferior (pid);
>> + }
>> }
>>
>
> Thanks,
> Pedro Alves
>
Thanks
--Don
This patch implements basic support for follow-fork and detach-on-fork on
extended-remote Linux targets. Only 'fork' is supported in this patch;
'vfork' support is added n a subsequent patch. This patch depends on
the previous patches in the patch series.
Sufficient extended-remote functionality has been implemented here to pass
gdb.base/multi-forks.exp, as well as gdb.base/foll-fork.exp with the
catchpoint tests commented out. Some other fork tests fail with this
patch because it doesn't provide the architecture support needed for
watchpoint inheritance or fork catchpoints.
The implementation follows the same general structure as for the native
implementation as much as possible.
This implementation includes:
* enabling fork events in linux-low.c in initialize_low and
linux_enable_extended_features
* handling fork events in gdbserver/linux-low.c:handle_extended_wait
- when a fork event occurs in gdbserver, we must do the full creation
of the new process, thread, lwp, and breakpoint lists. This is
required whether or not the new child is destined to be
detached-on-fork, because GDB will make target calls that require all
the structures. In particular we need the breakpoint lists in order
to remove the breakpoints from a detaching child. If we are not
detaching the child we will need all these structures anyway.
- as part of this event handling we store the target_waitstatus in a new
member of the parent lwp_info structure, 'waitstatus'. This
is used to store extended event information for reporting to GDB.
- handle_extended_wait is given a return value, denoting whether the
handled event should be reported to GDB. Previously it had only
handled clone events, which were never reported.
* using a new predicate in gdbserver to control handling of the fork event
(and eventually all extended events) in linux_wait_1. The predicate,
extended_event_reported, checks a target_waitstatus.kind for an
extended ptrace event.
* implementing a new RSP 'T' Stop Reply Packet stop reason: "fork", in
gdbserver/remote-utils.c and remote.c.
* implementing new target and RSP support for target_follow_fork with
target extended-remote. (The RSP components were actually defined in
patch 1, but they see their first use here).
- remote target routine remote_follow_fork, which just sends the 'D;pid'
detach packet to detach the new fork child cleanly. We can't just
call target_detach because the data structures for the forked child
have not been allocated on the host side.
Tested on x64 Ubuntu Lucid, native, remote, extended-remote.
gdb/gdbserver/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* linux-low.c (handle_extended_wait): Implement return value,
rename argument 'event_child' to 'event_lwp', handle
PTRACE_EVENT_FORK, call internal_error for unrecognized event.
(linux_low_ptrace_options): New function.
(linux_low_filter_event): Call linux_low_ptrace_options,
use different argument fo linux_enable_event_reporting,
use return value from handle_extended_wait.
(extended_event_reported): New function.
(linux_wait_1): Call extended_event_reported and set
status to report fork events.
(linux_write_memory): Add pid to debug message.
(reset_lwp_ptrace_options_callback): New function.
(linux_handle_new_gdb_connection): New function.
(linux_target_ops): Initialize new structure member.
* linux-low.h (struct lwp_info) <waitstatus>: New member.
* lynx-low.c: Initialize new structure member.
* remote-utils.c (prepare_resume_reply): Implement stop reason
"fork" for "T" stop message.
* server.c (handle_query): Call handle_new_gdb_connection.
* server.h (report_fork_events): Declare global flag.
* target.h (struct target_ops) <handle_new_gdb_connection>:
New member.
(target_handle_new_gdb_connection): New macro.
* win32-low.c: Initialize new structure member.
gdb/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* linux-nat.c (linux_nat_ptrace_options): New function.
(linux_init_ptrace, wait_lwp, linux_nat_filter_event):
Call linux_nat_ptrace_options and use different argument to
linux_enable_event_reporting.
(_initialize_linux_nat): Delete call to
linux_ptrace_set_additional_flags.
* nat/linux-ptrace.c (current_ptrace_options): Rename to
supported_ptrace_options.
(additional_flags): Delete variable.
(linux_check_ptrace_features): Use supported_ptrace_options.
(linux_test_for_tracesysgood, linux_test_for_tracefork):
Likewise, and remove additional_flags check.
(linux_enable_event_reporting): Change 'attached' argument to
'options'. Use supported_ptrace_options.
(ptrace_supports_feature): Change comment. Use
supported_ptrace_options.
(linux_ptrace_set_additional_flags): Delete function.
* nat/linux-ptrace.h (linux_ptrace_set_additional_flags):
Delete function prototype.
* remote.c (remote_fork_event_p): New function.
(remote_detach_pid): New function.
(remote_detach_1): Call remote_detach_pid, don't mourn inferior
if doing detach-on-fork.
(remote_follow_fork): New function.
(remote_parse_stop_reply): Handle new "T" stop reason "fork".
(remote_pid_to_str): Print "process" strings for pid/0/0 ptids.
(init_extended_remote_ops): Initialize to_follow_fork.
---
gdb/gdbserver/linux-low.c | 180 +++++++++++++++++++++++++++++++++++++++----
gdb/gdbserver/linux-low.h | 4 +
gdb/gdbserver/lynx-low.c | 1 +
gdb/gdbserver/remote-utils.c | 14 +++-
gdb/gdbserver/server.c | 3 +
gdb/gdbserver/server.h | 1 +
gdb/gdbserver/target.h | 7 ++
gdb/gdbserver/win32-low.c | 1 +
gdb/linux-nat.c | 37 ++++++---
gdb/nat/linux-ptrace.c | 85 ++++++++------------
gdb/nat/linux-ptrace.h | 1 -
gdb/remote.c | 130 +++++++++++++++++++++++++------
12 files changed, 358 insertions(+), 106 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index ddd0c69..7f61946 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -20,6 +20,7 @@
#include "linux-low.h"
#include "nat/linux-osdata.h"
#include "agent.h"
+#include "tdesc.h"
#include "nat/linux-nat.h"
#include "nat/linux-waitpid.h"
@@ -414,22 +415,23 @@ linux_add_process (int pid, int attached)
static CORE_ADDR get_pc (struct lwp_info *lwp);
/* Handle a GNU/Linux extended wait response. If we see a clone
- event, we need to add the new LWP to our list (and not report the
- trap to higher layers). */
+ event, we need to add the new LWP to our list (and return 0 so as
+ not to report the trap to higher layers). */
-static void
-handle_extended_wait (struct lwp_info *event_child, int wstat)
+static int
+handle_extended_wait (struct lwp_info *event_lwp, int wstat)
{
int event = linux_ptrace_get_extended_event (wstat);
- struct thread_info *event_thr = get_lwp_thread (event_child);
+ struct thread_info *event_thr = get_lwp_thread (event_lwp);
struct lwp_info *new_lwp;
- if (event == PTRACE_EVENT_CLONE)
+ if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_CLONE))
{
ptid_t ptid;
unsigned long new_pid;
int ret, status;
+ /* Get the pid of the new lwp. */
ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0,
&new_pid);
@@ -449,6 +451,57 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
warning ("wait returned unexpected status 0x%x", status);
}
+ if (event == PTRACE_EVENT_FORK)
+ {
+ struct process_info *parent_proc;
+ struct process_info *child_proc;
+ struct lwp_info *child_lwp;
+ struct target_desc *tdesc;
+
+ ptid = ptid_build (new_pid, new_pid, 0);
+
+ if (debug_threads)
+ {
+ debug_printf ("HEW: Got fork event from LWP %ld, "
+ "new child is %d\n",
+ ptid_get_lwp (ptid_of (event_thr)),
+ ptid_get_pid (ptid));
+ }
+
+ /* Add the new process to the tables and clone the breakpoint
+ lists of the parent. We need to do this even if the new process
+ will be detached, since we will need the process object and the
+ breakpoints to remove any breakpoints from memory when we
+ detach, and the client side will access registers. */
+ child_proc = linux_add_process (new_pid, 0);
+ gdb_assert (child_proc != NULL);
+ child_lwp = add_lwp (ptid);
+ gdb_assert (child_lwp != NULL);
+ child_lwp->stopped = 1;
+ parent_proc = get_thread_process (event_thr);
+ child_proc->attached = parent_proc->attached;
+ clone_all_breakpoints (&child_proc->breakpoints,
+ &child_proc->raw_breakpoints,
+ parent_proc->breakpoints);
+
+ tdesc = xmalloc (sizeof (struct target_desc));
+ copy_target_description (tdesc, parent_proc->tdesc);
+ child_proc->tdesc = tdesc;
+ child_lwp->must_set_ptrace_flags = 1;
+
+ /* Save fork info in the parent thread. */
+ event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
+ event_lwp->waitstatus.value.related_pid = ptid;
+ /* The status_pending field contains bits denoting the
+ extended event, so when the pending event is handled,
+ the handler will look at lwp->waitstatus. */
+ event_lwp->status_pending_p = 1;
+ event_lwp->status_pending = wstat;
+
+ /* Report the event. */
+ return 0;
+ }
+
if (debug_threads)
debug_printf ("HEW: Got clone event "
"from LWP %ld, new child is LWP %ld\n",
@@ -477,7 +530,12 @@ handle_extended_wait (struct lwp_info *event_child, int wstat)
new_lwp->status_pending_p = 1;
new_lwp->status_pending = status;
}
+
+ /* Don't report the event. */
+ return 1;
}
+
+ internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event);
}
/* Return the PC as read from the regcache of LWP, without any
@@ -1935,6 +1993,22 @@ check_stopped_by_watchpoint (struct lwp_info *child)
return child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT;
}
+/* Return the ptrace options that we want to try to enable. */
+
+static int
+linux_low_ptrace_options (int attached)
+{
+ int options = 0;
+
+ if (!attached)
+ options |= PTRACE_O_EXITKILL;
+
+ if (report_fork_events)
+ options |= PTRACE_O_TRACEFORK;
+
+ return options;
+}
+
/* Do low-level handling of the event, and check if we should go on
and pass it to caller code. Return the affected lwp if we are, or
NULL otherwise. */
@@ -2022,8 +2096,9 @@ linux_low_filter_event (int lwpid, int wstat)
if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags)
{
struct process_info *proc = find_process_pid (pid_of (thread));
+ int options = linux_low_ptrace_options (proc->attached);
- linux_enable_event_reporting (lwpid, proc->attached);
+ linux_enable_event_reporting (lwpid, options);
child->must_set_ptrace_flags = 0;
}
@@ -2033,8 +2108,12 @@ linux_low_filter_event (int lwpid, int wstat)
&& linux_is_extended_waitstatus (wstat))
{
child->stop_pc = get_pc (child);
- handle_extended_wait (child, wstat);
- return NULL;
+ if (handle_extended_wait (child, wstat))
+ {
+ /* The event has been handled, so just return without
+ reporting it. */
+ return NULL;
+ }
}
/* Check first whether this was a SW/HW breakpoint before checking
@@ -2622,6 +2701,18 @@ ignore_event (struct target_waitstatus *ourstatus)
return null_ptid;
}
+/* Return non-zero if WAITSTATUS reflects an extended linux
+ event. Otherwise, return zero. */
+
+static int
+extended_event_reported (const struct target_waitstatus *waitstatus)
+{
+ if (waitstatus == NULL)
+ return 0;
+
+ return (waitstatus->kind == TARGET_WAITKIND_FORKED);
+}
+
/* Wait for process, returns status. */
static ptid_t
@@ -2988,7 +3079,8 @@ linux_wait_1 (ptid_t ptid,
&& !bp_explains_trap && !trace_event)
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
- && gdb_no_commands_at_breakpoint (event_child->stop_pc)));
+ && gdb_no_commands_at_breakpoint (event_child->stop_pc))
+ || extended_event_reported (&event_child->waitstatus));
run_breakpoint_commands (event_child->stop_pc);
@@ -3010,6 +3102,13 @@ linux_wait_1 (ptid_t ptid,
paddress (event_child->stop_pc),
paddress (event_child->step_range_start),
paddress (event_child->step_range_end));
+ if (extended_event_reported (&event_child->waitstatus))
+ {
+ char *str = target_waitstatus_to_string (ourstatus);
+ debug_printf ("LWP %ld: extended event with waitstatus %s\n",
+ lwpid_of (get_lwp_thread (event_child)), str);
+ xfree (str);
+ }
}
/* We're not reporting this breakpoint to GDB, so apply the
@@ -3119,7 +3218,17 @@ linux_wait_1 (ptid_t ptid,
unstop_all_lwps (1, event_child);
}
- ourstatus->kind = TARGET_WAITKIND_STOPPED;
+ if (extended_event_reported (&event_child->waitstatus))
+ {
+ /* If the reported event is a fork, vfork or exec, let GDB know. */
+ ourstatus->kind = event_child->waitstatus.kind;
+ ourstatus->value = event_child->waitstatus.value;
+
+ /* Clear the event lwp's waitstatus since we handled it already. */
+ event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ }
+ else
+ ourstatus->kind = TARGET_WAITKIND_STOPPED;
/* Now that we've selected our final event LWP, un-adjust its PC if
it was a software breakpoint, and the client doesn't know we can
@@ -3152,7 +3261,7 @@ linux_wait_1 (ptid_t ptid,
but, it stopped for other reasons. */
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
- else
+ else if (ourstatus->kind == TARGET_WAITKIND_STOPPED)
{
ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (w));
}
@@ -4996,8 +5105,8 @@ linux_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
val = val & 0xffff;
else if (len == 3)
val = val & 0xffffff;
- debug_printf ("Writing %0*x to 0x%08lx\n", 2 * ((len < 4) ? len : 4),
- val, (long)memaddr);
+ debug_printf ("Writing %0*x to 0x%08lx in process %d\n",
+ 2 * ((len < 4) ? len : 4), val, (long)memaddr, pid);
}
/* Fill start and end extra bytes of buffer with existing memory data. */
@@ -5441,6 +5550,48 @@ linux_supports_vfork_events (void)
return linux_supports_tracefork ();
}
+/* Callback for 'find_inferior'. Set the (possibly changed) ptrace
+ options for the specified lwp. */
+
+static int
+reset_lwp_ptrace_options_callback (struct inferior_list_entry *entry,
+ void *args)
+{
+ struct thread_info *thread = (struct thread_info *) entry;
+ struct lwp_info *lwp = get_thread_lwp (thread);
+
+ if (!lwp->stopped)
+ {
+ /* Stop the lwp so we can modify its ptrace options. */
+ lwp->must_set_ptrace_flags = 1;
+ linux_stop_lwp (lwp);
+ }
+ else
+ {
+ /* Already stopped; go ahead and set the ptrace options. */
+ struct process_info *proc = find_process_pid (pid_of (thread));
+ int options = linux_low_ptrace_options (proc->attached);
+
+ linux_enable_event_reporting (lwpid_of (thread), options);
+ lwp->must_set_ptrace_flags = 0;
+ }
+
+ return 0;
+}
+
+/* Target hook for 'handle_new_gdb_connection'. Causes a reset of the
+ ptrace flags for all inferiors. This is in case the new GDB connection
+ doesn't support the same set of events that the previous one did. */
+
+static void
+linux_handle_new_gdb_connection (void)
+{
+ pid_t pid;
+
+ /* Request that all the lwps reset their ptrace options. */
+ find_inferior (&all_threads, reset_lwp_ptrace_options_callback , &pid);
+}
+
static int
linux_supports_disable_randomization (void)
{
@@ -6415,6 +6566,7 @@ static struct target_ops linux_target_ops = {
linux_supports_multi_process,
linux_supports_fork_events,
linux_supports_vfork_events,
+ linux_handle_new_gdb_connection,
#ifdef USE_THREAD_DB
thread_db_handle_monitor_command,
#else
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 1ae3701..41067d6 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -271,6 +271,10 @@ struct lwp_info
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
+ /* This is used to store extended ptrace event information until
+ it is reported to GDB. */
+ struct target_waitstatus waitstatus;
+
/* When stopped is set, this is where the lwp last stopped, with
decr_pc_after_break already accounted for. If the LWP is
running, this is the address at which the lwp was resumed. */
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 687cce0..38df1b7 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -760,6 +760,7 @@ static struct target_ops lynx_target_ops = {
NULL, /* supports_multi_process */
NULL, /* supports_fork_events */
NULL, /* supports_vfork_events */
+ NULL, /* handle_new_gdb_connection */
NULL, /* handle_monitor_command */
};
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1de86be..6078348 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1114,12 +1114,24 @@ prepare_resume_reply (char *buf, ptid_t ptid,
switch (status->kind)
{
case TARGET_WAITKIND_STOPPED:
+ case TARGET_WAITKIND_FORKED:
{
struct thread_info *saved_thread;
const char **regp;
struct regcache *regcache;
- sprintf (buf, "T%02x", status->value.sig);
+ if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
+ {
+ enum gdb_signal signal = GDB_SIGNAL_TRAP;
+
+ sprintf (buf, "T%02xfork:", signal);
+ buf += strlen (buf);
+ buf = write_ptid (buf, status->value.related_pid);
+ strcat (buf, ";");
+ }
+ else
+ sprintf (buf, "T%02x", status->value.sig);
+
buf += strlen (buf);
saved_thread = current_thread;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index c609db9..144e2f8 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2102,6 +2102,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_stopped_by_hw_breakpoint ())
strcat (own_buf, ";hwbreak+");
+ /* Reinitialize the target as needed for the new connection. */
+ target_handle_new_gdb_connection ();
+
return;
}
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 91d4080..696a24e 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -84,6 +84,7 @@ extern int disable_packet_qfThreadInfo;
extern int run_once;
extern int multi_process;
+extern int report_fork_events;
extern int non_stop;
/* True if the "swbreak+" feature is active. In that case, GDB wants
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index dc51627..ca010ff 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -283,6 +283,9 @@ struct target_ops
/* Returns true if vfork events are supported. */
int (*supports_vfork_events) (void);
+ /* Allows target to re-initialize connection-specific settings. */
+ void (*handle_new_gdb_connection) (void);
+
/* If not NULL, target-specific routine to process monitor command.
Returns 1 if handled, or 0 to perform default processing. */
int (*handle_monitor_command) (char *);
@@ -422,6 +425,10 @@ int kill_inferior (int);
(the_target->supports_vfork_events ? \
(*the_target->supports_vfork_events) () : 0)
+#define target_handle_new_gdb_connection() \
+ (the_target->handle_new_gdb_connection ? \
+ (*the_target->handle_new_gdb_connection) () : 0)
+
#define detach_inferior(pid) \
(*the_target->detach) (pid)
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 5f50e46..e7c04d6 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -1829,6 +1829,7 @@ static struct target_ops win32_target_ops = {
NULL, /* supports_multi_process */
NULL, /* supports_fork_events */
NULL, /* supports_vfork_events */
+ NULL, /* handle_new_gdb_connection */
NULL, /* handle_monitor_command */
NULL, /* core_of_thread */
NULL, /* read_loadmap */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 6c198cf..22990b9 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -368,6 +368,25 @@ pull_pid_from_list (struct simple_pid_list **listp, int pid, int *statusp)
return 0;
}
+/* Return the ptrace options that we want to try to enable. */
+
+static int
+linux_nat_ptrace_options (int attached)
+{
+ int options = 0;
+
+ if (!attached)
+ options |= PTRACE_O_EXITKILL;
+
+ options |= (PTRACE_O_TRACESYSGOOD
+ | PTRACE_O_TRACEVFORKDONE
+ | PTRACE_O_TRACEVFORK
+ | PTRACE_O_TRACEFORK
+ | PTRACE_O_TRACEEXEC);
+
+ return options;
+}
+
/* Initialize ptrace warnings and check for supported ptrace
features given PID.
@@ -376,7 +395,9 @@ pull_pid_from_list (struct simple_pid_list **listp, int pid, int *statusp)
static void
linux_init_ptrace (pid_t pid, int attached)
{
- linux_enable_event_reporting (pid, attached);
+ int options = linux_nat_ptrace_options (attached);
+
+ linux_enable_event_reporting (pid, options);
linux_ptrace_init_warnings ();
}
@@ -2301,8 +2322,9 @@ wait_lwp (struct lwp_info *lp)
if (lp->must_set_ptrace_flags)
{
struct inferior *inf = find_inferior_pid (ptid_get_pid (lp->ptid));
+ int options = linux_nat_ptrace_options (inf->attach_flag);
- linux_enable_event_reporting (ptid_get_lwp (lp->ptid), inf->attach_flag);
+ linux_enable_event_reporting (ptid_get_lwp (lp->ptid), options);
lp->must_set_ptrace_flags = 0;
}
@@ -3102,8 +3124,9 @@ linux_nat_filter_event (int lwpid, int status)
if (WIFSTOPPED (status) && lp->must_set_ptrace_flags)
{
struct inferior *inf = find_inferior_pid (ptid_get_pid (lp->ptid));
+ int options = linux_nat_ptrace_options (inf->attach_flag);
- linux_enable_event_reporting (ptid_get_lwp (lp->ptid), inf->attach_flag);
+ linux_enable_event_reporting (ptid_get_lwp (lp->ptid), options);
lp->must_set_ptrace_flags = 0;
}
@@ -5036,14 +5059,6 @@ Enables printf debugging output."),
sigdelset (&suspend_mask, SIGCHLD);
sigemptyset (&blocked_mask);
-
- /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
- support read-only process state. */
- linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD
- | PTRACE_O_TRACEVFORKDONE
- | PTRACE_O_TRACEVFORK
- | PTRACE_O_TRACEFORK
- | PTRACE_O_TRACEEXEC);
}
\f
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index aba3da8..fb12606 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -25,14 +25,10 @@
#include <stdint.h>
-/* Stores the currently supported ptrace options. A value of
- -1 means we did not check for features yet. A value of 0 means
- there are no supported features. */
-static int current_ptrace_options = -1;
-
-/* Additional flags to test. */
-
-static int additional_flags;
+/* Stores the ptrace options supported by the running kernel.
+ A value of -1 means we did not check for features yet. A value
+ of 0 means there are no supported features. */
+static int supported_ptrace_options = -1;
/* Find all possible reasons we could fail to attach PID and append
these as strings to the already initialized BUFFER. '\0'
@@ -343,7 +339,7 @@ linux_check_ptrace_features (void)
int child_pid, ret, status;
/* Initialize the options. */
- current_ptrace_options = 0;
+ supported_ptrace_options = 0;
/* Fork a child so we can do some testing. The child will call
linux_child_function and will get traced. The child will
@@ -387,14 +383,11 @@ linux_test_for_tracesysgood (int child_pid)
{
int ret;
- if ((additional_flags & PTRACE_O_TRACESYSGOOD) == 0)
- return;
-
ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
(PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
if (ret == 0)
- current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
+ supported_ptrace_options |= PTRACE_O_TRACESYSGOOD;
}
/* Determine if PTRACE_O_TRACEFORK can be used to follow fork
@@ -414,15 +407,12 @@ linux_test_for_tracefork (int child_pid)
if (ret != 0)
return;
- if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
- {
- /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
- ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
- (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
- | PTRACE_O_TRACEVFORKDONE));
- if (ret == 0)
- current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
- }
+ /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */
+ ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+ (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
+ | PTRACE_O_TRACEVFORKDONE));
+ if (ret == 0)
+ supported_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
/* Setting PTRACE_O_TRACEFORK did not cause an error, however we
don't know for sure that the feature is available; old
@@ -458,10 +448,10 @@ linux_test_for_tracefork (int child_pid)
/* We got the PID from the grandchild, which means fork
tracing is supported. */
- current_ptrace_options |= PTRACE_O_TRACECLONE;
- current_ptrace_options |= (additional_flags & (PTRACE_O_TRACEFORK
- | PTRACE_O_TRACEVFORK
- | PTRACE_O_TRACEEXEC));
+ supported_ptrace_options |= PTRACE_O_TRACECLONE;
+ supported_ptrace_options |= (PTRACE_O_TRACEFORK
+ | PTRACE_O_TRACEVFORK
+ | PTRACE_O_TRACEEXEC);
/* Do some cleanup and kill the grandchild. */
my_waitpid (second_pid, &second_status, 0);
@@ -489,33 +479,31 @@ linux_test_for_exitkill (int child_pid)
(PTRACE_TYPE_ARG4) PTRACE_O_EXITKILL);
if (ret == 0)
- current_ptrace_options |= PTRACE_O_EXITKILL;
+ supported_ptrace_options |= PTRACE_O_EXITKILL;
}
/* Enable reporting of all currently supported ptrace events.
- ATTACHED should be nonzero if we have attached to the inferior. */
+ OPTIONS is a bit mask of extended features we want enabled,
+ if supported by the kernel. PTRACE_O_TRACECLONE is always
+ enabled, if supported. */
void
-linux_enable_event_reporting (pid_t pid, int attached)
+linux_enable_event_reporting (pid_t pid, int options)
{
- int ptrace_options;
-
/* Check if we have initialized the ptrace features for this
target. If not, do it now. */
- if (current_ptrace_options == -1)
+ if (supported_ptrace_options == -1)
linux_check_ptrace_features ();
- ptrace_options = current_ptrace_options;
- if (attached)
- {
- /* When attached to our inferior, we do not want the inferior
- to die with us if we terminate unexpectedly. */
- ptrace_options &= ~PTRACE_O_EXITKILL;
- }
+ /* We always want clone events. */
+ options |= PTRACE_O_TRACECLONE;
+
+ /* Filter out unsupported options. */
+ options &= supported_ptrace_options;
/* Set the options. */
ptrace (PTRACE_SETOPTIONS, pid, (PTRACE_TYPE_ARG3) 0,
- (PTRACE_TYPE_ARG4) (uintptr_t) ptrace_options);
+ (PTRACE_TYPE_ARG4) (uintptr_t) options);
}
/* Disable reporting of all currently supported ptrace events. */
@@ -528,16 +516,16 @@ linux_disable_event_reporting (pid_t pid)
}
/* Returns non-zero if PTRACE_OPTIONS is contained within
- CURRENT_PTRACE_OPTIONS, therefore supported. Returns 0
+ SUPPORTED_PTRACE_OPTIONS, therefore supported. Returns 0
otherwise. */
static int
ptrace_supports_feature (int ptrace_options)
{
- if (current_ptrace_options == -1)
+ if (supported_ptrace_options == -1)
linux_check_ptrace_features ();
- return ((current_ptrace_options & ptrace_options) == ptrace_options);
+ return ((supported_ptrace_options & ptrace_options) == ptrace_options);
}
/* Returns non-zero if PTRACE_EVENT_FORK is supported by ptrace,
@@ -595,17 +583,6 @@ linux_ptrace_init_warnings (void)
linux_ptrace_test_ret_to_nx ();
}
-/* Set additional ptrace flags to use. Some such flags may be checked
- by the implementation above. This function must be called before
- any other function in this file; otherwise the flags may not take
- effect appropriately. */
-
-void
-linux_ptrace_set_additional_flags (int flags)
-{
- additional_flags = flags;
-}
-
/* Extract extended ptrace event from wait status. */
int
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 03d98c9..1db0cde 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -156,7 +156,6 @@ extern int linux_supports_tracefork (void);
extern int linux_supports_traceclone (void);
extern int linux_supports_tracevforkdone (void);
extern int linux_supports_tracesysgood (void);
-extern void linux_ptrace_set_additional_flags (int);
extern int linux_ptrace_get_extended_event (int wstat);
extern int linux_is_extended_waitstatus (int wstat);
extern int linux_wstatus_maybe_breakpoint (int wstat);
diff --git a/gdb/remote.c b/gdb/remote.c
index f939059..8e26e5e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1461,6 +1461,14 @@ remote_multi_process_p (struct remote_state *rs)
return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE;
}
+/* Returns true if fork events are supported. */
+
+static int
+remote_fork_event_p (struct remote_state *rs)
+{
+ return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
+}
+
/* Tokens for use by the asynchronous signal handlers for SIGINT. */
static struct async_signal_handler *async_sigint_remote_twice_token;
static struct async_signal_handler *async_sigint_remote_token;
@@ -4419,16 +4427,42 @@ remote_open_1 (const char *name, int from_tty,
wait_forever_enabled_p = 1;
}
-/* This takes a program previously attached to and detaches it. After
- this is done, GDB can be used to debug some other program. We
- better not have left any breakpoints in the target program or it'll
- die when it hits one. */
+/* Detach the specified process. */
+
+static void
+remote_detach_pid (int pid)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ if (remote_multi_process_p (rs))
+ xsnprintf (rs->buf, get_remote_packet_size (), "D;%x", pid);
+ else
+ strcpy (rs->buf, "D");
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+
+ if (rs->buf[0] == 'O' && rs->buf[1] == 'K')
+ ;
+ else if (rs->buf[0] == '\0')
+ error (_("Remote doesn't know how to detach"));
+ else
+ error (_("Can't detach process."));
+}
+
+/* This detaches a program to which we previously attached, using
+ inferior_ptid to identify the process. After this is done, GDB
+ can be used to debug some other program. We better not have left
+ any breakpoints in the target program or it'll die when it hits
+ one. */
static void
-remote_detach_1 (const char *args, int from_tty, int extended)
+remote_detach_1 (const char *args, int from_tty)
{
int pid = ptid_get_pid (inferior_ptid);
struct remote_state *rs = get_remote_state ();
+ struct thread_info *tp = find_thread_ptid (inferior_ptid);
+ int is_fork_parent;
if (args)
error (_("Argument given to \"detach\" when remotely debugging."));
@@ -4447,37 +4481,74 @@ remote_detach_1 (const char *args, int from_tty, int extended)
}
/* Tell the remote target to detach. */
- if (remote_multi_process_p (rs))
- xsnprintf (rs->buf, get_remote_packet_size (), "D;%x", pid);
- else
- strcpy (rs->buf, "D");
+ remote_detach_pid (pid);
- putpkt (rs->buf);
- getpkt (&rs->buf, &rs->buf_size, 0);
-
- if (rs->buf[0] == 'O' && rs->buf[1] == 'K')
- ;
- else if (rs->buf[0] == '\0')
- error (_("Remote doesn't know how to detach"));
- else
- error (_("Can't detach process."));
-
- if (from_tty && !extended)
+ if (from_tty && !rs->extended)
puts_filtered (_("Ending remote debugging.\n"));
- target_mourn_inferior ();
+ /* Check to see if we are detaching a fork parent. Note that if we
+ are detaching a fork child, tp == NULL. */
+ if (tp != NULL)
+ is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
+
+ /* If doing detach-on-fork, we don't mourn, because that will delete
+ breakpoints that should be available for the followed inferior. */
+ if (!is_fork_parent)
+ target_mourn_inferior ();
+ else
+ {
+ inferior_ptid = null_ptid;
+ detach_inferior (pid);
+ }
}
static void
remote_detach (struct target_ops *ops, const char *args, int from_tty)
{
- remote_detach_1 (args, from_tty, 0);
+ remote_detach_1 (args, from_tty);
}
static void
extended_remote_detach (struct target_ops *ops, const char *args, int from_tty)
{
- remote_detach_1 (args, from_tty, 1);
+ remote_detach_1 (args, from_tty);
+}
+
+/* Target follow-fork function for remote targets. On entry, and
+ at return, the current inferior is the fork parent.
+
+ Note that although this is currently only used for extended-remote,
+ it is named remote_follow_fork in anticipation of using it for the
+ remote target as well. */
+
+static int
+remote_follow_fork (struct target_ops *ops, int follow_child,
+ int detach_fork)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ if (remote_fork_event_p (rs))
+ {
+ /* When following the parent and detaching the child, we detach
+ the child here. For the case of following the child and
+ detaching the parent, the detach is done in the target-
+ independent follow fork code in infrun.c. We can't use
+ target_detach when detaching an unfollowed child because
+ the client side doesn't know anything about the child. */
+ if (detach_fork && !follow_child)
+ {
+ /* Detach the fork child. */
+ ptid_t child_ptid;
+ pid_t child_pid;
+
+ child_ptid = inferior_thread ()->pending_follow.value.related_pid;
+ child_pid = ptid_get_pid (child_ptid);
+
+ remote_detach_pid (child_pid);
+ detach_inferior (child_pid);
+ }
+ }
+ return 0;
}
/* Same as remote_detach, but don't send the "D" packet; just disconnect. */
@@ -5640,6 +5711,11 @@ Packet: '%s'\n"),
p = unpack_varlen_hex (++p1, &c);
event->core = c;
}
+ else if (strncmp (p, "fork", p1 - p) == 0)
+ {
+ event->ws.value.related_pid = read_ptid (++p1, &p);
+ event->ws.kind = TARGET_WAITKIND_FORKED;
+ }
else
{
ULONGEST pnum;
@@ -9489,8 +9565,11 @@ remote_pid_to_str (struct target_ops *ops, ptid_t ptid)
if (ptid_equal (magic_null_ptid, ptid))
xsnprintf (buf, sizeof buf, "Thread <main>");
else if (rs->extended && remote_multi_process_p (rs))
- xsnprintf (buf, sizeof buf, "Thread %d.%ld",
- ptid_get_pid (ptid), ptid_get_lwp (ptid));
+ if (ptid_get_lwp (ptid) == 0)
+ return normal_pid_to_str (ptid);
+ else
+ xsnprintf (buf, sizeof buf, "Thread %d.%ld",
+ ptid_get_pid (ptid), ptid_get_lwp (ptid));
else
xsnprintf (buf, sizeof buf, "Thread %ld",
ptid_get_lwp (ptid));
@@ -11817,6 +11896,7 @@ Specify the serial device it is connected to (e.g. /dev/ttya).";
extended_remote_ops.to_kill = extended_remote_kill;
extended_remote_ops.to_supports_disable_randomization
= extended_remote_supports_disable_randomization;
+ extended_remote_ops.to_follow_fork = remote_follow_fork;
}
static int
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 3/7] Extended-remote Linux follow fork
2015-04-17 20:59 ` [PATCH v8 3/7] Extended-remote Linux follow fork Don Breazeal
@ 2015-04-23 14:18 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2015-04-23 14:18 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 04/17/2015 09:58 PM, Don Breazeal wrote:
> + if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
> + {
> + enum gdb_signal signal = GDB_SIGNAL_TRAP;
> +
> + sprintf (buf, "T%02xfork:", signal);
> + buf += strlen (buf);
> + buf = write_ptid (buf, status->value.related_pid);
> + strcat (buf, ";");
We don't really need that ';', right?
> + }
> + else
> + sprintf (buf, "T%02x", status->value.sig);
I think we should not rely on the value of "status->value.sig"
here, and send GDB_SIGNAL_0 explicitly.
>>> - if (from_tty && !extended)
>>> + if (from_tty && !rs->extended)
>>> puts_filtered (_("Ending remote debugging.\n"));
>>>
>>> - target_mourn_inferior ();
>>> + /* Check to see if we are detaching a fork parent. Note that if we
>>> + are detaching a fork child, tp == NULL. */
>>> + if (tp != NULL)
>>> + is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
>>> +
>>> + /* If doing detach-on-fork, we don't mourn, because that will delete
>>> + breakpoints that should be available for the followed inferior. */
>>
>> How does this work in the native case then?
>
> In this scenario we are following the fork child and detaching from
> the parent, which is sending us through the target's detach code.
>
> In the native Linux case the detach code is linux_nat_detach, which in
> turn calls inf_ptrace_detach. There is no call to target_mourn_inferior
> in this sequence of function calls, so the breakpoints aren't affected
> at that point. The breakpoints are "fixed up" for the child by
> follow_inferior_reset_breakpoints at the very end of follow_fork, at
> which point the child inferior has been set up.
>
> The is_fork_parent condition in remote_detach_1 preventing the call
> to target_mourn_inferior essentially makes the procedure the same as
> for the native case.
But how come the native side doesn't ever need to call mourn from
detach then? Or put another way, why do we call generic_mourn_inferior
in remote.c's detach for?
I think something's still not very right in grand scheme of things.
> +
> +/* This detaches a program to which we previously attached, using
> + inferior_ptid to identify the process. After this is done, GDB
> + can be used to debug some other program. We better not have left
> + any breakpoints in the target program or it'll die when it hits
> + one. */
>
> static void
> -remote_detach_1 (const char *args, int from_tty, int extended)
> +remote_detach_1 (const char *args, int from_tty)
> {
> int pid = ptid_get_pid (inferior_ptid);
> struct remote_state *rs = get_remote_state ();
> + struct thread_info *tp = find_thread_ptid (inferior_ptid);
> + int is_fork_parent;
...
>
> - target_mourn_inferior ();
> + /* Check to see if we are detaching a fork parent. Note that if we
> + are detaching a fork child, tp == NULL. */
> + if (tp != NULL)
> + is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
Seems like is_fork_parent is left uninitialized in the fork child case.
Should probably be:
is_fork_parent = (tp != NULL
&& tp->pending_follow.kind == TARGET_WAITKIND_FORKED);
I'm still confused about this though.
How can we distinguish whether we're detaching the parent for
detach-on-fork, or whether detach-on-fork is off, vs the user
doing an explicit "detach" before following the fork?
I notice that the native side forgets to detach the child in
the latter case...
...
Catchpoint 2 (forked process 32471), 0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130
130 pid = ARCH_FORK ();
(gdb) detach
Detaching from program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork, process 32467
(gdb) shell cat /proc/32471/status
Name: foll-fork
State: t (tracing stop)
Tgid: 32471
> +
> + /* If doing detach-on-fork, we don't mourn, because that will delete
> + breakpoints that should be available for the followed inferior. */
> + if (!is_fork_parent)
> + target_mourn_inferior ();
> + else
> + {
> + inferior_ptid = null_ptid;
> + detach_inferior (pid);
> + }
> }
So I think the detach scenario is still in need of further work,
but given that the native code doesn't get it right either, we can
address that as follow ups.
Thus, this version is OK with the issues mentioned
above fixed.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 2/7] Clone remote breakpoints
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
` (2 preceding siblings ...)
2015-04-17 20:59 ` [PATCH v8 3/7] Extended-remote Linux follow fork Don Breazeal
@ 2015-04-17 20:59 ` Don Breazeal
2015-04-17 21:00 ` [PATCH v8 7/7] Extended-remote follow fork documentation Don Breazeal
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 20:59 UTC (permalink / raw)
To: gdb-patches, palves
This version of this patch is unchanged except possibly for merges from
the mainline. This patch was approved pending a few 'nits', fixes are
documented here: https://sourceware.org/ml/gdb-patches/2014-10/msg00883.html.
Thanks!
--Don
This patch implements gdbserver routines to clone the breakpoint lists of a
process, duplicating them for another process. In gdbserver, each process
maintains its own independent breakpoint list. When a fork call creates a
child, all of the breakpoints currently inserted in the parent process are
also inserted in the child process, but there is nothing to describe them
in the data structures related to the child. The child must have a
breakpoint list describing them so that they can be removed (if detaching)
or recognized (if following). Implementation is a mechanical process of
just cloning the lists in several new functions in gdbserver/mem-break.c.
Tested by building, since none of the new functions are called yet. This
was tested with another patch in the series that implements follow-fork.
gdb/gdbserver/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* mem-break.c (APPEND_TO_LIST): Define macro.
(clone_agent_expr): New function.
(clone_one_breakpoint): New function.
(clone_all_breakpoints): New function.
* mem-break.h: Declare new functions.
---
gdb/gdbserver/mem-break.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/mem-break.h | 6 +++
2 files changed, 111 insertions(+)
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 70fab2e..d1b66bf 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -28,6 +28,24 @@ int breakpoint_len;
#define MAX_BREAKPOINT_LEN 8
+/* Helper macro used in loops that append multiple items to a singly-linked
+ list instead of inserting items at the head of the list, as, say, in the
+ breakpoint lists. LISTPP is a pointer to the pointer that is the head of
+ the new list. ITEMP is a pointer to the item to be added to the list.
+ TAILP must be defined to be the same type as ITEMP, and initialized to
+ NULL. */
+
+#define APPEND_TO_LIST(listpp, itemp, tailp) \
+ do \
+ { \
+ if ((tailp) == NULL) \
+ *(listpp) = (itemp); \
+ else \
+ (tailp)->next = (itemp); \
+ (tailp) = (itemp); \
+ } \
+ while (0)
+
/* GDB will never try to install multiple breakpoints at the same
address. However, we can see GDB requesting to insert a breakpoint
at an address is had already inserted one previously in a few
@@ -1913,3 +1931,90 @@ free_all_breakpoints (struct process_info *proc)
while (proc->breakpoints)
delete_breakpoint_1 (proc, proc->breakpoints);
}
+
+/* Clone an agent expression. */
+
+static struct agent_expr *
+clone_agent_expr (const struct agent_expr *src_ax)
+{
+ struct agent_expr *ax;
+
+ ax = xcalloc (1, sizeof (*ax));
+ ax->length = src_ax->length;
+ ax->bytes = xcalloc (ax->length, 1);
+ memcpy (ax->bytes, src_ax->bytes, ax->length);
+ return ax;
+}
+
+/* Deep-copy the contents of one breakpoint to another. */
+
+static struct breakpoint *
+clone_one_breakpoint (const struct breakpoint *src)
+{
+ struct breakpoint *dest;
+ struct raw_breakpoint *dest_raw;
+ struct point_cond_list *current_cond;
+ struct point_cond_list *new_cond;
+ struct point_cond_list *cond_tail = NULL;
+ struct point_command_list *current_cmd;
+ struct point_command_list *new_cmd;
+ struct point_command_list *cmd_tail = NULL;
+
+ /* Clone the raw breakpoint. */
+ dest_raw = xcalloc (1, sizeof (*dest_raw));
+ dest_raw->raw_type = src->raw->raw_type;
+ dest_raw->refcount = src->raw->refcount;
+ dest_raw->pc = src->raw->pc;
+ dest_raw->size = src->raw->size;
+ memcpy (dest_raw->old_data, src->raw->old_data, MAX_BREAKPOINT_LEN);
+ dest_raw->inserted = src->raw->inserted;
+
+ /* Clone the high-level breakpoint. */
+ dest = xcalloc (1, sizeof (*dest));
+ dest->type = src->type;
+ dest->raw = dest_raw;
+ dest->handler = src->handler;
+
+ /* Clone the condition list. */
+ for (current_cond = src->cond_list; current_cond != NULL;
+ current_cond = current_cond->next)
+ {
+ new_cond = xcalloc (1, sizeof (*new_cond));
+ new_cond->cond = clone_agent_expr (current_cond->cond);
+ APPEND_TO_LIST (&dest->cond_list, new_cond, cond_tail);
+ }
+
+ /* Clone the command list. */
+ for (current_cmd = src->command_list; current_cmd != NULL;
+ current_cmd = current_cmd->next)
+ {
+ new_cmd = xcalloc (1, sizeof (*new_cmd));
+ new_cmd->cmd = clone_agent_expr (current_cmd->cmd);
+ new_cmd->persistence = current_cmd->persistence;
+ APPEND_TO_LIST (&dest->command_list, new_cmd, cmd_tail);
+ }
+
+ return dest;
+}
+
+/* Create a new breakpoint list NEW_LIST that is a copy of the
+ list starting at SRC_LIST. Create the corresponding new
+ raw_breakpoint list NEW_RAW_LIST as well. */
+
+void
+clone_all_breakpoints (struct breakpoint **new_list,
+ struct raw_breakpoint **new_raw_list,
+ const struct breakpoint *src_list)
+{
+ const struct breakpoint *bp;
+ struct breakpoint *new_bkpt;
+ struct breakpoint *bkpt_tail = NULL;
+ struct raw_breakpoint *raw_bkpt_tail = NULL;
+
+ for (bp = src_list; bp != NULL; bp = bp->next)
+ {
+ new_bkpt = clone_one_breakpoint (bp);
+ APPEND_TO_LIST (new_list, new_bkpt, bkpt_tail);
+ APPEND_TO_LIST (new_raw_list, new_bkpt->raw, raw_bkpt_tail);
+ }
+}
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index 8b010c1..b5a3208 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -253,4 +253,10 @@ int insert_memory_breakpoint (struct raw_breakpoint *bp);
int remove_memory_breakpoint (struct raw_breakpoint *bp);
+/* Create a new breakpoint list NEW_BKPT_LIST that is a copy of SRC. */
+
+void clone_all_breakpoints (struct breakpoint **new_bkpt_list,
+ struct raw_breakpoint **new_raw_bkpt_list,
+ const struct breakpoint *src);
+
#endif /* MEM_BREAK_H */
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v8 7/7] Extended-remote follow fork documentation
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
` (3 preceding siblings ...)
2015-04-17 20:59 ` [PATCH v8 2/7] Clone remote breakpoints Don Breazeal
@ 2015-04-17 21:00 ` Don Breazeal
2015-04-23 14:20 ` Pedro Alves
2015-04-17 21:00 ` [PATCH v8 6/7] Remote fork catch Don Breazeal
2015-04-17 21:00 ` [PATCH v8 5/7] Remote follow vfork Don Breazeal
6 siblings, 1 reply; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 21:00 UTC (permalink / raw)
To: gdb-patches, palves
This patch contains the accumulated documentation changes for the
rest of the extended-remote follow fork patchset.
On 4/15/2015 8:46 AM, Pedro Alves wrote:
> On 04/10/2015 09:43 PM, Don Breazeal wrote:
>
>> @tab @code{hbreak}
>>
>> +@item @code{fork-event-feature}
>> +@tab @code{fork stop reason}
>> +@tab @code{fork}
>> +
>> +@item @code{vfork-event-feature}
>> +@tab @code{vfork stop reason}
>> +@tab @code{vfork}
>> +
>> @end multitable
>>
>> @node Remote Stub
>> @@ -35286,6 +35297,45 @@ The @var{r} part must be left empty.
>>
>> The same remarks about @samp{qSupported} and non-stop mode above
>> apply.
>> +
>> +@cindex fork events, remote reply
>> +@item fork
>> +The packet indicates that @code{fork} was called, and @var{r}
>> +is the ptid of the new child process. This packet is only
>
> I don't think we document what a ptid is --- that's an internal
> gdb implementation detail. This should probably say that it's the
> thread id of the new child process, and cross reference
> to "thread-id syntax".
I've made this change.
>
>> +applicable to targets that support fork events.
>
> Thanks,
> Pedro Alves
>
Thanks
--Don
gdb/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* NEWS: Announce fork support in the RSP and support
for fork debugging in extended mode.
gdb/doc/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* gdb.texinfo (Forks): Note that fork debugging is
supported in extended mode.
(Remote Configuration): Add fork event features to table
of packet settings.
(Stop Reply Packets): Add fork events to list of stop reasons.
(General Query Packets): Add fork events to tables of
'gdbfeatures' and 'stub features' supported in the qSupported
packet, as well as to the list containing stub feature
details.
---
gdb/NEWS | 24 ++++++++++++++++
gdb/doc/gdb.texinfo | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/gdb/NEWS b/gdb/NEWS
index c24195e..93da9e9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -101,6 +101,30 @@ hwbreak stop reason
vFile:fstat:
Return information about files on the remote system.
+fork stop reason
+ Indicates that a fork system call was executed.
+
+vfork stop reason
+ Indicates that a vfork system call was executed.
+
+vforkdone stop reason
+ Indicates that a vfork child of the specified process has executed
+ an exec or exit, allowing the vfork parent to resume execution.
+
+fork-events and vfork-events features in qSupported
+ The qSupported packet allows GDB to request support for fork and
+ vfork events using new 'gdbfeatures' fork-events and vfork-events,
+ and the qSupported response can contain the corresponding
+ 'stubfeatures'. Set and show commands can be used to display
+ whether these features are enabled.
+
+* Extended-remote fork events
+
+ ** GDB now has support for fork events on extended-remote Linux
+ targets. For targets with Linux kernels 2.5.60 and later, this
+ enables follow-fork-mode and detach-on-fork for both fork and
+ vfork, as well as fork and vfork catchpoints.
+
* The info record command now shows the recording format and the
branch tracing configuration for the current thread when using
the btrace record target.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6255c14..688f594 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3135,6 +3135,9 @@ create additional processes using the @code{fork} or @code{vfork} functions.
Currently, the only platforms with this feature are HP-UX (11.x and later
only?) and @sc{gnu}/Linux (kernel version 2.5.60 and later).
+The fork debugging commands are supported in both native mode and when
+connected to @code{gdbserver} using @kbd{target extended-remote}.
+
By default, when a program forks, @value{GDBN} will continue to debug
the parent process and the child process will run unimpeded.
@@ -19919,6 +19922,14 @@ are:
@tab @code{hwbreak stop reason}
@tab @code{hbreak}
+@item @code{fork-event-feature}
+@tab @code{fork stop reason}
+@tab @code{fork}
+
+@item @code{vfork-event-feature}
+@tab @code{vfork stop reason}
+@tab @code{vfork}
+
@end multitable
@node Remote Stub
@@ -35357,6 +35368,49 @@ The @var{r} part must be left empty.
The same remarks about @samp{qSupported} and non-stop mode above
apply.
+
+@cindex fork events, remote reply
+@item fork
+The packet indicates that @code{fork} was called, and @var{r}
+is the thread ID of the new child process. Refer to
+@ref{thread-id syntax} for the format of the @var{thread-id}
+field. This packet is only applicable to targets that support
+fork events.
+
+This packet should not be sent by default; older @value{GDBN} versions
+did not support it. @value{GDBN} requests it, by supplying an
+appropriate @samp{qSupported} feature (@pxref{qSupported}). The
+remote stub must also supply the appropriate @samp{qSupported} feature
+indicating support.
+
+@cindex vfork events, remote reply
+@item vfork
+The packet indicates that @code{vfork} was called, and @var{r}
+is the thread ID of the new child process. Refer to
+@ref{thread-id syntax} for the format of the @var{thread-id}
+field. This packet is only applicable to targets that support
+vfork events.
+
+This packet should not be sent by default; older @value{GDBN} versions
+did not support it. @value{GDBN} requests it, by supplying an
+appropriate @samp{qSupported} feature (@pxref{qSupported}). The
+remote stub must also supply the appropriate @samp{qSupported} feature
+indicating support.
+
+@cindex vforkdone events, remote reply
+@item vforkdone
+The packet indicates that a child process created by a vfork
+has either called @code{exec} or terminated, so that the
+address spaces of the parent and child process are no longer
+shared. The @var{r} part is ignored. This packet is only
+applicable to targets that support vforkdone events.
+
+This packet should not be sent by default; older @value{GDBN} versions
+did not support it. @value{GDBN} requests it, by supplying an
+appropriate @samp{qSupported} feature (@pxref{qSupported}). The
+remote stub must also supply the appropriate @samp{qSupported} feature
+indicating support.
+
@end table
@item W @var{AA}
@@ -35949,6 +36003,18 @@ reason in stop replies. @xref{swbreak stop reason}, for details.
@item hwbreak
This feature indicates whether @value{GDBN} supports the hwbreak stop
reason in stop replies. @xref{swbreak stop reason}, for details.
+
+@item fork-events
+This feature indicates whether @value{GDBN} supports fork event
+extensions to the remote protocol. @value{GDBN} does not use such
+extensions unless the stub also reports that it supports them by
+including @samp{fork-events+} in its @samp{qSupported} reply.
+
+@item vfork-events
+This feature indicates whether @value{GDBN} supports vfork event
+extensions to the remote protocol. @value{GDBN} does not use such
+extensions unless the stub also reports that it supports them by
+including @samp{vfork-events+} in its @samp{qSupported} reply.
@end table
Stubs should ignore any unknown values for
@@ -36187,6 +36253,16 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No
+@item @samp{fork-events}
+@tab No
+@tab @samp{-}
+@tab No
+
+@item @samp{vfork-events}
+@tab No
+@tab @samp{-}
+@tab No
+
@end multitable
These are the currently defined stub features, in more detail:
@@ -36375,6 +36451,13 @@ breakpoints.
The remote stub reports the @samp{hwbreak} stop reason for hardware
breakpoints.
+@item fork-events
+The remote stub reports the @samp{fork} stop reason for fork events.
+
+@item vfork-events
+The remote stub reports the @samp{vfork} stop reason for vfork events
+and vforkdone events.
+
@end table
@item qSymbol::
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v8 6/7] Remote fork catch
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
` (4 preceding siblings ...)
2015-04-17 21:00 ` [PATCH v8 7/7] Extended-remote follow fork documentation Don Breazeal
@ 2015-04-17 21:00 ` Don Breazeal
2015-04-23 14:19 ` Pedro Alves
2015-04-17 21:00 ` [PATCH v8 5/7] Remote follow vfork Don Breazeal
6 siblings, 1 reply; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 21:00 UTC (permalink / raw)
To: gdb-patches, palves
Hi Pedro,
This version of the patch incorporates changes based on your comments on
the previous version, as outlined below.
On 4/15/2015 8:39 AM, Pedro Alves wrote:
> On 04/10/2015 06:09 PM, Don Breazeal wrote:
---snip snip---
>> +/* Determine if THREAD is a pending fork parent thread. ARG contains
>> + the pid of the process who's threads we want to check, or -1 if
>> + we want to check all threads. */
>> +
>> +static int
>> +pending_fork_parent_callback (struct thread_info *thread, void *arg)
>> +{
>> + int pid = *(int *) arg;
>> +
>> + if (thread->pending_follow.kind == TARGET_WAITKIND_FORKED
>> + || thread->pending_follow.kind == TARGET_WAITKIND_VFORKED)
>> + {
>> + if ((pid == -1) || (pid == ptid_get_pid (thread->ptid)))
>
> Unnecessary parens:
>
> if (pid == -1 || pid == ptid_get_pid (thread->ptid))
Fixed.
>
>
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/* If CONTEXT contains any fork child threads that have not been
>> + reported yet, remove them from the CONTEXT list. If such a
>> + thread exists it is because we are stopped at a fork catchpoint
>> + and have not yet called follow_fork, which will set up the
>> + host-side data structures for the new process. */
>> +
>> +static void
>> +remove_new_fork_child (struct threads_listing_context *context)
>> +{
>> + struct thread_info * thread;
>> + int pid = -1;
>> +
>> + /* Check to see if there is an in-progress fork parent. */
>> + thread = iterate_over_threads (pending_fork_parent_callback, &pid);
>> + if (thread != NULL)
>
> In non-stop mode, if you're debugging multiple process, multiple
> processes can fork at the same, and then we end up with multiple
> threads with an in-progress fork parent. So this needs to walk
> the whole thread list, not just stop at the first. Either
> use ALL_NON_EXITED_THREADS, or move the loop below to
> pending_fork_parent_callback (or to a helper function called
> by that).
Good point, thanks. I have reworked this to use ALL_NON_EXITED_THREADS.
>
>> + {
>> + ptid_t child_ptid = thread->pending_follow.value.related_pid;
>> + struct thread_item *item;
>> + int i;
>> +
>> + for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
>> + {
>> + if (ptid_equal (item->ptid, child_ptid))
>> + {
>> + VEC_ordered_remove (thread_item_t, context->items, i);
>> + break;
>> + }
>> + }
>> + }
>> +}
>> +
>> /* Implement the to_update_thread_list function for the remote
>> targets. */
>>
>> @@ -2874,6 +2964,10 @@ remote_update_thread_list (struct target_ops *ops)
>> }
>> }
>>
>> + /* Remove any unreported fork child from CONTEXT so that
>> + we don't interfere with follow fork. */
>> + remove_new_fork_child (&context);
>
> I think there's a race here, in non-stop mode. Consider:
>
> #1 - process forks just before gdb starts fetching the remote thread
> list.
> #2 - gdbserver adds the fork child its thread list.
> #3 - gdbserver queues the fork event, sends vStopped notification
> #4 - gdb/remote_update_thread_list pulls the thread list
> #5 - we're now in remove_new_fork_child, but we don't know
> about the fork event yet. It's still pending in the vStopped
> queue.
>
> So I think that we need to make remote_update_thread_list do,
> in this order:
>
> #1 - fetch the remote thread list
> #2 - fetch the pending vStopped notifications
> (remote_notif_get_pending_events)
> #3 - call remove_new_fork_child
> #4 - add threads we don't know about yet to our list.
>
> and make remove_new_fork_child also peek at the
> pending vStopped events queue (and in the future at
> any other layers of pending events in the core side.)
I think all that was necessary here was to insert a call to
remote_notif_get_pending_events before the call to remove_new_fork_child,
since remote_notif_get_pending_events ultimately calls
remote_parse_stop_reply on the responses from the target, handling the
pending events like normal events. Or am I missing something? I
inferred from your comment above that there might be more to it.
>
>> + child_pid = ptid_get_pid (thread->pending_follow.value.related_pid);
>> + res = remote_vkill (child_pid, rs);
>> + if (res != 0)
>> + error (_("Can't kill fork child process"));
>
> It'll probably be good to include the PID in the error message.
Done.
>
>> + }
>
> Thanks,
> Pedro Alves
>
Thanks
--Don
This patch implements catchpoints for fork events on extended-remote
Linux targets.
Implementation appeared to be straightforward, requiring four new functions
in remote.c to implement insert/remove of fork/vfork catchpoints. These
functions are essentially stubs that just return 0 ('success') if the
required features are enabled. If the fork events are being reported, then
catchpoints are set and hit.
However, there are some extra issues that arise with catchpoints.
1) Thread creation reporting -- fork catchpoints are hit before the
follow_fork has been completed. When stopped at a fork catchpoint
in the native implementation, the new process is not 'reported'
until after the follow is done. It doesn't show up in the inferiors
list or the threads list. However, in the gdbserver case, an
'info threads' while stopped at a fork catchpoint will retrieve the
new thread info from the target and add it to GDB's data structures,
prior to the follow operations. Because of this premature report,
things on the GDB side eventually get very confused.
So in remote.c:remote_update_thread_list, we check to see if there
are any pending fork parent threads. If there are we remove the
related fork child thread from the thread list sent by the target.
2) Kill process before fork is followed -- on the native side in
linux-nat.c:linux_nat_kill, there is some code to handle the case where
a fork has occurred but follow_fork hasn't been called yet. It does
this by using the last status to determine if a follow is pending, and
if it is, to kill the child task. The use of last_status is fragile
in situations like non-stop mode where other events may have occurred
after the fork event. This patch identifies a fork parent
in remote.c:extended_remote_kill in a way similar to that used in
thread creation reporting above. If one is found, it kills the new
child as well.
Tested on x64 Ubuntu Lucid, native, remote, extended-remote. Tested the
case of killing the forking process before the fork has been followed
manually.
Thanks
--Don
gdb/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* remote.c (remote_insert_fork_catchpoint): New function.
(remote_remove_fork_catchpoint): New function.
(remote_insert_vfork_catchpoint): New function.
(remote_remove_vfork_catchpoint): New function.
(pending_fork_parent_callback): New function.
(remove_new_fork_child): New function.
(remote_update_thread_list): Call remote_notif_get_pending_events
and remove_new_fork_child.
(extended_remote_kill): Kill fork child when killing the
parent before follow_fork completes.
(init_extended_remote_ops): Initialize target vector with
new fork catchpoint functions.
---
gdb/remote.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)
diff --git a/gdb/remote.c b/gdb/remote.c
index c0fa3b9..ec88afe 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1477,6 +1477,46 @@ remote_vfork_event_p (struct remote_state *rs)
return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
}
+/* Insert fork catchpoint target routine. If fork events are enabled
+ then return success, nothing more to do. */
+
+static int
+remote_insert_fork_catchpoint (struct target_ops *ops, int pid)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ return !remote_fork_event_p (rs);
+}
+
+/* Remove fork catchpoint target routine. Nothing to do, just
+ return success. */
+
+static int
+remote_remove_fork_catchpoint (struct target_ops *ops, int pid)
+{
+ return 0;
+}
+
+/* Insert vfork catchpoint target routine. If vfork events are enabled
+ then return success, nothing more to do. */
+
+static int
+remote_insert_vfork_catchpoint (struct target_ops *ops, int pid)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ return !remote_vfork_event_p (rs);
+}
+
+/* Remove vfork catchpoint target routine. Nothing to do, just
+ return success. */
+
+static int
+remote_remove_vfork_catchpoint (struct target_ops *ops, int pid)
+{
+ return 0;
+}
+
/* Tokens for use by the asynchronous signal handlers for SIGINT. */
static struct async_signal_handler *async_sigint_remote_twice_token;
static struct async_signal_handler *async_sigint_remote_token;
@@ -2815,6 +2855,59 @@ remote_get_threads_with_qthreadinfo (struct target_ops *ops,
return 0;
}
+/* Determine if THREAD is a pending fork parent thread. ARG contains
+ the pid of the process who's threads we want to check, or -1 if
+ we want to check all threads. */
+
+static struct thread_info *
+is_pending_fork_parent (struct thread_info *thread, void *arg)
+{
+ int pid = *(int *) arg;
+
+ if (thread->pending_follow.kind == TARGET_WAITKIND_FORKED
+ || thread->pending_follow.kind == TARGET_WAITKIND_VFORKED)
+ {
+ if (pid == -1 || pid == ptid_get_pid (thread->ptid))
+ return thread;
+ }
+
+ return NULL;
+}
+
+/* If CONTEXT contains any fork child threads that have not been
+ reported yet, remove them from the CONTEXT list. If such a
+ thread exists it is because we are stopped at a fork catchpoint
+ and have not yet called follow_fork, which will set up the
+ host-side data structures for the new process. */
+
+static void
+remove_new_fork_child (struct threads_listing_context *context)
+{
+ struct thread_info * thread;
+ int pid = -1;
+
+ /* For any in-progress fork parent threads, delete the corresponding
+ fork child threads from the CONTEXT list. */
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ if (is_pending_fork_parent (thread, &pid) != NULL)
+ {
+ ptid_t child_ptid = thread->pending_follow.value.related_pid;
+ struct thread_item *item;
+ int i;
+
+ for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
+ {
+ if (ptid_equal (item->ptid, child_ptid))
+ {
+ VEC_ordered_remove (thread_item_t, context->items, i);
+ break;
+ }
+ }
+ }
+ }
+}
+
/* Implement the to_update_thread_list function for the remote
targets. */
@@ -2839,6 +2932,7 @@ remote_update_thread_list (struct target_ops *ops)
int i;
struct thread_item *item;
struct thread_info *tp, *tmp;
+ struct notif_client *notif = ¬if_client_stop;
got_list = 1;
@@ -2874,6 +2968,14 @@ remote_update_thread_list (struct target_ops *ops)
}
}
+ /* Remove any unreported fork child threads from CONTEXT so
+ that we don't interfere with follow fork, which is where
+ creation of such threads is handled. We first call
+ remote_notif_get_pending_events to make sure that we
+ know about any pending (unreported) fork events. */
+ remote_notif_get_pending_events (notif);
+ remove_new_fork_child (&context);
+
/* And now add threads we don't know about yet to our list. */
for (i = 0;
VEC_iterate (thread_item_t, context.items, i, item);
@@ -8002,6 +8104,27 @@ extended_remote_kill (struct target_ops *ops)
int res;
int pid = ptid_get_pid (inferior_ptid);
struct remote_state *rs = get_remote_state ();
+ struct thread_info *thread;
+ struct notif_client *notif = ¬if_client_stop;
+
+ /* If we're stopped while forking and we haven't followed yet, kill the
+ child task. We need to do this before killing the parent task
+ because if this is a vfork then the parent will be sleeping. First
+ we need to call remote_notif_get_pending_events to make sure that we
+ know about any pending (unreported) fork events. */
+ remote_notif_get_pending_events (notif);
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ if (is_pending_fork_parent(thread, &pid))
+ {
+ int child_pid;
+
+ child_pid = ptid_get_pid (thread->pending_follow.value.related_pid);
+ res = remote_vkill (child_pid, rs);
+ if (res != 0)
+ error (_("Can't kill fork child process %d"), child_pid);
+ }
+ }
res = remote_vkill (pid, rs);
if (res == -1 && !(rs->extended && remote_multi_process_p (rs)))
@@ -11917,6 +12040,14 @@ Specify the serial device it is connected to (e.g. /dev/ttya).";
extended_remote_ops.to_supports_disable_randomization
= extended_remote_supports_disable_randomization;
extended_remote_ops.to_follow_fork = remote_follow_fork;
+ extended_remote_ops.to_insert_fork_catchpoint
+ = remote_insert_fork_catchpoint;
+ extended_remote_ops.to_remove_fork_catchpoint
+ = remote_remove_fork_catchpoint;
+ extended_remote_ops.to_insert_vfork_catchpoint
+ = remote_insert_vfork_catchpoint;
+ extended_remote_ops.to_remove_vfork_catchpoint
+ = remote_remove_vfork_catchpoint;
}
static int
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 6/7] Remote fork catch
2015-04-17 21:00 ` [PATCH v8 6/7] Remote fork catch Don Breazeal
@ 2015-04-23 14:19 ` Pedro Alves
2015-05-06 16:10 ` Don Breazeal
0 siblings, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2015-04-23 14:19 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 04/17/2015 09:58 PM, Don Breazeal wrote:
>> > I think there's a race here, in non-stop mode. Consider:
>> >
>> > #1 - process forks just before gdb starts fetching the remote thread
>> > list.
>> > #2 - gdbserver adds the fork child its thread list.
>> > #3 - gdbserver queues the fork event, sends vStopped notification
>> > #4 - gdb/remote_update_thread_list pulls the thread list
>> > #5 - we're now in remove_new_fork_child, but we don't know
>> > about the fork event yet. It's still pending in the vStopped
>> > queue.
>> >
>> > So I think that we need to make remote_update_thread_list do,
>> > in this order:
>> >
>> > #1 - fetch the remote thread list
>> > #2 - fetch the pending vStopped notifications
>> > (remote_notif_get_pending_events)
>> > #3 - call remove_new_fork_child
>> > #4 - add threads we don't know about yet to our list.
>> >
>> > and make remove_new_fork_child also peek at the
>> > pending vStopped events queue (and in the future at
>> > any other layers of pending events in the core side.)
> I think all that was necessary here was to insert a call to
> remote_notif_get_pending_events before the call to remove_new_fork_child,
> since remote_notif_get_pending_events ultimately calls
> remote_parse_stop_reply on the responses from the target, handling the
> pending events like normal events. Or am I missing something? I
> inferred from your comment above that there might be more to it.
It fetches the vStopped notifications out of the remote,
and parses them, but the events are left in the stop reply
queue, they're not handled. They're only handled once the
core next calls target_wait -> remote_wait_ns -> queued_stop_reply.
So we need to peek into the stop reply queue, and check if we have
pending fork events. We have peek_stop_reply for a similar use,
but it's not the same.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 6/7] Remote fork catch
2015-04-23 14:19 ` Pedro Alves
@ 2015-05-06 16:10 ` Don Breazeal
2015-05-08 10:14 ` Pedro Alves
2015-05-08 10:16 ` Pedro Alves
0 siblings, 2 replies; 18+ messages in thread
From: Don Breazeal @ 2015-05-06 16:10 UTC (permalink / raw)
To: gdb-patches, palves
On 4/23/2015 7:19 AM, Pedro Alves wrote:
> On 04/17/2015 09:58 PM, Don Breazeal wrote:
>
>>>> I think there's a race here, in non-stop mode. Consider:
>>>>
>>>> #1 - process forks just before gdb starts fetching the remote thread
>>>> list.
>>>> #2 - gdbserver adds the fork child its thread list.
>>>> #3 - gdbserver queues the fork event, sends vStopped notification
>>>> #4 - gdb/remote_update_thread_list pulls the thread list
>>>> #5 - we're now in remove_new_fork_child, but we don't know
>>>> about the fork event yet. It's still pending in the vStopped
>>>> queue.
>>>>
>>>> So I think that we need to make remote_update_thread_list do,
>>>> in this order:
>>>>
>>>> #1 - fetch the remote thread list
>>>> #2 - fetch the pending vStopped notifications
>>>> (remote_notif_get_pending_events)
>>>> #3 - call remove_new_fork_child
>>>> #4 - add threads we don't know about yet to our list.
>>>>
>>>> and make remove_new_fork_child also peek at the
>>>> pending vStopped events queue (and in the future at
>>>> any other layers of pending events in the core side.)
>> I think all that was necessary here was to insert a call to
>> remote_notif_get_pending_events before the call to remove_new_fork_child,
>> since remote_notif_get_pending_events ultimately calls
>> remote_parse_stop_reply on the responses from the target, handling the
>> pending events like normal events. Or am I missing something? I
>> inferred from your comment above that there might be more to it.
>
> It fetches the vStopped notifications out of the remote,
> and parses them, but the events are left in the stop reply
> queue, they're not handled. They're only handled once the
> core next calls target_wait -> remote_wait_ns -> queued_stop_reply.
>
> So we need to peek into the stop reply queue, and check if we have
> pending fork events. We have peek_stop_reply for a similar use,
> but it's not the same.
>
Hi Pedro,
I've reimplemented the mechanisms that (a) ignore new threads
that are as-yet-unreported fork children, and (b) kill as-yet-
unreported fork children when killing the fork parent. These both
now check the stop reply queue for unreported fork events. I changed
the names of the new functions to use 'ignore' instead of 'remove'
to more accurately reflect what they are doing, and moved
remote_update_thread_list to avoid having to add prototypes and
forward declarations to satisfy all the dependences.
I tested killing the unreported fork child manually on x86_64 Ubuntu.
I just relied on usual regression testing for ignoring the new
threads (knowing that things go bad if they are not ignored).
I have some concerns about my test results, though. Over the past
few weeks I've been seeing more intermittent failures than I expect,
on both the mainline and my branch. Sometimes I get clean test runs,
other times not. Is this something others have been seeing, or is it
just me?
I know that some of these tests are problematic (random-signal.exp),
but others I haven't been aware of. The failures I've seen include
(but aren't limited to):
gdb.base/random-signal.exp
gdb.mi/mi-nsmoribund.exp
gdb.threads/attach-many-short-lived-threads.exp
gdb.threads/interrupted-hand-call.exp
gdb.threads/thread-unwindonsignal.exp
gdb.trace/tspeed.exp
I've also seen failures in 'random' tests due to timeouts on the
qSupported packet, like this:
target remote localhost:3238^M
Remote debugging using localhost:3238^M
Ignoring packet error, continuing...^M
warning: unrecognized item "timeout" in "qSupported" response^M
I was particularly concerned about this since I had made changes to
qSupported, but I found that it often times out for me on the
mainline as well. I tried bumping the timeout up to 5 seconds for
the qSupported packet and still saw the error. I haven't identified
a root cause.
Most of these issues I've been able to reproduce on the mainline by
just running the test over and over in a loop until it fails.
So, all that said, I believe that my changes aren't introducing any
obvious regressions, but I'd feel a lot better about it if I could
get clean test runs more reliably.
Thanks
--Don
This patch implements catchpoints for fork events on extended-remote
Linux targets.
Implementation appeared to be straightforward, requiring four new functions
in remote.c to implement insert/remove of fork/vfork catchpoints. These
functions are essentially stubs that just return 0 ('success') if the
required features are enabled. If the fork events are being reported, then
catchpoints are set and hit.
However, there are some extra issues that arise with catchpoints.
1) Thread creation reporting -- fork catchpoints are hit before the
follow_fork has been completed. When stopped at a fork catchpoint
in the native implementation, the new process is not 'reported'
until after the follow is done. It doesn't show up in the inferiors
list or the threads list. However, in the gdbserver case, an
'info threads' while stopped at a fork catchpoint will retrieve the
new thread info from the target and add it to GDB's data structures,
prior to the follow operations. Because of this premature report,
things on the GDB side eventually get very confused.
So in remote.c:remote_update_thread_list, we call a new function,
ignore_new_fork_children, to see if there are any pending fork
parent thread(s). If there are we remove the related fork child
thread(s) from the thread list sent by the target.
2) Kill process before fork is followed -- on the native side in
linux-nat.c:linux_nat_kill, there is some code to handle the case where
a fork has occurred but follow_fork hasn't been called yet. It does
this by using the last status to determine if a follow is pending, and
if it is, to kill the child task. The use of last_status is fragile
in situations like non-stop mode where other events may have occurred
after the fork event. This patch identifies a fork parent
in remote.c:extended_remote_kill in a way similar to that used in
thread creation reporting above. Any new fork child threads that are
found are killed as well as the fork parent.
Tested on x64 Ubuntu Lucid, native, remote, extended-remote. Tested the
case of killing the forking process before the fork has been followed
manually.
Thanks
--Don
gdb/
2015-05-04 Don Breazeal <donb@codesourcery.com>
* gdb/remote.c (remote_insert_fork_catchpoint): New function.
(remote_remove_fork_catchpoint): New function.
(remote_insert_vfork_catchpoint): New function.
(remote_remove_vfork_catchpoint): New function.
(remote_update_thread_list): Call ignore_new_fork_children.
Move below ignore_new_fork_children in file.
(is_pending_fork_parent): New function.
(ignore_new_fork_child): New function.
(ignore_child_of_pending_fork): New function.
(ignore_new_fork_children): New function.
(kill_child_of_pending_fork): New function.
(kill_new_fork_children): New function.
(extended_remote_kill): Call kill_new_fork_children.
(init_extended_remote_ops): Initialize target vector with
new fork catchpoint functions.
---
gdb/remote.c | 413 ++++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 317 insertions(+), 96 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 8a9eb9b..bc55d65 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -86,6 +86,7 @@ static long target_buf_size;
enum { REMOTE_ALIGN_WRITES = 16 };
/* Prototypes for local functions. */
+static void remote_update_thread_list (struct target_ops *ops);
static void async_cleanup_sigint_signal_handler (void *dummy);
static int getpkt_sane (char **buf, long *sizeof_buf, int forever);
static int getpkt_or_notif_sane (char **buf, long *sizeof_buf,
@@ -104,6 +105,10 @@ static void remote_open_1 (const char *, int, struct target_ops *,
static void remote_close (struct target_ops *self);
+struct remote_state;
+
+static int remote_vkill (int pid, struct remote_state *rs);
+
static void remote_mourn (struct target_ops *ops);
static void extended_remote_restart (void);
@@ -181,7 +186,6 @@ static ptid_t read_ptid (char *buf, char **obuf);
static void remote_set_permissions (struct target_ops *self);
-struct remote_state;
static int remote_get_trace_status (struct target_ops *self,
struct trace_status *ts);
@@ -1478,6 +1482,46 @@ remote_vfork_event_p (struct remote_state *rs)
return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
}
+/* Insert fork catchpoint target routine. If fork events are enabled
+ then return success, nothing more to do. */
+
+static int
+remote_insert_fork_catchpoint (struct target_ops *ops, int pid)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ return !remote_fork_event_p (rs);
+}
+
+/* Remove fork catchpoint target routine. Nothing to do, just
+ return success. */
+
+static int
+remote_remove_fork_catchpoint (struct target_ops *ops, int pid)
+{
+ return 0;
+}
+
+/* Insert vfork catchpoint target routine. If vfork events are enabled
+ then return success, nothing more to do. */
+
+static int
+remote_insert_vfork_catchpoint (struct target_ops *ops, int pid)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ return !remote_vfork_event_p (rs);
+}
+
+/* Remove vfork catchpoint target routine. Nothing to do, just
+ return success. */
+
+static int
+remote_remove_vfork_catchpoint (struct target_ops *ops, int pid)
+{
+ return 0;
+}
+
/* Tokens for use by the asynchronous signal handlers for SIGINT. */
static struct async_signal_handler *async_sigint_remote_twice_token;
static struct async_signal_handler *async_sigint_remote_token;
@@ -2824,101 +2868,6 @@ remote_get_threads_with_qthreadinfo (struct target_ops *ops,
return 0;
}
-/* Implement the to_update_thread_list function for the remote
- targets. */
-
-static void
-remote_update_thread_list (struct target_ops *ops)
-{
- struct remote_state *rs = get_remote_state ();
- struct threads_listing_context context;
- struct cleanup *old_chain;
- int got_list = 0;
-
- context.items = NULL;
- old_chain = make_cleanup (clear_threads_listing_context, &context);
-
- /* We have a few different mechanisms to fetch the thread list. Try
- them all, starting with the most preferred one first, falling
- back to older methods. */
- if (remote_get_threads_with_qxfer (ops, &context)
- || remote_get_threads_with_qthreadinfo (ops, &context)
- || remote_get_threads_with_ql (ops, &context))
- {
- int i;
- struct thread_item *item;
- struct thread_info *tp, *tmp;
-
- got_list = 1;
-
- if (VEC_empty (thread_item_t, context.items)
- && remote_thread_always_alive (ops, inferior_ptid))
- {
- /* Some targets don't really support threads, but still
- reply an (empty) thread list in response to the thread
- listing packets, instead of replying "packet not
- supported". Exit early so we don't delete the main
- thread. */
- do_cleanups (old_chain);
- return;
- }
-
- /* CONTEXT now holds the current thread list on the remote
- target end. Delete GDB-side threads no longer found on the
- target. */
- ALL_THREADS_SAFE (tp, tmp)
- {
- for (i = 0;
- VEC_iterate (thread_item_t, context.items, i, item);
- ++i)
- {
- if (ptid_equal (item->ptid, tp->ptid))
- break;
- }
-
- if (i == VEC_length (thread_item_t, context.items))
- {
- /* Not found. */
- delete_thread (tp->ptid);
- }
- }
-
- /* And now add threads we don't know about yet to our list. */
- for (i = 0;
- VEC_iterate (thread_item_t, context.items, i, item);
- ++i)
- {
- if (!ptid_equal (item->ptid, null_ptid))
- {
- struct private_thread_info *info;
- /* In non-stop mode, we assume new found threads are
- running until proven otherwise with a stop reply. In
- all-stop, we can only get here if all threads are
- stopped. */
- int running = non_stop ? 1 : 0;
-
- remote_notice_new_inferior (item->ptid, running);
-
- info = demand_private_info (item->ptid);
- info->core = item->core;
- info->extra = item->extra;
- item->extra = NULL;
- }
- }
- }
-
- if (!got_list)
- {
- /* If no thread listing method is supported, then query whether
- each known thread is alive, one by one, with the T packet.
- If the target doesn't support threads at all, then this is a
- no-op. See remote_thread_alive. */
- prune_threads ();
- }
-
- do_cleanups (old_chain);
-}
-
/*
* Collect a descriptive string about the given thread.
* The target may say anything it wants to about the thread
@@ -5427,6 +5376,202 @@ struct queue_iter_param
struct stop_reply *output;
};
+/* Determine if THREAD is a pending fork parent thread. ARG contains
+ the pid of the process that owns the threads we want to check, or
+ -1 if we want to check all threads. */
+
+static int
+is_pending_fork_parent (struct target_waitstatus *ws, int event_pid,
+ ptid_t thread_ptid)
+{
+ if (ws->kind == TARGET_WAITKIND_FORKED
+ || ws->kind == TARGET_WAITKIND_VFORKED)
+ {
+ if (event_pid == -1 || event_pid == ptid_get_pid (thread_ptid))
+ return 1;
+ }
+
+ return 0;
+}
+
+/* Remove the thread specified as the related_pid field of WS
+ from the CONTEXT list. */
+
+static void
+ignore_new_fork_child (struct target_waitstatus *ws,
+ struct threads_listing_context *context)
+{
+ struct thread_item *item;
+ int i;
+ ptid_t child_ptid = ws->value.related_pid;
+
+ for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
+ {
+ if (ptid_equal (item->ptid, child_ptid))
+ {
+ VEC_ordered_remove (thread_item_t, context->items, i);
+ break;
+ }
+ }
+}
+
+/* Check whether EVENT is a fork event, and if it is, remove the
+ fork child from the context list passed in DATA. */
+
+static int
+ignore_child_of_pending_fork (QUEUE (stop_reply_p) *q,
+ QUEUE_ITER (stop_reply_p) *iter,
+ stop_reply_p event,
+ void *data)
+{
+ struct queue_iter_param *param = data;
+ struct threads_listing_context *context = param->input;
+
+ if (event->ws.kind == TARGET_WAITKIND_FORKED
+ || event->ws.kind == TARGET_WAITKIND_VFORKED)
+ {
+ ignore_new_fork_child (&event->ws, context);
+ }
+
+ return 1;
+}
+
+/* If CONTEXT contains any fork child threads that have not been
+ reported yet, remove them from the CONTEXT list. If such a
+ thread exists it is because we are stopped at a fork catchpoint
+ and have not yet called follow_fork, which will set up the
+ host-side data structures for the new process. */
+
+static void
+ignore_new_fork_children (struct threads_listing_context *context)
+{
+ struct thread_info * thread;
+ int pid = -1;
+ struct notif_client *notif = ¬if_client_stop;
+ struct queue_iter_param param;
+
+ /* For any threads stopped at a fork event, delete the corresponding
+ fork child threads from the CONTEXT list. */
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ struct target_waitstatus *ws = &thread->pending_follow;
+
+ if (is_pending_fork_parent (ws, pid, thread->ptid))
+ {
+ ignore_new_fork_child (ws, context);
+ }
+ }
+
+ /* Check for any pending fork events (not reported or processed yet)
+ in process PID and remove those fork child threads from the
+ CONTEXT list as well. */
+ remote_notif_get_pending_events (notif);
+ param.input = context;
+ param.output = NULL;
+ QUEUE_iterate (stop_reply_p, stop_reply_queue,
+ ignore_child_of_pending_fork, ¶m);
+}
+
+/* Implement the to_update_thread_list function for the remote
+ targets. */
+
+static void
+remote_update_thread_list (struct target_ops *ops)
+{
+ struct remote_state *rs = get_remote_state ();
+ struct threads_listing_context context;
+ struct cleanup *old_chain;
+ int got_list = 0;
+
+ context.items = NULL;
+ old_chain = make_cleanup (clear_threads_listing_context, &context);
+
+ /* We have a few different mechanisms to fetch the thread list. Try
+ them all, starting with the most preferred one first, falling
+ back to older methods. */
+ if (remote_get_threads_with_qxfer (ops, &context)
+ || remote_get_threads_with_qthreadinfo (ops, &context)
+ || remote_get_threads_with_ql (ops, &context))
+ {
+ int i;
+ struct thread_item *item;
+ struct thread_info *tp, *tmp;
+
+ got_list = 1;
+
+ if (VEC_empty (thread_item_t, context.items)
+ && remote_thread_always_alive (ops, inferior_ptid))
+ {
+ /* Some targets don't really support threads, but still
+ reply an (empty) thread list in response to the thread
+ listing packets, instead of replying "packet not
+ supported". Exit early so we don't delete the main
+ thread. */
+ do_cleanups (old_chain);
+ return;
+ }
+
+ /* CONTEXT now holds the current thread list on the remote
+ target end. Delete GDB-side threads no longer found on the
+ target. */
+ ALL_THREADS_SAFE (tp, tmp)
+ {
+ for (i = 0;
+ VEC_iterate (thread_item_t, context.items, i, item);
+ ++i)
+ {
+ if (ptid_equal (item->ptid, tp->ptid))
+ break;
+ }
+
+ if (i == VEC_length (thread_item_t, context.items))
+ {
+ /* Not found. */
+ delete_thread (tp->ptid);
+ }
+ }
+
+ /* Remove any unreported fork child threads from CONTEXT so
+ that we don't interfere with follow fork, which is where
+ creation of such threads is handled. */
+ ignore_new_fork_children (&context);
+
+ /* And now add threads we don't know about yet to our list. */
+ for (i = 0;
+ VEC_iterate (thread_item_t, context.items, i, item);
+ ++i)
+ {
+ if (!ptid_equal (item->ptid, null_ptid))
+ {
+ struct private_thread_info *info;
+ /* In non-stop mode, we assume new found threads are
+ running until proven otherwise with a stop reply. In
+ all-stop, we can only get here if all threads are
+ stopped. */
+ int running = non_stop ? 1 : 0;
+
+ remote_notice_new_inferior (item->ptid, running);
+
+ info = demand_private_info (item->ptid);
+ info->core = item->core;
+ info->extra = item->extra;
+ item->extra = NULL;
+ }
+ }
+ }
+
+ if (!got_list)
+ {
+ /* If no thread listing method is supported, then query whether
+ each known thread is alive, one by one, with the T packet.
+ If the target doesn't support threads at all, then this is a
+ no-op. See remote_thread_alive. */
+ prune_threads ();
+ }
+
+ do_cleanups (old_chain);
+}
+
/* Remove stop replies in the queue if its pid is equal to the given
inferior's pid. */
@@ -7945,6 +8090,69 @@ getpkt_or_notif_sane (char **buf, long *sizeof_buf, int forever,
is_notif);
}
+/* Check whether EVENT is a fork event for the process specified
+ by the pid passed in DATA, and if it is, kill the fork child. */
+
+static int
+kill_child_of_pending_fork (QUEUE (stop_reply_p) *q,
+ QUEUE_ITER (stop_reply_p) *iter,
+ stop_reply_p event,
+ void *data)
+{
+ struct queue_iter_param *param = data;
+ int parent_pid = *(int *) param->input;
+
+ if (is_pending_fork_parent (&event->ws, parent_pid, event->ptid))
+ {
+ struct remote_state *rs = get_remote_state ();
+ int child_pid = ptid_get_pid (event->ws.value.related_pid);
+ int res;
+
+ res = remote_vkill (child_pid, rs);
+ if (res != 0)
+ error (_("Can't kill fork child process %d"), child_pid);
+ }
+
+ return 1;
+}
+
+/* Kill any new fork children of process PID that haven't been
+ processed by follow_fork. */
+
+static void
+kill_new_fork_children (int pid, struct remote_state *rs)
+{
+ struct thread_info *thread;
+ struct notif_client *notif = ¬if_client_stop;
+ struct queue_iter_param param;
+
+ /* Kill the fork child threads of any threads in process PID
+ that are stopped at a fork event. */
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ struct target_waitstatus *ws = &thread->pending_follow;
+
+ if (is_pending_fork_parent(ws, pid, thread->ptid))
+ {
+ struct remote_state *rs = get_remote_state ();
+ int child_pid = ptid_get_pid (ws->value.related_pid);
+ int res;
+
+ res = remote_vkill (child_pid, rs);
+ if (res != 0)
+ error (_("Can't kill fork child process %d"), child_pid);
+ }
+ }
+
+ /* Check for any pending fork events (not reported or processed yet)
+ in process PID and kill those fork child threads as well. */
+ remote_notif_get_pending_events (notif);
+ param.input = &pid;
+ param.output = NULL;
+ QUEUE_iterate (stop_reply_p, stop_reply_queue,
+ kill_child_of_pending_fork, ¶m);
+}
+
\f
static void
remote_kill (struct target_ops *ops)
@@ -8014,6 +8222,11 @@ extended_remote_kill (struct target_ops *ops)
int pid = ptid_get_pid (inferior_ptid);
struct remote_state *rs = get_remote_state ();
+ /* If we're stopped while forking and we haven't followed yet, kill the
+ child task. We need to do this before killing the parent task
+ because if this is a vfork then the parent will be sleeping. */
+ kill_new_fork_children (pid, rs);
+
res = remote_vkill (pid, rs);
if (res == -1 && !(rs->extended && remote_multi_process_p (rs)))
{
@@ -11957,6 +12170,14 @@ Specify the serial device it is connected to (e.g. /dev/ttya).";
extended_remote_ops.to_supports_disable_randomization
= extended_remote_supports_disable_randomization;
extended_remote_ops.to_follow_fork = remote_follow_fork;
+ extended_remote_ops.to_insert_fork_catchpoint
+ = remote_insert_fork_catchpoint;
+ extended_remote_ops.to_remove_fork_catchpoint
+ = remote_remove_fork_catchpoint;
+ extended_remote_ops.to_insert_vfork_catchpoint
+ = remote_insert_vfork_catchpoint;
+ extended_remote_ops.to_remove_vfork_catchpoint
+ = remote_remove_vfork_catchpoint;
}
static int
--
1.7.0.4
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 6/7] Remote fork catch
2015-05-06 16:10 ` Don Breazeal
@ 2015-05-08 10:14 ` Pedro Alves
2015-05-08 16:42 ` Don Breazeal
2015-05-08 10:16 ` Pedro Alves
1 sibling, 1 reply; 18+ messages in thread
From: Pedro Alves @ 2015-05-08 10:14 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 05/06/2015 05:10 PM, Don Breazeal wrote:
> I've reimplemented the mechanisms that (a) ignore new threads
> that are as-yet-unreported fork children, and (b) kill as-yet-
> unreported fork children when killing the fork parent. These both
> now check the stop reply queue for unreported fork events. I changed
> the names of the new functions to use 'ignore' instead of 'remove'
> to more accurately reflect what they are doing, and moved
> remote_update_thread_list to avoid having to add prototypes and
> forward declarations to satisfy all the dependences.
I understand what you were trying to avoid, but I really don't like
the move: the function is currently alongside a bunch of related code
and ends up somewhere quite unrelated. I'd rather have the
forward declarations. You're already adding other forward
declarations anyway.
Also, why does "ignore" more accurately reflect what the functions
are doing? E.g.,:
> +/* Remove the thread specified as the related_pid field of WS
> + from the CONTEXT list. */
> +
> +static void
> +ignore_new_fork_child (struct target_waitstatus *ws,
> + struct threads_listing_context *context)
> +{
> + struct thread_item *item;
> + int i;
> + ptid_t child_ptid = ws->value.related_pid;
> +
> + for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
> + {
> + if (ptid_equal (item->ptid, child_ptid))
> + {
> + VEC_ordered_remove (thread_item_t, context->items, i);
> + break;
> + }
> + }
> +}
> +
That's clearly removing something from a list, as even the comment says.
How about instead adding:
static void
threads_listing_context_remove (struct threads_listing_context *context,
ptid_t remove_ptid)
{
struct thread_item *item;
int i;
for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
{
if (ptid_equal (item->ptid, remove_ptid))
{
VEC_ordered_remove (thread_item_t, context->items, i);
break;
}
}
}
after clear_threads_listing_context, and then doing:
/* Check whether EVENT is a fork event, and if it is, remove the
fork child from the context list passed in DATA. */
static int
remove_child_of_pending_fork (QUEUE (stop_reply_p) *q,
QUEUE_ITER (stop_reply_p) *iter,
stop_reply_p event,
void *data)
{
struct queue_iter_param *param = data;
struct threads_listing_context *context = param->input;
if (event->ws.kind == TARGET_WAITKIND_FORKED
|| event->ws.kind == TARGET_WAITKIND_VFORKED)
{
threads_listing_context_remove (context,
&event->ws->value.related_pid);
}
return 1;
}
etc.
> +
> +static void
> +kill_new_fork_children (int pid, struct remote_state *rs)
> +{
> + struct thread_info *thread;
> + struct notif_client *notif = ¬if_client_stop;
> + struct queue_iter_param param;
> +
> + /* Kill the fork child threads of any threads in process PID
> + that are stopped at a fork event. */
> + ALL_NON_EXITED_THREADS (thread)
> + {
> + struct target_waitstatus *ws = &thread->pending_follow;
> +
> + if (is_pending_fork_parent(ws, pid, thread->ptid))
Space before parens.
Otherwise this is good to go. Thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 6/7] Remote fork catch
2015-05-08 10:14 ` Pedro Alves
@ 2015-05-08 16:42 ` Don Breazeal
2015-05-08 17:47 ` Pedro Alves
0 siblings, 1 reply; 18+ messages in thread
From: Don Breazeal @ 2015-05-08 16:42 UTC (permalink / raw)
To: Pedro Alves, Breazeal, Don, gdb-patches
On 5/8/2015 3:13 AM, Pedro Alves wrote:
> On 05/06/2015 05:10 PM, Don Breazeal wrote:
>
>> I've reimplemented the mechanisms that (a) ignore new threads
>> that are as-yet-unreported fork children, and (b) kill as-yet-
>> unreported fork children when killing the fork parent. These both
>> now check the stop reply queue for unreported fork events. I changed
>> the names of the new functions to use 'ignore' instead of 'remove'
>> to more accurately reflect what they are doing, and moved
>> remote_update_thread_list to avoid having to add prototypes and
>> forward declarations to satisfy all the dependences.
>
> I understand what you were trying to avoid, but I really don't like
> the move: the function is currently alongside a bunch of related code
> and ends up somewhere quite unrelated. I'd rather have the
> forward declarations. You're already adding other forward
> declarations anyway.
>
> Also, why does "ignore" more accurately reflect what the functions
> are doing? E.g.,:
>
>> +/* Remove the thread specified as the related_pid field of WS
>> + from the CONTEXT list. */
>> +
>> +static void
>> +ignore_new_fork_child (struct target_waitstatus *ws,
>> + struct threads_listing_context *context)
>> +{
>> + struct thread_item *item;
>> + int i;
>> + ptid_t child_ptid = ws->value.related_pid;
>> +
>> + for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
>> + {
>> + if (ptid_equal (item->ptid, child_ptid))
>> + {
>> + VEC_ordered_remove (thread_item_t, context->items, i);
>> + break;
>> + }
>> + }
>> +}
>> +
>
> That's clearly removing something from a list, as even the comment says.
>
> How about instead adding:
>
> static void
> threads_listing_context_remove (struct threads_listing_context *context,
> ptid_t remove_ptid)
> {
> struct thread_item *item;
> int i;
>
> for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
> {
> if (ptid_equal (item->ptid, remove_ptid))
> {
> VEC_ordered_remove (thread_item_t, context->items, i);
> break;
> }
> }
> }
>
> after clear_threads_listing_context, and then doing:
>
> /* Check whether EVENT is a fork event, and if it is, remove the
> fork child from the context list passed in DATA. */
>
> static int
> remove_child_of_pending_fork (QUEUE (stop_reply_p) *q,
> QUEUE_ITER (stop_reply_p) *iter,
> stop_reply_p event,
> void *data)
> {
> struct queue_iter_param *param = data;
> struct threads_listing_context *context = param->input;
>
> if (event->ws.kind == TARGET_WAITKIND_FORKED
> || event->ws.kind == TARGET_WAITKIND_VFORKED)
> {
> threads_listing_context_remove (context,
> &event->ws->value.related_pid);
> }
>
> return 1;
> }
>
> etc.
>
>> +
>> +static void
>> +kill_new_fork_children (int pid, struct remote_state *rs)
>> +{
>> + struct thread_info *thread;
>> + struct notif_client *notif = ¬if_client_stop;
>> + struct queue_iter_param param;
>> +
>> + /* Kill the fork child threads of any threads in process PID
>> + that are stopped at a fork event. */
>> + ALL_NON_EXITED_THREADS (thread)
>> + {
>> + struct target_waitstatus *ws = &thread->pending_follow;
>> +
>> + if (is_pending_fork_parent(ws, pid, thread->ptid))
>
> Space before parens.
>
> Otherwise this is good to go. Thanks!
>
Thanks Pedro. Your suggested function naming makes sense; it eliminates
any ambiguity about whether the thread is the actual running thread or
an element in the thread list.
I think that with this patch completed we are done with this patchset.
I will make these changes and push it all in. My plan after that is to:
-> resurrect the extended-remote follow exec patches (minus the exit
event stuff) and work through any issues with those
-> then revisit the 'target remote' patch for both follow fork and
follow exec
Let me know if there's any reason for me to do things in a different order.
Thanks again for all the reviews and help!
--Don
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 6/7] Remote fork catch
2015-05-08 16:42 ` Don Breazeal
@ 2015-05-08 17:47 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2015-05-08 17:47 UTC (permalink / raw)
To: Don Breazeal, Breazeal, Don, gdb-patches
On 05/08/2015 05:41 PM, Don Breazeal wrote:
> I think that with this patch completed we are done with this patchset.
I'm afraid I had lost track. Congrats! :-)
> I will make these changes and push it all in.
(Per usual policy, please post the final patch you push in to
the list, for the archives.)
> My plan after that is to:
>
> -> resurrect the extended-remote follow exec patches (minus the exit
> event stuff) and work through any issues with those
>
> -> then revisit the 'target remote' patch for both follow fork and
> follow exec
>
> Let me know if there's any reason for me to do things in a different order.
That sounds good. I agree with making the feature fully work first
with extended-remote.
>
> Thanks again for all the reviews and help!
Thanks for working on this and for the patience!
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 6/7] Remote fork catch
2015-05-06 16:10 ` Don Breazeal
2015-05-08 10:14 ` Pedro Alves
@ 2015-05-08 10:16 ` Pedro Alves
1 sibling, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2015-05-08 10:16 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 05/06/2015 05:10 PM, Don Breazeal wrote:
> I have some concerns about my test results, though. Over the past
> few weeks I've been seeing more intermittent failures than I expect,
> on both the mainline and my branch. Sometimes I get clean test runs,
> other times not. Is this something others have been seeing, or is it
> just me?
Yeah, some tests are racy still. See the buildbot results
here, for example.
https://sourceware.org/ml/gdb-testers/2015-q2/msg03532.html
https://sourceware.org/ml/gdb-testers/2015-q2/msg03533.html
etc. That list is high volume, since results are threaded by commit,
it's manageable in a mail client.
>
> I know that some of these tests are problematic (random-signal.exp),
> but others I haven't been aware of. The failures I've seen include
> (but aren't limited to):
>
> gdb.base/random-signal.exp
The buildbots show:
FAIL: gdb.base/random-signal.exp: stop with control-c (timeout)
Haven't investigated it.
> gdb.mi/mi-nsmoribund.exp
This one's fixed by:
https://sourceware.org/ml/gdb-patches/2015-05/msg00117.html
That may fix some others. Not sure.
> gdb.threads/attach-many-short-lived-threads.exp
The buildbots show this failing randomly too. As far as I've
seen, most often, gdb fails to attach to the process,
saying that the leader thread is zombie, on first iteration.
It looks like either gdb or kernel bug, but I haven't managed to
pinpoint it.
> gdb.threads/interrupted-hand-call.exp
> gdb.threads/thread-unwindonsignal.exp
These are both caused by the same bug:
PASS -> FAIL: gdb.threads/thread-unwindonsignal.exp: continue until exit
PASS -> FAIL: gdb.threads/interrupted-hand-call.exp: continue until exit
It's related to:
https://sourceware.org/ml/gdb-patches/2015-03/msg00597.html
That is, in those tests, a thread other than the main thread exits
the process while gdbserver is still iterating over all threads
continuing them. gdbserver needs more fixing.
Really the proper way to handle this would be to file bugs in
bugzilla, pasting there the relevant gdb.log bits, and mark the
tests with KFAIL.
> gdb.trace/tspeed.exp
No idea. Given the test's purpose, I'd suspect a test bug.
>
> I've also seen failures in 'random' tests due to timeouts on the
> qSupported packet, like this:
>
> target remote localhost:3238^M
> Remote debugging using localhost:3238^M
> Ignoring packet error, continuing...^M
> warning: unrecognized item "timeout" in "qSupported" response^M
>
> I was particularly concerned about this since I had made changes to
> qSupported, but I found that it often times out for me on the
> mainline as well. I tried bumping the timeout up to 5 seconds for
> the qSupported packet and still saw the error. I haven't identified
> a root cause.
No idea.
>
> Most of these issues I've been able to reproduce on the mainline by
> just running the test over and over in a loop until it fails.
>
> So, all that said, I believe that my changes aren't introducing any
> obvious regressions, but I'd feel a lot better about it if I could
> get clean test runs more reliably.
I'd love to get clean test results too. All the help would be
much appreciated!
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 5/7] Remote follow vfork
2015-04-17 20:59 [PATCH v8 0/7] Don Breazeal
` (5 preceding siblings ...)
2015-04-17 21:00 ` [PATCH v8 6/7] Remote fork catch Don Breazeal
@ 2015-04-17 21:00 ` Don Breazeal
2015-04-23 14:19 ` Pedro Alves
6 siblings, 1 reply; 18+ messages in thread
From: Don Breazeal @ 2015-04-17 21:00 UTC (permalink / raw)
To: gdb-patches, palves
Hi Pedro,
This version of the patch incorporates changes based on your comments on
the previous version, as outlined here:
https://sourceware.org/ml/gdb-patches/2015-04/msg00574.html
On 4/15/2015 8:38 AM, Pedro Alves wrote:
> On 04/10/2015 06:09 PM, Don Breazeal wrote:
>> Hi Pedro,
>>
>> This version of the patch incorporates changes based on your comments on
>> the previous version, as outlined below.
>>
>> On 3/24/2015 5:28 AM, Pedro Alves wrote:
>>> On 03/17/2015 08:56 PM, Don Breazeal wrote:
>>>
>>>> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
>>>> index dc43e38..42c3ec5 100644
>>>> --- a/gdb/gdbserver/remote-utils.c
>>>> +++ b/gdb/gdbserver/remote-utils.c
>>>> @@ -1115,15 +1115,19 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>>>> {
>>>> case TARGET_WAITKIND_STOPPED:
>>>> case TARGET_WAITKIND_FORKED:
>>>> + case TARGET_WAITKIND_VFORKED:
>>>> {
>>>> struct thread_info *saved_thread;
>>>> const char **regp;
>>>> struct regcache *regcache;
>>>>
>>>> - if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
>>>> + if ((status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
>>>> + || (status->kind == TARGET_WAITKIND_VFORKED
>>>> + && report_vfork_events))
>>>> {
>>>> enum gdb_signal signal = GDB_SIGNAL_TRAP;
>>>> - const char *event = "xfork";
>>>> + const char *event = (status->kind == TARGET_WAITKIND_FORKED
>>>> + ? "xfork" : "vfork");
>>>>
>>>> sprintf (buf, "T%02x%s:", signal, event);
>>>> buf += strlen (buf);
>>>> @@ -1245,6 +1249,15 @@ prepare_resume_reply (char *buf, ptid_t ptid,
>>>> else
>>>> sprintf (buf, "X%02x", status->value.sig);
>>>> break;
>>>> + case TARGET_WAITKIND_VFORK_DONE:
>>>> + if (multi_process)
>>>> + {
>>>> + enum gdb_signal signal = GDB_SIGNAL_TRAP;
>>>> + const char *event = "vforkdone";
>>>> +
>>>
>>> Should only include vforkdone if report_vfork_events is true, I'd think.
>>
>> If we get one of these events (fork, vfork, vforkdone) and report_xxx_events
>> is not set, then it is a bug, so I put those flags into asserts for all
>> three events to ensure proper behavior.
>
> I don't think that's a good idea. For instance, what if
> a thread/lwp has already stopped for VFORK_DONE but the event is left
> pending to report to the gdb. Meanwhile, gdb disconnects before the
> event is sent. Now a new gdb reconnects, without support for vfork-done.
> See server.c:handle_status, both non-stop and all-stop. That would
> trigger that assertion.
Wow, disconnect/reconnect again, I need to internalize that. I've removed
the assertions and moved the checking of the report_xxx_events into the 'if'
conditions, like
if ((status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
|| (status->kind == TARGET_WAITKIND_VFORKED && report_vfork_events))
and added the check for report_vfork_events to the TARGET_WAITKIND_VFORK_DONE
case.
Thanks,
--Don
This patch implements follow-fork for vfork on remote and extended-remote
Linux targets.
The implementation follows the native implementation as much as possible.
Most of the work is done on the GDB side in the existing code now in
infrun.c. GDBserver just has to report the events and do a little
bookkeeping.
Implementation includes:
* enabling VFORK events by adding ptrace options for VFORK and VFORK_DONE
to linux-low.c:linux_low_ptrace_options.
* handling VFORK and VFORK_DONE events in linux-low.c:handle_extended_wait
and reporting them to GDB.
* including VFORK and VFORK_DONE events in the predicate
linux-low.c:extended_event_reported.
* adding support for VFORK and VFORK_DONE events in RSP by adding stop
reasons "vfork" and "vforkdone" to the 'T' Stop Reply Packet in both
gdbserver/remote-utils.c and gdb/remote.c.
Tested on x64 Ubuntu Lucid, native, remote, extended-remote.
Thanks
--Don
gdb/gdbserver/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* linux-low.c (handle_extended_wait): Handle PTRACE_EVENT_FORK and
PTRACE_EVENT_VFORK_DONE.
(linux_low_ptrace_options, extended_event_reported): Add vfork
events.
* remote-utils.c (prepare_resume_reply): New stop reasons "vfork"
and "vforkdone" for RSP 'T' Stop Reply Packet.
* server.h (report_vfork_events): Declare
global variable.
gdb/
2015-04-17 Don Breazeal <donb@codesourcery.com>
* remote.c (remove_vfork_event_p): New function.
(remote_follow_fork): Add vfork event type to event checking.
(remote_parse_stop_reply): New stop reasons "vfork" and
"vforkdone" for RSP 'T' Stop Reply Packet.
---
gdb/gdbserver/linux-low.c | 26 ++++++++++++++++++++++----
gdb/gdbserver/remote-utils.c | 17 +++++++++++++++--
gdb/gdbserver/server.h | 1 +
gdb/remote.c | 22 +++++++++++++++++++++-
4 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index bf41eab..7faad44 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -425,7 +425,8 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
struct thread_info *event_thr = get_lwp_thread (event_lwp);
struct lwp_info *new_lwp;
- if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_CLONE))
+ if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_VFORK)
+ || (event == PTRACE_EVENT_CLONE))
{
ptid_t ptid;
unsigned long new_pid;
@@ -451,7 +452,7 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
warning ("wait returned unexpected status 0x%x", status);
}
- if (event == PTRACE_EVENT_FORK)
+ if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK)
{
struct process_info *parent_proc;
struct process_info *child_proc;
@@ -494,8 +495,13 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
the_low_target.new_fork (parent_proc, child_proc);
/* Save fork info in the parent thread. */
- event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
+ if (event == PTRACE_EVENT_FORK)
+ event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
+ else if (event == PTRACE_EVENT_VFORK)
+ event_lwp->waitstatus.kind = TARGET_WAITKIND_VFORKED;
+
event_lwp->waitstatus.value.related_pid = ptid;
+
/* The status_pending field contains bits denoting the
extended event, so when the pending event is handled,
the handler will look at lwp->waitstatus. */
@@ -538,6 +544,13 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat)
/* Don't report the event. */
return 1;
}
+ else if (event == PTRACE_EVENT_VFORK_DONE)
+ {
+ event_lwp->waitstatus.kind = TARGET_WAITKIND_VFORK_DONE;
+
+ /* Report the event. */
+ return 0;
+ }
internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event);
}
@@ -2010,6 +2023,9 @@ linux_low_ptrace_options (int attached)
if (report_fork_events)
options |= PTRACE_O_TRACEFORK;
+ if (report_vfork_events)
+ options |= (PTRACE_O_TRACEVFORK | PTRACE_O_TRACEVFORKDONE);
+
return options;
}
@@ -2714,7 +2730,9 @@ extended_event_reported (const struct target_waitstatus *waitstatus)
if (waitstatus == NULL)
return 0;
- return (waitstatus->kind == TARGET_WAITKIND_FORKED);
+ return (waitstatus->kind == TARGET_WAITKIND_FORKED
+ || waitstatus->kind == TARGET_WAITKIND_VFORKED
+ || waitstatus->kind == TARGET_WAITKIND_VFORK_DONE);
}
/* Wait for process, returns status. */
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 6078348..8e00491 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1115,16 +1115,20 @@ prepare_resume_reply (char *buf, ptid_t ptid,
{
case TARGET_WAITKIND_STOPPED:
case TARGET_WAITKIND_FORKED:
+ case TARGET_WAITKIND_VFORKED:
{
struct thread_info *saved_thread;
const char **regp;
struct regcache *regcache;
- if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
+ if ((status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
+ || (status->kind == TARGET_WAITKIND_VFORKED && report_vfork_events))
{
enum gdb_signal signal = GDB_SIGNAL_TRAP;
+ const char *event = (status->kind == TARGET_WAITKIND_FORKED
+ ? "fork" : "vfork");
- sprintf (buf, "T%02xfork:", signal);
+ sprintf (buf, "T%02x%s:", signal, event);
buf += strlen (buf);
buf = write_ptid (buf, status->value.related_pid);
strcat (buf, ";");
@@ -1244,6 +1248,15 @@ prepare_resume_reply (char *buf, ptid_t ptid,
else
sprintf (buf, "X%02x", status->value.sig);
break;
+ case TARGET_WAITKIND_VFORK_DONE:
+ if (report_vfork_events)
+ {
+ enum gdb_signal signal = GDB_SIGNAL_TRAP;
+ const char *event = "vforkdone";
+
+ sprintf (buf, "T%02x%s:;", signal, event);
+ }
+ break;
default:
error ("unhandled waitkind");
break;
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 696a24e..09a5624 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -85,6 +85,7 @@ extern int disable_packet_qfThreadInfo;
extern int run_once;
extern int multi_process;
extern int report_fork_events;
+extern int report_vfork_events;
extern int non_stop;
/* True if the "swbreak+" feature is active. In that case, GDB wants
diff --git a/gdb/remote.c b/gdb/remote.c
index 8e26e5e..c0fa3b9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1469,6 +1469,14 @@ remote_fork_event_p (struct remote_state *rs)
return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
}
+/* Returns true if vfork events are supported. */
+
+static int
+remote_vfork_event_p (struct remote_state *rs)
+{
+ return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
+}
+
/* Tokens for use by the asynchronous signal handlers for SIGINT. */
static struct async_signal_handler *async_sigint_remote_twice_token;
static struct async_signal_handler *async_sigint_remote_token;
@@ -4526,8 +4534,10 @@ remote_follow_fork (struct target_ops *ops, int follow_child,
int detach_fork)
{
struct remote_state *rs = get_remote_state ();
+ enum target_waitkind kind = inferior_thread ()->pending_follow.kind;
- if (remote_fork_event_p (rs))
+ if ((kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
+ || (kind == TARGET_WAITKIND_VFORKED && remote_vfork_event_p (rs)))
{
/* When following the parent and detaching the child, we detach
the child here. For the case of following the child and
@@ -5716,6 +5726,16 @@ Packet: '%s'\n"),
event->ws.value.related_pid = read_ptid (++p1, &p);
event->ws.kind = TARGET_WAITKIND_FORKED;
}
+ else if (strncmp (p, "vfork", p1 - p) == 0)
+ {
+ event->ws.value.related_pid = read_ptid (++p1, &p);
+ event->ws.kind = TARGET_WAITKIND_VFORKED;
+ }
+ else if (strncmp (p, "vforkdone", p1 - p) == 0)
+ {
+ event->ws.kind = TARGET_WAITKIND_VFORK_DONE;
+ p = skip_to_semicolon (p1 + 1);
+ }
else
{
ULONGEST pnum;
--
1.8.1.1
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v8 5/7] Remote follow vfork
2015-04-17 21:00 ` [PATCH v8 5/7] Remote follow vfork Don Breazeal
@ 2015-04-23 14:19 ` Pedro Alves
0 siblings, 0 replies; 18+ messages in thread
From: Pedro Alves @ 2015-04-23 14:19 UTC (permalink / raw)
To: Don Breazeal, gdb-patches
On 04/17/2015 09:58 PM, Don Breazeal wrote:
> This patch implements follow-fork for vfork on remote and extended-remote
> Linux targets.
Only extended-remote, I think.
> --- a/gdb/gdbserver/remote-utils.c
> +++ b/gdb/gdbserver/remote-utils.c
> @@ -1115,16 +1115,20 @@ prepare_resume_reply (char *buf, ptid_t ptid,
> {
> case TARGET_WAITKIND_STOPPED:
> case TARGET_WAITKIND_FORKED:
> + case TARGET_WAITKIND_VFORKED:
> {
> struct thread_info *saved_thread;
> const char **regp;
> struct regcache *regcache;
>
> - if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
> + if ((status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
> + || (status->kind == TARGET_WAITKIND_VFORKED && report_vfork_events))
> {
> enum gdb_signal signal = GDB_SIGNAL_TRAP;
> + const char *event = (status->kind == TARGET_WAITKIND_FORKED
> + ? "fork" : "vfork");
>
> - sprintf (buf, "T%02xfork:", signal);
> + sprintf (buf, "T%02x%s:", signal, event);
> buf += strlen (buf);
> buf = write_ptid (buf, status->value.related_pid);
> strcat (buf, ";");
> @@ -1244,6 +1248,15 @@ prepare_resume_reply (char *buf, ptid_t ptid,
> else
> sprintf (buf, "X%02x", status->value.sig);
> break;
> + case TARGET_WAITKIND_VFORK_DONE:
> + if (report_vfork_events)
> + {
> + enum gdb_signal signal = GDB_SIGNAL_TRAP;
> + const char *event = "vforkdone";
static const char event[] = "vforkdone";
> +
> + sprintf (buf, "T%02x%s:", signal, event);
Or just simply:
sprintf (buf, "T%02xvforkdone:", GDB_SIGNAL_TRAP);
> + }
I think this should have a fallback like the TARGET_WAITKIND_FORKED
TARGET_WAITKIND_VFORKED cases:
else
sprintf (buf, "T%02x", GDB_SIGNAL_0);
... in case this was an event that was pending already
when GDB connected, and the connecting gdb doesn't want
or understand those events.
OK with that fixed.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 18+ messages in thread