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

["Pierre Muller" <muller@ics.u-strasbg.fr>]

> >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;