From patchwork Thu Aug 7 17:59:54 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 2348 Received: (qmail 27235 invoked by alias); 7 Aug 2014 18:01:07 -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 27170 invoked by uid 89); 7 Aug 2014 18:01:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00 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; Thu, 07 Aug 2014 18:01:04 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1XFRzp-0007gT-8I from donb@codesourcery.com for gdb-patches@sourceware.org; Thu, 07 Aug 2014 11:01:01 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 7 Aug 2014 11:01:01 -0700 Received: from build4-lucid-cs (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.2.247.3; Thu, 7 Aug 2014 11:01:00 -0700 Received: by build4-lucid-cs (Postfix, from userid 1905) id 5A85940A74; Thu, 7 Aug 2014 11:00:59 -0700 (PDT) From: Don Breazeal To: Subject: [PATCH 09/10] Extended-remote fork catchpoints Date: Thu, 7 Aug 2014 10:59:54 -0700 Message-ID: <1407434395-19089-10-git-send-email-donb@codesourcery.com> In-Reply-To: <1407434395-19089-1-git-send-email-donb@codesourcery.com> References: <1407434395-19089-1-git-send-email-donb@codesourcery.com> MIME-Version: 1.0 X-IsSubscribed: yes 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 fork events were being reported, then catchpoints were 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. 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 gdbserver, an 'info threads' will retrieve the new thread info from the target and add it to GDB's data structures. Because of this premature report, things on the GDB side eventually get very confused. So in gdbserver, in server.c:handle_qxfer_threads_worker, we check 'last_status' and if it shows a FORK event, we know that we are in an unfollowed fork and we do not report the new (forked) thread to GDB. 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. I implemented similar code in linux-low.c:linux_kill. 3) One of the tests related to fork catchpoints, gdb.threads/fork-thread-pending.exp, depended on the threads being reported in a specific order. GDBserver reported the threads in a different order, that is, 'info threads' showed the same threads, but in a different order. The test used a hard-coded thread number to find a threads that (a) was not the main thread (blocked in pthread_join), and (b) was not the forking thread (stopped in fork). I implemented a new proc, find_unforked_thread, that uses a pretty brute-force method of finding a thread. I considered just hard-coding another number (the native case used thread 2, which was the forking thread in the remote case), but that didn't seem future-proof. Suggestions on how to do this better would be welcome. Tested on x64 Ubuntu Lucid, native, remote, extended-remote. Tested the case of killing the forking process before the fork has been followed manually. It wasn't clear to me how to check that a process had actually been killed from a dejagnu test. Thanks, --Don gdb/ 2014-08-06 Don Breazeal * remote.c (extended_remote_insert_fork_catchpoint): New function. (extended_remote_remove_fork_catchpoint): New function. (extended_remote_insert_vfork_catchpoint): New function. (extended_remote_remove_vfork_catchpoint): New function. (init_extended_remote_ops): Initialize target vector with new fork catchpoint functions. gdb/gdbserver/ 2014-08-06 Don Breazeal * linux-low.c (linux_kill): Kill fork child when between fork and follow_fork. * server.c (handle_qxfer_threads_worker): Skip fork child thread when between fork and follow_fork. (get_last_target_status): New function. * server.h: gdb/testsuite/ 2014-08-06 Don Breazeal * gdb.threads/fork-thread-pending.exp (find_unforked_thread): New proc. (main): Call new proc instead of using hard-coded thread number. --- gdb/gdbserver/linux-low.c | 18 ++++++ gdb/gdbserver/server.c | 18 ++++++ gdb/gdbserver/server.h | 2 + gdb/remote.c | 66 +++++++++++++++++++++ gdb/testsuite/gdb.threads/fork-thread-pending.exp | 23 +++++++- 5 files changed, 126 insertions(+), 1 deletions(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 69cad15..64b5528 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -1066,6 +1066,24 @@ linux_kill (int pid) { struct process_info *process; struct lwp_info *lwp; + struct target_waitstatus last; + ptid_t last_ptid; + + /* If we're stopped while forking and we haven't followed yet, + kill the child task. We need to do this first because the + parent will be sleeping if this is a vfork. */ + + get_last_target_status (&last_ptid, &last); + + if (last.kind == TARGET_WAITKIND_FORKED + || last.kind == TARGET_WAITKIND_VFORKED) + { + lwp = find_lwp_pid (last.value.related_pid); + gdb_assert (lwp != NULL); + kill_wait_lwp (lwp); + process = find_process_pid (ptid_get_pid (last.value.related_pid)); + the_target->mourn (process); + } process = find_process_pid (pid); if (process == NULL) diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index eb95dbf..66a53ca 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -1316,6 +1316,15 @@ handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg) int core = target_core_of_thread (ptid); char core_s[21]; + /* Skip new threads created as the result of a fork if we are not done + handling that fork event. We won't know whether to tell GDB about + the new thread until we are done following the fork. */ + if ((last_status.kind == TARGET_WAITKIND_FORKED + || last_status.kind == TARGET_WAITKIND_VFORKED) + && (ptid_get_pid (last_status.value.related_pid) + == ptid_get_pid (ptid))) + return; + write_ptid (ptid_s, ptid); if (core != -1) @@ -4021,3 +4030,12 @@ handle_target_event (int err, gdb_client_data client_data) return 0; } + +/* Retrieve the last waitstatus reported to GDB. */ + +void +get_last_target_status (ptid_t *ptid, struct target_waitstatus *last) +{ + *ptid = last_ptid; + *last = last_status; +} diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index ef66a32..e17eb04 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -139,6 +139,8 @@ typedef int gdb_fildes_t; /* Functions from server.c. */ extern int handle_serial_event (int err, gdb_client_data client_data); extern int handle_target_event (int err, gdb_client_data client_data); +extern void get_last_target_status (ptid_t *ptid, + struct target_waitstatus *last); #include "remote-utils.h" diff --git a/gdb/remote.c b/gdb/remote.c index 1304949..2918e54 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -112,6 +112,18 @@ static void remote_mourn (struct target_ops *ops); static void extended_remote_restart (void); +static int extended_remote_insert_fork_catchpoint (struct target_ops *ops, + int pid); + +static int extended_remote_remove_fork_catchpoint (struct target_ops *ops, + int pid); + +static int extended_remote_insert_vfork_catchpoint (struct target_ops *ops, + int pid); + +static int extended_remote_remove_vfork_catchpoint (struct target_ops *ops, + int pid); + static int extended_remote_follow_fork (struct target_ops *ops, int follow_child, int detach_fork); @@ -7812,6 +7824,52 @@ extended_remote_kill (struct target_ops *ops) target_mourn_inferior (); } +/* Insert fork catchpoint target routine. If follow-fork is enabled + then fork events are supported by the OS, and if we are in this + routine then extended mode is enabled, so the fork events will be + reported. Nothing further is required. */ + +static int +extended_remote_insert_fork_catchpoint (struct target_ops *ops, int pid) +{ + if (remote_protocol_packets[PACKET_vFollowFork].support != PACKET_DISABLE) + return 0; + + return 1; +} + +/* Remove fork catchpoint target routine. Nothing to do, just + return success. */ + +static int +extended_remote_remove_fork_catchpoint (struct target_ops *ops, int pid) +{ + return 0; +} + +/* Insert vfork catchpoint target routine. If follow-fork is enabled + then fork events are supported by the OS, and if we are in this + routine then extended mode is enabled, so fork events will be + reported. Nothing further is required. */ + +static int +extended_remote_insert_vfork_catchpoint (struct target_ops *ops, int pid) +{ + if (remote_protocol_packets[PACKET_vFollowFork].support != PACKET_DISABLE) + return 0; + + return 1; +} + +/* Remove vfork catchpoint target routine. Nothing to do, just + return success. */ + +static int +extended_remote_remove_vfork_catchpoint (struct target_ops *ops, int pid) +{ + return 0; +} + /* Target routine for follow-fork. */ static int @@ -11609,6 +11667,14 @@ 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_insert_fork_catchpoint + = extended_remote_insert_fork_catchpoint; + extended_remote_ops.to_remove_fork_catchpoint + = extended_remote_remove_fork_catchpoint; + extended_remote_ops.to_insert_vfork_catchpoint + = extended_remote_insert_vfork_catchpoint; + extended_remote_ops.to_remove_vfork_catchpoint + = extended_remote_remove_vfork_catchpoint; extended_remote_ops.to_follow_fork = extended_remote_follow_fork; extended_remote_ops.to_mourn_inferior = extended_remote_mourn; extended_remote_ops.to_detach = extended_remote_detach; diff --git a/gdb/testsuite/gdb.threads/fork-thread-pending.exp b/gdb/testsuite/gdb.threads/fork-thread-pending.exp index 57e45c9..e8c4830 100644 --- a/gdb/testsuite/gdb.threads/fork-thread-pending.exp +++ b/gdb/testsuite/gdb.threads/fork-thread-pending.exp @@ -31,6 +31,26 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executab return -1 } +# Find a thread that did not fork and is not the main thread and +# return its thread number. We can't just hard-code the thread +# number since we have no guarantee as to the ordering of the threads +# in gdb. We know that the main thread is in pthread_join and the +# forking thread is in fork, so we use this rather ungainly regexp +# to capture an entry from 'info threads' that doesn't show one of +# those routines, then extract the thread number. + +proc find_unforked_thread { } { + gdb_test_multiple "info threads" "find unforked thread" { + -re "(\[^\r]*Thread\[^\r]* in \[^fp]\[^ot]\[^rh]\[^kr]\[^e]\[^a]\[^d]\[^_]\[^j]\[^\r]*\r\n)" { + regexp "(\[ ]*)(\[0-9]*)(\[ ]*Thread\[^\r]*\r\n)" $expect_out(0,string) ignore lead_spc threadnum rest + } + timeout { + set threadnum -1 + } + } + return $threadnum +} + clean_restart ${binfile} if ![runto_main] then { @@ -46,7 +66,8 @@ gdb_test "continue" "Catchpoint.*" "1, get to the fork event" gdb_test "info threads" " Thread .* Thread .* Thread .* Thread .*" "1, multiple threads found" -gdb_test "thread 2" ".*" "1, switched away from event thread" +set threadnum [find_unforked_thread] +gdb_test "thread $threadnum" ".*" "1, switched away from event thread to thread $threadnum" gdb_test "continue" "Not resuming.*" "1, refused to resume"