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

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

[Pierre Muller]

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;

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

[Christopher Faylor]

On Sat, Jun 21, 2008 at 01:47:48AM +0200, Pierre Muller wrote:
>@@ -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");

This seems like a race condition to me.  You can't be guaranteed
that the breakpoint you're receiving is not a valid breakpoint.
It would be better to check if it is a valid breakpoint and, if
not, and iff CtrlBreakSent was true then do this.

But two other observations:

1. I believe that MixedCase like this is against GNU coding standards but,
even if it isn't, the convention in win32-nat.c is to use lowercase and
underscores: ctrl_break_sent.

2.  What is newconsole good for?  I never use it and wonder if it
shouldn't just be nuked.  My usual refrain applies here:  Is there
anything similar in regular gdb?

cgf

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

[Daniel Jacobowitz]

On Sat, Jun 21, 2008 at 01:05:21PM -0400, Christopher Faylor wrote:
> 2.  What is newconsole good for?  I never use it and wonder if it
> shouldn't just be nuked.  My usual refrain applies here:  Is there
> anything similar in regular gdb?

I believe this is similar to "set inferior-tty", though I'm not sure
exactly what the Windows version does.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Corinna Vinschen]

On Jun 21 13:21, Daniel Jacobowitz wrote:
> On Sat, Jun 21, 2008 at 01:05:21PM -0400, Christopher Faylor wrote:
> > 2.  What is newconsole good for?  I never use it and wonder if it
> > shouldn't just be nuked.  My usual refrain applies here:  Is there
> > anything similar in regular gdb?
> 
> I believe this is similar to "set inferior-tty", though I'm not sure
> exactly what the Windows version does.

It starts the inferior in a new console window, different from the
console window GDB is running in.  It's quite similar to what "set
inferior-tty" is for.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat

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

[Christopher Faylor]

On Sat, Jun 21, 2008 at 01:21:15PM -0400, Daniel Jacobowitz wrote:
>On Sat, Jun 21, 2008 at 01:05:21PM -0400, Christopher Faylor wrote:
>> 2.  What is newconsole good for?  I never use it and wonder if it
>> shouldn't just be nuked.  My usual refrain applies here:  Is there
>> anything similar in regular gdb?
>
>I believe this is similar to "set inferior-tty", though I'm not sure
>exactly what the Windows version does.

Huh.  Learn something new...  I wonder how many times I could have used
this.

It is similar but inferior-tty is more flexible.

cgf

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

[Christopher Faylor]

On Sat, Jun 21, 2008 at 08:36:48PM +0200, Corinna Vinschen wrote:
>On Jun 21 13:21, Daniel Jacobowitz wrote:
>> On Sat, Jun 21, 2008 at 01:05:21PM -0400, Christopher Faylor wrote:
>> > 2.  What is newconsole good for?  I never use it and wonder if it
>> > shouldn't just be nuked.  My usual refrain applies here:  Is there
>> > anything similar in regular gdb?
>> 
>> I believe this is similar to "set inferior-tty", though I'm not sure
>> exactly what the Windows version does.
>
>It starts the inferior in a new console window, different from the
>console window GDB is running in.  It's quite similar to what "set
>inferior-tty" is for.

So it actually is useful and used then?

cgf

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

[Corinna Vinschen]

On Jun 21 23:18, Christopher Faylor wrote:
> On Sat, Jun 21, 2008 at 08:36:48PM +0200, Corinna Vinschen wrote:
> >On Jun 21 13:21, Daniel Jacobowitz wrote:
> >> On Sat, Jun 21, 2008 at 01:05:21PM -0400, Christopher Faylor wrote:
> >> > 2.  What is newconsole good for?  I never use it and wonder if it
> >> > shouldn't just be nuked.  My usual refrain applies here:  Is there
> >> > anything similar in regular gdb?
> >> 
> >> I believe this is similar to "set inferior-tty", though I'm not sure
> >> exactly what the Windows version does.
> >
> >It starts the inferior in a new console window, different from the
> >console window GDB is running in.  It's quite similar to what "set
> >inferior-tty" is for.
> 
> So it actually is useful and used then?

Yes, for instance for debugging applications which grab the whole
console (editors etc).


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat

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

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Christopher Faylor
> Envoyé : Sunday, June 22, 2008 5:18 AM
> À : gdb-patches@sourceware.org; Pierre Muller
> Objet : Re: [RFC] win32-nat.c 'set new-console' and interruption
> 
> On Sat, Jun 21, 2008 at 08:36:48PM +0200, Corinna Vinschen wrote:
> >On Jun 21 13:21, Daniel Jacobowitz wrote:
> >> On Sat, Jun 21, 2008 at 01:05:21PM -0400, Christopher Faylor wrote:
> >> > 2.  What is newconsole good for?  I never use it and wonder if it
> >> > shouldn't just be nuked.  My usual refrain applies here:  Is there
> >> > anything similar in regular gdb?
> >>
> >> I believe this is similar to "set inferior-tty", though I'm not sure
> >> exactly what the Windows version does.
> >
> >It starts the inferior in a new console window, different from the
> >console window GDB is running in.  It's quite similar to what "set
> >inferior-tty" is for.
> 
> So it actually is useful and used then?

  Under the Free Pascal IDE (project on which I 
worked for quite some time) I placed the
new-console as an equivalent of the 'set tty'
and it is much easier to have the debuggee to run in a separate console
that to do all the switching back and forth of all console parameters.

  I run gdb mainly in that mode for cygwin,
but the lack of capability of interrupting the
debuggee from the GDB console was annoying me for a long time already!

  I will resend a new patch proposal that complies with
the lowercase coding standard rule shortly.

Pierre Muller
Pascal language support maintainer for GDB

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

[Pierre Muller]

> >     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");
> 
> This seems like a race condition to me.  You can't be guaranteed
> that the breakpoint you're receiving is not a valid breakpoint.
> It would be better to check if it is a valid breakpoint and, if
> not, and iff CtrlBreakSent was true then do this.
  Yes, I tought about this, but the problem here is that I
don't know how to identify this EXCEPTION_BREAKPOINT as being
created by the call to DebugBreakProcess as opposed to 
an exception coming from some other GDB installed breakpoint...
  Should we test if 'int 3' is at current eip? I am not really sure
this would work, in particular, I don't know what win32 API
exception is generated if we use hardware breakpoints!
  Any ideas?   

> 1. I believe that MixedCase like this is against GNU coding standards
> but,
> even if it isn't, the convention in win32-nat.c is to use lowercase and
> underscores: ctrl_break_sent.

  I tried to remove all these MixedCase variables and functions,
with the exceptions of the functions variables that are used to
check for the existence of exported functions in system DLLs
that are named as the export function itself. There were several
such instances in the code already.
  
> 2.  What is newconsole good for?  I never use it and wonder if it
> shouldn't just be nuked.  My usual refrain applies here:  Is there
> anything similar in regular gdb?

  I hope we answered that part of your email!

  I am still concerned about the terminal functions:
>  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.



Pierre Muller
Pascal language support maintainer for GDB




ChangeLog entry:

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

        * win32-nat.c (group_id, ctrl_break_sent, start_flags): New
