This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[commit] Cleanup inf-ptrace.c


This provides some cleanups that touch real code.  I finally
understand why sometimes we see two exit events.  This happens if we
fork a child.  You'll then see an exit event through ptrace(), and
because we're the parent of the process.  You won't see the second
event though if you attached to the process.  It gets real trocky if
you detach from a forked child.  Then, whenever the detached inferior
exits, you'll get a spurious exit event.  The old code did handle
these, but I it seems only by accident.  The new code in
inf_ptrace_wait() is much more explicit about it.

I've tested this on all OS'es that use this file: FreeBSD, NetBSD,
OpenBSD, HP-UX, Ultrix.  It should be fine for Linux too, but I guess
with all the threading and vfork stuff, Linux will never again use
inf-ptrace.c.

Committed,

Mark

Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* gdb_ptrace.h (PT_TRACE_ME): Define to zero if not already
	defined.
	* inf-ptrace.c: Tweak comments.
	(inf_ptrace_me): Use PT_TRACE_ME instead of hardcoded zero.
	(inf_ptrace_mourn_inferior): Call waitpid.
	(inf_ptrace_attach): Use pid_t, Remove unnecessary cast.
	(inf_ptrace_detach): Use pid_t.  Use ptid_get_pid instead of
	PIDGET.
	(inf_ptrace_kill): Rename from inf_ptrace_kill_inferior.  Use
	pid_t.  Use ptid_get_pid instead of PIDGET.
	(inf_ptrace_kill): Call waitpid instead of wait.
	(inf_ptrace_resume): Use pid_t.  Use ptid_get_pid instead of
	PIDGET.
	(inf_ptrace_wait): Use waitpid instead wait.  Use pid_t.  Don't
	call target_has_exited or target_thread_alive.  Properly ignore
	terminated detached child processes.
	(inf_ptrace_has_exited): Remove function.
	(inf_ptrace_xfer_partial): Use pid_t.  Use ptid_get_pid instead of
	PIDGET.  Use gdb_byte instead of `unsigned char'.
	(inf_ptrace_thread_alive): Use ptid_get_pid instead of PIDGET.
	(inf_ptrace_pid_to_str): Remove function.
	(inf_ptrace_target): Use inf_ptrace_kill instead of
	inf_ptrace_kill_inferior.  Use normal_pid_to_str instead of
	inf_ptrace_pid_to_str.  Don't set to_has_exited.
	(inf_ptrace_fetch_register, inf_ptrace_store_register): Reformat
	long lines.

2005-07-25  Mark Kettenis  <kettenis@gnu.org>

Index: inf-ptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ptrace.c,v
retrieving revision 1.21
diff -u -p -r1.21 inf-ptrace.c
--- inf-ptrace.c 25 Jul 2005 20:19:37 -0000 1.21
+++ inf-ptrace.c 25 Jul 2005 20:33:15 -0000
@@ -41,18 +41,16 @@
 static struct target_ops *ptrace_ops_hack;
 
 
-/* Stub function which causes the inferior that runs it, to be ptrace-able
-   by its parent process.  */
+/* Prepare to be traced.  */
 
 static void
 inf_ptrace_me (void)
 {
   /* "Trace me, Dr. Memory!"  */
-  ptrace (0, 0, (PTRACE_TYPE_ARG3) 0, 0);
+  ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3)0, 0);
 }
 
-/* Stub function which causes the GDB that runs it, to start ptrace-ing
-   the child process.  */
+/* Start tracing PID.  */
 
 static void
 inf_ptrace_him (int pid)
@@ -76,10 +74,10 @@ inf_ptrace_him (int pid)
   target_post_startup_inferior (pid_to_ptid (pid));
 }
 
-/* Start an inferior Unix child process and sets inferior_ptid to its
-   pid.  EXEC_FILE is the file to run.  ALLARGS is a string containing
-   the arguments to the program.  ENV is the environment vector to
-   pass.  Errors reported with error().  */
+/* Start a new inferior Unix child process.  EXEC_FILE is the file to
+   run, ALLARGS is a string containing the arguments to the program.
+   ENV is the environment vector to pass.  If FROM_TTY is non-zero, be
+   chatty about it.  */
 
 static void
 inf_ptrace_create_inferior (char *exec_file, char *allargs, char **env,
@@ -87,26 +85,39 @@ inf_ptrace_create_inferior (char *exec_f
 {
   fork_inferior (exec_file, allargs, env, inf_ptrace_me, inf_ptrace_him,
 		 NULL, NULL);
+
   /* We are at the first instruction we care about.  */
   observer_notify_inferior_created (&current_target, from_tty);
+
   /* Pedal to the metal...  */
   proceed ((CORE_ADDR) -1, TARGET_SIGNAL_0, 0);
 }
 
+/* Clean up a rotting corpse of an inferior after it died.  */
+
 static void
 inf_ptrace_mourn_inferior (void)
 {
+  int status;
+
+  /* Wait just one more time to collect the inferior's exit status.
+     Don not check whether this succeeds though, since we may be
+     dealing with a process that we attached to.  Such a process will
+     only report its exit status to its origional parent.  */
+  waitpid (ptid_get_pid (inferior_ptid), &status, 0);
+
   unpush_target (ptrace_ops_hack);
   generic_mourn_inferior ();
 }
 
-/* Attach to process PID, then initialize for debugging it.  */
+/* Attach to the process specified by ARGS.  If FROM_TTY is non-zero,
+   be chatty about it.  */
 
 static void
 inf_ptrace_attach (char *args, int from_tty)
 {
   char *exec_file;
-  int pid;
+  pid_t pid;
   char *dummy;
 
   if (!args)
@@ -123,7 +134,7 @@ inf_ptrace_attach (char *args, int from_
 
   if (from_tty)
     {
-      exec_file = (char *) get_exec_file (0);
+      exec_file = get_exec_file (0);
 
       if (exec_file)
 	printf_unfiltered (_("Attaching to program: %s, %s\n"), exec_file,
@@ -137,7 +148,7 @@ inf_ptrace_attach (char *args, int from_
 
 #ifdef PT_ATTACH
   errno = 0;
-  ptrace (PT_ATTACH, pid, (PTRACE_TYPE_ARG3) 0, 0);
+  ptrace (PT_ATTACH, pid, (PTRACE_TYPE_ARG3)0, 0);
   if (errno != 0)
     perror_with_name (("ptrace"));
   attach_flag = 1;
@@ -153,18 +164,14 @@ inf_ptrace_attach (char *args, int from_
   observer_notify_inferior_created (&current_target, from_tty);
 }
 
-/* Take a program previously attached to and detaches it.  The program
-   resumes execution and will no longer stop on signals, etc.  We'd
-   better not have left any breakpoints in the program or it'll die
-   when it hits one.  For this to work, it may be necessary for the
-   process to have been previously attached.  It *might* work if the
-   program was started via the normal ptrace (PTRACE_TRACEME).  */
+/* Detach from the inferior, optionally passing it the signal
+   specified ARGS.  If FROM_TTY is non-zero, be chatty about it.  */
 
 static void
 inf_ptrace_detach (char *args, int from_tty)
 {
+  pid_t pid = ptid_get_pid (inferior_ptid);
   int sig = 0;
-  int pid = PIDGET (inferior_ptid);
 
   if (from_tty)
     {
@@ -179,8 +186,12 @@ inf_ptrace_detach (char *args, int from_
     sig = atoi (args);
 
 #ifdef PT_DETACH
+  /* We'd better not have left any breakpoints in the program or it'll
+     die when it hits one.  Alsno note that this may only work if we
+     previously attached to the inferior.  It *might* work if we
+     started the process ourselves.  */
   errno = 0;
-  ptrace (PT_DETACH, pid, (PTRACE_TYPE_ARG3) 1, sig);
+  ptrace (PT_DETACH, pid, (PTRACE_TYPE_ARG3)1, sig);
   if (errno != 0)
     perror_with_name (("ptrace"));
   attach_flag = 0;
@@ -192,56 +203,50 @@ inf_ptrace_detach (char *args, int from_
   unpush_target (ptrace_ops_hack);
 }
 
+/* Kill the inferior.  */
+
 static void
-inf_ptrace_kill_inferior (void)
+inf_ptrace_kill (void)
 {
+  pid_t pid = ptid_get_pid (inferior_ptid);
   int status;
-  int pid = PIDGET (inferior_ptid);
 
   if (pid == 0)
     return;
 
-  /* This once used to call "kill" to kill the inferior just in case
-     the inferior was still running.  As others have noted in the past
-     (kingdon) there shouldn't be any way to get here if the inferior
-     is still running -- else there's a major problem elsewere in GDB
-     and it needs to be fixed.
-
-     The kill call causes problems under HP-UX 10, so it's been
-     removed; if this causes problems we'll deal with them as they
-     arise.  */
-  ptrace (PT_KILL, pid, (PTRACE_TYPE_ARG3) 0, 0);
-  wait (&status);
+  ptrace (PT_KILL, pid, (PTRACE_TYPE_ARG3)0, 0);
+  waitpid (pid, &status, 0);
+
   target_mourn_inferior ();
 }
 
-/* Send a SIGINT to the process group.  This acts just like the user
-   typed a ^C on the controlling terminal.
-
-   FIXME: This may not be correct for all systems.  Some may want to
-   use killpg() instead of kill (-pgrp).  */
+/* Stop the inferior.  */
 
 static void
 inf_ptrace_stop (void)
 {
+  /* Send a SIGINT to the process group.  This acts just like the user
+     typed a ^C on the controlling terminal.  Note that using a
+     negative process number in kill() is a System V-ism.  The proper
+     BSD interface is killpg().  However, all modern BSDs support the
+     System V interface too.  */
   kill (-inferior_process_group, SIGINT);
 }
 
-/* Resume execution of the inferior process.  If STEP is nonzero,
-   single-step it.  If SIGNAL is nonzero, give it that signal.  */
+/* Resume execution of thread PTID, or all threads if PTID is -1.  If
+   STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
+   that signal.  */
 
 static void
 inf_ptrace_resume (ptid_t ptid, int step, enum target_signal signal)
 {
+  pid_t pid = ptid_get_pid (ptid);
   int request = PT_CONTINUE;
-  int pid = PIDGET (ptid);
 
   if (pid == -1)
-    /* Resume all threads.  */
-    /* I think this only gets used in the non-threaded case, where
-       "resume all threads" and "resume inferior_ptid" are the
-       same.  */
-    pid = PIDGET (inferior_ptid);
+    /* Resume all threads.  Traditionally ptrace() only supports
+       single-threaded processes, so simply resume the inferior.  */
+    pid = ptid_get_pid (inferior_ptid);
 
   if (step)
     {
@@ -255,99 +260,63 @@ inf_ptrace_resume (ptid_t ptid, int step
 
   /* An address of (PTRACE_TYPE_ARG3)1 tells ptrace to continue from
      where it was.  If GDB wanted it to start some other way, we have
-     already written a new PC value to the child.  */
+     already written a new program counter value to the child.  */
   errno = 0;
-  ptrace (request, pid, (PTRACE_TYPE_ARG3) 1, target_signal_to_host (signal));
+  ptrace (request, pid, (PTRACE_TYPE_ARG3)1, target_signal_to_host (signal));
   if (errno != 0)
     perror_with_name (("ptrace"));
 }
 
-/* Wait for child to do something.  Return pid of child, or -1 in case
-   of error; store status through argument pointer OURSTATUS.  */
+/* Wait for the child specified by PTID to do something.  Return the
+   process ID of the child, or MINUS_ONE_PTID in case of error; store
+   the status in *OURSTATUS.  */
 
 static ptid_t
 inf_ptrace_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
 {
-  int save_errno;
-  int status;
-  char *execd_pathname = NULL;
-  int exit_status;
-  int related_pid;
-  int syscall_id;
-  enum target_waitkind kind;
-  int pid;
+  pid_t pid;
+  int status, save_errno;
 
   do
     {
-      set_sigint_trap ();	/* Causes SIGINT to be passed on to the
-				   attached process. */
+      set_sigint_trap ();
       set_sigio_trap ();
 
-      pid = wait (&status);
-
-      save_errno = errno;
+      do
+	{
+	  pid = waitpid (ptid_get_pid (ptid), &status, 0);
+	  save_errno = errno;
+	}
+      while (pid == -1 && errno == EINTR);
 
       clear_sigio_trap ();
-
       clear_sigint_trap ();
 
       if (pid == -1)
 	{
-	  if (save_errno == EINTR)
-	    continue;
-
 	  fprintf_unfiltered (gdb_stderr,
-			      "Child process unexpectedly missing: %s.\n",
+			      _("Child process unexpectedly missing: %s.\n"),
 			      safe_strerror (save_errno));
 
 	  /* Claim it exited with unknown signal.  */
 	  ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
 	  ourstatus->value.sig = TARGET_SIGNAL_UNKNOWN;
-	  return pid_to_ptid (-1);
+	  return minus_one_ptid;
 	}
 
-      /* Did it exit?  */
-      if (target_has_exited (pid, status, &exit_status))
-	{
-	  /* ??rehrauer: For now, ignore this. */
-	  continue;
-	}
-
-      if (!target_thread_alive (pid_to_ptid (pid)))
-	{
-	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
-	  return pid_to_ptid (pid);
-	}
+      /* Ignore terminated detached child processes.  */
+      if (!WIFSTOPPED (status) && pid != ptid_get_pid (inferior_ptid))
+	pid = -1;
     }
-  while (pid != PIDGET (inferior_ptid)); /* Some other child died or
-                                            stopped.  */
+  while (pid == -1);
 
   store_waitstatus (ourstatus, status);
   return pid_to_ptid (pid);
 }
 
-static int
-inf_ptrace_has_exited (int pid, int wait_status, int *exit_status)
-{
-  if (WIFEXITED (wait_status))
-    {
-      *exit_status = WEXITSTATUS (wait_status);
-      return 1;
-    }
-
-  if (WIFSIGNALED (wait_status))
-    {
-      *exit_status = 0;		/* ?? Don't know what else to say here. */
-      return 1;
-    }
-
-  /* ??? Do we really need to consult the event state, too?
-     Assume the wait_state alone suffices.  */
-  return 0;
-}
-
-/* Perform a partial transfer to/from the specified object.  For
-   memory transfers, fall back to the old memory xfer functions.  */
+/* Attempt a transfer all LEN bytes starting at OFFSET between the
+   inferior's OBJECT:ANNEX space and GDB's READBUF/WRITEBUF buffer.
+   Return the number of bytes actually transferred.  */
 
 static LONGEST
 inf_ptrace_xfer_partial (struct target_ops *ops, enum target_object object,
@@ -355,6 +324,8 @@ inf_ptrace_xfer_partial (struct target_o
 			 const gdb_byte *writebuf,
 			 ULONGEST offset, LONGEST len)
 {
+  pid_t pid = ptid_get_pid (inferior_ptid);
+
   switch (object)
     {
     case TARGET_OBJECT_MEMORY:
@@ -364,7 +335,7 @@ inf_ptrace_xfer_partial (struct target_o
 	 and writing data in the traced process's address space.  */
       {
 	struct ptrace_io_desc piod;
-	
+
 	/* NOTE: We assume that there are no distinct address spaces
 	   for instruction and data.  */
 	piod.piod_op = writebuf ? PIOD_WRITE_D : PIOD_READ_D;
@@ -373,7 +344,7 @@ inf_ptrace_xfer_partial (struct target_o
 	piod.piod_len = len;
 
 	errno = 0;
-	if (ptrace (PT_IO, PIDGET (inferior_ptid), (caddr_t) &piod, 0) == 0)
+	if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0)
 	  /* Return the actual number of bytes read or written.  */
 	  return piod.piod_len;
 	/* If the PT_IO request is somehow not supported, fallback on
@@ -387,15 +358,15 @@ inf_ptrace_xfer_partial (struct target_o
 	union
 	{
 	  PTRACE_TYPE_RET word;
-	  unsigned char byte[sizeof (PTRACE_TYPE_RET)];
+	  gdb_byte byte[sizeof (PTRACE_TYPE_RET)];
 	} buffer;
 	ULONGEST rounded_offset;
 	LONGEST partial_len;
-	
+
 	/* Round the start offset down to the next long word
 	   boundary.  */
 	rounded_offset = offset & -(ULONGEST) sizeof (PTRACE_TYPE_RET);
-	
+
 	/* Since ptrace will transfer a single word starting at that
 	   rounded_offset the partial_len needs to be adjusted down to
 	   that (remember this function only does a single transfer).
@@ -404,7 +375,7 @@ inf_ptrace_xfer_partial (struct target_o
 	partial_len = (rounded_offset + sizeof (PTRACE_TYPE_RET)) - offset;
 	if (partial_len > len)
 	  partial_len = len;
-	
+
 	if (writebuf)
 	  {
 	    /* If OFFSET:PARTIAL_LEN is smaller than
@@ -414,42 +385,41 @@ inf_ptrace_xfer_partial (struct target_o
 		|| (offset + partial_len
 		    < rounded_offset + sizeof (PTRACE_TYPE_RET)))
 	      /* Need part of initial word -- fetch it.  */
-	      buffer.word = ptrace (PT_READ_I, PIDGET (inferior_ptid),
-				    (PTRACE_TYPE_ARG3) (long) rounded_offset,
-				    0);
-	    
+	      buffer.word = ptrace (PT_READ_I, pid,
+				    (PTRACE_TYPE_ARG3)(long)rounded_offset, 0);
+
 	    /* Copy data to be written over corresponding part of
 	       buffer.  */
 	    memcpy (buffer.byte + (offset - rounded_offset),
 		    writebuf, partial_len);
-	    
+
 	    errno = 0;
-	    ptrace (PT_WRITE_D, PIDGET (inferior_ptid),
-		    (PTRACE_TYPE_ARG3) (long) rounded_offset,
-		    buffer.word);
+	    ptrace (PT_WRITE_D, pid,
+		    (PTRACE_TYPE_ARG3)(long)rounded_offset, buffer.word);
 	    if (errno)
 	      {
 		/* Using the appropriate one (I or D) is necessary for
 		   Gould NP1, at least.  */
 		errno = 0;
-		ptrace (PT_WRITE_I, PIDGET (inferior_ptid),
-			(PTRACE_TYPE_ARG3) (long) rounded_offset,
-			buffer.word);
+		ptrace (PT_WRITE_I, pid,
+			(PTRACE_TYPE_ARG3)(long)rounded_offset, buffer.word);
 		if (errno)
 		  return 0;
 	      }
 	  }
+
 	if (readbuf)
 	  {
 	    errno = 0;
-	    buffer.word = ptrace (PT_READ_I, PIDGET (inferior_ptid),
-				  (PTRACE_TYPE_ARG3) (long) rounded_offset, 0);
+	    buffer.word = ptrace (PT_READ_I, pid,
+				  (PTRACE_TYPE_ARG3)(long)rounded_offset, 0);
 	    if (errno)
 	      return 0;
 	    /* Copy appropriate bytes out of the buffer.  */
 	    memcpy (readbuf, buffer.byte + (offset - rounded_offset),
 		    partial_len);
 	  }
+
 	return partial_len;
       }
 
@@ -467,18 +437,13 @@ inf_ptrace_xfer_partial (struct target_o
     }
 }
 
-/* Check to see if the given thread is alive.
-
-   FIXME: Is kill() ever the right way to do this?  I doubt it, but
-   for now we're going to try and be compatable with the old thread
-   code.  */
+/* Return non-zero if the thread specified by PTID is alive.  */
 
 static int
 inf_ptrace_thread_alive (ptid_t ptid)
 {
-  pid_t pid = PIDGET (ptid);
-
-  return (kill (pid, 0) != -1);
+  /* ??? Is kill the right way to do this?  */
+  return (kill (ptid_get_pid (ptid), 0) != -1);
 }
 
 /* Print status information about what we're accessing.  */
@@ -486,15 +451,9 @@ inf_ptrace_thread_alive (ptid_t ptid)
 static void
 inf_ptrace_files_info (struct target_ops *ignore)
 {
-  printf_unfiltered (_("\tUsing the running image of %s %s.\n"),
-		     attach_flag ? "attached" : "child",
-		     target_pid_to_str (inferior_ptid));
-}
-
-static char *
-inf_ptrace_pid_to_str (ptid_t ptid)
-{
-  return normal_pid_to_str (ptid);
+  printf_filtered (_("\tUsing the running image of %s %s.\n"),
+		   attach_flag ? "attached" : "child",
+		   target_pid_to_str (inferior_ptid));
 }
 
 /* Create a prototype ptrace target.  The client can override it with
@@ -510,22 +469,20 @@ inf_ptrace_target (void)
   t->to_resume = inf_ptrace_resume;
   t->to_wait = inf_ptrace_wait;
   t->to_files_info = inf_ptrace_files_info;
-  t->to_kill = inf_ptrace_kill_inferior;
+  t->to_kill = inf_ptrace_kill;
   t->to_create_inferior = inf_ptrace_create_inferior;
   t->to_mourn_inferior = inf_ptrace_mourn_inferior;
   t->to_thread_alive = inf_ptrace_thread_alive;
-  t->to_pid_to_str = inf_ptrace_pid_to_str;
+  t->to_pid_to_str = normal_pid_to_str;
   t->to_stop = inf_ptrace_stop;
   t->to_xfer_partial = inf_ptrace_xfer_partial;
 
-  t->to_has_exited = inf_ptrace_has_exited;
-
   ptrace_ops_hack = t;
   return t;
 }
 
 
-/* Pointer to a function that returns the oggset within the user area
+/* Pointer to a function that returns the offset within the user area
    where a particular register is stored.  */
 static CORE_ADDR (*inf_ptrace_register_u_offset)(int);
 
@@ -556,10 +513,10 @@ inf_ptrace_fetch_register (int regnum)
   for (i = 0; i < size / sizeof (PTRACE_TYPE_RET); i++)
     {
       errno = 0;
-      buf[i] = ptrace (PT_READ_U, pid, (PTRACE_TYPE_ARG3) addr, 0);
+      buf[i] = ptrace (PT_READ_U, pid, (PTRACE_TYPE_ARG3)addr, 0);
       if (errno != 0)
-	error (_("Couldn't read register %s (#%d): %s."), REGISTER_NAME (regnum),
-	       regnum, safe_strerror (errno));
+	error (_("Couldn't read register %s (#%d): %s."),
+	       REGISTER_NAME (regnum), regnum, safe_strerror (errno));
 
       addr += sizeof (PTRACE_TYPE_RET);
     }
@@ -607,10 +564,10 @@ inf_ptrace_store_register (int regnum)
   for (i = 0; i < size / sizeof (PTRACE_TYPE_RET); i++)
     {
       errno = 0;
-      ptrace (PT_WRITE_U, pid, (PTRACE_TYPE_ARG3) addr, buf[i]);
+      ptrace (PT_WRITE_U, pid, (PTRACE_TYPE_ARG3)addr, buf[i]);
       if (errno != 0)
-	error (_("Couldn't write register %s (#%d): %s."), REGISTER_NAME (regnum),
-	       regnum, safe_strerror (errno));
+	error (_("Couldn't write register %s (#%d): %s."),
+	       REGISTER_NAME (regnum), regnum, safe_strerror (errno));
 
       addr += sizeof (PTRACE_TYPE_RET);
     }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]