From 17d8546e6067d0133f19275d1ccc25cf93cfc7bc Mon Sep 17 00:00:00 2001 From: Don Breazeal Date: Wed, 26 Aug 2015 13:31:59 -0700 Subject: [PATCH] Fix native follow-exec-mode "new" This patch fixes a segmentation fault in native GDB when handling an exec event with follow-exec-mode set to "new". The stack trace from the segfault was this: 0 0x0000000000669594 in gdbarch_data (gdbarch=0x0, data=0x20da7a0) at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/gdbarch.c:4847 1 0x00000000004d430e in get_remote_arch_state () at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/remote.c:603 2 0x00000000004d431e in get_remote_state () at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/remote.c:616 3 0x00000000004dda8b in discard_pending_stop_replies (inf=0x217c710) at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/remote.c:5775 4 0x00000000006a5928 in observer_inferior_exit_notification_stub ( data=0x4dda7a , args_data=0x7fff12c258f0) at ./observer.inc:1137 5 0x00000000006a419a in generic_observer_notify (subject=0x21dfbe0, args=0x7fff12c258f0) at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/observer.c:167 6 0x00000000006a59ba in observer_notify_inferior_exit (inf=0x217c710) at ./observer.inc:1162 7 0x00000000007981d5 in exit_inferior_1 (inftoex=0x217c710, silent=1) at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/inferior.c:244 8 0x00000000007982f2 in exit_inferior_num_silent (num=1) at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/inferior.c:286 9 0x000000000062f93d in follow_exec (ptid=..., execd_pathname=0x7fff12c259a0 "/scratch/dbreazea/sandbox/exec-nat/build/gdb/testsuite/gdb.base/execd-prog") at /scratch/dbreazea/sandbox/exec-nat/binutils-gdb/gdb/infrun.c:1195 In follow_exec we were creating a new inferior for the execd program, as required by the exec mode, but we were doing it before calling exit_inferior_num_silent on the original inferior. So on entry to exit_inferior_num_silent we had two inferiors with the same ptid. In the calls made by exit_inferior_num_silent, the current inferior is temporarily saved and replaced in order to make use of functions that only operate on the current inferior (for example, in do_all_continuations, called while deleting the threads of the original inferior). When we restored the original inferior, we just took the first inferior that matched the ptid of the original and got the new (wrong) one. It hadn't been initialized yet and had no gdbarch pointer, and GDB segfaulted. The fix for that is to call exit_inferior_num_silent before adding the new inferior, so that we never have two inferiors with the same ptid. Then exit_inferior_num_silent uses the original inferior as the current inferior throughout, and can find a valid gdbarch pointer. Once we have finished with the exit of the old inferior and added the new one, we need to create a new thread for the new inferior. In the function that called follow_exec, handle_inferior_event_1, ecs->event_thread now points to the thread that was deleted with the exit of the original inferior. To remedy this we create the new thread, and once we return from follow_exec we reset ecs->event_thread. Note that we are guaranteed that we can reset ecs->event_thread safely using inferior_thread because we have set the current inferior in follow_exec, and inferior_ptid was set by the call to context_switch at the beginning of exec event handling. gdb/ChangeLog: * infrun.c (follow_exec): Re-order operations for handling follow-exec-mode "new". (handle_inferior_event_1): Assign ecs->event_thread to the current thread. * remote.c (get_remote_arch_state): Add an assertion. --- gdb/ChangeLog | 8 ++++++++ gdb/infrun.c | 15 ++++++++++++--- gdb/remote.c | 1 + 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 6fa5ba5..b9493e4 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2015-08-25 Don Breazeal + + * infrun.c (follow_exec): Re-order operations for + handling follow-exec-mode "new". + (handle_inferior_event_1): Assign ecs->event_thread + to the current thread. + * remote.c (get_remote_arch_state): Add an assertion. + 2015-08-26 Pedro Alves * MAINTAINERS: Add Markus Metzger as btrace maintainer. diff --git a/gdb/infrun.c b/gdb/infrun.c index 1e224b1..c3e7dd7 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1187,12 +1187,16 @@ follow_exec (ptid_t ptid, char *execd_pathname) /* The user wants to keep the old inferior and program spaces around. Create a new fresh one, and switch to it. */ - inf = add_inferior (current_inferior ()->pid); + /* Do exit processing for the original inferior before adding + the new inferior so we don't have two active inferiors with + the same ptid, which can confuse find_inferior_ptid. */ + exit_inferior_num_silent (current_inferior ()->num); + + inf = add_inferior (pid); pspace = add_program_space (maybe_new_address_space ()); inf->pspace = pspace; inf->aspace = pspace->aspace; - - exit_inferior_num_silent (current_inferior ()->num); + add_thread (ptid); set_current_inferior (inf); set_current_program_space (pspace); @@ -4978,6 +4982,11 @@ Cannot fill $_exitsignal with the correct signal number.\n")); stop. */ follow_exec (inferior_ptid, ecs->ws.value.execd_pathname); + /* In follow_exec we may have deleted the original thread and + created a new one. Make sure that the event thread is the + execd thread for that case (this is a nop otherwise). */ + ecs->event_thread = inferior_thread (); + ecs->event_thread->control.stop_bpstat = bpstat_stop_status (get_regcache_aspace (get_current_regcache ()), stop_pc, ecs->ptid, &ecs->ws); diff --git a/gdb/remote.c b/gdb/remote.c index e019277..6c9a1b3 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -600,6 +600,7 @@ static struct gdbarch_data *remote_gdbarch_data_handle; static struct remote_arch_state * get_remote_arch_state (void) { + gdb_assert (target_gdbarch () != NULL); return gdbarch_data (target_gdbarch (), remote_gdbarch_data_handle); } -- 2.7.4