gdb: centralize "[Thread ...exited]" notifications

Currently, each target backend is responsible for printing "[Thread
...exited]" before deleting a thread.  This leads to unnecessary
differences between targets, like e.g. with the remote target, we
never print such messages, even though we do print "[New Thread ...]".

E.g., debugging the gdb.threads/attach-many-short-lived-threads.exp
with gdbserver, letting it run for a bit, and then pressing Ctrl-C, we
currently see:

 (gdb) c
 Continuing.
 ^C[New Thread 3850398.3887449]
 [New Thread 3850398.3887500]
 [New Thread 3850398.3887551]
 [New Thread 3850398.3887602]
 [New Thread 3850398.3887653]
 ...

 Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
 0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
     at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
 78      in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
 (gdb)

Above, we only see "New Thread" notifications, even though threads
were deleted.

After this patch, we'll see:

 (gdb) c
 Continuing.
 ^C[Thread 3558643.3577053 exited]
 [Thread 3558643.3577104 exited]
 [Thread 3558643.3577155 exited]
 [Thread 3558643.3579603 exited]
 ...
 [New Thread 3558643.3597415]
 [New Thread 3558643.3600015]
 [New Thread 3558643.3599965]
 ...

 Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
 0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
     at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
 78      in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
 (gdb) q

This commit fixes this by moving the thread exit printing to common
code instead, triggered from within delete_thread (or rather,
set_thread_exited).

There's one wrinkle, though.  While most targest want to print:

 [Thread ... exited]

the Windows target wants to print:

 [Thread ... exited with code <exit_code>]

... and sometimes wants to suppress the notification for the main
thread.  To address that, this commits adds a delete_thread_with_code
function, only used by that target (so far).

This fix was originally posted as part of a larger series:

  https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/

But didn't really need to be part of that series.  In order to get
this fix merged sooner, I (Andrew Burgess) have rebased this commit
outside of the original series.  Any bugs introduced while splitting
this patch out and rebasing, are entirely my own.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30129
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
This commit is contained in:
Pedro Alves 2022-12-12 20:31:00 +00:00 committed by Andrew Burgess
parent 9324bfeab9
commit 9d7d58e726
18 changed files with 100 additions and 88 deletions

View File