variables.
        (handle_exception): Recognize signal sent with DebugBreakProcess
        and treat as TARGET_SIGNAL_INT.
        (gdb_ctrl_handler): New function. Sends a interrupting signal to
        debuggee.
        (win32_wait): Use gdb_ctrl_handler if debuggee started in a 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 group_id.
        (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.




Pierre@d620-muller ~/gdbcvs/purecvs
$ cat win32term.patch
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     23 Jun 2008 07:16:44 -0000
@@ -137,6 +137,8 @@ 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 group_id = 0;
+static int ctrl_break_sent = 0;

 /* Counts of things. */
 static int exception_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 CreateProcess flags */

 /* 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 (ctrl_break_sent)
+       {
+         ctrl_break_sent = 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
+gdb_ctrl_handler (DWORD dw_ctrl_type)
+{
+  DEBUG_EVENTS(("gdb_ctrl_handler called with dwCtrType=%u\n",
+               (unsigned int) dw_ctrl_type));
+  switch (dw_ctrl_type)
+    {
+      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 (&gdb_ctrl_handler, TRUE);
+      else
+       SetConsoleCtrlHandler (NULL, TRUE);
       retval = get_win32_debug_event (pid, ourstatus);
       SetConsoleCtrlHandler (NULL, FALSE);
+      if (start_flags & CREATE_NEW_CONSOLE)
+       SetConsoleCtrlHandler (&gdb_ctrl_handler, 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)
+    group_id = pi.dwProcessId;
+  else
+    group_id = 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,
+                                      group_id));
+    }
+  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));
+         ctrl_break_sent = 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;

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

[Christopher Faylor]

On Mon, Jun 23, 2008 at 09:17:39AM +0200, Pierre Muller wrote:
>I will resend a new patch proposal that complies with the lowercase
>coding standard rule shortly.

That was only the most minor problem.  The race condition is something
that needs to be addressed before the patch can be considered.

cgf

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

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Christopher Faylor
> Envoyé : Monday, June 23, 2008 1:45 PM
> À : gdb-patches@sourceware.org; Pierre Muller
> Objet : Re: [RFC] win32-nat.c 'set new-console' and interruption
> 
> On Mon, Jun 23, 2008 at 09:17:39AM +0200, Pierre Muller wrote:
> >I will resend a new patch proposal that complies with the lowercase
> >coding standard rule shortly.
> 
> That was only the most minor problem.  The race condition is something
> that needs to be addressed before the patch can be considered.

  I agree with you on this.
I have a proposal to remove that possible race condition:
The exception record has a field that contains the exception
address, if I test that there is no GDB inserted breakpoint at
that location before converting the TARGET_SIGNAL_TRAP
into a TARGET_SIGNAL_INT, it should fix most problems, no?

  The one case that it would still not catch would be 
a 'int 3' instruction that is in the debuggee code from the start
but other than at startup, such instructions are quite unlikely, no?

  Is this a sufficient fix for the possible race or
should I try harder?


Pierre Muller
Pascal language support maintainer for GDB

PS: I have two code formatting issues in my modified code:

1) the CORE_ADDR addr assignment line goes past the 80 row boundary,
even if I put the current_event on a separate line
would
+          CORE_ADDR addr = (CORE_ADDR)
+            current_event.u.Exception.ExceptionRecord.ExceptionAddress; 
be acceptable to avoid going past 80?

2) For the DEBUG_EXCEPT macro use that I added,
I am also unsure about the formatting, as I had
to cut the line within the string.



Modified code to remove the race condition
between an exception create by DebugBreakProcess
and an exception created by the 'int 3' instruction
inserted by GDB for breakpoints.

@@ -1076,7 +1079,22 @@ handle_exception (struct target_waitstat
       break;
     case EXCEPTION_BREAKPOINT:
       DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_BREAKPOINT");
-      ourstatus->value.sig = TARGET_SIGNAL_TRAP;
+      if (ctrl_break_sent)
+
+       {
+          CORE_ADDR addr =
+           (CORE_ADDR)
current_event.u.Exception.ExceptionRecord.ExceptionAddress;
+
+          if (!breakpoint_inserted_here_p (addr))
+           {
+             DEBUG_EXCEPT (("EXCEPTION_BREAKPOINT at 0x%lx converted to \
+TARGET_SIGNAL_INT\n", (DWORD) addr));
+             ctrl_break_sent = 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");

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

[Pedro Alves]

A Monday 23 June 2008 15:23:06, Pierre Muller wrote:

> I have a proposal to remove that possible race condition:
> The exception record has a field that contains the exception
> address, if I test that there is no GDB inserted breakpoint at
> that location before converting the TARGET_SIGNAL_TRAP
> into a TARGET_SIGNAL_INT, it should fix most problems, no?
>
>   The one case that it would still not catch would be
> a 'int 3' instruction that is in the debuggee code from the start
> but other than at startup, such instructions are quite unlikely, no?
>

IIRC, DebugBreakProcess injects a thread in the debuggee and
always breaks at the same address / in the same function -- I don't
know if there's a hardcoded 0xcc at the break address you
could check, or if the exception is generated programatically,
but at least you could conditionalize on the function name (if there's
no hardcoded break, you still can't distiguish by name only a user
break placed in that special break function)

Another option is to use SuspendThread on all threads to stop
the process, which is what I believe Visual Studio uses.
gdbserver has that implemented for systems that don't have
DebugBreakProcess.

-- 
Pedro Alves

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

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Monday, June 23, 2008 4:41 PM
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFC] win32-nat.c 'set new-console' and interruption
> 
> A Monday 23 June 2008 15:23:06, Pierre Muller wrote:
> 
> > I have a proposal to remove that possible race condition:
> > The exception record has a field that contains the exception
> > address, if I test that there is no GDB inserted breakpoint at
> > that location before converting the TARGET_SIGNAL_TRAP
> > into a TARGET_SIGNAL_INT, it should fix most problems, no?
> >
> >   The one case that it would still not catch would be
> > a 'int 3' instruction that is in the debuggee code from the start
> > but other than at startup, such instructions are quite unlikely, no?
> >
> 
> IIRC, DebugBreakProcess injects a thread in the debuggee and
> always breaks at the same address / in the same function -- I don't
> know if there's a hardcoded 0xcc at the break address you
> could check, or if the exception is generated programatically,
> but at least you could conditionalize on the function name (if there's
> no hardcoded break, you still can't distiguish by name only a user
> break placed in that special break function)

  I checked this: on windows XP it does use an 'int 3' instruction
that is situated in ntdll DLL at an exported function named
DbgUiConnectToDbg, but I would really like to avoid hard-coding this into
win32-nat.c as there is no assurance that other versions of
Microsoft OS will use the same function.

> Another option is to use SuspendThread on all threads to stop
> the process, which is what I believe Visual Studio uses.
> gdbserver has that implemented for systems that don't have
> DebugBreakProcess.

That is also a nice idea...
The only problem in that case is, which thread do we then
use as the current_thread? 
Do we default on main_thread_id?
But the current win32-nat source code has a particularity
here: it never removes the main_thread_idd thread
even if that thread exits, I don't really know 
why it was coded like this but that means that using
main_thread_id might put us in a 'dead' thread.

  I never managed to send a patch for this, 
and I suspect that there are some hidden reason for
this strange behavior!

  We could insert code that stops all threads into
win32_wait at the location of the 
deprecated_ui_loop_hook code,
but I don't know really how to handle this:
if you suspend manually all threads, you can of course
set ourstatus, but the question is how do you coordinate this 
with the next call to win32_resume?
Is it enough to add a global variable
threads_suspended_manually (not sure this is the best name)
and to add something like
if (!threads_suspended_manually)
  res = ContinueDebugEvent (...)
else
  /* Here we add the code to resume all threads */

If fear that we might need more special code, for instance if
win32_resume is called with a non zero signal to pass,
which would of course be impossible in that case.

  I would personally prefer to postpone this 
to a later patch, but Christopher should 
tell us what the best way is.


Pierre Muller
Pascal language support maintainer for GDB


 

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

[Christopher Faylor]

On Mon, Jun 23, 2008 at 03:41:26PM +0100, Pedro Alves wrote:
>Another option is to use SuspendThread on all threads to stop the
>process, which is what I believe Visual Studio uses.  gdbserver has
>that implemented for systems that don't have DebugBreakProcess.

It may be ok in Windows XP and beyond (I haven't checked) but I don't
believe you can reliably use SuspendThread otherwise.  If you suspend a
thread at the wrong point you can cause problems.

cgf

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

[Pedro Alves]

A Monday 23 June 2008 18:36:20, Christopher Faylor wrote:
> On Mon, Jun 23, 2008 at 03:41:26PM +0100, Pedro Alves wrote:
> >Another option is to use SuspendThread on all threads to stop the
> >process, which is what I believe Visual Studio uses.  gdbserver has
> >that implemented for systems that don't have DebugBreakProcess.
>
> It may be ok in Windows XP and beyond (I haven't checked) but I don't
> believe you can reliably use SuspendThread otherwise.  If you suspend a
> thread at the wrong point you can cause problems.

It may have been a problem with earlier Windows versions (maybe deadlocking
on some global internal lock, but evidence is scarse).  Not sure it's a
real problem.  Would pretty much make it useless if you can't use it from a
debugger.  google is riddled with articles claiming not to use it, but
those are aimed at application developers who confuse it with a locking
primitive.  I don't believe that when WaitForDebugEvent returns
with all threads stopped, there has been any kind of special magic
done that SuspendThread wouldn't trigger.  Sure, it is dangerous to use 
SuspendThreads to suspend threads in the current process (as
some stack walking libs do), as the deadlock risk is higher
then, like deadlocking on output or painting routines; but that is a
non-issue for a debugger.

I know for sure MSFT's Windows CE debugger (I bet it's the same
codebase as the desktop debugger) uses SuspendThread to stop the process,
as enabling its debug output tells the whole world.  :-)  I did notice
that it was GetThreadContext that seemed to do the blocking
waiting for the thread to suspend in user-code (and noticed other
weirdnesses around that, that seemed CE specific).

I know you've been all over this before, but for the archives:

Quoting from MSDN:
 "If the function succeeds, execution of the specified thread is suspended and  
 the thread's suspend count is incremented. Suspending a thread causes the  
 thread to stop executing *user-mode* (application) code."

 "This function is primarily designed for use by debuggers. It is not intended 
 to be used for thread synchronization. (...)"

I sure hope we can use it if/when we get to implement non-stop mode for
Windows.

Anyway, back to the original issue: do we really need to translate
that SIGTRAP into a SIGINT (from a DebugBreakProcess) ?

-- 
Pedro Alves

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

[Pierre Muller]

> Anyway, back to the original issue: do we really need to translate
> that SIGTRAP into a SIGINT (from a DebugBreakProcess) ?


This is what I get with my patch:

(top-gdb) set new-c
(top-gdb) r ./gdb
Starting program: /usr/local/src/gdbcvs/build-bare/gdb/gdb.exe ./gdb
[New Thread 480.0x2ec]
[New Thread 480.0xc60]
[New Thread 480.0xae4]
[New Thread 480.0xe7c]

Program received signal SIGINT, Interrupt.
[Switching to Thread 480.0xe7c]
0x7c901231 in ntdll!DbgUiConnectToDbg ()
   from /cygdrive/d/WINDOWS/system32/ntdll.dll
(top-gdb)


If I removed the code that transforms the
EXCEPTION_BREAKPOINT into a TARGET_SIGNAL_INT
I get this:

Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 2548.0xf1c]
0x7c901231 in ntdll!DbgUiConnectToDbg ()
   from /cygdrive/d/WINDOWS/system32/ntdll.dll
