From patchwork Mon Dec 14 19:29:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 10006 Received: (qmail 123172 invoked by alias); 14 Dec 2015 19:29:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 123159 invoked by uid 89); 14 Dec 2015 19:29:58 -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; Mon, 14 Dec 2015 19:29:48 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1a8Yob-0005hs-Ab from Don_Breazeal@mentor.com ; Mon, 14 Dec 2015 11:29:45 -0800 Received: from [172.30.5.189] (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; Mon, 14 Dec 2015 11:29:44 -0800 Subject: Re: [PATCH v2 2/3] Target remote mode fork and exec events To: Pedro Alves , "gdb-patches@sourceware.org" References: <564F1A6A.3030301@redhat.com> <1449526447-10039-1-git-send-email-donb@codesourcery.com> <1449526447-10039-3-git-send-email-donb@codesourcery.com> <5666D32F.2040007@redhat.com> From: Don Breazeal Message-ID: <566F18A8.2020006@codesourcery.com> Date: Mon, 14 Dec 2015 11:29:44 -0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <5666D32F.2040007@redhat.com> X-IsSubscribed: yes On 12/8/2015 4:55 AM, Pedro Alves wrote: > On 12/07/2015 10:14 PM, Don Breazeal wrote: > >> On 11/20/2015 5:04 AM, Pedro Alves wrote: > >>>> Note that follow-exec-mode is not supported, because target >>>> remote mode does not support the 'run' command. >>> >>> Not sure I don't understand this part/comment. >> >> As I noted in my previous reply, this was an overly-pessimistic way of >> putting it. I revised the docs etc to say something like this: >> "follow-exec-mode is of limited used in target remote mode, since the 'run' >> command is not supported." The point being that the main reason a user >> would want to set follow-exec-mode would be to get control over which >> inferior is run when a 'run' command is executed. It is supported in >> that the inferiors are created or not created as specified, and one could >> (for example) use follow-exec-mode to keep a record of the programs that >> were executed. > > Right. Note that we could support "run" with "target remote", as long > as the remote side supports vRun. The problem with simply enabling > "run" is that: > > (gdb) target remote > ... > (gdb) run > The program being debugged has been started already. > Start it from the beginning? (y or n) > > ... saying yes will kill the inferior and close the connection, before > there's a chance to start the new process... > > > But, with "set detach-on-fork off", say the program forks, and then the child > execs, and then it exits. The parent is still running so the "target remote" > connection is still up. Switch to the child inferior (inferior 2), and > issue "run". I can see people wondering why that doesn't work. > > Likewise the simpler: > > (gdb) target remote ... > *inferior 1 is now bound to progress N (the only process) running on the target* > (gdb) add-inferior > *inferior 2 added* > (gdb) inferior 2 > (gdb) run > > Should we allow that "run" to actually succeed in this case? I'm a believer in "either it always works, or it never works". > > > (((( > Actually trying the detach-on-fork scenario above, we see very > confusing behavior (this is with your series, but I think > current master will have similar problems): > > $ gdb -ex "tar rem :9999" > 0x0000003615a011f0 in _start () from target:/lib64/ld-linux-x86-64.so.2 > (gdb) set detach-on-fork off > (gdb) c > Continuing. > Reading /lib64/libm.so.6 from remote target... > Reading /lib64/libc.so.6 from remote target... > [New Thread 32274.32274] > Reading /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork from remote target... > Reading /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork from remote target... > Reading /lib64/libm.so.6 from remote target... > Reading /lib64/libc.so.6 from remote target... > Reading /lib64/ld-linux-x86-64.so.2 from remote target... > Reading /lib64/ld-linux-x86-64.so.2 from remote target... > [Inferior 1 (process 32258) exited normally] > Reading /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork from remote target... > (gdb) info inferiors > Num Description Executable > * 1 target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > 2 process 32274 target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > (gdb) maint print target-stack > The current target stack is: > - remote (Remote serial target in gdb-specific protocol) > - exec (Local exec file) > - None (None) > (gdb) run > Reading /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork from remote target... > `target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork' has disappeared; keeping its symbols. > Starting program: target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > Detaching after fork from child process 32279. > /bin/bash: /home/pedro/gdb/mygit/build/gdb/target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork: No such file or directory > During startup program exited with code 127. > (gdb) maint print target-stack > The current target stack is: > - exec (Local exec file) > - None (None) > (gdb) > > > Note that that last run tried to create the inferior in the > native target and its the native target that silently closes > the remote connection...: > > (top-gdb) bt > #0 remote_close (self=0xdde300 ) at /home/pedro/gdb/mygit/src/gdb/remote.c:3403 > #1 0x0000000000696101 in target_close (targ=0xdde300 ) at /home/pedro/gdb/mygit/src/gdb/target.c:3308 > #2 0x0000000000691e8d in push_target (t=0xece890) at /home/pedro/gdb/mygit/src/gdb/target.c:697 > #3 0x00000000004b9495 in inf_ptrace_create_inferior (ops=0xece890, exec_file=0x1bfa190 "target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork", > allargs=0x1c09f60 "", env=0x12d0220, from_tty=1) at /home/pedro/gdb/mygit/src/gdb/inf-ptrace.c:105 > #4 0x00000000004bf6dc in linux_nat_create_inferior (ops=0xece890, exec_file=0x1bfa190 "target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork", > allargs=0x1c09f60 "", env=0x12d0220, from_tty=1) at /home/pedro/gdb/mygit/src/gdb/linux-nat.c:1194 > #5 0x000000000062c018 in run_command_1 (args=0x0, from_tty=1, tbreak_at_main=0) at /home/pedro/gdb/mygit/src/gdb/infcmd.c:604 > #6 0x000000000062c121 in run_command (args=0x0, from_tty=1) at /home/pedro/gdb/mygit/src/gdb/infcmd.c:639 > > > > I tried the "add-inferior" scenario too now, and it's also > quite confusing: > > (gdb) info inferiors > Num Description Executable > * 1 process 32274 target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > (gdb) add-inferior > Added inferior 2 > (gdb) inferior 2 > [Switching to inferior 2 [] ()] > (gdb) file /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > Reading symbols from /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork...done. > (gdb) set remote exec-file /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > (gdb) start > Temporary breakpoint 1 at 0x400671: main. (2 locations) > Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > Detaching after fork from child process 32353. > Detaching after fork from child process 32354. > Detaching after fork from child process 32356. > Error in re-setting breakpoint 1: Cannot access memory at address 0x400669 > Error in re-setting breakpoint 1: Cannot access memory at address 0x400669 > Error in re-setting breakpoint 1: Cannot access memory at address 0x400669 > Error in re-setting breakpoint 1: Cannot access memory at address 0x400669 > Error in re-setting breakpoint 1: Cannot access memory at address 0x400669 > [Inferior 2 (process 32353) exited normally] > (gdb) info inferiors > Num Description Executable > 1 target:/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > * 2 /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork > (gdb) maint print target-stack > The current target stack is: > - exec (Local exec file) > - None (None) > (gdb) > > Likewise here start closed the remote connection, and then > find_run_target() returns the native target. > > "set auto-connect-native-target off" works around these issues. > > )))) Wow. I have a backlog of new tests that we've talked about related to the fork/exec event support. I can likely make some other tests out of the above as well. > >> Tested on x86_64 native, native-gdbserver, native-extended-gdbserver. No >> regressions, other than one issue with gdb.reverse/waitpid-reverse.exp. >> My version of GDB seemed to evoke an existing KFAIL, and after that I >> would get a new FAIL. I didn't see the KFAIL on "master", and so the >> subsequent FAIL wouldn't show up either. > > Sorry, I'm confused. Does this series cause a new FAIL, or not? Sorry about the gibberish. It is a FAIL that didn't occur with target remote mode before, but now it behaves identically to how native and extended-remote behave by generating the FAIL. > >> >> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c >> index 5e053b3..2e0e1d5 100644 >> --- a/gdb/gdbserver/server.c >> +++ b/gdb/gdbserver/server.c >> @@ -3959,9 +3959,11 @@ process_serial_event (void) >> discard_queued_stop_replies (pid_to_ptid (pid)); >> write_ok (own_buf); >> >> - if (extended_protocol) >> + if (extended_protocol || get_first_inferior (&all_threads) != NULL) > > Replace that get_first_inferior() by get_first_thread() or even target_running(). Done. > > > >> @@ -8800,41 +8797,7 @@ kill_new_fork_children (int pid, struct remote_state *rs) >> } >> >> >> -static void >> -remote_kill (struct target_ops *ops) >> -{ >> - >> - /* Catch errors so the user can quit from gdb even when we >> - aren't on speaking terms with the remote system. */ >> - TRY >> - { >> - putpkt ("k"); >> - } >> - CATCH (ex, RETURN_MASK_ERROR) >> - { >> - if (ex.error == TARGET_CLOSE_ERROR) >> - { >> - /* If we got an (EOF) error that caused the target >> - to go away, then we're done, that's what we wanted. >> - "k" is susceptible to cause a premature EOF, given >> - that the remote server isn't actually required to >> - reply to "k", and it can happen that it doesn't >> - even get to reply ACK to the "k". */ >> - return; >> - } >> - >> - /* Otherwise, something went wrong. We didn't actually kill >> - the target. Just propagate the exception, and let the >> - user or higher layers decide what to do. */ >> - throw_exception (ex); >> - } >> - END_CATCH >> - >> - /* We've killed the remote end, we get to mourn it. Since this is >> - target remote, single-process, mourning the inferior also >> - unpushes remote_ops. */ >> - target_mourn_inferior (); >> -} >> +/* Send a kill request to the target using the 'vKill' packet. */ >> >> static int >> remote_vkill (int pid, struct remote_state *rs) >> @@ -8861,56 +8824,99 @@ remote_vkill (int pid, struct remote_state *rs) >> } >> } >> >> +/* Send a kill request to the target using the 'k' packet. */ >> + >> static void >> -extended_remote_kill (struct target_ops *ops) >> +remote_kill_k (void) >> { >> - int res; >> + /* Catch errors so the user can quit from gdb even when we >> + aren't on speaking terms with the remote system. */ >> + TRY >> + { >> + putpkt ("k"); >> + } >> + CATCH (ex, RETURN_MASK_ERROR) >> + { >> + if (ex.error == TARGET_CLOSE_ERROR) >> + { >> + /* If we got an (EOF) error that caused the target >> + to go away, then we're done, that's what we wanted. >> + "k" is susceptible to cause a premature EOF, given >> + that the remote server isn't actually required to >> + reply to "k", and it can happen that it doesn't >> + even get to reply ACK to the "k". */ >> + return; >> + } >> + >> + /* Otherwise, something went wrong. We didn't actually kill >> + the target. Just propagate the exception, and let the >> + user or higher layers decide what to do. */ >> + throw_exception (ex); >> + } >> + END_CATCH >> +} > > Can't tell why you moved the function below rather than rename it > in place. I suspect that it was just a consequence of how v1 was > written. If there's no reason for the move, please leave the function > where it was (and check for spurious changes afterwards), which helps > git blame archaeology. This is back in place. > >> + >> +/* Target hook to kill the current inferior. */ >> + >> +static void >> +remote_kill (struct target_ops *ops) >> +{ > > >> -static void >> -remote_mourn (struct target_ops *target) >> -{ >> - unpush_target (target); >> + return; >> + } >> >> - /* remote_close takes care of doing most of the clean up. */ >> - generic_mourn_inferior (); >> + error (_("Can't kill process")); >> + return; > > "return" is unreachable after "error". Fixed. > > Otherwise looks good to me. > > Thanks, > Pedro Alves > In testing after making the changes above I ran into a failure I hadn't seen before, which turned out to be an uninitialized variable that had been exposed by the new functionality. That resulted in this valgrind output: ==12133== Invalid free() / delete / delete[] / realloc() ==12133== at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==12133== by 0x7943FF: xfree (common-utils.c:100) ==12133== by 0x4D0DEE: free_private_thread_info (remote.c:457) ==12133== by 0x62F0A2: free_thread (thread.c:200) ==12133== by 0x62F8DF: delete_thread_1 (thread.c:464) ==12133== by 0x62F93B: delete_thread_silent (thread.c:480) ==12133== by 0x783386: delete_thread_of_inferior (inferior.c:189) ==12133== by 0x62FA2E: iterate_over_threads (thread.c:531) ==12133== by 0x7834EF: exit_inferior_1 (inferior.c:250) ==12133== by 0x7835EC: exit_inferior_silent (inferior.c:288) ==12133== by 0x7836DE: discard_all_inferiors (inferior.c:328) ==12133== by 0x4D5D3C: remote_close (remote.c:3432) ==12133== Address 0x746e69206669206b is not stack'd, malloc'd or (recently) free'd I've included the fix in my commit as obvious: q = remote_query_supported_append (q, "vContSupported+"); @@ -4975,7 +4973,8 @@ remote_detach_1 (const char *args, int from_tty) /* Tell the remote target to detach. */ remote_detach_pid (pid); - if (from_tty && !rs->extended) + /* Exit only if this is the only active inferior. */ + if (from_tty && !rs->extended && number_of_live_inferiors () == 1) puts_filtered (_("Ending remote debugging.\n")); /* Check to see if we are detaching a fork parent. Note that if we @@ -5071,10 +5070,11 @@ remote_disconnect (struct target_ops *target, const char *args, int from_tty) if (args) error (_("Argument given to \"disconnect\" when remotely debugging.")); - /* Make sure we unpush even the extended remote targets; mourn - won't do it. So call remote_mourn directly instead of - target_mourn_inferior. */ - remote_mourn (target); + /* Make sure we unpush even the extended remote targets. Calling + target_mourn_inferior won't unpush, and remote_mourn won't + unpush if there is more than one inferior left. */ + unpush_target (target); + generic_mourn_inferior (); if (from_tty) puts_filtered ("Ending remote debugging.\n"); @@ -8800,42 +8800,53 @@ kill_new_fork_children (int pid, struct remote_state *rs) } +/* Target hook to kill the current inferior. */ + static void remote_kill (struct target_ops *ops) { + int res = -1; + int pid = ptid_get_pid (inferior_ptid); + struct remote_state *rs = get_remote_state (); - /* Catch errors so the user can quit from gdb even when we - aren't on speaking terms with the remote system. */ - TRY + if (packet_support (PACKET_vKill) != PACKET_DISABLE) { - putpkt ("k"); - } - CATCH (ex, RETURN_MASK_ERROR) - { - if (ex.error == TARGET_CLOSE_ERROR) + /* 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 == 0) { - /* If we got an (EOF) error that caused the target - to go away, then we're done, that's what we wanted. - "k" is susceptible to cause a premature EOF, given - that the remote server isn't actually required to - reply to "k", and it can happen that it doesn't - even get to reply ACK to the "k". */ + target_mourn_inferior (); return; } + } - /* Otherwise, something went wrong. We didn't actually kill - the target. Just propagate the exception, and let the - user or higher layers decide what to do. */ - throw_exception (ex); + /* If we are in 'target remote' mode and we are killing the only + inferior, then we will tell gdbserver to exit and unpush the + target. */ + if (res == -1 && !remote_multi_process_p (rs) + && number_of_live_inferiors () == 1) + { + remote_kill_k (); + + /* We've killed the remote end, we get to mourn it. If we are + not in extended mode, mourning the inferior also unpushes + remote_ops from the target stack, which closes the remote + connection. */ + target_mourn_inferior (); + + return; } - END_CATCH - /* We've killed the remote end, we get to mourn it. Since this is - target remote, single-process, mourning the inferior also - unpushes remote_ops. */ - target_mourn_inferior (); + error (_("Can't kill process")); } +/* Send a kill request to the target using the 'vKill' packet. */ + static int remote_vkill (int pid, struct remote_state *rs) { @@ -8861,55 +8872,52 @@ remote_vkill (int pid, struct remote_state *rs) } } +/* Send a kill request to the target using the 'k' packet. */ + static void -extended_remote_kill (struct target_ops *ops) +remote_kill_k (void) { - int res; - 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))) + /* Catch errors so the user can quit from gdb even when we + aren't on speaking terms with the remote system. */ + TRY { - /* Don't try 'k' on a multi-process aware stub -- it has no way - to specify the pid. */ - putpkt ("k"); -#if 0 - getpkt (&rs->buf, &rs->buf_size, 0); - if (rs->buf[0] != 'O' || rs->buf[0] != 'K') - res = 1; -#else - /* Don't wait for it to die. I'm not really sure it matters whether - we do or not. For the existing stubs, kill is a noop. */ - res = 0; -#endif } + CATCH (ex, RETURN_MASK_ERROR) + { + if (ex.error == TARGET_CLOSE_ERROR) + { + /* If we got an (EOF) error that caused the target + to go away, then we're done, that's what we wanted. + "k" is susceptible to cause a premature EOF, given + that the remote server isn't actually required to + reply to "k", and it can happen that it doesn't + even get to reply ACK to the "k". */ + return; + } - if (res != 0) - error (_("Can't kill process")); - - target_mourn_inferior (); + /* Otherwise, something went wrong. We didn't actually kill + the target. Just propagate the exception, and let the + user or higher layers decide what to do. */ + throw_exception (ex); + } + END_CATCH } static void remote_mourn (struct target_ops *target) { - unpush_target (target); + struct remote_state *rs = get_remote_state (); - /* remote_close takes care of doing most of the clean up. */ - generic_mourn_inferior (); -} + /* In 'target remote' mode with one inferior, we close the connection. */ + if (!rs->extended && number_of_live_inferiors () <= 1) + { + unpush_target (target); -static void -extended_remote_mourn (struct target_ops *target) -{ - struct remote_state *rs = get_remote_state (); + /* remote_close takes care of doing most of the clean up. */ + generic_mourn_inferior (); + return; + } /* In case we got here due to an error, but we're going to stay connected. */ @@ -8940,10 +8948,7 @@ extended_remote_mourn (struct target_ops *target) current thread. */ record_currthread (rs, minus_one_ptid); - /* Unlike "target remote", we do not want to unpush the target; then - the next time the user says "run", we won't be connected. */ - - /* Call common code to mark the inferior as not running. */ + /* Call common code to mark the inferior as not running. */ generic_mourn_inferior (); if (!have_inferiors ()) @@ -10465,7 +10470,7 @@ remote_pid_to_str (struct target_ops *ops, ptid_t ptid) { if (ptid_equal (magic_null_ptid, ptid)) xsnprintf (buf, sizeof buf, "Thread
"); - else if (rs->extended && remote_multi_process_p (rs)) + else if (remote_multi_process_p (rs)) if (ptid_get_lwp (ptid) == 0) return normal_pid_to_str (ptid); else @@ -11635,11 +11640,7 @@ remote_supports_multi_process (struct target_ops *self) { struct remote_state *rs = get_remote_state (); - /* Only extended-remote handles being attached to multiple - processes, even though plain remote can use the multi-process - thread id extensions, so that GDB knows the target process's - PID. */ - return rs->extended && remote_multi_process_p (rs); + return remote_multi_process_p (rs); } static int @@ -13071,6 +13072,14 @@ Specify the serial device it is connected to\n\ remote_ops.to_btrace_conf = remote_btrace_conf; remote_ops.to_augmented_libraries_svr4_read = remote_augmented_libraries_svr4_read; + remote_ops.to_follow_fork = remote_follow_fork; + remote_ops.to_follow_exec = remote_follow_exec; + remote_ops.to_insert_fork_catchpoint = remote_insert_fork_catchpoint; + remote_ops.to_remove_fork_catchpoint = remote_remove_fork_catchpoint; + remote_ops.to_insert_vfork_catchpoint = remote_insert_vfork_catchpoint; + remote_ops.to_remove_vfork_catchpoint = remote_remove_vfork_catchpoint; + remote_ops.to_insert_exec_catchpoint = remote_insert_exec_catchpoint; + remote_ops.to_remove_exec_catchpoint = remote_remove_exec_catchpoint; } /* Set up the extended remote vector by making a copy of the standard @@ -13089,27 +13098,11 @@ init_extended_remote_ops (void) Specify the serial device it is connected to (e.g. /dev/ttya)."; extended_remote_ops.to_open = extended_remote_open; extended_remote_ops.to_create_inferior = extended_remote_create_inferior; - extended_remote_ops.to_mourn_inferior = extended_remote_mourn; extended_remote_ops.to_detach = extended_remote_detach; extended_remote_ops.to_attach = extended_remote_attach; extended_remote_ops.to_post_attach = extended_remote_post_attach; - 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; - extended_remote_ops.to_follow_exec = remote_follow_exec; - 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; - extended_remote_ops.to_insert_exec_catchpoint - = remote_insert_exec_catchpoint; - extended_remote_ops.to_remove_exec_catchpoint - = remote_remove_exec_catchpoint; } static int --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1918,7 +1918,8 @@ demand_private_info (ptid_t ptid) info->priv = XNEW (struct private_thread_info); info->private_dtor = free_private_thread_info; info->priv->core = -1; - info->priv->extra = 0; + info->priv->extra = NULL; + info->priv->name = NULL; This is now pushed; the updated patch is attached. I think with this, basic remote fork and exec events are complete. I appreciate all your help with this project, Pedro. Thanks for hanging in there with me! --Don From: Don Breazeal Subject: [pushed][PATCH v3 2/3] Target remote mode fork and exec events This is the version of the patch that was pushed. The only differences from v2 are: * server.c:process_serial_event, use target_running instead of get_first_inferior * replace remote_kill in its original location in remote.c * remote.c:remote_kill, remove unreachable "return" I also ran into an intermittent failure that was caused by an uninitialized variable, exposed by the new functionality. I just included the change here as obvious: --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1918,7 +1918,8 @@ demand_private_info (ptid_t ptid) info->priv = XNEW (struct private_thread_info); info->private_dtor = free_private_thread_info; info->priv->core = -1; - info->priv->extra = 0; + info->priv->extra = NULL; + info->priv->name = NULL; Here is the commit message: This patch implements support for fork and exec events with target remote mode Linux targets. For such targets with Linux kernels 2.5.46 and later, this enables follow-fork-mode, detach-on-fork and fork and exec catchpoints. The changes required to implement this included: * Don't exit from gdbserver if there are still active inferiors. * Allow changing the active process in remote mode. * Enable fork and exec events in remote mode. * Print "Ending remote debugging" only when disconnecting. * Combine remote_kill and extended_remote_kill into a single function that can handle the multiple inferior case for target remote. Also, the same thing for remote_mourn and extended_remote_mourn. * Enable process-style ptids in target remote. * Remove restriction on multiprocess mode in target remote. gdb/gdbserver/ChangeLog: * server.c (process_serial_event): Don't exit from gdbserver in remote mode if there are still active inferiors. gdb/ChangeLog: * inferior.c (number_of_live_inferiors): New function. (have_live_inferiors): Use number_of_live_inferiors in place of duplicate code. * inferior.h (number_of_live_inferiors): Declare new function. * remote.c (set_general_process): Remove restriction on target remote mode. (remote_query_supported): Likewise. (remote_detach_1): Exit in target remote mode only when there is just one live inferior left. (remote_disconnect): Unpush the target directly instead of calling remote_mourn. (remote_kill): Rewrite function to handle both target remote and extended-remote. Call remote_kill_k. (remote_kill_k): New function. (extended_remote_kill): Delete function. (remote_mourn, extended_remote_mourn): Combine functions into one, remote_mourn, and enable extended functionality for target remote. (remote_pid_to_str): Enable "process" style ptid string for target remote. (remote_supports_multi_process): Remove restriction on target remote mode. --- gdb/ChangeLog | 25 ++++++ gdb/gdbserver/ChangeLog | 5 ++ gdb/gdbserver/server.c | 6 +- gdb/inferior.c | 31 ++++++-- gdb/inferior.h | 3 + gdb/remote.c | 205 +++++++++++++++++++++++------------------------- 6 files changed, 161 insertions(+), 114 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f9ed66d..9ded377 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,28 @@ +2015-12-11 Don Breazeal + + * inferior.c (number_of_live_inferiors): New function. + (have_live_inferiors): Use number_of_live_inferiors in place + of duplicate code. + * inferior.h (number_of_live_inferiors): Declare new function. + * remote.c (set_general_process): Remove restriction on target + remote mode. + (remote_query_supported): Likewise. + (remote_detach_1): Exit in target remote mode only when there + is just one live inferior left. + (remote_disconnect): Unpush the target directly instead of + calling remote_mourn. + (remote_kill): Rewrite function to handle both target remote + and extended-remote. Call remote_kill_k. + (remote_kill_k): New function. + (extended_remote_kill): Delete function. + (remote_mourn, extended_remote_mourn): Combine functions into + one, remote_mourn, and enable extended functionality for target + remote. + (remote_pid_to_str): Enable "process" style ptid string for + target remote. + (remote_supports_multi_process): Remove restriction on target + remote mode. + 2015-12-14 Andrew Burgess * i386-tdep.c (i386_mpx_info_bounds): Use TYPE_LENGTH. diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 617c249..63bb250 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,8 @@ +2015-12-11 Don Breazeal + + * server.c (process_serial_event): Don't exit from gdbserver + in remote mode if there are still active inferiors. + 2015-12-11 Yao Qi * linux-aarch64-low.c (aarch64_breakpoint_at): Call diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index a09841c..8f097e5 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -3959,9 +3959,11 @@ process_serial_event (void) discard_queued_stop_replies (pid_to_ptid (pid)); write_ok (own_buf); - if (extended_protocol) + if (extended_protocol || target_running ()) { - /* Treat this like a normal program exit. */ + /* There is still at least one inferior remaining or + we are in extended mode, so don't terminate gdbserver, + and instead treat this like a normal program exit. */ last_status.kind = TARGET_WAITKIND_EXITED; last_status.value.integer = 0; last_ptid = pid_to_ptid (pid); diff --git a/gdb/inferior.c b/gdb/inferior.c index 157e236..a0296c8 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -459,22 +459,41 @@ have_inferiors (void) return 0; } +/* Return the number of live inferiors. We account for the case + where an inferior might have a non-zero pid but no threads, as + in the middle of a 'mourn' operation. */ + int -have_live_inferiors (void) +number_of_live_inferiors (void) { struct inferior *inf; + int num_inf = 0; for (inf = inferior_list; inf; inf = inf->next) if (inf->pid != 0) { struct thread_info *tp; - - tp = any_thread_of_process (inf->pid); - if (tp && target_has_execution_1 (tp->ptid)) - break; + + ALL_NON_EXITED_THREADS (tp) + if (tp && ptid_get_pid (tp->ptid) == inf->pid) + if (target_has_execution_1 (tp->ptid)) + { + /* Found a live thread in this inferior, go to the next + inferior. */ + ++num_inf; + break; + } } - return inf != NULL; + return num_inf; +} + +/* Return true if there is at least one live inferior. */ + +int +have_live_inferiors (void) +{ + return number_of_live_inferiors () > 0; } /* Prune away any unused inferiors, and then prune away no longer used diff --git a/gdb/inferior.h b/gdb/inferior.h index d3cf615..78a5ed3 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -490,6 +490,9 @@ extern struct inferior *iterate_over_inferiors (int (*) (struct inferior *, /* Returns true if the inferior list is not empty. */ extern int have_inferiors (void); +/* Returns the number of live inferiors (real live processes). */ +extern int number_of_live_inferiors (void); + /* Returns true if there are any live inferiors in the inferior list (not cores, not executables, real live processes). */ extern int have_live_inferiors (void); diff --git a/gdb/remote.c b/gdb/remote.c index 52c5df8..1190522 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -119,12 +119,12 @@ struct remote_state; static int remote_vkill (int pid, struct remote_state *rs); +static void remote_kill_k (void); + static void remote_mourn (struct target_ops *ops); static void extended_remote_restart (void); -static void extended_remote_mourn (struct target_ops *); - static void remote_send (char **buf, long *sizeof_buf_p); static int readchar (int timeout); @@ -1918,7 +1918,8 @@ demand_private_info (ptid_t ptid) info->priv = XNEW (struct private_thread_info); info->private_dtor = free_private_thread_info; info->priv->core = -1; - info->priv->extra = 0; + info->priv->extra = NULL; + info->priv->name = NULL; } return info->priv; @@ -2097,7 +2098,7 @@ set_general_process (void) struct remote_state *rs = get_remote_state (); /* If the remote can't handle multiple processes, don't bother. */ - if (!rs->extended || !remote_multi_process_p (rs)) + if (!remote_multi_process_p (rs)) return; /* We only need to change the remote current thread if it's pointing @@ -4609,18 +4610,15 @@ 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+"); - if (packet_set_cmd_state (PACKET_exec_event_feature) - != AUTO_BOOLEAN_FALSE) - q = remote_query_supported_append (q, "exec-events+"); - } + 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+"); + if (packet_set_cmd_state (PACKET_exec_event_feature) + != AUTO_BOOLEAN_FALSE) + q = remote_query_supported_append (q, "exec-events+"); if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE)