This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix "PC register is not available" issue
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: palves at redhat dot com, gdb-patches at sourceware dot org
- Date: Thu, 27 Mar 2014 19:41:39 +0200
- Subject: Re: [PATCH] Fix "PC register is not available" issue
- Authentication-results: sourceware.org; auth=none
- References: <83txawa9wk dot fsf at gnu dot org> <20140318161608 dot GD4282 at adacore dot com> <83pplja2h9 dot fsf at gnu dot org> <20140318165413 dot GE4282 at adacore dot com> <834n2kztfw dot fsf at gnu dot org> <20140327125646 dot GA4030 at adacore dot com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> Date: Thu, 27 Mar 2014 05:56:46 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org
>
> > 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.
>
> Perhaps a simpler theory is that these threads are typically
> short-lived, and so you'd be always be seeing their exit soon
> after they are created.
They live for several minutes, so not so short-lived.
> > 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
>
> Yay! :)
>
> > 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.
>
> I am missing the command that caused the first error. From what you
> are saying, was it "kill"?
No, it was "continue", which caused the inferior to run until a
breakpoint, at which point I typed "q". The thread exit event
happened while the inferior was running.
> it'd be interesting to understand why we send a TerminateProcess
> first, and then try to SetThreadContext later on. It does not seem
> to make sense.
That happens because windows_kill_inferior does this:
static void
windows_kill_inferior (struct target_ops *ops)
{
CHECK (TerminateProcess (current_process_handle, 0));
for (;;)
{
if (!windows_continue (DBG_CONTINUE, -1, 1))
break;
if (!WaitForDebugEvent (¤t_event, INFINITE))
break;
if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
break;
}
target_mourn_inferior (); /* Or just windows_mourn_inferior? */
}
IOW, we resume the inferior (perhaps to collect all the debug
events?).
> Perhaps one way to address the problem more globally is to have
> a version of the CHECK macro that ignores access-denied errors,
> and use this one on operations where we know we might get that
> error?
We only have one or 2 places for that right now, so I wouldn't think a
separate macro is justified.
> > Finally, here's the full patch. I hope this research answered all the
> > questions, and we can now get the patch in.
>
> I'm good with the first half of the patch that handles SuspendThread
> more gracefully. For the additional argument to windows_continue,
> I propose we handle that as a separate patch. It's the right thing
> to do procedurally anyway, and it'll give us a chance to investigate
> more without holding the first half up.
Given what I told above, what additional investigations are needed?
Note that the second part is not entirely separate: those phantom
threads hit the problem with SetThreadContext as well, and checking
whether the thread already exited does let through fewer of those
warnings.