@ -232,7 +232,9 @@ annotate_thread_changed (void)
/* Emit notification on thread exit. */
static void
annotate_thread_exited (struct thread_info *t, int silent)
annotate_thread_exited (thread_info *t,
gdb::optional<ULONGEST> exit_code,
bool /* silent */)
{
if (annotation_level > 1)
{

View File

@ -3362,7 +3362,9 @@ remove_breakpoints (void)
that thread. */
static void
remove_threaded_breakpoints (struct thread_info *tp, int silent)
remove_threaded_breakpoints (thread_info *tp,
gdb::optional<ULONGEST> /* exit_code */,
int /* silent */)
{
for (breakpoint &b : all_breakpoints_safe ())
{

View File

@ -1394,9 +1394,6 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
{
fbsd_lwp_debug_printf ("deleting thread for LWP %u",
pl.pl_lwpid);
if (print_thread_events)
gdb_printf (_("[%s exited]\n"),
target_pid_to_str (wptid).c_str ());
low_delete_thread (thr);
delete_thread (thr);
fbsd_inf->num_lwps--;

View File

@ -623,16 +623,30 @@ extern struct thread_info *add_thread_with_info (process_stratum_target *targ,
/* Delete thread THREAD and notify of thread exit. If the thread is
currently not deletable, don't actually delete it but still tag it
as exited and do the notification. */
extern void delete_thread (struct thread_info *thread);
as exited and do the notification. EXIT_CODE is the thread's exit
code. If SILENT, don't actually notify the CLI. THREAD must not
be NULL or an assertion will fail. */
extern void delete_thread_with_exit_code (thread_info *thread,
ULONGEST exit_code,
bool silent = false);
/* Delete thread THREAD and notify of thread exit. If the thread is
currently not deletable, don't actually delete it but still tag it
as exited and do the notification. THREAD must not be NULL or an
assertion will fail. */
extern void delete_thread (thread_info *thread);
/* Like delete_thread, but be quiet about it. Used when the process
this thread belonged to has already exited, for example. */
extern void delete_thread_silent (struct thread_info *thread);
/* Mark the thread exited, but don't delete it or remove it from the
inferior thread list. */
extern void set_thread_exited (thread_info *tp, bool silent);
inferior thread list. EXIT_CODE is the thread's exit code, if
available. If SILENT, then don't inform the CLI about the
exit. */
extern void set_thread_exited (thread_info *tp,
gdb::optional<ULONGEST> exit_code = {},
bool silent = false);
/* Delete a step_resume_breakpoint from the thread database. */
extern void delete_step_resume_breakpoint (struct thread_info *);

View File

@ -253,7 +253,7 @@ inferior::clear_thread_list ()
{
threads_debug_printf ("deleting thread %s",
thr->ptid.to_string ().c_str ());
set_thread_exited (thr, true /* silent */);
set_thread_exited (thr, {}, true /* silent */);
if (thr->deletable ())
delete thr;
});

View File

@ -457,9 +457,11 @@ interps_notify_new_thread (thread_info *t)
/* See interps.h. */
void
interps_notify_thread_exited (thread_info *t, int silent)
interps_notify_thread_exited (thread_info *t,
gdb::optional<ULONGEST> exit_code,
int silent)
{
interps_notify (&interp::on_thread_exited, t, silent);
interps_notify (&interp::on_thread_exited, t, exit_code, silent);
}
/* See interps.h. */

View File

@ -122,7 +122,9 @@ public:
virtual void on_new_thread (thread_info *t) {}
/* Notify the interpreter that thread T has exited. */
virtual void on_thread_exited (thread_info *, int silent) {}
virtual void on_thread_exited (thread_info *,
gdb::optional<ULONGEST> exit_code,
int silent) {}
/* Notify the interpreter that inferior INF was added. */
virtual void on_inferior_added (inferior *inf) {}
@ -297,7 +299,9 @@ extern void interps_notify_user_selected_context_changed
extern void interps_notify_new_thread (thread_info *t);
/* Notify all interpreters that thread T has exited. */
extern void interps_notify_thread_exited (thread_info *t, int silent);
extern void interps_notify_thread_exited (thread_info *t,
gdb::optional<ULONGEST> exit_code,
int silent);
/* Notify all interpreters that inferior INF was added. */
extern void interps_notify_inferior_added (inferior *inf);

View File

@ -906,13 +906,7 @@ exit_lwp (struct lwp_info *lp)
struct thread_info *th = linux_target->find_thread (lp->ptid);
if (th)
{
if (print_thread_events)
gdb_printf (_("[%s exited]\n"),
target_pid_to_str (lp->ptid).c_str ());
delete_thread (th);
}
delete_thread (th);
delete_lwp (lp->ptid);
}

View File

@ -277,7 +277,9 @@ mi_interp::on_new_thread (thread_info *t)
}
void
mi_interp::on_thread_exited (thread_info *t, int silent)
mi_interp::on_thread_exited (thread_info *t,
gdb::optional<ULONGEST> /* exit_code */,
int /* silent */)
{
target_terminal::scoped_restore_terminal_state term_state;
target_terminal::ours_for_output ();

View File

@ -51,7 +51,8 @@ public:
void on_command_error () override;
void on_user_selected_context_changed (user_selected_what selection) override;
void on_new_thread (thread_info *t) override;
void on_thread_exited (thread_info *t, int silent) override;
void on_thread_exited (thread_info *t, gdb::optional<ULONGEST> exit_code,
int silent) override;
void on_inferior_added (inferior *inf) override;
void on_inferior_appeared (inferior *inf) override;
void on_inferior_disappeared (inferior *inf) override;

View File

@ -626,9 +626,6 @@ nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
/* NetBSD does not store an LWP exit status. */
ourstatus->set_thread_exited (0);
if (print_thread_events)
gdb_printf (_("[%s exited]\n"),
target_pid_to_str (wptid).c_str ());
delete_thread (thr);
}

View File

@ -109,10 +109,13 @@ extern observable<struct objfile */* objfile */> free_objfile;
/* The thread specified by T has been created. */
extern observable<struct thread_info */* t */> new_thread;
/* The thread specified by T has exited. The SILENT argument
indicates that gdb is removing the thread from its tables without
wanting to notify the user about it. */
extern observable<struct thread_info */* t */, int /* silent */> thread_exit;
/* The thread specified by T has exited. EXIT_CODE is the thread's
exit code, if available. The SILENT argument indicates that GDB is
removing the thread from its tables without wanting to notify the
CLI about it. */
extern observable<thread_info */* t */,
gdb::optional<ULONGEST> /* exit_code */,
bool /* silent */> thread_exit;
/* An explicit stop request was issued to PTID. If PTID equals
minus_one_ptid, the request applied to all threads. If

View File

@ -2117,9 +2117,6 @@ wait_again:
case PR_SYSENTRY:
if (what == SYS_lwp_exit)
{
if (print_thread_events)
gdb_printf (_("[%s exited]\n"),
target_pid_to_str (retval).c_str ());
delete_thread (this->find_thread (retval));
proc_resume (pi, ptid, 0, GDB_SIGNAL_0);
goto wait_again;
@ -2223,9 +2220,6 @@ wait_again:
}
else if (what == SYS_lwp_exit)
{
if (print_thread_events)
gdb_printf (_("[%s exited]\n"),
target_pid_to_str (retval).c_str ());
delete_thread (this->find_thread (retval));
status->set_spurious ();
return retval;

View File

@ -361,7 +361,9 @@ add_thread_object (struct thread_info *tp)
}
static void
delete_thread_object (struct thread_info *tp, int ignore)
delete_thread_object (thread_info *tp,
gdb::optional<ULONGEST> /* exit_code */,
bool /* silent */)
{
if (!gdb_python_initialized)
return;

View File

@ -235,17 +235,9 @@ foreach_mi_ui_mode mode {
# The output has arrived! Check how we did. There are other bugs
# that come into play here which change what output we'll see.
if { $saw_mi_thread_exited && $saw_mi_bp_deleted \
&& $saw_cli_thread_exited \
&& $saw_cli_bp_deleted } {
kpass "gdb/30129" $gdb_test_name
} elseif { $saw_mi_thread_exited && $saw_mi_bp_deleted \
&& !$saw_cli_thread_exited \
&& $saw_cli_bp_deleted } {
kfail "gdb/30129" $gdb_test_name
} else {
fail "$gdb_test_name"
}
gdb_assert { $saw_mi_thread_exited && $saw_mi_bp_deleted \
&& $saw_cli_thread_exited \
&& $saw_cli_bp_deleted } $gdb_test_name
}
}

View File

@ -155,17 +155,7 @@ if {$is_remote} {
exp_continue
}
# When PR gdb/30129 is fixed then this can all be collapsed down
# into a single gdb_assert call. This is split out like this
# because the SAW_BP_DELETED part is working, and we want to
# spot if that stops working.
if { $saw_thread_exited && $saw_bp_deleted } {
kpass "gdb/30129" $gdb_test_name
} elseif {!$saw_thread_exited && $saw_bp_deleted} {
kfail "gdb/30129" $gdb_test_name
} else {
fail $gdb_test_name
}
gdb_assert { $saw_thread_exited && $saw_bp_deleted } $gdb_test_name
}
}
} else {

View File

@ -194,16 +194,29 @@ clear_thread_inferior_resources (struct thread_info *tp)
/* Notify interpreters and observers that thread T has exited. */
static void
notify_thread_exited (thread_info *t, int silent)
notify_thread_exited (thread_info *t, gdb::optional<ULONGEST> exit_code,
int silent)
{
interps_notify_thread_exited (t, silent);
gdb::observers::thread_exit.notify (t, silent);
if (!silent && print_thread_events)
{
if (exit_code.has_value ())
gdb_printf (_("[%s exited with code %s]\n"),
target_pid_to_str (t->ptid).c_str (),
pulongest (*exit_code));
else
gdb_printf (_("[%s exited]\n"),
target_pid_to_str (t->ptid).c_str ());
}
interps_notify_thread_exited (t, exit_code, silent);
gdb::observers::thread_exit.notify (t, exit_code, silent);
}
/* See gdbthread.h. */
void
set_thread_exited (thread_info *tp, bool silent)
set_thread_exited (thread_info *tp, gdb::optional<ULONGEST> exit_code,
bool silent)
{
/* Dead threads don't need to step-over. Remove from chain. */
if (thread_is_in_step_over_chain (tp))
@ -222,7 +235,7 @@ set_thread_exited (thread_info *tp, bool silent)
if (proc_target != nullptr)
proc_target->maybe_remove_resumed_with_pending_wait_status (tp);
notify_thread_exited (tp, silent);
notify_thread_exited (tp, exit_code, silent);
/* Tag it as exited. */
tp->state = THREAD_EXITED;
@ -470,20 +483,22 @@ global_thread_step_over_chain_remove (struct thread_info *tp)
global_thread_step_over_list.erase (it);
}
/* Delete the thread referenced by THR. If SILENT, don't notify
the observer of this exit.
THR must not be NULL or a failed assertion will be raised. */
/* Helper for the different delete_thread variants. */
static void
delete_thread_1 (thread_info *thr, bool silent)
delete_thread_1 (thread_info *thr, gdb::optional<ULONGEST> exit_code,
bool silent)
{
gdb_assert (thr != nullptr);
threads_debug_printf ("deleting thread %s, silent = %d",
thr->ptid.to_string ().c_str (), silent);
threads_debug_printf ("deleting thread %s, exit_code = %s, silent = %d",
thr->ptid.to_string ().c_str (),
(exit_code.has_value ()
? pulongest (*exit_code)
: "<none>"),
silent);
set_thread_exited (thr, silent);
set_thread_exited (thr, exit_code, silent);
if (!thr->deletable ())
{
@ -499,16 +514,25 @@ delete_thread_1 (thread_info *thr, bool silent)
/* See gdbthread.h. */
void
delete_thread_with_exit_code (thread_info *thread, ULONGEST exit_code,
bool silent)
{
delete_thread_1 (thread, exit_code, silent);
}
/* See gdbthread.h. */
void
delete_thread (thread_info *thread)
{
delete_thread_1 (thread, false /* not silent */);
delete_thread_1 (thread, {}, false /* not silent */);
}
void
delete_thread_silent (thread_info *thread)
{
delete_thread_1 (thread, true /* silent */);
delete_thread_1 (thread, {}, true /* not silent */);
}
struct thread_info *

View File

@ -612,21 +612,13 @@ windows_nat_target::delete_thread (ptid_t ptid, DWORD exit_code,
id = ptid.lwp ();
/* Emit a notification about the thread being deleted.
/* Note that no notification was printed when the main thread was
created, and thus, unless in verbose mode, we should be symmetrical,
and avoid an exit notification for the main thread here as well. */
Note that no notification was printed when the main thread
was created, and thus, unless in verbose mode, we should be
symmetrical, and avoid that notification for the main thread
here as well. */
if (info_verbose)
gdb_printf ("[Deleting %s]\n", target_pid_to_str (ptid).c_str ());
else if (print_thread_events && !main_thread_p)
gdb_printf (_("[%s exited with code %u]\n"),
target_pid_to_str (ptid).c_str (),
(unsigned) exit_code);
::delete_thread (this->find_thread (ptid));
bool silent = (main_thread_p && !info_verbose);
thread_info *to_del = this->find_thread (ptid);
delete_thread_with_exit_code (to_del, exit_code, silent);
auto iter = std::find_if (windows_process.thread_list.begin (),
windows_process.thread_list.end (),