(top-gdb) c
Continuing.
[New Thread 2548.0xd10]

Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 2548.0xd10]
0x7c901231 in ntdll!DbgUiConnectToDbg ()
   from /cygdrive/d/WINDOWS/system32/ntdll.dll


I find it cleaner with signal SIGINT, 
but it's true that the other option is simpler
and not that different!

  Christopher, should I just remove the 
ctrl_break_sent variable and let 
GDB use SIGTRAP signal?


Pierre Muller
Pascal language support maintainer for GDB

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

[Joel Brobecker]

> I find it cleaner with signal SIGINT, but it's true that the other
> option is simpler and not that different!

I think most people would expect to see SIGINT. Also, seeing a SIGINT
would make it consistent with what the user would see if the console
was shared.

>   Christopher, should I just remove the ctrl_break_sent variable and
>   let GDB use SIGTRAP signal?

So, I would personally prefered if we kept the SIGTRAP to SIGINT
translation... Just my 2 cents.

-- 
Joel

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

[Christopher Faylor]

On Mon, Jun 23, 2008 at 07:22:10PM +0100, Pedro Alves wrote:
>A Monday 23 June 2008 18:36:20, Christopher Faylor wrote:
>> On Mon, Jun 23, 2008 at 03:41:26PM +0100, Pedro Alves wrote:
>> >Another option is to use SuspendThread on all threads to stop the
>> >process, which is what I believe Visual Studio uses.  gdbserver has
>> >that implemented for systems that don't have DebugBreakProcess.
>>
>> It may be ok in Windows XP and beyond (I haven't checked) but I don't
>> believe you can reliably use SuspendThread otherwise.  If you suspend a
>> thread at the wrong point you can cause problems.
>
>It may have been a problem with earlier Windows versions (maybe deadlocking
>on some global internal lock, but evidence is scarse).  Not sure it's a
>real problem.  Would pretty much make it useless if you can't use it from a
>debugger.  google is riddled with articles claiming not to use it, but
>those are aimed at application developers who confuse it with a locking
>primitive.

Which is not me.

If you want gdb to be usable on systems other than Windows XP and beyond
then you can't use SuspendThread.

I'm not speaking from theory.  I'm speaking from experience.

If this wasn't something that we wanted to do then we shouldn't be
carefully autoloading functions that only exist in XP in win32-nat.c.

cgf

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

[Christopher Faylor]

On Mon, Jun 23, 2008 at 03:51:53PM -0400, Joel Brobecker wrote:
>> I find it cleaner with signal SIGINT, but it's true that the other
>> option is simpler and not that different!
>
>I think most people would expect to see SIGINT. Also, seeing a SIGINT
>would make it consistent with what the user would see if the console
>was shared.
>
>>   Christopher, should I just remove the ctrl_break_sent variable and
>>   let GDB use SIGTRAP signal?
>
>So, I would personally prefered if we kept the SIGTRAP to SIGINT
>translation... Just my 2 cents.

I think that makes 4 cents.  A SIGTRAP would be confusing.

I haven't looked at the patch yet, however.

cgf

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

[Pierre Muller]

> >So, I would personally prefered if we kept the SIGTRAP to SIGINT
> >translation... Just my 2 cents.
> 
> I think that makes 4 cents.  A SIGTRAP would be confusing.
> 
> I haven't looked at the patch yet, however.
> 
> cgf

In that case, let me send you a new version of the patch then:
version 3 superseeds the two previous versions in this thread.

 It contains the attempt to distinguish
if EXCEPTION_BREAKPOINT comes from the win32_stop call,
to eliminate the race condition you raised:
if ctrl_break_sent is set and the location of the 
exception addr is not a breakpoint set by GDB,
then we assume that the exception was generated by
the call to DebugBreakProcess.

  I also added code to reset ctrl_break_sent to zero 
in do_initial_win32_stuff.

  Finally I also set start_flags to CREATE_NEW_CONSOLE
if we attach to a running process rather than
start one. It seems rather odd to me to try to
attach to a process running in the same console, it
is much better to start GDB in an new console and
to attach from there.
  Nevertheless, I must admit that I don't know if
there is away to know if the process to which we are attaching
is running in the console or not.


Pierre Muller
Pascal language support maintainer for GDB


$ cat win32term.log
ChangeLog entry:

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

        * win32-nat.c (group_id, ctrl_break_sent, start_flags): New
variables.
        (handle_exception): Recognize signal sent with DebugBreakProcess
        and treat as TARGET_SIGNAL_INT.
        (gdb_ctrl_handler): New function. Sends a interrupting signal to
        debuggee.
        (win32_wait): Use gdb_ctrl_handler if debuggee started in a new
console.
        (do_initial_win32_stuff): Reset ctrl_break_sent.
        (kernel32): HANDLE variable moved from has_detach_ability to main
level.
        (win32_attach): set start_flags to CREATE_NEW_CONSOLE.
        (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 group_id.
        (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.


$ cat  win32term.patch
Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.155
diff -u -p -r1.155 win32-nat.c
--- gdb/win32-nat.c     24 Jun 2008 02:33:17 -0000      1.155
+++ gdb/win32-nat.c     24 Jun 2008 07:43:13 -0000
@@ -137,6 +137,8 @@ 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 group_id = 0;
+static int ctrl_break_sent = 0;

 /* Counts of things. */
 static int exception_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 CreateProcess flags */

 /* This vector maps GDB's idea of a register's number into an address
    in the win32 exception context vector.
@@ -1074,7 +1077,22 @@ handle_exception (struct target_waitstat
       break;
     case EXCEPTION_BREAKPOINT:
       DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_BREAKPOINT");
-      ourstatus->value.sig = TARGET_SIGNAL_TRAP;
+      if (ctrl_break_sent)
+
+       {
+          CORE_ADDR addr =
+           (CORE_ADDR)
current_event.u.Exception.ExceptionRecord.ExceptionAddre
ss;
+
+          if (!breakpoint_inserted_here_p (addr))
+           {
+             DEBUG_EXCEPT (("EXCEPTION_BREAKPOINT at 0x%lx converted to \
+TARGET_SIGNAL_INT\n", (DWORD) addr));
+             ctrl_break_sent = 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");
@@ -1445,6 +1463,30 @@ out:
   return retval;
 }

+
+static BOOL WINAPI
+gdb_ctrl_handler (DWORD dw_ctrl_type)
+{
+  DEBUG_EVENTS(("gdb_ctrl_handler called with dwCtrType=%u\n",
+               (unsigned int) dw_ctrl_type));
+  switch (dw_ctrl_type)
+    {
+      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)
@@ -1476,9 +1518,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 (&gdb_ctrl_handler, TRUE);
+      else
+       SetConsoleCtrlHandler (NULL, TRUE);
       retval = get_win32_debug_event (pid, ourstatus);
       SetConsoleCtrlHandler (NULL, FALSE);
+      if (start_flags & CREATE_NEW_CONSOLE)
+       SetConsoleCtrlHandler (&gdb_ctrl_handler, FALSE);
+      else
+       SetConsoleCtrlHandler (NULL, FALSE);

       if (retval)
        return pid_to_ptid (retval);
@@ -1505,6 +1554,8 @@ do_initial_win32_stuff (DWORD pid)
   event_count = 0;
   exception_count = 0;
   open_process_used = 0;
+  ctrl_break_sent = 0;
+
   debug_registers_changed = 0;
   debug_registers_used = 0;
   for (i = 0; i < sizeof (dr) / sizeof (dr[0]); i++)
@@ -1546,12 +1597,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)
@@ -1686,6 +1737,10 @@ win32_attach (char *args, int from_tty)

   attach_flag = 1;

+  /* Attaching to a process on the same console seems not the right thing
to do
.
+     Let's assume the debuggee runs on a separate console.  */
+  start_flags = CREATE_NEW_CONSOLE;
+
   if (from_tty)
     {
       char *exec_file = (char *) get_exec_file (0);
@@ -1906,6 +1961,13 @@ win32_create_inferior (char *exec_file,
   else
     saw_create = 0;

+  start_flags = flags;
+
+  if (flags & CREATE_NEW_PROCESS_GROUP)
+    group_id = pi.dwProcessId;
+  else
+    group_id = 0;
+
   do_initial_win32_stuff (pi.dwProcessId);

   /* win32_continue (DBG_CONTINUE, -1); */
@@ -1921,6 +1983,7 @@ win32_mourn_inferior (void)
       CHECK (CloseHandle (current_process_handle));
       open_process_used = 0;
     }
+  start_flags = 0;
   unpush_target (&win32_ops);
   generic_mourn_inferior ();
 }
@@ -1931,11 +1994,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,
+                                      group_id));
+    }
+  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));
+         ctrl_break_sent = 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,
@@ -2076,6 +2163,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)
@@ -2098,9 +2215,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;

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

