From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20579 invoked by alias); 22 Oct 2007 13:44:08 -0000 Received: (qmail 20568 invoked by uid 22791); 22 Oct 2007 13:44:07 -0000 X-Spam-Check-By: sourceware.org Received: from ics.u-strasbg.fr (HELO ics.u-strasbg.fr) (130.79.112.250) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 22 Oct 2007 13:44:00 +0000 Received: from ICSMULLER (laocoon.u-strasbg.fr [130.79.112.72]) by ics.u-strasbg.fr (Postfix) with ESMTP id BB25318701D; Mon, 22 Oct 2007 15:48:28 +0200 (CEST) From: "Pierre Muller" To: Cc: "'Pedro Alves'" , "'Daniel Jacobowitz'" Subject: [RFC] win32-nat.c: Handle EXCEPTION_INVALID_HANDLE as SIGSYS Date: Mon, 22 Oct 2007 14:14:00 -0000 Message-ID: <008101c814b1$9aeb2dd0$d0c18970$@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 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: 2007-10/txt/msg00503.txt.bz2 This patch does several things, that should probably be separated into several smaller patches, but I send it here together as a RFC. The patch mainly fixes the problems described in the thread starting with: http://sourceware.org/ml/gdb/2007-10/msg00131.html The patch first implements recognition of the exception EXCEPTION_INVALID_HANDLE generated by a call to CloseHandle with an invalid parameter. The new variable stop_on_invalid_handle, which can be set via "set stoponinvalidhandle", allows to either ignore that exception completely (with just a comment if verbose is set, default behavior), or to map it to SIGSYS, which can then be handled as any target signal via "handle SIGSYS ..." Combined to this I integrated the new variable old_behavior, which allowed me to disable all CloseHandle calls present in the win32-nat.c source that resulted in later INVALID_HANDLE exception. These exceptions come from the fact that the win32 API specifications say that the thread and process handles are closed by the system after the corresponding debug events, and thus they should never be closed by the debugger itself. I also changed the type of handle_exception function, to try to better explain the behavior of that code. The resulting behavior is still non trivial, especially, I found that I still needed to differentiate between the Cygwin Kernel32!IsBad check that calls directly win32_continue, and the unknown_exception handling that uses win32_resume. Trying to call win32_continue directly in that case did not work as expected. I tried to check that no handle was left open, but a separate tool would probably be needed for that. I tested kill and got no invalid handle exceptions within GDB with the disabled calls introduced with the old_behavior variable. Comments welcome, Pierre Muller ChangeLog entry: 2007-10-22 Pierre Muller * win32-nat.c (stop_on_invalid_handle): New variable. (old_behavior): New variable. (win32_delete_thread): Do not close thread handle. (handle_output_debug_string): Add output if verbose is set. (exception_answer): New enum. (handle_exception): Change return type to enum exception_answer. (win32_continue): Clarify debug event output. Move call to SetThreadContext before ResumeThread calls. Check ResumeThread return values. (get_win32_debug_event): Check return from WaitForDebugEvent. Do not close process handle on EXIT_PROCESS_DEBUG_EVENT. Adapt code to new return type of handle_exception function. (initialize_win32_nat): Add "set/show StopOnInvalidHandle" commands. Index: win32-nat.c =================================================================== RCS file: /cvs/src/src/gdb/win32-nat.c,v retrieving revision 1.138 diff -u -p -r1.138 win32-nat.c --- win32-nat.c 16 Oct 2007 18:43:24 -0000 1.138 +++ win32-nat.c 22 Oct 2007 13:06:57 -0000 @@ -152,6 +152,8 @@ 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 int stop_on_invalid_handle = 0; /* stop on STATUS_INVALID_HANDLE */ +static int old_behavior = 0; /* This vector maps GDB's idea of a register's number into an address in the win32 exception context vector. @@ -316,7 +318,8 @@ win32_init_thread_list (void) { thread_info *here = th->next; th->next = here->next; - (void) CloseHandle (here->h); + if (old_behavior) + (void) CloseHandle (here->h); xfree (here); } thread_head.next = NULL; @@ -341,7 +344,13 @@ win32_delete_thread (DWORD id) { thread_info *here = th->next; th->next = here->next; - CloseHandle (here->h); + if (old_behavior) + { + if (info_verbose) + printf_unfiltered ("Closing thread handle (h=0x%lx)\n", + (DWORD) here->h); + CloseHandle (here->h); + } xfree (here); } } @@ -822,8 +831,13 @@ handle_output_debug_string (struct targe if (!target_read_string ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024, 0) || !s || !*s) - /* nothing to do */; - else if (strncmp (s, _CYGWIN_SIGNAL_STRING, sizeof (_CYGWIN_SIGNAL_STRING) - 1) != 0) + /* nothing to do */ + return retval; + if (info_verbose) + { + printf_unfiltered ("Debug string: \"%s\"\n", s); + } + if (strncmp (s, _CYGWIN_SIGNAL_STRING, sizeof (_CYGWIN_SIGNAL_STRING) - 1) != 0) { #ifdef __CYGWIN__ if (strncmp (s, "cYg", 3) != 0) @@ -989,7 +1003,15 @@ info_w32_command (char *args, int from_t printf_unfiltered ("gdb: Target exception %s at 0x%08lx\n", x, \ (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionAddress) -static int +enum exception_answer +{ + pass_unknown_exception_to_inferior_directly = -1, + pass_exception_to_inferior_directly = 0, + stop_on_exception_as_signalled = 1, + ignore_exception_and_continue = 2 +}; + +static enum exception_answer handle_exception (struct target_waitstatus *ourstatus) { thread_info *th; @@ -1018,7 +1040,7 @@ handle_exception (struct target_waitstat if ((!cygwin_exceptions && (addr >= cygwin_load_start && addr < cygwin_load_end)) || (find_pc_partial_function (addr, &fn, NULL, NULL) && strncmp (fn, "KERNEL32!IsBad", strlen ("KERNEL32!IsBad")) == 0)) - return 0; + return pass_exception_to_inferior_directly; } #endif break; @@ -1094,10 +1116,26 @@ handle_exception (struct target_waitstat DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_NONCONTINUABLE_EXCEPTION"); ourstatus->value.sig = TARGET_SIGNAL_ILL; break; - default: + case STATUS_INVALID_HANDLE: + if (stop_on_invalid_handle) { + /* Map this to bad SysCall */ + DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_INVALID_HANDLE"); + ourstatus->value.sig = TARGET_SIGNAL_SYS; + break; + } + else + { + printf_unfiltered ("Ignoring EXCEPTION_INVALID_HANDLE exception\n"); + return ignore_exception_and_continue; + } + break; + default: /* Treat unhandled first chance exceptions specially. */ if (current_event.u.Exception.dwFirstChance) - return -1; + { + DEBUG_EXCEPTION_SIMPLE ("unknown (first chance)"); + return pass_unknown_exception_to_inferior_directly; + } printf_unfiltered ("gdb: unknown target exception 0x%08lx at 0x%08lx\n", current_event.u.Exception.ExceptionRecord.ExceptionCode, (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionAddress); @@ -1106,7 +1144,7 @@ handle_exception (struct target_waitstat } exception_count++; last_sig = ourstatus->value.sig; - return 1; + return stop_on_exception_as_signalled; } /* Resume all artificially suspended threads if we are continuing @@ -1117,11 +1155,20 @@ win32_continue (DWORD continue_status, i int i; thread_info *th; BOOL res; - + char *status_name; + switch (continue_status) { + case DBG_CONTINUE: + status_name = "DBG_CONTINUE"; + break; + case DBG_EXCEPTION_NOT_HANDLED: + status_name = "DBG_EXCEPTION_NOT_HANDLED"; + break; + default: + printf_unfiltered("Unknown continue status 0x%lx\n", continue_status); + } DEBUG_EVENTS (("ContinueDebugEvent (cpid=%ld, ctid=%ld, %s);\n", current_event.dwProcessId, current_event.dwThreadId, - continue_status == DBG_CONTINUE ? - "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED")); + status_name)); res = ContinueDebugEvent (current_event.dwProcessId, current_event.dwThreadId, continue_status); @@ -1129,10 +1176,6 @@ win32_continue (DWORD continue_status, i for (th = &thread_head; (th = th->next) != NULL;) if (((id == -1) || (id == (int) th->id)) && th->suspend_count) { - - for (i = 0; i < th->suspend_count; i++) - (void) ResumeThread (th->h); - th->suspend_count = 0; if (debug_registers_changed) { /* Only change the value of the debug registers */ @@ -1147,8 +1190,24 @@ win32_continue (DWORD continue_status, i CHECK (SetThreadContext (th->h, &th->context)); th->context.ContextFlags = 0; } - } + for (i = 0; i < th->suspend_count; i++) + { + res = ResumeThread (th->h); + if (res == -1) + { + printf_unfiltered ("ResumeThread call failed %lu\n", + GetLastError ()); + } + if (info_verbose && (i != res)) + { + printf_unfiltered ("Suspend count mismatch in win32_continue\n"); + } + if (res == 1) + break; + } + th->suspend_count = 0; + } debug_registers_changed = 0; return res; } @@ -1259,13 +1318,21 @@ get_win32_debug_event (int pid, struct t thread_info *th; static thread_info dummy_thread_info; int retval = 0; + int ignore_exception = 0; ptid_t ptid = {-1}; last_sig = TARGET_SIGNAL_0; - if (!(debug_event = WaitForDebugEvent (¤t_event, 1000))) - goto out; + debug_event = WaitForDebugEvent (¤t_event, 1000); + if (!debug_event) + { + DWORD res = GetLastError (); + if (res != ERROR_SEM_TIMEOUT) + printf_unfiltered("WaitForDebugEvent failed. GetLastError=%lu\n", + res); + return 0; + } event_count++; continue_status = DBG_CONTINUE; @@ -1346,7 +1413,8 @@ get_win32_debug_event (int pid, struct t break; ourstatus->kind = TARGET_WAITKIND_EXITED; ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode; - CloseHandle (current_process_handle); + if (old_behavior) + CloseHandle (current_process_handle); retval = main_thread_id; break; @@ -1386,14 +1454,17 @@ get_win32_debug_event (int pid, struct t break; switch (handle_exception (ourstatus)) { - case 0: + case pass_exception_to_inferior_directly: continue_status = DBG_EXCEPTION_NOT_HANDLED; break; - case 1: + case stop_on_exception_as_signalled: retval = current_event.dwThreadId; break; - case -1: - last_sig = 1; + case ignore_exception_and_continue: + ignore_exception = 1; + break; + case pass_unknown_exception_to_inferior_directly: + last_sig = TARGET_SIGNAL_UNKNOWN; continue_status = -1; break; } @@ -1422,8 +1493,10 @@ get_win32_debug_event (int pid, struct t if (!retval || saw_create != 1) { - if (continue_status == -1) - win32_resume (ptid, 0, 1); + if (ignore_exception) + win32_resume (ptid, 0, TARGET_SIGNAL_0); + else if (continue_status == -1) + win32_resume (ptid, 0, TARGET_SIGNAL_UNKNOWN); else CHECK (win32_continue (continue_status, -1)); } @@ -1949,11 +2022,11 @@ win32_kill_inferior (void) if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT) break; } - - CHECK (CloseHandle (current_process_handle)); + if (old_behavior) + CHECK (CloseHandle (current_process_handle)); /* this may fail in an attached process so don't check. */ - if (current_thread && current_thread->h) + if (old_behavior && current_thread && current_thread->h) (void) CloseHandle (current_thread->h); target_mourn_inferior (); /* or just win32_mourn_inferior? */ } @@ -2173,6 +2246,15 @@ Show whether to display kernel exception NULL, /* FIXME: i18n: */ &setlist, &showlist); + add_setshow_boolean_cmd ("stoponinvalidhandle", class_support, + &stop_on_invalid_handle, _("\ +Set whether to stop on STATUS_INVALID_HANDLE exception."), _("\ +Show whether to stop on STATUS_INVALID_HANDLE exception."), NULL, + NULL, + NULL, /* FIXME: i18n: */ + &setlist, &showlist); + + add_prefix_cmd ("w32", class_info, info_w32_command, _("Print information specific to Win32 debugging."), &info_w32_cmdlist, "info w32 ", 0, &infolist);