[25/31] Ignore failure to read PC when resuming

Message ID 20221212203101.1034916-26-pedro@palves.net
State New
Series Step over thread clone and thread exit |

Commit Message

Pedro Alves Dec. 12, 2022, 8:30 p.m. UTC
  If GDB sets a GDB_THREAD_OPTION_EXIT option on a thread, and the
thread exits, the server reports the corresponding thread exit event,
and forgets about the thread, i.e., removes the exited thread from its
thread list.

On the GDB side, GDB set the GDB_THREAD_OPTION_EXIT option on a
thread, GDB delays deleting the thread from its thread list until it
sees the corresponding thread exit event, as that event needs special
handling in infrun.

When a thread disappears from the target, but it still exists on GDB's
thread list, in all-stop RSP mode, it can happen that GDB ends up
trying to resume such an already-exited-thread that GDB doesn't yet
know is gone.  When that happens, against GDBserver, typically the
ongoing execution command fails with this error:

 PC register is not available

At the remote protocol level, we may see e.g., this:

      [remote] Packet received: w0;p97479.978d2
    [remote] wait: exit
    [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
    [infrun] print_target_wait_results:   619641.620754.0 [Thread 619641.620754],
    [infrun] print_target_wait_results:   status->kind = THREAD_EXITED, exit_status = 0
    [infrun] handle_inferior_event: status->kind = THREAD_EXITED, exit_status = 0
    [infrun] context_switch: Switching context from 0.0.0 to 619641.620754.0
    [infrun] clear_proceed_status_thread: 619641.620754.0

GDB saw an exit event for thread 619641.620754.  After processing it,
infrun decides to re-resume the target again.  To do that, infrun
picks some other thread that isn't exited yet from GDB's perspective,
switches to it, and calls keep_going.  Below, infrun happens to pick
thread p97479.97479, the leader, which also exited, but GDB doesn't
know yet:

    [remote] Sending packet: $Hgp97479.97479#75
    [remote] Packet received: OK
    [remote] Sending packet: $g#67
    [remote] Packet received: xxxxxxxxxxxxxxxxx (...snip...) [1120 bytes omitted]
    [infrun] reset: reason=handling event
    [infrun] maybe_set_commit_resumed_all_targets: not requesting commit-resumed for target remote, no resumed threads
  [infrun] fetch_inferior_event: exit
  PC register is not available

The Linux backends, both in GDB and in GDBserver, already silently
ignore failures to resume, with the understanding that we'll see an
exit event soon.  Core of GDB doesn't do that yet, though.

This patch is a small step in that direction.  It swallows the error
when thrown from within resume_1.  There are likely are spots where we
will need similar treatment, but we can tackle them as we find them.

After this patch, we'll see something like this instead:

    [infrun] resume_1: step=0, signal=GDB_SIGNAL_0, trap_expected=0, current thread [640478.640478.0] at 0x0
    [infrun] do_target_resume: resume_ptid=640478.0.0, step=0, sig=GDB_SIGNAL_0
    [remote] Sending packet: $vCont;c:p9c5de.-1#78
    [infrun] prepare_to_wait: prepare_to_wait
    [infrun] reset: reason=handling event
    [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target remote
    [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target remote
  [infrun] fetch_inferior_event: exit
  [infrun] fetch_inferior_event: enter
    [infrun] scoped_disable_commit_resumed: reason=handling event
    [infrun] random_pending_event_thread: None found.
    [remote] wait: enter
      [remote] Packet received: W0;process:9c5de
    [remote] wait: exit
    [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
    [infrun] print_target_wait_results:   640478.0.0 [process 640478],
    [infrun] print_target_wait_results:   status->kind = EXITED, exit_status = 0
    [infrun] handle_inferior_event: status->kind = EXITED, exit_status = 0
  [Inferior 1 (process 640478) exited normally]
    [infrun] stop_waiting: stop_waiting
    [infrun] reset: reason=handling event
  (gdb) [infrun] fetch_inferior_event: exit

Change-Id: I7f1c7610923435c4e98e70acc5ebe5ebbac581e2
 gdb/infrun.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)


diff --git a/gdb/infrun.c b/gdb/infrun.c
index 09391d85256..21e5aa0f50e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2595,7 +2595,28 @@  resume_1 (enum gdb_signal sig)
       step = false;
-  CORE_ADDR pc = regcache_read_pc (regcache);
+  CORE_ADDR pc = 0;
+  try
+    {
+      pc = regcache_read_pc (regcache);
+    }
+  catch (const gdb_exception_error &err)
+    {
+      /* Swallow errors as it may be that the current thread exited
+	 and we've haven't seen its exit status yet.  Let the
+	 resumption continue and we'll collect the exit event
+	 shortly.  */
+      if (err.error == TARGET_CLOSE_ERROR)
+	throw;
+      if (debug_infrun)
+	{
+	  string_file buf;
+	  exception_print (&buf, err);
+	  infrun_debug_printf ("resume: swallowing error: %s",
+			       buf.string ().c_str ());
+	}
+    }
   infrun_debug_printf ("step=%d, signal=%s, trap_expected=%d, "
 		       "current thread [%s] at %s",