]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Centralize "[Thread ...exited]" notifications
authorPedro Alves <pedro@palves.net>
Tue, 21 Jun 2022 18:30:48 +0000 (19:30 +0100)
committerPedro Alves <pedro@palves.net>
Mon, 18 Jul 2022 16:34:41 +0000 (17:34 +0100)
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).

Change-Id: I06ec07b7c51527872a9713dd11cf7867b50fc5ff

15 files changed:
gdb/annotate.c
gdb/breakpoint.c
gdb/fbsd-nat.c
gdb/gdbthread.h
gdb/inferior.c
gdb/inferior.h
gdb/infrun.c
gdb/linux-nat.c
gdb/mi/mi-interp.c
gdb/netbsd-nat.c
gdb/observable.h
gdb/procfs.c
gdb/python/py-inferior.c
gdb/thread.c
gdb/windows-nat.c

index 33805dcdb307b3a5d9a7d9917e5da1592da5a490..b45384ddb159e87a1854f63af8638e88dc2c8453 100644 (file)
@@ -233,7 +233,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)
     {
index 74f53368464a20e4c0c8cf78ebca79d630ca0631..62a1850c144eaeac3b0e995a3fdea991d57e0fca 100644 (file)
@@ -3219,7 +3219,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,
+                            bool /* silent */)
 {
   for (breakpoint *b : all_breakpoints_safe ())
     {
index 32b289f3bbc1e58c99d4bc2fb74f618020d3e662..f5427741156ed43ea57ffe3dfd4cdc915b262e30 100644 (file)
@@ -1300,9 +1300,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);
                }
index 43e9d6ea484994c2873e05c470848a554bde3d3f..7ab02873f1730639e4cf58ace39b6f3f3f6517a3 100644 (file)
@@ -636,16 +636,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 *);
index c1d2be6e3a2ab1548b8f521af694fd43ca2ab41f..56fd1d09abc6a9367d8b7968c4b3e9e4fcfe26a1 100644 (file)
@@ -183,7 +183,7 @@ inferior::clear_thread_list ()
     {
       threads_debug_printf ("deleting thread %s",
                            thr->ptid.to_string ().c_str ());
-      set_thread_exited (thr, true);
+      set_thread_exited (thr, {}, true);
       if (thr->deletable ())
        delete thr;
     });
index 9c1dcec0cea80c2013266d4a06a02f18f8f8e9df..6a677d7ab9e2c5098bd59b63f2f58054b77907ed 100644 (file)
@@ -636,6 +636,8 @@ extern void detach_inferior (inferior *inf);
 
 extern void exit_inferior (inferior *inf);
 
+/* Like exit_inferior, but be quiet -- don't announce the exit of the
+   inferior's threads to the CLI.  */
 extern void exit_inferior_silent (inferior *inf);
 
 extern void exit_inferior_num_silent (int num);
index 44e0a7c232a8269f6ae78883e242964f9eff1ac6..0f2f033676b3d916a99f47dea3683afb8f7e003c 100644 (file)
@@ -3703,7 +3703,9 @@ infrun_thread_stop_requested (ptid_t ptid)
 }
 
 static void
-infrun_thread_thread_exit (struct thread_info *tp, int silent)
+infrun_thread_thread_exit (thread_info *tp,
+                          gdb::optional<ULONGEST> /* exit_code */,
+                          bool /* silent */)
 {
   if (target_last_proc_target == tp->inf->process_target ()
       && target_last_wait_ptid == tp->ptid)
index 0d0cbf5552158f08ace3dd0df3185390015ec83b..6e9cc93668cd3e030dfe3d154c3ed39ad21bfe7c 100644 (file)
@@ -913,15 +913,10 @@ linux_nat_switch_fork (ptid_t new_ptid)
 static void
 exit_lwp (struct lwp_info *lp, bool del_thread = true)
 {
-  struct thread_info *th = find_thread_ptid (linux_target, lp->ptid);
-
-  if (th)
+  if (del_thread)
     {
-      if (print_thread_events)
-       gdb_printf (_("[%s exited]\n"),
-                   target_pid_to_str (lp->ptid).c_str ());
-
-      if (del_thread)
+      thread_info *th = find_thread_ptid (linux_target, lp->ptid);
+      if (th != nullptr)
        delete_thread (th);
     }
 
index ae15177890c4781d7071a142fc9acd2264dc86fe..9bb86138e3709390bbc24b11afa37f4704ee1629 100644 (file)
@@ -68,7 +68,9 @@ static void mi_on_normal_stop (struct bpstat *bs, int print_frame);
 static void mi_on_no_history (void);
 
 static void mi_new_thread (struct thread_info *t);
-static void mi_thread_exit (struct thread_info *t, int silent);
+static void mi_thread_exit (thread_info *t,
+                           gdb::optional<ULONGEST> exit_code,
+                           bool silent);
 static void mi_record_changed (struct inferior*, int, const char *,
                               const char *);
 static void mi_inferior_added (struct inferior *inf);
@@ -355,8 +357,10 @@ mi_new_thread (struct thread_info *t)
     }
 }
 
