Fix "PC register is not available" issue
Commit Message
> Date: Fri, 11 Apr 2014 13:06:09 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org
>
> I was wondering what the status of the GDB-side of the patch was.
> I thought we were all set to commit it, while also looking at
> applying the same patch on the GDBserver side? I don't think we are
> in any hurry to get anything committed, but I'd had to see all
> this work get forgotten and lost... Also, I don't think the patch
> is all that risky, but the earlier we put it in, the more exposure
> it'll get before we release 7.8!
As I wrote on gdb@, I have committed the patch to fix the GDB side of
the problem. See below for what I actually committed, both to master
and to the GDB 7.7 branch.
Should we now close the Bugzilla PR? If I can do that, any pointers
as to how?
commit 17617f2d366ca969ccbc784be4f75931a1afd20f
Author: Eli Zaretskii <eliz@gnu.org>
Date: Sat Apr 19 11:12:19 2014 +0300
PR gdb/14018 -- avoid "PC register not available" errors.
gdb/windows-nat.c (thread_rec): Don't display a warning when
SuspendThread fails with ERROR_ACCESS_DENIED. If SuspendThread
fails for any reason, set th->suspended to -1, so that we don't
try to resume such a thread. Also, don't return NULL in these
cases, to avoid completely ruin the session due to "PC register is
not available" error.
(do_windows_fetch_inferior_registers): Check errors in
GetThreadContext call.
(windows_continue): Accept an additional argument KILLED; if not
zero, ignore errors in the SetThreadContext call, since the
inferior was killed and is shutting down.
(windows_resume, get_windows_debug_event)
(windows_create_inferior, windows_mourn_inferior)
(windows_kill_inferior): All callers of windows_continue changed
to adjust to its new calling sequence.
Comments
> As I wrote on gdb@, I have committed the patch to fix the GDB side of
> the problem. See below for what I actually committed, both to master
> and to the GDB 7.7 branch.
I didn't know we were planning on putting the change in the GDB 7.7
branch as well, but since you've tested it for several weeks, and
it's been seen by Pedro as well, we can indeed try to have it for
7.7.1 as well. The one thing I do ask when pushing a patch to the branch
is to update the release Wiki page to add a reference to the fix:
https://sourceware.org/gdb/wiki/GDB_7.7_Release
I've done it for you, since there was already a PR.
> Should we now close the Bugzilla PR? If I can do that, any pointers
> as to how?
Just log in, and change the state to "RESOLVED" + "FIXED".
I was already logged in, so did it as well.
> Date: Mon, 21 Apr 2014 08:43:33 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: palves@redhat.com, gdb-patches@sourceware.org
>
> > As I wrote on gdb@, I have committed the patch to fix the GDB side of
> > the problem. See below for what I actually committed, both to master
> > and to the GDB 7.7 branch.
>
> I didn't know we were planning on putting the change in the GDB 7.7
> branch as well, but since you've tested it for several weeks, and
> it's been seen by Pedro as well, we can indeed try to have it for
> 7.7.1 as well. The one thing I do ask when pushing a patch to the branch
> is to update the release Wiki page to add a reference to the fix:
> https://sourceware.org/gdb/wiki/GDB_7.7_Release
>
> I've done it for you, since there was already a PR.
>
> > Should we now close the Bugzilla PR? If I can do that, any pointers
> > as to how?
>
> Just log in, and change the state to "RESOLVED" + "FIXED".
> I was already logged in, so did it as well.
Thanks, on both accounts.
@@ -1,3 +1,22 @@
+2014-04-19 Eli Zaretskii <eliz@gnu.org>
+
+ PR gdb/14018
+ * windows-nat.c (thread_rec): Don't display a warning when
+ SuspendThread fails with ERROR_ACCESS_DENIED. If SuspendThread
+ fails for any reason, set th->suspended to -1, so that we don't
+ try to resume such a thread. Also, don't return NULL in these
+ cases, to avoid completely ruin the session due to "PC register is
+ not available" error.
+ (do_windows_fetch_inferior_registers): Check errors in
+ GetThreadContext call.
+ (windows_continue): Accept an additional argument KILLED; if not
+ zero, ignore errors in the SetThreadContext call, since the
+ inferior was killed and is shutting down.
+ (windows_resume, get_windows_debug_event)
+ (windows_create_inferior, windows_mourn_inferior)
+ (windows_kill_inferior): All callers of windows_continue changed
+ to adjust to its new calling sequence.
+
2014-04-19 Yao Qi <yao@codesourcery.com>
* ctf.c (ctf_open): Call post_create_inferior.
@@ -309,12 +309,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;
@@ -444,7 +450,7 @@ do_windows_fetch_inferior_registers (struct regcache *regcache, int r)
{
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 */
@@ -1181,10 +1187,12 @@ handle_exception (struct target_waitstatus *ourstatus)
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;
@@ -1212,7 +1220,16 @@ windows_continue (DWORD continue_status, int id)
}
if (th->context.ContextFlags)
{
- CHECK (SetThreadContext (th->h, &th->context));
+ DWORD ec = 0;
+
+ if (GetExitCodeThread (th->h, &ec)
+ && ec == STILL_ACTIVE)
+ {
+ BOOL status = SetThreadContext (th->h, &th->context);
+
+ if (!killed)
+ CHECK (status);
+ }
th->context.ContextFlags = 0;
}
if (th->suspended > 0)
@@ -1340,9 +1357,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
@@ -1560,7 +1577,7 @@ get_windows_debug_event (struct target_ops *ops,
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
{
@@ -2337,13 +2354,13 @@ windows_create_inferior (struct target_ops *ops, char *exec_file,
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)
{
@@ -2412,7 +2429,7 @@ windows_kill_inferior (struct target_ops *ops)
for (;;)
{
- if (!windows_continue (DBG_CONTINUE, -1))
+ if (!windows_continue (DBG_CONTINUE, -1, 1))
break;
if (!WaitForDebugEvent (¤t_event, INFINITE))
break;