[4/8] Step over thread exit, always delete the thread non-silently

Message ID 20231214202238.1065676-5-pedro@palves.net
State New
Headers
Series Step over thread exit improvements/fixes + AMD GPU |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Pedro Alves Dec. 14, 2023, 8:22 p.m. UTC
  With AMD GPU debugging, I noticed that when stepping over a breakpoint
placed on top of the s_endpgm instruction inline (displaced=off), GDB
would behave differently -- it wouldn't print the wave exit.  E.g:

With displaced stepping, or no breakpoint at all:

 stepi
 [AMDGPU Wave 1:4:1:1 (0,0,0)/0 exited]
 Command aborted, thread exited.
 (gdb)

With inline stepping:

 stepi
 Command aborted, thread exited.
 (gdb)

In the cases we see the "exited" notification, handle_thread_exit is
what first called delete_thread on the exiting thread, which is
non-silent.

With inline stepping, however, handle_thread_exit ends up in
update_thread_list (via restart_threads) before any delete_thread
call.  Thus, amd_dbgapi_target::update_thread_list notices that the
wave is gone and deletes it with delete_thread_silent.

This commit fixes it, by making handle_thread_exited call
set_thread_exited (with the default silent=false) early, which emits
the user-visible notification.

Change-Id: I22ab3145e18d07c99dace45576307b9f9d5d966f
---
 gdb/infrun.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
  

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 76693a30611..1d863896c40 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5885,6 +5885,13 @@  handle_thread_exited (execution_control_state *ecs)
      update the thread list and delete the event thread.  */
   bool abort_cmd = (ecs->event_thread->thread_fsm () != nullptr);
 
+  /* Mark the thread exited right now, because finish_step_over may
+     update the thread list and that may delete the thread silently
+     (depending on target), while we always want to emit the "[Thread
+     ... exited]" notification.  Don't actually delete the thread yet,
+     because we need to pass its pointer down to finish_step_over.  */
+  set_thread_exited (ecs->event_thread);
+
   /* Maybe the thread was doing a step-over, if so release
      resources and start any further pending step-overs.
 
@@ -5904,10 +5911,6 @@  handle_thread_exited (execution_control_state *ecs)
 	 event thread again, as finish_step_over may have switched
 	 threads.  */
       switch_to_thread (ecs->event_thread);
-
-      /* Emit [Thread ... exited] notification.  */
-      delete_thread (ecs->event_thread);
-
       ecs->event_thread = nullptr;
       return false;
     }