RFA: close-on-exec internal file descriptors

[Tom Tromey <tromey at redhat dot com>]

A few weeks ago, someone on irc pointed out that gdb leaks file
descriptors to its child processes.  One simple way to see this is to
start gdb with an exec and then shell out:

    opsy. gdb /tmp/r
    [...]
    (gdb) shell ls -l /proc/$$/fd
    total 0
    lrwx------ 1 tromey tromey 64 2008-12-05 17:10 0 -> /dev/pts/1
    lrwx------ 1 tromey tromey 64 2008-12-05 17:10 1 -> /dev/pts/1
    lrwx------ 1 tromey tromey 64 2008-12-05 17:10 2 -> /dev/pts/1
    lr-x------ 1 tromey tromey 64 2008-12-05 17:10 3 -> pipe:[1100229]
    l-wx------ 1 tromey tromey 64 2008-12-05 17:10 4 -> pipe:[1100229]
    lr-x------ 1 tromey tromey 64 2008-12-05 17:10 5 -> /proc/8096/fd

I believe those 'pipe' entries are from the call to pipe in
linux-nat.c:linux_nat_set_async.

This patch fixes this problem by introducing new wrapper functions
which create close-on-exec file descriptors.  Then, these functions
are used everywhere in gdb.  After this patch, these wrapper functions
should be used in all new code as well.

I chose to take advantage of the new glibc flags like O_CLOEXEC when
they are available.  This is friendlier in the Python case -- here,
gdb might have multiple threads, and the glibc flags enable us to
avoid a window where a file descriptor is not marked close-on-exec.
If the new flags or functions are not available, we fall back to
fcntl.

If you're following the binutils list, you may note that I did not
take this approach in libbfd.  I plan to submit a follow-up patch
there, because I now believe this approach to be better.

Built & regtested on x86-64 (compile farm).  I believe that machine
does not have the O_CLOEXEC support.  My machine here does, though.

I didn't write a new test case.  I don't think my build includes
remote-mips.c, so that has not actually been compiled.  Also, I did
not touch gdbserver.

Please review.

thanks,
Tom

2008-11-20  Tom Tromey  <tromey@redhat.com>

	* configure, config.in: Rebuild.
	* configure.ac: Check for fileno, pipe2.
	* xml-tdesc.c (fetch_xml_from_file): Use fopen_cloexec.
	* tracepoint.c (tracepoint_save_command): Use fopen_cloexec.
	* remote.c (remote_file_put): Use fopen_cloexec.
	(remote_file_get): Likewise.
	* remote-mips.c (pmon_start_download): Use fopen_cloexec.
	* defs.h (open_cloexec, fopen_cloexec): Declare.
	(pipe_cloexec): Likewise.
	* ser-tcp.c (net_open): Use socket_cloexec.
	(socket_cloexec): New function.
	* ser-unix.c (hardwire_open): Use open_cloexec.
	* remote-fileio.c (remote_fileio_func_open): Use open_cloexec.
	* cli/cli-dump.c (fopen_with_cleanup): Use fopen_cloexec.
	* ui-file.c (gdb_fopen): Use fopen_cloexec
	* tui/tui-io.c (tui_initialize_io): Use pipe_cloexec.
	* source.c (openp): Use open_cloexec.
	(find_and_open_source): Likewise.
	* solib.c (solib_bfd_open): Use open_cloexec.
	* linux-nat.c (linux_nat_setup_async): Use pipe_cloexec.
	(pid_is_stopped): Use fopen_cloexec.
	(linux_nat_find_memory_regions): Likewise.
	(linux_nat_info_proc_cmd): Likewise.
	(linux_proc_pending_signals): Likewise.
	* corelow.c (core_open): Use open_cloexec.
	* utils.c (FD_CLOEXEC): New define.
	(open_cloexec): New function.
	(fopen_cloexec): Likewise.
	(pipe_cloexec): Likewise.
	(close_on_exec): Likewise.

diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index d5dbc97..ba78202 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -105,7 +105,7 @@ scan_filename_with_cleanup (char **cmd, const char *defname)
 FILE *
 fopen_with_cleanup (const char *filename, const char *mode)
 {
-  FILE *file = fopen (filename, mode);
+  FILE *file = fopen_cloexec (filename, mode);
   if (file == NULL)
     perror_with_name (filename);
   make_cleanup_fclose (file);
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 8725aa6..0b046f6 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -775,7 +775,7 @@ AC_FUNC_VFORK
 AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
                 getgid poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
-		ttrace wborder])
+		ttrace wborder pipe2 fileno])
 
 # Check the return and argument types of ptrace.  No canned test for
 # this, so roll our own.
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 35c998c..c03a130 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -300,7 +300,7 @@ core_open (char *filename, int from_tty)
     flags |= O_RDWR;
   else
     flags |= O_RDONLY;