[Pedro Alves]

A Tuesday 24 June 2008 02:13:37, Christopher Faylor wrote:

> Which is not me.

I know.  I really didn't mean to imply you were one of them.

> If you want gdb to be usable on systems other than Windows XP and beyond
> then you can't use SuspendThread.
>
> I'm not speaking from theory.  I'm speaking from experience.

As I said, I know you have been all over this before.  I'm not
trying to be picky on you.  :-)

Wasn't the conclusion that calling GetThreadContext blocks
until the thread really suspends, or that GetThreadContext fails
if the thread hasn't suspend yet?  Like here:

http://cygwin.com/ml/cygwin-developers/2005-12/msg00005.html

What was the outcome of that?

I really would like to know of a problem it has caused by using
it from a debugger, that is, from a process which isn't the
owner of the thread; instead of a problem caused by calling
SuspendThread on a thread of the current process.

> If this wasn't something that we wanted to do then we shouldn't be
> carefully autoloading functions that only exist in XP in win32-nat.c.

Right, but that's a bit of a different issue.  SuspendThread has
been available in win32 since ever.  DebugBreakProcess hasn't.


I happen to have written a patch last week that implements scheduler
locking for win32-nat.c, that relies on SuspendThread to leave
the other non-resumed threads suspended (the original suspending
is done internally by windows before returning from
WaitForDebugEvent).  I'm hoping that this kind of usage SuspendThread
usage is accepted...

