Re: [PATCH] Fix "PC register is not available" issue

[Eli Zaretskii <eliz at gnu dot org>]

> 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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fd9677b..23ca6c0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -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.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index fe40c4d..bad7408 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -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 (&current_event, INFINITE))
 	break;