[RFC] [gdbserver/win32] Enable detaching on MinGW.

[RFC] [gdbserver/win32] Enable detaching on MinGW.

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Hi all,

Currently detach support is skipped on MinGW (and mingw32ce),
because there is no waitpid on Windows.  This patch enables it
by implementing the waiting for the inferior to exit in a new
target_op::join.

One thing that annoys me, is that requesting for detach kills the
inferior if detaching isn't supported.  The patch makes gdbserver
return an error (E packet) to gdb, so gdb knows it shouldn't
stop debugging.  The user can then kill the inferior is he wants.

What do you think of this behavior?

Is the patch OK?

Cheers,
Pedro Alves



[-- Attachment #2: gdbserver_detach.diff --]
[-- Type: text/x-diff, Size: 8160 bytes --]

2007-05-08  Pedro Alves  <pedro_alves@portugalmail.pt>

gdb/
	* remote.c (remote_detach): Error out if remote can't detach.

gdb/gdbserver/

	* target.h (target_ops): Change return type of detach to int.
	Add join.
	(join_inferior): New.
	* server.c (main): Don't skip detach support on mingw32.
	If the inferior doesn't support detaching return error.
	Call join_inferior instead of using waitpid.

	* linux-low.c (linux_join): New.
	(linux_target_op): Add linux_join.
	* spu-low.c (spu_join): New.
	(spu_target_ops): Add spu_join.
	* win32-low.c (win32_detach): Adapt to new interface.
	Reopen current_process_handle before detaching.  Issue a child
	continue before detaching.
	(win32_join): New.
	(win32_target_op): Add win32_join.

---
 gdb/gdbserver/linux-low.c |   14 +++++++++++
 gdb/gdbserver/server.c    |   37 ++++++++++++-----------------
 gdb/gdbserver/spu-low.c   |   13 ++++++++++
 gdb/gdbserver/target.h    |   12 ++++++++-
 gdb/gdbserver/win32-low.c |   57 ++++++++++++++++++++++++++++++++++++++++------
 gdb/remote.c              |    6 ++++
 6 files changed, 108 insertions(+), 31 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2007-05-08 01:52:40.000000000 +0100
+++ src/gdb/gdbserver/linux-low.c	2007-05-08 02:01:02.000000000 +0100
@@ -293,6 +293,19 @@ linux_detach (void)
   for_each_inferior (&all_threads, linux_detach_one_process);
 }
 
+static void
+linux_join (void)
+{
+  extern unsigned long signal_pid;
+  int status, ret;
+
+  do {
+    ret = waitpid (signal_pid, &status, 0);
+    if (WIFEXITED (status) || WIFSIGNALED (status))
+      break;
+  } while (ret != -1 || errno != ECHILD);
+}
+
 /* Return nonzero if the given thread is still alive.  */
 static int
 linux_thread_alive (unsigned long tid)
@@ -1656,6 +1669,7 @@ static struct target_ops linux_target_op
   linux_attach,
   linux_kill,
   linux_detach,
+  linux_join,
   linux_thread_alive,
   linux_resume,
   linux_wait,
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2007-05-08 01:52:40.000000000 +0100
+++ src/gdb/gdbserver/server.c	2007-05-08 02:01:02.000000000 +0100
@@ -805,32 +805,27 @@ main (int argc, char *argv[])
 	    case 'Q':
 	      handle_general_set (own_buf);
 	      break;
-#ifndef USE_WIN32API
-	    /* Skip "detach" support on mingw32, since we don't have
-	       waitpid.  */
 	    case 'D':
 	      fprintf (stderr, "Detaching from inferior\n");
