[RFC] win32-nat.c 'set new-console' and interruption

["Pierre Muller" <muller at ics dot u-strasbg dot fr>]

Hi,

  The current cygwin/mingw32 has an annoying problem
when used with the debuggee running in a separate console
(which is done using the 'set new-console on' command).

  In current CVS HEAD gdb, there is no way to interrupt the running debuggee
from the GDB console.

  This patch tries to address this issue,
all changes are limited to win32-nat.c source file.
  
  The first problem is that a recent patch in win32-nat.c 
simply disabled GDB Ctrl-C handling completely while the debuggee was
running.
While this is reasonable if GDB and the debuggee are in the same console,
there is no reason to do this if the debuggee is running in a new
console.

  The patch installs a new function GDBCtrlHandler as the Control Handler
if the current debuggee was started with CREATE_NEW_CONSOLE.
  This allows to catch Ctrl-C or Ctrl-Break in the GDB console.
  
  I first thought that simply calling win32_stop 
would then work without problems, but it appears that
the win32 API function GenerateConsoleCtrlEvent 
only works if the signal is sent to a process running in the same
console as the caller...
  There is now a new win32 API function called DebugBreakProcess 
(available from Windows XP) that is able to overcome that limitation.
I thus extended win32_stop in order to
use this new function.
  I was wondering if the 'kill' function could be used 
for systems that do not provide this function (windows 2000 for instance).
  Using 'signal SIGINT' from the interruption generated
by win32_stop, it is possible to send the exception to the debuggee.
  I wrote a small test for this, but when I tried to 
write an expect file for it, it became quite messy, and the
test do not work as it should at all :( 
  I also added a new variable called GroupId that is 
supposed to get the correct process group ID according to the
specifications given by the GenerateConsoleCtrlEvent  reference,
which is not the case in current code. If new-group is off,
CreateProcess is called without CREATE_NEW_GROUP,
and thus the new process id is not a process group id.
In that case, zero should be used to send the signal to 
the own group of GDB, which contains the debuggee.
  
  I also added code that basically removed all the terminal
state switching that is not useful if the debuggee runs in a separate
console,
but I not sure I understand that code part sufficiently to claim that
I did that part right.
  It might have been easier to simply set gdb_has_a_terminal to zero
to stop all that switching.

  Comments most welcome,


Pierre Muller
Pascal language support maintainer for GDB




ChangeLog entry:

2008-06-20  Pierre Muller  <muller@ics.u-strasbg.fr>

	* win32-nat.c (GroupId, CtrlBreakSent, start_flags): New variables.
	(handle_exception): Recognize signal sent with DebugBreakProcess
	and treat as TARGET_SIGNAL_INT.
	(GDBCtrlHandler): New function. Sends a interrupting signal to
debuggee.
	(win32_wait): Use GDBCtrlHandler if debuggee started in new console.
	(kernel32): HANDLE variable moved from has_detach_ability to main
level.
	(DebugBreakProcess): New function variable.
	(win32_stop): Only use GenerateConsoleCtrlEvent if debuggee on same 
	console. Otherwise test for kernel32 DebugBreakProcess function and 
	use if available.
	(win32_create_inferior): Set start_flags and GroupId.
	(win32_terminal_inferior, win32_terminal_ours): New functions.
	Do nothing if debuggee was started in a new console.
	(win32_terminal_ours_for_output): New function, as above.
	(init_win32_ops): Set to_terminal_inferior to
win32_terminal_inferior
	and to_terminal_ours to win32_terminal_ours.


Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.154
diff -u -p -r1.154 win32-nat.c
--- gdb/win32-nat.c	19 Jun 2008 06:36:45 -0000	1.154
+++ gdb/win32-nat.c	20 Jun 2008 22:46:54 -0000
@@ -137,7 +137,9 @@ static DEBUG_EVENT current_event;	/* The
 static HANDLE current_process_handle;	/* Currently executing process */
 static thread_info *current_thread;	/* Info on currently selected thread
*/
 static DWORD main_thread_id;		/* Thread ID of the main thread */
-
+static DWORD GroupId = 0;
+static int CtrlBreakSent = 0;
+ 
 /* Counts of things. */
 static int exception_count = 0;
 static int event_count = 0;
@@ -155,6 +157,7 @@ static int debug_events = 0;		/* show ev
 static int debug_memory = 0;		/* show target memory accesses */
 static int debug_exceptions = 0;	/* show target exceptions */
 static int useshell = 0;		/* use shell for subprocesses */
+static DWORD start_flags = 0;		/* remember flags used to start
process */
 
 /* This vector maps GDB's idea of a register's number into an address
    in the win32 exception context vector.
@@ -1076,7 +1079,13 @@ handle_exception (struct target_waitstat
       break;
     case EXCEPTION_BREAKPOINT:
       DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_BREAKPOINT");
-      ourstatus->value.sig = TARGET_SIGNAL_TRAP;
+      if (CtrlBreakSent)
+	{
+	  CtrlBreakSent = 0;
+	  ourstatus->value.sig = TARGET_SIGNAL_INT;
+	}
+      else
+	ourstatus->value.sig = TARGET_SIGNAL_TRAP;
       break;
     case DBG_CONTROL_C:
       DEBUG_EXCEPTION_SIMPLE ("DBG_CONTROL_C");
@@ -1447,6 +1456,30 @@ out:
   return retval;
 }
 
+
+static BOOL WINAPI
+GDBCtrlHandler (DWORD dwCtrlType)
+{
+  DEBUG_EVENTS(("GDBCtrlHandler called with dwCtrType=%u\n",
+		(unsigned int) dwCtrlType));
+  switch (dwCtrlType)
+    {
+      case CTRL_C_EVENT:
+      case CTRL_BREAK_EVENT:
+	win32_stop();
+	/* Notify that event is handled.  */
+	return TRUE;
+	break;
+      case CTRL_CLOSE_EVENT:
+      case CTRL_LOGOFF_EVENT:
+      case CTRL_SHUTDOWN_EVENT:
+	/* Notify that event is not handled.  */
+	return FALSE;
+        break;
+   }
+  return FALSE; 
+}
+ 
 /* Wait for interesting events to occur in the target process. */
 static ptid_t
 win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
