From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24053 invoked by alias); 23 Jun 2008 07:28:37 -0000 Received: (qmail 24043 invoked by uid 22791); 23 Jun 2008 07:28:36 -0000 X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.157) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 23 Jun 2008 07:28:16 +0000 Received: from baal.u-strasbg.fr (baal.u-strasbg.fr [IPv6:2001:660:2402::41]) by mailhost.u-strasbg.fr (8.14.2/jtpda-5.5pre1) with ESMTP id m5N7SCuj096617 for ; Mon, 23 Jun 2008 09:28:12 +0200 (CEST) Received: from mailserver.u-strasbg.fr (ms2.u-strasbg.fr [IPv6:2001:660:2402::142]) by baal.u-strasbg.fr (8.14.0/jtpda-5.5pre1) with ESMTP id m5N7SCKX077056 for ; Mon, 23 Jun 2008 09:28:12 +0200 (CEST) Received: from d620muller (www-ics.u-strasbg.fr [130.79.210.225]) by mailserver.u-strasbg.fr (8.13.8/jtpda-5.5pre1) with ESMTP id m5N7S99g052488 for ; Mon, 23 Jun 2008 09:28:12 +0200 (CEST) From: "Pierre Muller" To: References: <000001c8d330$0c6b51f0$2541f5d0$@u-strasbg.fr> <20080621170521.GB2470@ednor.casa.cgf.cx> In-Reply-To: <20080621170521.GB2470@ednor.casa.cgf.cx> Subject: [RFC v2] win32-nat.c 'set new-console' and interruption Date: Mon, 23 Jun 2008 08:52:00 -0000 Message-ID: <000201c8d502$b3fa8610$1bef9230$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Content-Language: en-us X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.0 (mailhost.u-strasbg.fr [IPv6:2001:660:2402::157]); Mon, 23 Jun 2008 09:28:12 +0200 (CEST) X-Virus-Status: Clean Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-06/txt/msg00377.txt.bz2 > > 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 * 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;