[Patch] Win32 gdbserver new interrupt support, and attach to process fix.

[Lerele <lerele@champenstudios.com>]

Hello,

I've added remote interrupt support for gdbserver on win32.
Also fixed gdbserver attach functionality, it wasn't even working at all.

Daniel, adding this support was much simpler than what I proposed some 
time ago.
You can also see I have added in server.h a new function so that win32 
code can call 'input_interrupt' function from remote-utils.c. I don't 
know if you're going to like that (having win32-i386-low.c call server 
code), but it needs to check for socket data availability, to be able to 
interrupt the child process.

Also, interrupt support is done by having the gdbserver constantly check 
(4 times a second) for debug events from child, and also check for 
socket data to interrupt it. This was how win32 gdbserver was working 
before anyway, except it was doing it once a second, and without 
checking socket data.
The alternative I think would be to, as Eli Zaretskii pointed out some 
days ago in gdb list, to have a separate thread check for socket 
availability and have it send the signal to the child, however this 
option would need some kind of thread synchronization to access 
gdbserver data from the thread (mainly read socket data from several 
threads).
It works fine as it is, without making it too complex. Only problem is 
that gdbserver may consume *some* cpu.

Hope patch is Ok.

Regards,
Leo.


PD: Patch follows:

2007-02-24    Leo Zayas
    * server.h, remote-utils.c: Expose input_interrupt through
    check_remote_input_interrupt_request for gdbserver.
    * win32-i386-low.c: Fix gdbserver attach support on win32.
    * win32-i386-low.c: Add remote interrupt support.
===================================================================
diff -u src_orig/gdb/gdbserver/remote-utils.c 
src/gdb/gdbserver/remote-utils.c
--- src_orig/gdb/gdbserver/remote-utils.c    2007-02-16 
21:01:14.000000000 +0100
+++ src/gdb/gdbserver/remote-utils.c    2007-02-23 23:53:44.796875000 +0100
@@ -78,7 +78,11 @@
 int remote_debug = 0;
 struct ui_file *gdb_stdlog;
 
-static int remote_desc;
+#ifdef USE_WIN32API
+static int remote_desc=INVALID_SOCKET;
+#else
+static int remote_desc=-1;
+#endif
 
 /* FIXME headerize? */
 extern int using_threads;
@@ -567,8 +571,6 @@
   return putpkt_binary (buf, strlen (buf));
 }
 
-#ifndef USE_WIN32API
-
 /* Come here when we get an input interrupt from the remote side.  This
    interrupt should only be active while we are waiting for the child to do
    something.  About the only thing that should come through is a ^C, which
@@ -580,16 +582,30 @@
   fd_set readset;
   struct timeval immediate = { 0, 0 };
 
+  /* We will be calling input_interrupt on win32 to check for remote 
interrupt,
+     via exposed function 'check_remote_input_interrupt_request'.
+     This function may be called before establishing communications;
+     need to validate socket object. */
+
+#ifdef USE_WIN32API
+  if (remote_desc==INVALID_SOCKET)
+    return;
+#else
+  if (remote_desc==-1)
+    return;
+#endif
+
   /* Protect against spurious interrupts.  This has been observed to
      be a problem under NetBSD 1.4 and 1.5.  */
-
+ 
   FD_ZERO (&readset);
   FD_SET (remote_desc, &readset);
+
   if (select (remote_desc + 1, &readset, 0, 0, &immediate) > 0)
     {
       int cc;
       char c = 0;
-     
+
       cc = read (remote_desc, &c, 1);
 
       if (cc != 1 || c != '\003')
@@ -602,7 +618,13 @@
       (*the_target->send_signal) (SIGINT);
     }
 }
