This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]