[v8,5/7] Remote follow vfork
Commit Message
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(-)
Comments
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
@@ -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. */
@@ -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;
@@ -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
@@ -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;