-  scratch_chan = open (filename, flags, 0);
+  scratch_chan = open_cloexec (filename, flags, 0);
   if (scratch_chan < 0)
     perror_with_name (filename);
 
diff --git a/gdb/defs.h b/gdb/defs.h
index b047266..47516b3 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -409,6 +409,14 @@ char *ldirname (const char *filename);
 
 char **gdb_buildargv (const char *);
 
+int open_cloexec (const char *, int, int);
+
+int pipe_cloexec (int[2]);
+
+FILE *fopen_cloexec (const char *, const char *);
+
+void close_on_exec (int);
+
 /* From demangle.c */
 
 extern void set_demangling_style (char *);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 913bfec..6c45bb7 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1152,7 +1152,7 @@ pid_is_stopped (pid_t pid)
   int retval = 0;
 
   snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
-  status_file = fopen (buf, "r");
+  status_file = fopen_cloexec (buf, "r");
   if (status_file != NULL)
     {
       int have_state = 0;
@@ -3339,7 +3339,7 @@ linux_nat_find_memory_regions (int (*func) (CORE_ADDR,
 
   /* Compose the filename for the /proc memory map, and open it.  */
   sprintf (mapsfilename, "/proc/%lld/maps", pid);
-  if ((mapsfile = fopen (mapsfilename, "r")) == NULL)
+  if ((mapsfile = fopen_cloexec (mapsfilename, "r")) == NULL)
     error (_("Could not open %s."), mapsfilename);
   cleanup = make_cleanup_fclose (mapsfile);
 
@@ -3663,7 +3663,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
   if (cmdline_f || all)
     {
       sprintf (fname1, "/proc/%lld/cmdline", pid);
-      if ((procfile = fopen (fname1, "r")) != NULL)
+      if ((procfile = fopen_cloexec (fname1, "r")) != NULL)
 	{
 	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
 	  fgets (buffer, sizeof (buffer), procfile);
@@ -3694,7 +3694,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
   if (mappings_f || all)
     {
       sprintf (fname1, "/proc/%lld/maps", pid);
-      if ((procfile = fopen (fname1, "r")) != NULL)
+      if ((procfile = fopen_cloexec (fname1, "r")) != NULL)
 	{
 	  long long addr, endaddr, size, offset, inode;
 	  char permissions[8], device[8], filename[MAXPATHLEN];
@@ -3756,7 +3756,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
   if (status_f || all)
     {
       sprintf (fname1, "/proc/%lld/status", pid);
-      if ((procfile = fopen (fname1, "r")) != NULL)
+      if ((procfile = fopen_cloexec (fname1, "r")) != NULL)
 	{
 	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
 	  while (fgets (buffer, sizeof (buffer), procfile) != NULL)
@@ -3769,7 +3769,7 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
   if (stat_f || all)
     {
       sprintf (fname1, "/proc/%lld/stat", pid);
-      if ((procfile = fopen (fname1, "r")) != NULL)
+      if ((procfile = fopen_cloexec (fname1, "r")) != NULL)
 	{
 	  int itmp;
 	  char ctmp;
@@ -3966,7 +3966,7 @@ linux_proc_pending_signals (int pid, sigset_t *pending, sigset_t *blocked, sigse
   sigemptyset (blocked);
   sigemptyset (ignored);
   sprintf (fname, "/proc/%d/status", pid);
-  procfile = fopen (fname, "r");
+  procfile = fopen_cloexec (fname, "r");
   if (procfile == NULL)
     error (_("Could not open %s"), fname);
   cleanup = make_cleanup_fclose (procfile);
@@ -4552,7 +4552,7 @@ linux_nat_get_siginfo (ptid_t ptid)
 static void
 linux_nat_setup_async (void)
 {
-  if (pipe (linux_nat_event_pipe) == -1)
+  if (pipe_cloexec (linux_nat_event_pipe) == -1)
     internal_error (__FILE__, __LINE__,
 		    "creating event pipe failed.");
   fcntl (linux_nat_event_pipe[0], F_SETFL, O_NONBLOCK);
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 05a78be..65d01fd 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -655,7 +655,7 @@ remote_fileio_func_open (char *buf)
     }
 
   remote_fio_no_longjmp = 1;
-  fd = open (pathname, flags, mode);
+  fd = open_cloexec (pathname, flags, mode);
   if (fd < 0)
     {
       remote_fileio_return_errno (-1);
diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c
index b4423d5..bfb1da8 100644
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -2992,7 +2992,7 @@ pmon_start_download (void)
   if (tftp_in_use)
     {
       /* Create the temporary download file.  */
-      if ((tftp_file = fopen (tftp_localname, "w")) == NULL)
+      if ((tftp_file = fopen_cloexec (tftp_localname, "w")) == NULL)
 	perror_with_name (tftp_localname);
     }
   else
diff --git a/gdb/remote.c b/gdb/remote.c
index 06ab783..b44bdd7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8418,7 +8418,7 @@ remote_file_put (const char *local_file, const char *remote_file, int from_tty)
   if (!remote_desc)
     error (_("command can only be used with remote target"));
 
-  file = fopen (local_file, "rb");
+  file = fopen_cloexec (local_file, "rb");
   if (file == NULL)
     perror_with_name (local_file);
   back_to = make_cleanup_fclose (file);
@@ -8508,7 +8508,7 @@ remote_file_get (const char *remote_file, const char *local_file, int from_tty)
   if (fd == -1)
     remote_hostio_error (remote_errno);
 
-  file = fopen (local_file, "wb");
+  file = fopen_cloexec (local_file, "wb");
   if (file == NULL)
     perror_with_name (local_file);
   back_to = make_cleanup_fclose (file);
diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index dcf288d..8f51f45 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -61,6 +61,20 @@ void _initialize_ser_tcp (void);
 /* how many times per second to poll deprecated_ui_loop_hook */
 #define POLL_INTERVAL 2
 
+/* Like socket, but ensure that the resulting file descriptor is
+   marked close-on-exec, if possible.  */
+static int
+socket_cloexec (int domain, int type, int protocol)
+{
+#ifdef SOCK_CLOEXEC
+  return socket (domain, type | SOCK_CLOEXEC, protocol);
+#else
+  int result = socket (domain, type, protocol);
+  close_on_exec (result);
+  return result;
+#endif
+}
+
 /* Open a tcp socket */
 
 int
@@ -109,9 +123,9 @@ net_open (struct serial *scb, const char *name)
     }
 
   if (use_udp)
-    scb->fd = socket (PF_INET, SOCK_DGRAM, 0);
+    scb->fd = socket_cloexec (PF_INET, SOCK_DGRAM, 0);
   else
-    scb->fd = socket (PF_INET, SOCK_STREAM, 0);
+    scb->fd = socket_cloexec (PF_INET, SOCK_STREAM, 0);
 
   if (scb->fd < 0)
     return -1;
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 679d23e..4e920f2 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -108,7 +108,7 @@ void _initialize_ser_hardwire (void);
 static int
 hardwire_open (struct serial *scb, const char *name)
 {
-  scb->fd = open (name, O_RDWR);
+  scb->fd = open_cloexec (name, O_RDWR, 0);
   if (scb->fd < 0)
     return -1;
 
diff --git a/gdb/solib.c b/gdb/solib.c
index d04a907..acc8bdd 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -194,7 +194,7 @@ solib_bfd_open (char *in_pathname)
     }
 
   /* Now see if we can open it.  */
-  found_file = open (temp_pathname, O_RDONLY | O_BINARY, 0);
+  found_file = open_cloexec (temp_pathname, O_RDONLY | O_BINARY, 0);
 
   /* We try to find the library in various ways.  After each attempt
      (except for the one above), either found_file >= 0 and
diff --git a/gdb/source.c b/gdb/source.c
index c41c193..fbab97e 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -708,7 +708,7 @@ openp (const char *path, int opts, const char *string,
 	{
 	  filename = alloca (strlen (string) + 1);
 	  strcpy (filename, string);
-	  fd = open (filename, mode, prot);
+	  fd = open_cloexec (filename, mode, prot);
 	  if (fd >= 0)
 	    goto done;
 	}
@@ -775,7 +775,7 @@ openp (const char *path, int opts, const char *string,
 
       if (is_regular_file (filename))
 	{
-	  fd = open (filename, mode);
+	  fd = open_cloexec (filename, mode, 0);
 	  if (fd >= 0)
 	    break;
 	}
@@ -964,7 +964,7 @@ find_and_open_source (struct objfile *objfile,
           *fullname = rewritten_fullname;
         }
 
-      result = open (*fullname, OPEN_MODE);
+      result = open_cloexec (*fullname, OPEN_MODE, 0);
       if (result >= 0)
 	return result;
       /* Didn't work -- free old one, try again. */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 5c8c205..a397bb1 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2307,7 +2307,7 @@ tracepoint_save_command (char *args, int from_tty)
 
   pathname = tilde_expand (args);
   cleanup = make_cleanup (xfree, pathname);
-  if (!(fp = fopen (pathname, "w")))
+  if (!(fp = fopen_cloexec (pathname, "w")))
     error (_("Unable to open file '%s' for saving tracepoints (%s)"),
 	   args, safe_strerror (errno));
   make_cleanup_fclose (fp);
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index e25263c..3636f51 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -617,7 +617,7 @@ tui_initialize_io (void)
   /* Temporary solution for readline writing to stdout: redirect
      readline output in a pipe, read that pipe and output the content
      in the curses command window.  */
-  if (pipe (tui_readline_pipe) != 0)
+  if (pipe_cloexec (tui_readline_pipe) != 0)
     {
       fprintf_unfiltered (gdb_stderr, "Cannot create pipe for readline");
       exit (1);
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index 9a1d892..ad757be 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -515,7 +515,7 @@ stdio_fileopen (FILE *file)
 struct ui_file *
 gdb_fopen (char *name, char *mode)
 {
-  FILE *f = fopen (name, mode);
+  FILE *f = fopen_cloexec (name, mode);
   if (f == NULL)
     return NULL;
   return stdio_file_new (f, 1);
diff --git a/gdb/utils.c b/gdb/utils.c
index d14009f..2324fb0 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -66,6 +66,11 @@
 
 #include <sys/time.h>
 #include <time.h>
+#include <fcntl.h>
+
+#ifndef FD_CLOEXEC
+#define FD_CLOEXEC 1
+#endif
 
 #if !HAVE_DECL_MALLOC
 extern PTR malloc ();		/* OK: PTR */
@@ -3441,3 +3446,73 @@ gdb_buildargv (const char *s)
     nomem (0);
   return argv;
 }
+
+/* A helper function to mark a file descriptor close-on-exec.  Only
+   call this if you cannot mark the file descriptor close-on-exec at
+   creation.  */
+void
+close_on_exec (int fd)
+{
+#ifdef F_GETFD
+  if (fd >= 0)
+    {
+      int old = fcntl (fd, F_GETFD, 0);
+      if (old >= 0)
+	fcntl (fd, F_SETFD, old | FD_CLOEXEC);
+    }
+#endif
+}
+
+/* Like open, but ensure that the resulting file descriptor is marked
+   close-on-exec, if possible.  */
+int
+open_cloexec (const char *path, int flags, int mode)
+{
+#ifdef O_CLOEXEC
+  return open (path, flags | O_CLOEXEC, mode);
+#else
+  int fd = open (path, flags, mode);
+  close_on_exec (fd);
+  return fd;
+#endif
+}
+
+/* Like fopen, but ensure that the resulting file is marked
+   close-on-exec, if possible.  */
+FILE *
+fopen_cloexec (const char *path, const char *mode)
+{
+#ifdef O_CLOEXEC
+  /* We assume that O_CLOEXEC also implies the availability of the "e"
+     flag to fopen.  */
+  char new_mode[20];
+  strcpy (new_mode, mode);
+  strcat (new_mode, "e");
+  return fopen (path, new_mode);
+#else
+  FILE *result = fopen (path, mode);
+#if defined (HAVE_FILENO)
+  if (result)
+    close_on_exec (fileno (result));
+#endif
+  return result;
+#endif
+}
+
+/* Like pipe, but ensure that the resulting file descriptors are
+   marked close-on-exec, if possible.  */
+int
+pipe_cloexec (int fds[2])
+{
+#if defined (HAVE_PIPE2) && defined (O_CLOEXEC)
+  return pipe2 (fds, O_CLOEXEC);
+#else
+  int result = pipe (fds);
+  if (result != -1)
+    {
+      close_on_exec (fds[0]);
+      close_on_exec (fds[1]);
+    }
+  return result;
+#endif
+}
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 2e8c1f5..eeb85e0 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -438,11 +438,11 @@ fetch_xml_from_file (const char *filename, void *baton)
       char *fullname = concat (dirname, "/", filename, (char *) NULL);
       if (fullname == NULL)
 	nomem (0);
-      file = fopen (fullname, FOPEN_RT);
+      file = fopen_cloexec (fullname, FOPEN_RT);
       xfree (fullname);
     }
   else
-    file = fopen (filename, FOPEN_RT);
+    file = fopen_cloexec (filename, FOPEN_RT);
 
   if (file == NULL)
     return NULL;