-#endif
+
+/* Expose 'static void input_interrupt (int unused)' function to enable 
checking for a
+   remote interrupt request. */
+void check_remote_input_interrupt_request (void)
+{
+  input_interrupt(0);
+}
 
 /* Asynchronous I/O support.  SIGIO must be enabled when waiting, in 
order to
    accept Control-C from the client, and must be disabled when talking to
diff -u src_orig/gdb/gdbserver/server.h src/gdb/gdbserver/server.h
--- src_orig/gdb/gdbserver/server.h    2007-01-09 18:59:08.000000000 +0100
+++ src/gdb/gdbserver/server.h    2007-02-23 12:53:56.859375000 +0100
@@ -147,6 +147,7 @@
 void disable_async_io (void);
 void unblock_async_io (void);
 void block_async_io (void);
+void check_remote_input_interrupt_request (void);
 void convert_ascii_to_int (char *from, unsigned char *to, int n);
 void convert_int_to_ascii (unsigned char *from, char *to, int n);
 void new_thread_notify (int id);
diff -u src_orig/gdb/gdbserver/win32-i386-low.c 
src/gdb/gdbserver/win32-i386-low.c
--- src_orig/gdb/gdbserver/win32-i386-low.c    2007-01-09 
18:59:08.000000000 +0100
+++ src/gdb/gdbserver/win32-i386-low.c    2007-02-24 00:26:26.171875000 
+0100
@@ -37,6 +37,10 @@
 
 #define LOG 0
 
+#if LOG
+#include "../gdb_wait.h"
+#endif
+
 #define OUTMSG(X) do { printf X; fflush (stdout); } while (0)
 #if LOG
 #define OUTMSG2(X) do { printf X; fflush (stdout); } while (0)
@@ -238,7 +242,13 @@
 
   /* Nothing happened, but we stopped anyway.  This perhaps should be 
handled
      within target_wait, but I'm not sure target_wait should be 
resuming the
-     inferior.  */
+     inferior.
+
+     It should be safe to continue child given this wait status.
+     See function get_child_debug_event. Default wait status is spurious,
+     and it gets modified if any important debug events get received.
+     More specifically, this status gets returned in the wait loop to
+     allow socket pooling/resuming, to allow for remote interruption. */
   TARGET_WAITKIND_SPURIOUS,
 };
 
@@ -574,7 +584,7 @@
 
   FreeLibrary (kernel32);
 
-  return res;
+  return res? 0:-1;
 }
 
 /* Kill all inferiors.  */
@@ -840,7 +850,12 @@
   last_sig = TARGET_SIGNAL_0;
   ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
 
-  if (!(debug_event = WaitForDebugEvent (&current_event, 1000)))
+  /* Check for remote interruption request */
+  check_remote_input_interrupt_request();
+
+  /* 250 ms wait time for debug event to allow for more precise
+     remote interruption. */
+  if (!(debug_event = WaitForDebugEvent (&current_event, 250)))
     goto out;
 
   current_inferior =
@@ -1004,6 +1019,10 @@
 
       return our_status.value.sig;
     }
+      else if (our_status.kind == TARGET_WAITKIND_SPURIOUS)
+    {
+      /* do nothing, just continue */
+    }
       else
     OUTMSG (("Ignoring unknown internal event, %d\n", our_status.kind));
 
@@ -1054,6 +1073,35 @@
   return child_xfer_memory (memaddr, (char *) myaddr, len, 1, 0) != len;
 }
 
+/* Send a signal to the inferior process, however is appropriate. */
+static void
+win32_send_signal (int signo)
+{
+  if ( signo == TARGET_SIGNAL_INT )
+    {
+      if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, current_process_id))
+    {
+      /* fallback to XP/Vista 'DebugBreakProcess'.
+         GenerateConsoleCtrlEvent can fail if process id being debugged is
+         not a process group id.
+         Note if the above function fails, the code bellow will
+         generate a breakpoint exception to stop. */
+
+      typedef BOOL winapi_DebugBreakProcess(HANDLE);
+      HMODULE kernel32 = LoadLibrary ("KERNEL32.DLL");
+      winapi_DebugBreakProcess *DebugBreakProcess = NULL;
+      DebugBreakProcess =
+        (winapi_DebugBreakProcess *) GetProcAddress (kernel32,
+                             "DebugBreakProcess");
+      if (DebugBreakProcess==NULL || !DebugBreakProcess 
(current_process_handle))
+        {
+          OUTMSG ( ("Could not interrupt process.\n") );
+        }
+      FreeLibrary(kernel32);
+    }
+    }
+}
+
 static struct target_ops win32_target_ops = {
   win32_create_inferior,
   win32_attach,
@@ -1067,7 +1115,7 @@
   win32_read_inferior_memory,
   win32_write_inferior_memory,
   0,
-  0
+  win32_send_signal
 };
 
 /* Initialize the Win32 backend.  */