-- 
Pedro Alves

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

[Christopher Faylor]

On Tue, Jun 24, 2008 at 01:31:22PM +0100, Pedro Alves wrote:
>A Tuesday 24 June 2008 02:13:37, Christopher Faylor wrote:
>
>> Which is not me.
>
>I know.  I really didn't mean to imply you were one of them.
>
>> If you want gdb to be usable on systems other than Windows XP and beyond
>> then you can't use SuspendThread.
>>
>> I'm not speaking from theory.  I'm speaking from experience.
>
>As I said, I know you have been all over this before.  I'm not
>trying to be picky on you.  :-)
>
>Wasn't the conclusion that calling GetThreadContext blocks
>until the thread really suspends, or that GetThreadContext fails
>if the thread hasn't suspend yet?  Like here:
>
>http://cygwin.com/ml/cygwin-developers/2005-12/msg00005.html
>
>What was the outcome of that?

I have no idea.  Cygwin does use SuspendThread but only when it has no
other alternative.  And, it can still cause problems on older windows.

>I really would like to know of a problem it has caused by using
>it from a debugger, that is, from a process which isn't the
>owner of the thread; instead of a problem caused by calling
>SuspendThread on a thread of the current process.
>
>> If this wasn't something that we wanted to do then we shouldn't be
>> carefully autoloading functions that only exist in XP in win32-nat.c.
>
>Right, but that's a bit of a different issue.  SuspendThread has
>been available in win32 since ever.  DebugBreakProcess hasn't.

This has nothing to do with the availability of SuspendThread.  It has
to do with how well SuspendThread works.

Once again: we go out of the way to make sure that gdb works on older
windows.  That means that you can't assume that something that works on
newer windows is ok to use.  SuspendThread does not work reliably on
older windows.

If the consensus here is that we can give up on Windows NT4 (and maybe
even Windows 2000) and before that's fine.  Otherwise, either
SuspendThread calls have to be guarded with OS tests or not used at all.

cgf