From patchwork Wed Mar 26 18:49:23 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eli Zaretskii X-Patchwork-Id: 301 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (caibbdcaabij.dreamhost.com [208.113.200.189]) by wilcox.dreamhost.com (Postfix) with ESMTP id 8CE34360213 for ; Wed, 26 Mar 2014 11:49:36 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id 503E643C2D06; Wed, 26 Mar 2014 11:49:36 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 0EE8D4F787D4 for ; Wed, 26 Mar 2014 11:49:36 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:subject:in-reply-to:to:cc:reply-to :message-id:references; q=dns; s=default; b=lprj/qAbOqms11WgH8Oz yywKrVl4pT33J2nKwaveIYv7hXBEC5R0jSbfe7afhHcJ2sUts69FvNEMWSiSpLOc VZdK1k2QBvJifvIL1ECKNYQq18vwBfeH5sXwtXxBTGhqTQmb7Yr/dpkf2K3/wN+w V35GXGnfka3GACv4wK3Ix1Y= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:subject:in-reply-to:to:cc:reply-to :message-id:references; s=default; bh=Vp18jTwBdBH2Cj/25/nk+Pbs1s U=; b=MLS10TPGxaMKMyfZjVIuJBLHvBAeHGUaDbkdt+ov/8fiKAGBavyzdONxgq xtjxNW3yKfVd8dPK9HVHVxpZrUWBea16HIDrESwH27GhmobSY5795Owc0KBOFpGv UYopqJj2cOl65ICixeEXNwrsIVfk0FMlPcso0cr4uONIQOgsY= Received: (qmail 17442 invoked by alias); 26 Mar 2014 18:49:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 17430 invoked by uid 89); 26 Mar 2014 18:49:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL, BAYES_00, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout26.012.net.il Received: from mtaout26.012.net.il (HELO mtaout26.012.net.il) (80.179.55.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Mar 2014 18:49:31 +0000 Received: from conversion-daemon.mtaout26.012.net.il by mtaout26.012.net.il (HyperSendmail v2007.08) id <0N3200L003XWVQ00@mtaout26.012.net.il> for gdb-patches@sourceware.org; Wed, 26 Mar 2014 20:48:26 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout26.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0N3200J2B48O9Y40@mtaout26.012.net.il>; Wed, 26 Mar 2014 20:48:26 +0200 (IST) Date: Wed, 26 Mar 2014 20:49:23 +0200 From: Eli Zaretskii Subject: Re: [PATCH] Fix "PC register is not available" issue In-reply-to: <20140318165413.GE4282@adacore.com> To: Joel Brobecker , Pedro Alves Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <834n2kztfw.fsf@gnu.org> References: <83txawa9wk.fsf@gnu.org> <20140318161608.GD4282@adacore.com> <83pplja2h9.fsf@gnu.org> <20140318165413.GE4282@adacore.com> X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in This describes the results of my looking into this issue, given the comments and suggestions by Joel and Pedro. Sorry about the length. > I didn't mean to change the behavior - only hide the warning. > In this case, if it is normal that we can't suspend the thread, > then there is no point in warning (scaring) the user about it. > I would only generate a warning if something abnormal that we should > fix occured. The patch near the end of this message indeed includes code to ignore the warning in these cases. > I see that the GetThreadContext call (do_windows_fetch_inferior_registers) > doesn't check for errors (I think it should (*)). It'd be interesting to know whether gdb can > actually read the registers off of this thread, and if so, what's the > thread's backtrace like. I added CHECK to that call to GetThreadContext. It never produced a warning in all my testing, and it looks like we do succeed to get the registers. At least the registers of 2 such threads show different contents, and the EIP value is consistent with what "info threads" displays. > >> and if so, what's the thread's backtrace like. > > > > Why would we be interested in that info? > > It'll likely show us the thread is stopped at some ntdll.dll function > or some such, and from the function name we will likely > be able to infer what/which thread is this, like, e.g., whether > it's a thread injected with DebugBreakProcess or some such > (internally by one of the system dlls or the dlls your app > links with). I can show you 2 typical examples. This is from Emacs, where the application has 3 threads, and one more is started by the debugger. The rest, threads 5 and 6 in these examples, are those mysterious threads we are talking about. (gdb) info threads Id Target Id Frame 6 Thread 15492.0x1f28 0x77a41f46 in ntdll!ZwWaitForWorkViaWorkerFactory () from C:\Windows\system32\ntdll.dll 5 Thread 15492.0x73c0 0x77a41f46 in ntdll!ZwWaitForWorkViaWorkerFactory () from C:\Windows\system32\ntdll.dll 4 Thread 15492.0x2300 0x75ac78d7 in USER32!DispatchMessageW () from C:\Windows\syswow64\user32.dll 3 Thread 15492.0x1860 0x77a3fd91 in ntdll!ZwDelayExecution () from C:\Windows\system32\ntdll.dll 2 Thread 15492.0x2410 0x77a4015d in ntdll!ZwWaitForMultipleObjects () from C:\Windows\system32\ntdll.dll * 1 Thread 15492.0x44a0 cleanup_vector (vector=0x62daeb0) at alloc.c:2917 (gdb) info threads Id Target Id Frame 6 Thread 15492.0x1f28 0x77a3f8d1 in ntdll!ZwWaitForSingleObject () from C:\Windows\system32\ntdll.dll 5 Thread 15492.0x73c0 0x77a72880 in ntdll!RtlFillMemoryUlong () from C:\Windows\system32\ntdll.dll 4 Thread 15492.0x2300 0x75ac78d7 in USER32!DispatchMessageW () from C:\Windows\syswow64\user32.dll 3 Thread 15492.0x1860 0x77a3fd91 in ntdll!ZwDelayExecution () from C:\Windows\system32\ntdll.dll 2 Thread 15492.0x2410 0x77a4015d in ntdll!ZwWaitForMultipleObjects () from C:\Windows\system32\ntdll.dll * 1 Thread 15492.0x44a0 cleanup_vector (vector=0x388ca58) at alloc.c:2917 The first display is what I usually see: several (I've seen up to 4) threads waiting inside ZwWaitForWorkViaWorkerFactory. But sometimes they do perform some work, as can be seen from the second display. My theory is that we get those Access Denied (winerr = 5) errors when we try to suspend these threads when they are about to exit. That's because I normally see a "Thread exited" message immediately after the warning with the error code. However, testing whether the thread exited, and avoiding the SuspendThread call if it did, didn't stop the warnings, so I guess the issue is more subtle. E.g., it could be that during the final stages of thread shutdown, the thread runs some privileged code or something, which precludes suspension. It might also be related to the fact that we are debugging 32-bit threads on a 64-bit OS, so WOW64 is at work here. In any case, the patch below ignores Access Denied errors from SuspendThread. In addition, I tried to solve warnings like these: error return windows-nat.c:1306 was 5 [Thread 15492.0x68a0 exited with code 0] (gdb) q A debugging session is active. Inferior 1 [process 15492] will be killed. Quit anyway? (y or n) y error return windows-nat.c:1306 was 5 These come from SetThreadContext in windows_continue. The second occurrence is because we already killed the inferior by calling TerminateProcess, so its threads begin to shut down, and trying to set their context might indeed fail. The first warning is about one of those same threads, and again happens just before the thread exit is announced. My solution, which you can see below, is (a) pass an additional parameter to windows_continue telling it that the inferior was killed, in which case it doesn't bother checking errors from the SetThreadContext call; and (b) check if the thread already exited, and if so, avoid calling SetThreadContext on it. This completely avoids the warning when killing the inferior, and removes most (but not all) of the warnings for the other threads. Finally, here's the full patch. I hope this research answered all the questions, and we can now get the patch in. --- gdb/windows-nat.c~0 2014-02-06 04:21:29.000000000 +0200 +++ gdb/windows-nat.c 2014-03-25 19:18:16.814250000 +0200 @@ -310,12 +310,18 @@ thread_rec (DWORD id, int get_context) { DWORD err = GetLastError (); - warning (_("SuspendThread (tid=0x%x) failed." - " (winerr %u)"), - (unsigned) id, (unsigned) err); - return NULL; + /* We get Access Denied (5) when trying to suspend + threads that Windows started on behalf of the + debuggee, usually when those threads are just + about to exit. */ + if (err != ERROR_ACCESS_DENIED) + warning (_("SuspendThread (tid=0x%x) failed." + " (winerr %u)"), + (unsigned) id, (unsigned) err); + th->suspended = -1; } - th->suspended = 1; + else + th->suspended = 1; } else if (get_context < 0) th->suspended = -1; @@ -445,7 +451,7 @@ do_windows_fetch_inferior_registers (str { thread_info *th = current_thread; th->context.ContextFlags = CONTEXT_DEBUGGER_DR; - GetThreadContext (th->h, &th->context); + CHECK (GetThreadContext (th->h, &th->context)); /* Copy dr values from that thread. But only if there were not modified since last stop. PR gdb/2388 */ @@ -1266,10 +1272,12 @@ handle_exception (struct target_waitstat return 1; } -/* Resume all artificially suspended threads if we are continuing - execution. */ +/* Resume thread specified by ID, or all artificially suspended + threads, if we are continuing execution. KILLED non-zero means we + have killed the inferior, so we should ignore weird errors due to + threads shutting down. */ static BOOL -windows_continue (DWORD continue_status, int id) +windows_continue (DWORD continue_status, int id, int killed) { int i; thread_info *th; @@ -1297,7 +1305,16 @@ windows_continue (DWORD continue_status, } if (th->context.ContextFlags) { - CHECK (SetThreadContext (th->h, &th->context)); + DWORD ec = 0; + + if (GetExitCodeThread (th->h, &ec) + && ec == STILL_ACTIVE) + { + if (killed) + SetThreadContext (th->h, &th->context); + else + CHECK (SetThreadContext (th->h, &th->context)); + } th->context.ContextFlags = 0; } if (th->suspended > 0) @@ -1425,9 +1442,9 @@ windows_resume (struct target_ops *ops, Otherwise complain. */ if (resume_all) - windows_continue (continue_status, -1); + windows_continue (continue_status, -1, 0); else - windows_continue (continue_status, ptid_get_tid (ptid)); + windows_continue (continue_status, ptid_get_tid (ptid), 0); } /* Ctrl-C handler used when the inferior is not run in the same console. The @@ -1645,7 +1662,7 @@ get_windows_debug_event (struct target_o if (continue_status == -1) windows_resume (ops, minus_one_ptid, 0, 1); else - CHECK (windows_continue (continue_status, -1)); + CHECK (windows_continue (continue_status, -1, 0)); } else { @@ -2382,13 +2399,13 @@ windows_create_inferior (struct target_o do_initial_windows_stuff (ops, pi.dwProcessId, 0); - /* windows_continue (DBG_CONTINUE, -1); */ + /* windows_continue (DBG_CONTINUE, -1, 0); */ } static void windows_mourn_inferior (struct target_ops *ops) { - (void) windows_continue (DBG_CONTINUE, -1); + (void) windows_continue (DBG_CONTINUE, -1, 0); i386_cleanup_dregs(); if (open_process_used) { @@ -2456,7 +2473,7 @@ windows_kill_inferior (struct target_ops for (;;) { - if (!windows_continue (DBG_CONTINUE, -1)) + if (!windows_continue (DBG_CONTINUE, -1, 1)) break; if (!WaitForDebugEvent (¤t_event, INFINITE)) break;