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

@@ -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.

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) ();
       }

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.) */

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) {

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.) */

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.