From 77ea74b82e78974fffbfae8ba4a7046554f0ed5b Mon Sep 17 00:00:00 2001 From: Julian Seward Date: Tue, 17 May 2011 18:14:53 +0000 Subject: [PATCH] gdbserver: (#214909 c 82) ensure proper cleanup of gdbsrv FIFOs/shmem files with untraced fork/exec * syswrap-{generic|darwin|aix5}.c : in PRE(sys_execve) : terminate gdbserver * pub_core_gdbserver.h and m_gdbserver.c : add VG_(gdbserver_prerun_action), factorising the actions to do by gdbserver at "startup" (i.e. a traced fork or a traced exec). * scheduler.c : implement startup action using VG_(gdbserver_prerun_action) (Philippe Waroquiers, philippe.waroquiers@skynet.be) git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11771 --- coregrind/m_gdbserver/m_gdbserver.c | 23 +++++++++++++++++++++++ coregrind/m_scheduler/scheduler.c | 13 +------------ coregrind/m_syswrap/syswrap-aix5.c | 10 ++++++++++ coregrind/m_syswrap/syswrap-darwin.c | 10 ++++++++++ coregrind/m_syswrap/syswrap-generic.c | 11 +++++++++++ coregrind/pub_core_gdbserver.h | 8 ++++++++ 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/coregrind/m_gdbserver/m_gdbserver.c b/coregrind/m_gdbserver/m_gdbserver.c index cfde922eff..e3b0ffb71a 100644 --- a/coregrind/m_gdbserver/m_gdbserver.c +++ b/coregrind/m_gdbserver/m_gdbserver.c @@ -494,6 +494,24 @@ static void invalidate_current_ip (ThreadId tid, char *who) invalidate_if_jump_not_yet_gdbserved (VG_(get_IP) (tid), who); } +void VG_(gdbserver_prerun_action) (ThreadId tid) +{ + // Using VG_(dyn_vgdb_error) allows the user to control if gdbserver + // stops after a fork. + if (VG_(dyn_vgdb_error) == 0) { + /* The below call allows gdb to attach at startup + before the first guest instruction is executed. */ + VG_(umsg)("(action at startup) vgdb me ... \n"); + VG_(gdbserver)(tid); + } else { + /* User has activated gdbserver => initialize now the FIFOs + to let vgdb/gdb contact us either via the scheduler poll + mechanism or via vgdb ptrace-ing valgrind. */ + if (VG_(gdbserver_activity) (tid)) + VG_(gdbserver) (tid); + } +} + /* when fork is done, various cleanup is needed in the child process. In particular, child must have its own connection to avoid stealing data from its parent */ @@ -521,6 +539,11 @@ static void gdbserver_cleanup_in_child_after_fork(ThreadId me) vg_assert (gs_addresses == NULL); vg_assert (gs_watches == NULL); } + + + if (VG_(clo_trace_children)) { + VG_(gdbserver_prerun_action) (me); + } } /* If reason is init_reason, creates the connection resources (e.g. diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index 6d06947238..a5d1cfb14c 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -1042,18 +1042,7 @@ VgSchedReturnCode VG_(scheduler) ( ThreadId tid ) /* As we are initializing, VG_(dyn_vgdb_error) can't have been changed yet. */ - if (VG_(dyn_vgdb_error) == 0) { - /* The below call allows gdb to attach at startup - before the first guest instruction is executed. */ - VG_(umsg)("(action at startup) vgdb me ... \n"); - VG_(gdbserver)(1); - } else { - /* User has activated gdbserver => initialize now the FIFOs - to let vgdb/gdb contact us either via the scheduler poll - mechanism or via vgdb ptrace-ing valgrind. */ - if (VG_(gdbserver_activity) (1)) - VG_(gdbserver) (1); - } + VG_(gdbserver_prerun_action) (1); } else { VG_(disable_vgdb_poll) (); } diff --git a/coregrind/m_syswrap/syswrap-aix5.c b/coregrind/m_syswrap/syswrap-aix5.c index 8c4560682c..c765a3bdb1 100644 --- a/coregrind/m_syswrap/syswrap-aix5.c +++ b/coregrind/m_syswrap/syswrap-aix5.c @@ -45,6 +45,7 @@ #include "pub_core_xarray.h" #include "pub_core_clientstate.h" #include "pub_core_debuglog.h" +#include "pub_tool_gdbserver.h" // VG_(gdbserver) #include "pub_core_libcbase.h" #include "pub_core_libcassert.h" #include "pub_core_libcfile.h" @@ -892,6 +893,15 @@ PRE(sys_execve) /* After this point, we can't recover if the execve fails. */ VG_(debugLog)(1, "syswrap", "Exec of %s\n", (Char*)ARG1); + // Terminate gdbserver if it is active. + if (VG_(clo_vgdb) != Vg_VgdbNo) { + // If the child will not be traced, we need to terminate gdbserver + // to cleanup the gdbserver resources (e.g. the FIFO files). + // If child will be traced, we also terminate gdbserver: the new + // Valgrind will start a fresh gdbserver after exec. + VG_(gdbserver) (0); + } + /* Resistance is futile. Nuke all other threads. POSIX mandates this. (Really, nuke them all, since the new process will make its own new thread.) */ diff --git a/coregrind/m_syswrap/syswrap-darwin.c b/coregrind/m_syswrap/syswrap-darwin.c index c1884dfa92..463b33fbdb 100644 --- a/coregrind/m_syswrap/syswrap-darwin.c +++ b/coregrind/m_syswrap/syswrap-darwin.c @@ -41,6 +41,7 @@ #include "pub_core_debuglog.h" #include "pub_core_debuginfo.h" // VG_(di_notify_*) #include "pub_core_transtab.h" // VG_(discard_translations) +#include "pub_tool_gdbserver.h" // VG_(gdbserver) #include "pub_core_libcbase.h" #include "pub_core_libcassert.h" #include "pub_core_libcfile.h" @@ -2787,6 +2788,15 @@ PRE(posix_spawn) /* Ok. So let's give it a try. */ VG_(debugLog)(1, "syswrap", "Posix_spawn of %s\n", (Char*)ARG2); + // Terminate gdbserver if it is active. + if (VG_(clo_vgdb) != Vg_VgdbNo) { + // If the child will not be traced, we need to terminate gdbserver + // to cleanup the gdbserver resources (e.g. the FIFO files). + // If child will be traced, we also terminate gdbserver: the new + // Valgrind will start a fresh gdbserver after exec. + VG_(gdbserver) (tid); + } + // Set up the child's exe path. // if (trace_this_child) { diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index 4362140933..fd6927fd01 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -43,6 +43,7 @@ #include "pub_core_clientstate.h" // VG_(brk_base), VG_(brk_limit) #include "pub_core_debuglog.h" #include "pub_core_errormgr.h" +#include "pub_tool_gdbserver.h" // VG_(gdbserver) #include "pub_core_libcbase.h" #include "pub_core_libcassert.h" #include "pub_core_libcfile.h" @@ -2603,6 +2604,16 @@ PRE(sys_execve) /* After this point, we can't recover if the execve fails. */ VG_(debugLog)(1, "syswrap", "Exec of %s\n", (Char*)ARG1); + + // Terminate gdbserver if it is active. + if (VG_(clo_vgdb) != Vg_VgdbNo) { + // If the child will not be traced, we need to terminate gdbserver + // to cleanup the gdbserver resources (e.g. the FIFO files). + // If child will be traced, we also terminate gdbserver: the new + // Valgrind will start a fresh gdbserver after exec. + VG_(gdbserver) (0); + } + /* Resistance is futile. Nuke all other threads. POSIX mandates this. (Really, nuke them all, since the new process will make its own new thread.) */ diff --git a/coregrind/pub_core_gdbserver.h b/coregrind/pub_core_gdbserver.h index 27ed10e582..202d89bc84 100644 --- a/coregrind/pub_core_gdbserver.h +++ b/coregrind/pub_core_gdbserver.h @@ -32,6 +32,14 @@ #include "pub_tool_gdbserver.h" + +// After a fork or after an exec, call the below to (possibly) terminate +// the previous gdbserver and then activate a new gdbserver +// before any guest code execution, to e.g. allow the user to set +// breakpoints before execution. +// If VG_(clo_vgdb) == No, the below has no effect. +void VG_(gdbserver_prerun_action) (ThreadId tid); + // True if there is some activity from vgdb // If it returns True, then extern void VG_(gdbserver) can be called // to handle this incoming vgdb request. -- 2.47.2