@@ -1478,9 +1511,16 @@ win32_wait (ptid_t ptid, struct target_w
          in the inferior.  This is a classic race, and it would be nice
          to find a better solution to that problem.  But in the meantime,
          the current approach already greatly mitigate this issue.  */
-      SetConsoleCtrlHandler (NULL, TRUE);
+      if (start_flags & CREATE_NEW_CONSOLE)
+	SetConsoleCtrlHandler (&GDBCtrlHandler, TRUE);
+      else
+	SetConsoleCtrlHandler (NULL, TRUE);
       retval = get_win32_debug_event (pid, ourstatus);
       SetConsoleCtrlHandler (NULL, FALSE);
+      if (start_flags & CREATE_NEW_CONSOLE)
+	SetConsoleCtrlHandler (&GDBCtrlHandler, FALSE);
+      else
+	SetConsoleCtrlHandler (NULL, FALSE);
 
       if (retval)
 	return pid_to_ptid (retval);
@@ -1545,12 +1585,12 @@ do_initial_win32_stuff (DWORD pid)
    detach has worked. */
 static BOOL WINAPI (*DebugSetProcessKillOnExit)(BOOL);
 static BOOL WINAPI (*DebugActiveProcessStop)(DWORD);
-
+static BOOL WINAPI (*DebugBreakProcess)(HANDLE);
+static HMODULE kernel32 = NULL;
+ 
 static int
 has_detach_ability (void)
 {
-  static HMODULE kernel32 = NULL;
-
   if (!kernel32)
     kernel32 = LoadLibrary ("kernel32.dll");
   if (kernel32)
@@ -1905,6 +1945,13 @@ win32_create_inferior (char *exec_file, 
   else
     saw_create = 0;
 
+  start_flags = flags;
+
+  if (flags & CREATE_NEW_PROCESS_GROUP)
+    GroupId = pi.dwProcessId;
+  else
+    GroupId = 0;
+
   do_initial_win32_stuff (pi.dwProcessId);
 
   /* win32_continue (DBG_CONTINUE, -1); */
@@ -1920,6 +1967,7 @@ win32_mourn_inferior (void)
       CHECK (CloseHandle (current_process_handle));
       open_process_used = 0;
     }
+  start_flags = 0;
   unpush_target (&win32_ops);
   generic_mourn_inferior ();
 }
@@ -1930,11 +1978,35 @@ win32_mourn_inferior (void)
 static void
 win32_stop (void)
 {
-  DEBUG_EVENTS (("gdb: GenerateConsoleCtrlEvent (CTRLC_EVENT, 0)\n"));
-  CHECK (GenerateConsoleCtrlEvent (CTRL_C_EVENT,
current_event.dwProcessId));
+  if ((start_flags & CREATE_NEW_CONSOLE) == 0)
+    {
+      DEBUG_EVENTS (("gdb: GenerateConsoleCtrlEvent (CTRL_BREAK_EVENT,
%lu)\n",
+	 	    current_event.dwProcessId));
+      CHECK (GenerateConsoleCtrlEvent (CTRL_BREAK_EVENT, 
+				       GroupId));
+    }
+  else
+    {
+      if (!kernel32)
+	kernel32 = LoadLibrary ("kernel32.dll");
+      if (kernel32)
+	{
+	  if (!DebugBreakProcess)
+	    DebugBreakProcess = GetProcAddress (kernel32,
"DebugBreakProcess");
+	}
+      if (DebugBreakProcess && current_process_handle)
+	{
+	  DEBUG_EVENTS (("DebugBreakProcess (%d)\n",
+			 (int) current_process_handle));
+	  CtrlBreakSent = 1;
+	  DebugBreakProcess (current_process_handle);
+	}
+      else
+        DEBUG_EVENTS (("Unable to interrupt debuggee in another
console\n"));
+    }
   registers_changed ();		/* refresh register state */
 }
-
+ 
 static int
 win32_xfer_memory (CORE_ADDR memaddr, gdb_byte *our, int len,
 		   int write, struct mem_attrib *mem,
@@ -2075,6 +2147,36 @@ win32_xfer_partial (struct target_ops *o
       return -1;
     }
 }
+static void
+win32_terminal_inferior ()
+{
+  /* If the debuggee is not on the same console, 
+     we don't need to do anything.  */
+  if (start_flags & CREATE_NEW_CONSOLE)
+    return;
+  terminal_inferior ();
+}
+
+static void
+win32_terminal_ours_for_output ()
+{
+  /* If the debuggee is not on the same console, 
+     we don't need to do anything.  */
+  if (start_flags & CREATE_NEW_CONSOLE)
+    return;
+  terminal_ours_for_output ();
+}
+
+
+static void
+win32_terminal_ours ()
+{
+  /* If the debuggee is not on the same console, 
+     we don't need to do anything.  */
+  if (start_flags & CREATE_NEW_CONSOLE)
+    return;
+  terminal_ours ();
+}
 
 static void
 init_win32_ops (void)
@@ -2097,9 +2199,9 @@ init_win32_ops (void)
   win32_ops.to_insert_breakpoint = memory_insert_breakpoint;
   win32_ops.to_remove_breakpoint = memory_remove_breakpoint;
   win32_ops.to_terminal_init = terminal_init_inferior;
-  win32_ops.to_terminal_inferior = terminal_inferior;
-  win32_ops.to_terminal_ours_for_output = terminal_ours_for_output;
-  win32_ops.to_terminal_ours = terminal_ours;
+  win32_ops.to_terminal_inferior = win32_terminal_inferior;
+  win32_ops.to_terminal_ours_for_output = win32_terminal_ours_for_output;
+  win32_ops.to_terminal_ours = win32_terminal_ours;
   win32_ops.to_terminal_save_ours = terminal_save_ours;
   win32_ops.to_terminal_info = child_terminal_info;
   win32_ops.to_kill = win32_kill_inferior;