From 465a859e0a991d3bfe8a9ee65a29a223c42e2ce7 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 21 Aug 2015 19:52:36 +0100 Subject: [PATCH] Fix gdbserver crash exposed by gdb.threads/process-dies-while-handling-bp.exp Running that test in a loop, I found a gdbserver core dump with the following back trace: Core was generated by `../gdbserver/gdbserver --once --multi :2346'. Program terminated with signal SIGSEGV, Segmentation fault. #0 0x0000000000406ab6 in inferior_regcache_data (inferior=0x0) at src/gdb/gdbserver/inferiors.c:236 236 return inferior->regcache_data; (gdb) up #1 0x0000000000406d7f in get_thread_regcache (thread=0x0, fetch=1) at src/gdb/gdbserver/regcache.c:31 31 regcache = (struct regcache *) inferior_regcache_data (thread); (gdb) bt #0 0x0000000000406ab6 in inferior_regcache_data (inferior=0x0) at src/gdb/gdbserver/inferiors.c:236 #1 0x0000000000406d7f in get_thread_regcache (thread=0x0, fetch=1) at src/gdb/gdbserver/regcache.c:31 #2 0x0000000000409271 in prepare_resume_reply (buf=0x20dd593 "", ptid=..., status=0x20edce0) at src/gdb/gdbserver/remote-utils.c:1147 #3 0x000000000040ab0a in vstop_notif_reply (event=0x20edcc0, own_buf=0x20dd590 "T05") at src/gdb/gdbserver/server.c:183 #4 0x0000000000426b38 in notif_write_event (notif=0x66e6c0 , own_buf=0x20dd590 "T05") at src/gdb/gdbserver/notif.c:69 #5 0x0000000000426c55 in handle_notif_ack (own_buf=0x20dd590 "T05", packet_len=8) at src/gdb/gdbserver/notif.c:113 #6 0x000000000041118f in handle_v_requests (own_buf=0x20dd590 "T05", packet_len=8, new_packet_len=0x7fff742c77b8) at src/gdb/gdbserver/server.c:2862 #7 0x0000000000413850 in process_serial_event () at src/gdb/gdbserver/server.c:4148 #8 0x0000000000413945 in handle_serial_event (err=0, client_data=0x0) at src/gdb/gdbserver/server.c:4196 #9 0x000000000041a1ef in handle_file_event (event_file_desc=5) at src/gdb/gdbserver/event-loop.c:429 #10 0x00000000004199b6 in process_event () at src/gdb/gdbserver/event-loop.c:184 #11 0x000000000041a735 in start_event_loop () at src/gdb/gdbserver/event-loop.c:547 #12 0x00000000004123d2 in captured_main (argc=4, argv=0x7fff742c7ac8) at src/gdb/gdbserver/server.c:3562 #13 0x000000000041252e in main (argc=4, argv=0x7fff742c7ac8) at src/gdb/gdbserver/server.c:3631 Clearly this means that a thread pushed a stop reply in the event queue, and then before GDB confused the event, the whole process died, along with its thread. But the pending thread event was left dangling. When GDB fetched that event, gdbserver looked up the corresponding thread, but found NULL; not expecting this, gdbserver crashes when it tries to read this thread's registers. gdb/gdbserver/ 2015-08-21 Pedro Alves PR gdb/18749 * inferiors.c (remove_thread): Discard any pending stop reply for this thread. * server.c (remove_all_on_match_pid): Rename to ... (remove_all_on_match_ptid): ... this. Work with a filter ptid instead of a pid. (discard_queued_stop_replies): Change parameter to a ptid. Now extern. (handle_v_kill, kill_inferior_callback) (process_serial_event): Adjust. (captured_main): Call initialize_notif before starting the program, thus before threads are created. * server.h (discard_queued_stop_replies): Declare. --- gdb/gdbserver/ChangeLog | 14 ++++++++++++++ gdb/gdbserver/inferiors.c | 1 + gdb/gdbserver/server.c | 36 +++++++++++++++++------------------- gdb/gdbserver/server.h | 3 +++ 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 20b3e1c1c39..a11461ddb23 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,17 @@ +2015-08-21 Pedro Alves + + PR gdb/18749 + * inferiors.c (remove_thread): Discard any pending stop reply for + this thread. + * server.c (remove_all_on_match_pid): Rename to ... + (remove_all_on_match_ptid): ... this. Work with a filter ptid + instead of a pid. + (discard_queued_stop_replies): Change parameter to a ptid. Now + extern. + (handle_v_kill, kill_inferior_callback, captured_main) + (process_serial_event): Adjust. + * server.h (discard_queued_stop_replies): Declare. + 2015-08-21 Pedro Alves * linux-low.c (wait_for_sigstop): Always switch to no thread diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c index d7ad347eeed..3659df14394 100644 --- a/gdb/gdbserver/inferiors.c +++ b/gdb/gdbserver/inferiors.c @@ -165,6 +165,7 @@ remove_thread (struct thread_info *thread) if (thread->btrace != NULL) target_disable_btrace (thread->btrace); + discard_queued_stop_replies (ptid_of (thread)); remove_inferior (&all_threads, (struct inferior_list_entry *) thread); free_one_thread (&thread->entry); } diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 58907ec4a9f..34fcd2b0083 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -146,15 +146,15 @@ queue_stop_reply (ptid_t ptid, struct target_waitstatus *status) } static int -remove_all_on_match_pid (QUEUE (notif_event_p) *q, - QUEUE_ITER (notif_event_p) *iter, - struct notif_event *event, - void *data) +remove_all_on_match_ptid (QUEUE (notif_event_p) *q, + QUEUE_ITER (notif_event_p) *iter, + struct notif_event *event, + void *data) { - int *pid = data; + ptid_t filter_ptid = *(ptid_t *) data; + struct vstop_notif *vstop_event = (struct vstop_notif *) event; - if (*pid == -1 - || ptid_get_pid (((struct vstop_notif *) event)->ptid) == *pid) + if (ptid_match (vstop_event->ptid, filter_ptid)) { if (q->free_func != NULL) q->free_func (event); @@ -165,14 +165,13 @@ remove_all_on_match_pid (QUEUE (notif_event_p) *q, return 1; } -/* Get rid of the currently pending stop replies for PID. If PID is - -1, then apply to all processes. */ +/* See server.h. */ -static void -discard_queued_stop_replies (int pid) +void +discard_queued_stop_replies (ptid_t ptid) { QUEUE_iterate (notif_event_p, notif_stop.queue, - remove_all_on_match_pid, &pid); + remove_all_on_match_ptid, &ptid); } static void @@ -2784,7 +2783,7 @@ handle_v_kill (char *own_buf) last_status.kind = TARGET_WAITKIND_SIGNALLED; last_status.value.sig = GDB_SIGNAL_KILL; last_ptid = pid_to_ptid (pid); - discard_queued_stop_replies (pid); + discard_queued_stop_replies (last_ptid); write_ok (own_buf); return 1; } @@ -3211,7 +3210,7 @@ kill_inferior_callback (struct inferior_list_entry *entry) int pid = ptid_get_pid (process->entry.id); kill_inferior (pid); - discard_queued_stop_replies (pid); + discard_queued_stop_replies (pid_to_ptid (pid)); } /* Callback for for_each_inferior to detach or kill the inferior, @@ -3230,7 +3229,7 @@ detach_or_kill_inferior_callback (struct inferior_list_entry *entry) else kill_inferior (pid); - discard_queued_stop_replies (pid); + discard_queued_stop_replies (pid_to_ptid (pid)); } /* for_each_inferior callback for detach_or_kill_for_exit to print @@ -3487,6 +3486,7 @@ captured_main (int argc, char *argv[]) initialize_event_loop (); if (target_supports_tracepoints ()) initialize_tracepoint (); + initialize_notif (); own_buf = xmalloc (PBUFSIZ + 1); mem_buf = xmalloc (PBUFSIZ); @@ -3523,8 +3523,6 @@ captured_main (int argc, char *argv[]) } make_cleanup (detach_or_kill_for_exit_cleanup, NULL); - initialize_notif (); - /* Don't report shared library events on the initial connection, even if some libraries are preloaded. Avoids the "stopped by shared library event" notice on gdb side. */ @@ -3575,7 +3573,7 @@ captured_main (int argc, char *argv[]) /* Get rid of any pending statuses. An eventual reconnection (by the same GDB instance or another) will refresh all its state from scratch. */ - discard_queued_stop_replies (-1); + discard_queued_stop_replies (minus_one_ptid); for_each_inferior (&all_threads, clear_pending_status_callback); @@ -3823,7 +3821,7 @@ process_serial_event (void) write_enn (own_buf); else { - discard_queued_stop_replies (pid); + discard_queued_stop_replies (pid_to_ptid (pid)); write_ok (own_buf); if (extended_protocol) diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index 9080151dd9d..6020d7206d6 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -115,6 +115,9 @@ typedef int gdb_fildes_t; extern int handle_serial_event (int err, gdb_client_data client_data); extern int handle_target_event (int err, gdb_client_data client_data); +/* Get rid of the currently pending stop replies that match PTID. */ +extern void discard_queued_stop_replies (ptid_t ptid); + #include "remote-utils.h" #include "utils.h" -- 2.30.2