From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39033 invoked by alias); 15 Sep 2015 15:46:57 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 39022 invoked by uid 89); 15 Sep 2015 15:46:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Sep 2015 15:46:50 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1ZbsRS-0006sI-Vk from Don_Breazeal@mentor.com ; Tue, 15 Sep 2015 08:46:47 -0700 Received: from [172.30.14.241] (147.34.91.1) by SVR-ORW-FEM-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server (TLS) id 14.3.224.2; Tue, 15 Sep 2015 08:46:46 -0700 Subject: Re: problem with new vforkdone stop reply in 7.10 To: Pedro Alves References: <55F828D9.8050100@redhat.com> <55F8307D.5090305@redhat.com> CC: "gdb@sourceware.org" From: Don Breazeal Message-ID: <55F83D66.1010603@codesourcery.com> Date: Tue, 15 Sep 2015 15:46:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <55F8307D.5090305@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00007.txt.bz2 On 9/15/2015 7:51 AM, Pedro Alves wrote: > On 09/15/2015 03:19 PM, Pedro Alves wrote: >> Hi Don, >> >> Unfortunately, I missed in review that the vforkdone stop reply >> misses indicating the thread ID of the vfork parent which the >> event relates to: > > I'm testing this. I wonder whether I'm missing some reason > to keep the vforkdone switch case separate? > At least foll-vfork.exp still passes. Thanks for taking care of this. I don't think you are missing anything. I think I just over-simplified the VFORK_DONE packet starting in my first implementation. The changes make sense to me. It seems like you have this covered, but let me know if I can help at all. > > I should split this to a separate patch, but I also noticed that > the TARGET_WAITKIND_EXECD handling checks multi_process instead of > report_exec_events: > > - else if (status->kind == TARGET_WAITKIND_EXECD && multi_process) > + else if (status->kind == TARGET_WAITKIND_EXECD && report_exec_events) > > which I think would be the check we want. Absolutely, that should be report_exec_events. I missed making that change from a previous version of the patchset that didn't use the report_xxx_events variables. > > Thanks, > Pedro Alves > > From 85989d3ce1cc931aa7025e21952136ef037388a1 Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Tue, 15 Sep 2015 15:43:12 +0100 > Subject: [PATCH] Fix vforkdone stop reply > > The vforkdone stop reply misses indicating the thread ID of the vfork > parent which the event relates to: > > @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. > > @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. > > Unfortunately, this is not just a documentation issue. GDBserver > is really not specifying the thread ID. I noticed because > in non-stop mode, gdb complains: > > [Thread 6089.6089] #1 stopped. > #0 0x0000003615a011f0 in ?? () > 0x0000003615a011f0 in ?? () > (gdb) set debug remote 1 > (gdb) c > Continuing. > Sending packet: $QPassSignals:e;10;14;17;1a;1b;1c;21;24;25;2c;4c;#5f...Packet received: OK > Sending packet: $vCont;c:p17c9.17c9#88...Packet received: OK > Notification received: Stop:T05vfork:p17ce.17ce;06:40d7ffffff7f0000;07:30d7ffffff7f0000;10:e4c9eb1536000000;thread:p17c9.17c9;core:2; > Sending packet: $vStopped#55...Packet received: OK > Sending packet: $D;17ce#af...Packet received: OK > Sending packet: $vCont;c:p17c9.17c9#88...Packet received: OK > Notification received: Stop:T05vforkdone:; > No process or thread specified in stop reply: T05vforkdone:; > (gdb) > > This is not non-stop-mode-specific, however. Consider e.g., that in > all-stop, you may be debugging more than one process at the same time. > You continue, and both processes vfork. So when you next get a > T05vforkdone, there's no way to tell which of the parent processes is > done with the vfork. > I've started looking at adding some tests to plug some of the gaps in fork/exec event testing. Currently I'm looking at a test that kills the fork parent while stopped at a fork catchpoint. It seems like I should add non-stop cases to foll-fork.exp and foll-vfork.exp as well. And maybe the case where there are multiple vforks in progress at the same time, if I can come up with something that is reliable. > gdb/ChangeLog: > 2015-09-15 Pedro Alves > > * remote-utils.c (prepare_resume_reply): Merge > TARGET_WAITKIND_VFORK_DONE switch case with the > TARGET_WAITKIND_FORKED case. > > gdb/doc/ChangeLog: > 2015-09-15 Pedro Alves > > * gdb.texinfo: Explain that vforkdone's 'r' part indicates the > parent process. > --- > gdb/doc/gdb.texinfo | 9 +++++---- > gdb/gdbserver/remote-utils.c | 19 ++++++++----------- > 2 files changed, 13 insertions(+), 15 deletions(-) > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 395f0d4..3ac8c43 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -35498,10 +35498,11 @@ 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 > +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 > +the thread ID of the parent 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 vforkdone events. > > This packet should not be sent by default; older @value{GDBN} versions > diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c > index ac86dd5..7cf66cc 100644 > --- a/gdb/gdbserver/remote-utils.c > +++ b/gdb/gdbserver/remote-utils.c > @@ -1117,6 +1117,7 @@ prepare_resume_reply (char *buf, ptid_t ptid, > case TARGET_WAITKIND_STOPPED: > case TARGET_WAITKIND_FORKED: > case TARGET_WAITKIND_VFORKED: > + case TARGET_WAITKIND_VFORK_DONE: > case TARGET_WAITKIND_EXECD: > { > struct thread_info *saved_thread; > @@ -1135,7 +1136,13 @@ prepare_resume_reply (char *buf, ptid_t ptid, > buf = write_ptid (buf, status->value.related_pid); > strcat (buf, ";"); > } > - else if (status->kind == TARGET_WAITKIND_EXECD && multi_process) > + else if (status->kind == TARGET_WAITKIND_VFORK_DONE && report_vfork_events) > + { > + enum gdb_signal signal = GDB_SIGNAL_TRAP; > + > + sprintf (buf, "T%02xvforkdone:;", signal); > + } > + else if (status->kind == TARGET_WAITKIND_EXECD && report_exec_events) > { > enum gdb_signal signal = GDB_SIGNAL_TRAP; > const char *event = "exec"; > @@ -1269,16 +1276,6 @@ 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; > - > - sprintf (buf, "T%02xvforkdone:;", signal); > - } > - else > - sprintf (buf, "T%02x", GDB_SIGNAL_0); > - break; > default: > error ("unhandled waitkind"); > break; >