+/* Observer for the thread_exit notification.  */
+
 static void
-mi_thread_exit (struct thread_info *t, int silent)
+mi_thread_exit (thread_info *t, gdb::optional<ULONGEST> exit_code, bool silent)
 {
   SWITCH_THRU_ALL_UIS ()
     {
index c45df391afc6a2eaaa04747d5460e310871b68a0..5224b8a895f025f124ac5c6737a3c07a06f4fd21 100644 (file)
@@ -624,10 +624,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);
        }
 
index 796bf2a43c60572c42325d4cfcc54063e6fb963b..9706deb58740d88a8f747d4af3a516efdaca1f15 100644 (file)
@@ -126,10 +126,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
index ffb4d180290a202815d1bcb4c4b1d6338d564b21..09d37b4cc6fe27ad97d50670443da16bf85a15b1 100644 (file)
@@ -2122,9 +2122,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 (find_thread_ptid (this, retval));
                    target_continue_no_signal (ptid);
                    goto wait_again;
@@ -2229,9 +2226,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 (find_thread_ptid (this, retval));
                    status->set_spurious ();
                    return retval;
index ebcd5b0a70fc31976b2b29cd244cddf1b5b5a657..76a0cfc45fbbf34bb5c60b2323a8718d069df432 100644 (file)
@@ -340,7 +340,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 */)
 {
   struct threadlist_entry **entry, *tmp;
 
index 9aa2189160e79b6aad6024b770439fe07038df7a..f8c8b9bc6c03bc802e01251cd11f28e68862eb2b 100644 (file)
@@ -192,7 +192,8 @@ clear_thread_inferior_resources (struct thread_info *tp)
 /* 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))
@@ -211,7 +212,22 @@ set_thread_exited (thread_info *tp, bool silent)
       if (proc_target != nullptr)
        proc_target->maybe_remove_resumed_with_pending_wait_status (tp);
 
-      gdb::observers::thread_exit.notify (tp, silent);
+      if (!silent && print_thread_events)
+       {
+         if (exit_code.has_value ())
+           {
+             gdb_printf (_("[%s exited with code %s]\n"),
+                         target_pid_to_str (tp->ptid).c_str (),
+                         pulongest (*exit_code));
+           }
+         else
+           {
+             gdb_printf (_("[%s exited]\n"),
+                         target_pid_to_str (tp->ptid).c_str ());
+           }
+       }
+
+      gdb::observers::thread_exit.notify (tp, exit_code, silent);
 
       /* Tag it as exited.  */
       tp->state = THREAD_EXITED;
@@ -467,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 ())
     {
@@ -496,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, false /* not 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 *
index 9705fa33cc2f52f094d9362224d932ab30d2f6d1..8371de010134a5e7c107a6d2680cd350f10aa1b4 100644 (file)
@@ -434,21 +434,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
+  /* 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 (find_thread_ptid (&the_windows_nat_target, ptid));
+  bool silent = (main_thread_p && !info_verbose);
+  thread_info *todel = find_thread_ptid (&the_windows_nat_target, ptid);
+  delete_thread_with_exit_code (todel, exit_code, silent);
 
   auto iter = std::find_if (windows_process.thread_list.begin (),
                            windows_process.thread_list.end (),