-	      detach_inferior ();
-	      write_ok (own_buf);
-	      putpkt (own_buf);
-	      remote_close ();
-
-	      /* If we are attached, then we can exit.  Otherwise, we need to
-		 hang around doing nothing, until the child is gone.  */
-	      if (!attached)
-		{
-		  int status, ret;
-
-		  do {
-		    ret = waitpid (signal_pid, &status, 0);
-		    if (WIFEXITED (status) || WIFSIGNALED (status))
-		      break;
-		  } while (ret != -1 || errno != ECHILD);
+	      if (detach_inferior () != 0)
+		{
+		  write_enn (own_buf);
+		  putpkt (own_buf);
 		}
+	      else
+		{
+		  write_ok (own_buf);
+		  putpkt (own_buf);
+		  remote_close ();
 
-	      exit (0);
-#endif
+		  /* If we are attached, then we can exit.  Otherwise, we
+		     need to hang around doing nothing, until the child
+		     is gone.  */
+		  if (!attached)
+		    join_inferior ();
 
+		  exit (0);
+		}
 	    case '!':
 	      if (attached == 0)
 		{
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2007-05-08 01:52:40.000000000 +0100
+++ src/gdb/gdbserver/spu-low.c	2007-05-08 02:01:02.000000000 +0100
@@ -320,6 +320,18 @@ spu_detach (void)
   ptrace (PTRACE_DETACH, current_tid, 0, 0);
 }
 
+static void
+spu_join (void)
+{
+  int status, ret;
+
+  do {
+    ret = waitpid (current_tid, &status, 0);
+    if (WIFEXITED (status) || WIFSIGNALED (status))
+      break;
+  } while (ret != -1 || errno != ECHILD);
+}
+
 /* Return nonzero if the given thread is still alive.  */
 static int
 spu_thread_alive (unsigned long tid)
@@ -567,6 +579,7 @@ static struct target_ops spu_target_ops 
   spu_attach,
   spu_kill,
   spu_detach,
+  spu_join,
   spu_thread_alive,
   spu_resume,
   spu_wait,
Index: src/gdb/gdbserver/target.h
===================================================================
--- src.orig/gdb/gdbserver/target.h	2007-05-08 01:56:06.000000000 +0100
+++ src/gdb/gdbserver/target.h	2007-05-08 02:01:02.000000000 +0100
@@ -69,9 +69,14 @@ struct target_ops
 
   void (*kill) (void);
 
-  /* Detach from all inferiors.  */
+  /* Detach from all inferiors.
+     Return -1 on failure, and 0 on success.  */
 
-  void (*detach) (void);
+  int (*detach) (void);
+
+  /* Wait for inferiors to end.  */
+
+  void (*join) (void);
 
   /* Return 1 iff the thread with process ID PID is alive.  */
 
@@ -207,6 +212,9 @@ void set_target_ops (struct target_ops *
 #define store_inferior_registers(regno) \
   (*the_target->store_registers) (regno)
 
+#define join_inferior() \
+  (*the_target->join) ()
+
 unsigned char mywait (char *statusp, int connected_wait);
 
 int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
Index: src/gdb/gdbserver/win32-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-low.c	2007-05-08 01:52:40.000000000 +0100
+++ src/gdb/gdbserver/win32-low.c	2007-05-08 02:01:02.000000000 +0100
@@ -683,9 +683,11 @@ win32_kill (void)
 }
 
 /* Detach from all inferiors.  */
-static void
+static int
 win32_detach (void)
 {
+  HANDLE h;
+
   winapi_DebugActiveProcessStop DebugActiveProcessStop = NULL;
   winapi_DebugSetProcessKillOnExit DebugSetProcessKillOnExit = NULL;
 #ifdef _WIN32_WCE
@@ -696,13 +698,53 @@ win32_detach (void)
   DebugActiveProcessStop = GETPROCADDRESS (dll, DebugActiveProcessStop);
   DebugSetProcessKillOnExit = GETPROCADDRESS (dll, DebugSetProcessKillOnExit);
 
-  if (DebugSetProcessKillOnExit != NULL)
-    DebugSetProcessKillOnExit (FALSE);
+  if (DebugSetProcessKillOnExit == NULL
+      || DebugActiveProcessStop == NULL)
+    return -1;
+
+  /* We need a new handle, since DebugActiveProcessStop
+     closes all the ones that came through the events.  */
+  if ((h = OpenProcess (PROCESS_ALL_ACCESS,
+			FALSE,
+			current_process_id)) == NULL)
+    {
+      /* The process died.  */
+      return -1;
+    }
+
+  {
+    struct thread_resume resume;
+    resume.thread = -1;
+    resume.step = 0;
+    resume.sig = 0;
+    resume.leave_stopped = 0;
+    win32_resume (&resume);
+  }
 
-  if (DebugActiveProcessStop != NULL)
-    DebugActiveProcessStop (current_process_id);
-  else
-    win32_kill ();
+  if (!DebugActiveProcessStop (current_process_id))
+    {
+      CloseHandle (h);
+      return -1;
+    }
+  DebugSetProcessKillOnExit (FALSE);
+
+  current_process_handle = h;
+  return 0;
+}
+
+/* Wait for inferiors to end.  */
+static void
+win32_join (void)
+{
+  if (current_process_id == 0
+      || current_process_handle == NULL)
+    return;
+
+  WaitForSingleObject (current_process_handle, INFINITE);
+  CloseHandle (current_process_handle);
+
+  current_process_handle = NULL;
+  current_process_id = 0;
 }
 
 /* Return 1 iff the thread with thread ID TID is alive.  */
@@ -1160,6 +1202,7 @@ static struct target_ops win32_target_op
   win32_attach,
   win32_kill,
   win32_detach,
+  win32_join,
   win32_thread_alive,
   win32_resume,
   win32_wait,
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2007-05-08 01:52:40.000000000 +0100
+++ src/gdb/remote.c	2007-05-08 02:01:02.000000000 +0100
@@ -2600,7 +2600,11 @@ remote_detach (char *args, int from_tty)
 
   /* Tell the remote target to detach.  */
   strcpy (rs->buf, "D");
-  remote_send (&rs->buf, &rs->buf_size);
+  putpkt (rs->buf);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+
+  if (rs->buf[0] == 'E')
+    error (_("Can't detach process."));
 
   /* Unregister the file descriptor from the event loop.  */
   if (target_is_async_p ())

Re: [RFC] [gdbserver/win32] Enable detaching on MinGW.

[Daniel Jacobowitz]

On Tue, May 08, 2007 at 03:10:36AM +0100, Pedro Alves wrote:
> Hi all,
> 
> Currently detach support is skipped on MinGW (and mingw32ce),
> because there is no waitpid on Windows.  This patch enables it
> by implementing the waiting for the inferior to exit in a new
> target_op::join.
> 
> One thing that annoys me, is that requesting for detach kills the
> inferior if detaching isn't supported.  The patch makes gdbserver
> return an error (E packet) to gdb, so gdb knows it shouldn't
> stop debugging.  The user can then kill the inferior is he wants.
> 
> What do you think of this behavior?
> 
> Is the patch OK?

The gdbserver parts are fine.

> -  remote_send (&rs->buf, &rs->buf_size);
> +  putpkt (rs->buf);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +
> +  if (rs->buf[0] == 'E')
> +    error (_("Can't detach process."));

Isn't this basically what happens already in remote_send?  Oh, but we
have a better error message now.  This part's OK too.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [gdbserver/win32] Enable detaching on MinGW.

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

Daniel Jacobowitz wrote:
> On Tue, May 08, 2007 at 03:10:36AM +0100, Pedro Alves wrote:
>> Hi all,
>>
>> Currently detach support is skipped on MinGW (and mingw32ce),
>> because there is no waitpid on Windows.  This patch enables it
>> by implementing the waiting for the inferior to exit in a new
>> target_op::join.
>>
>> One thing that annoys me, is that requesting for detach kills the
>> inferior if detaching isn't supported.  The patch makes gdbserver
>> return an error (E packet) to gdb, so gdb knows it shouldn't
>> stop debugging.  The user can then kill the inferior is he wants.
>>
>> What do you think of this behavior?
>>
>> Is the patch OK?
> 
> The gdbserver parts are fine.
> 
>> -  remote_send (&rs->buf, &rs->buf_size);
>> +  putpkt (rs->buf);
>> +  getpkt (&rs->buf, &rs->buf_size, 0);
>> +
>> +  if (rs->buf[0] == 'E')
>> +    error (_("Can't detach process."));
> 
> Isn't this basically what happens already in remote_send?  Oh, but we
> have a better error message now.  This part's OK too.
> 

Thanks.

I've also committed the following as obvious.

Cheers,
Pedro Alves


[-- Attachment #2: fix_detach.diff --]
[-- Type: text/x-diff, Size: 1254 bytes --]

2007-05-10  Pedro Alves  <pedro_alves@portugalmail.pt>

	* linux-low.c (linux_detach): Change return type to int.  Return 0.
	* spu-low.c (spu_detach): Likewise.
---
 gdb/gdbserver/linux-low.c |    3 ++-
 gdb/gdbserver/spu-low.c   |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2007-05-10 21:55:20.000000000 +0100
+++ src/gdb/gdbserver/linux-low.c	2007-05-10 22:22:04.000000000 +0100
@@ -287,10 +287,11 @@ linux_detach_one_process (struct inferio
   ptrace (PTRACE_DETACH, pid_of (process), 0, 0);
 }
 
-static void
+static int
 linux_detach (void)
 {
   for_each_inferior (&all_threads, linux_detach_one_process);
+  return 0;
 }
 
 static void
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2007-05-10 21:55:20.000000000 +0100
+++ src/gdb/gdbserver/spu-low.c	2007-05-10 22:22:46.000000000 +0100
@@ -314,10 +314,11 @@ spu_kill (void)
 }
 
 /* Detach from inferior process.  */
-static void
+static int
 spu_detach (void)
 {
   ptrace (PTRACE_DETACH, current_tid, 0, 0);
+  return 0;
 }
 
 static void