From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) by sourceware.org (Postfix) with ESMTPS id C1C3D393F959 for ; Wed, 13 Jul 2022 22:25:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C1C3D393F959 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f43.google.com with SMTP id d16so8511wrv.10 for ; Wed, 13 Jul 2022 15:25:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=1v+x70mqjwQmAz8uH3ng9RWT3I2EizxkNPCOXfcvwcA=; b=rBhJk0tKAlggesS/6Ni1Wyw0dnGbkNqNmNzX5JDZS+bffawPMW82tdRPPQcYhQ70WA OgWH53KfESkqBCmCJ+SnO68hK/FZB+A9deQa74kY2fPVUdF8x2+nO1ixEtdkMUwcBjWv QoC7R9c531yPxrytp6ZLfdCajxXX3TyQtM5IK0d0up4XGs3DyMN2DslcZhCEQPxZeEUW sJwk60nYy66fNAzVhOhCJXkq6FiG71Ip2MH9VYr2LSTBsSa670Dy8dHnyEPlmgJNHR9w /P/+bbRpVg5hX3as36u6do7h5ue5fLRZ6deUeKZkXyugFFvNMRXxAsztbzRwrQmXnME5 Rdpg== X-Gm-Message-State: AJIora8J6XHH9LGYGdTM2W/7SPUIFtH+Jeuurd3UUfdLhXAf+cgaTIsX faTTrZwM2kct4vu+IBmrW/t50iR+rVs= X-Google-Smtp-Source: AGRyM1tmqn/As7RHFZ92gcVoV2lZShp/a0FhSdfrLgqITgKY9uv8GF67YoHLk916iRkx494NBhEUrg== X-Received: by 2002:a5d:4d92:0:b0:21d:6f02:d971 with SMTP id b18-20020a5d4d92000000b0021d6f02d971mr5062861wru.300.1657751138482; Wed, 13 Jul 2022 15:25:38 -0700 (PDT) Received: from localhost ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id o38-20020a05600c512600b003a2e2e965absm3347520wms.20.2022.07.13.15.25.37 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 13 Jul 2022 15:25:37 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH v2 28/29] Centralize "[Thread ...exited]" notifications Date: Wed, 13 Jul 2022 23:24:32 +0100 Message-Id: <20220713222433.374898-29-pedro@palves.net> X-Mailer: git-send-email 2.36.0 In-Reply-To: <20220713222433.374898-1-pedro@palves.net> References: <20220713222433.374898-1-pedro@palves.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jul 2022 22:25:42 -0000 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 ] ... 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 --- gdb/annotate.c | 4 +++- gdb/breakpoint.c | 4 +++- gdb/fbsd-nat.c | 3 --- gdb/gdbthread.h | 22 +++++++++++++---- gdb/inferior.c | 2 +- gdb/inferior.h | 2 ++ gdb/infrun.c | 4 +++- gdb/linux-nat.c | 11 +++------ gdb/mi/mi-interp.c | 8 +++++-- gdb/netbsd-nat.c | 4 ---- gdb/observable.h | 11 +++++---- gdb/procfs.c | 6 ----- gdb/python/py-inferior.c | 4 +++- gdb/thread.c | 51 ++++++++++++++++++++++++++++++---------- gdb/windows-nat.c | 16 ++++--------- 15 files changed, 92 insertions(+), 60 deletions(-) diff --git a/gdb/annotate.c b/gdb/annotate.c index 33805dcdb30..b45384ddb15 100644 --- a/gdb/annotate.c +++ b/gdb/annotate.c @@ -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 exit_code, + bool /* silent */) { if (annotation_level > 1) { diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 74f53368464..62a1850c144 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -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 exit_code, + bool /* silent */) { for (breakpoint *b : all_breakpoints_safe ()) { diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c index 32b289f3bbc..f5427741156 100644 --- a/gdb/fbsd-nat.c +++ b/gdb/fbsd-nat.c @@ -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); } diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 43e9d6ea484..7ab02873f17 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -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 exit_code = {}, + bool silent = false); /* Delete a step_resume_breakpoint from the thread database. */ extern void delete_step_resume_breakpoint (struct thread_info *); diff --git a/gdb/inferior.c b/gdb/inferior.c index c1d2be6e3a2..56fd1d09abc 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -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; }); diff --git a/gdb/inferior.h b/gdb/inferior.h index 9c1dcec0cea..6a677d7ab9e 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -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); diff --git a/gdb/infrun.c b/gdb/infrun.c index e565c9b0e33..c410dcdeb20 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -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 /* exit_code */, + bool /* silent */) { if (target_last_proc_target == tp->inf->process_target () && target_last_wait_ptid == tp->ptid) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 0d0cbf55521..6e9cc93668c 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -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); } diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 937a3d9d6f5..a930002a2b9 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -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 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 exit_code, bool silent) { SWITCH_THRU_ALL_UIS () { diff --git a/gdb/netbsd-nat.c b/gdb/netbsd-nat.c index c45df391afc..5224b8a895f 100644 --- a/gdb/netbsd-nat.c +++ b/gdb/netbsd-nat.c @@ -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); } diff --git a/gdb/observable.h b/gdb/observable.h index 796bf2a43c6..9706deb5874 100644 --- a/gdb/observable.h +++ b/gdb/observable.h @@ -126,10 +126,13 @@ extern observable free_objfile; /* The thread specified by T has been created. */ extern observable 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 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 /* 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 diff --git a/gdb/procfs.c b/gdb/procfs.c index ffb4d180290..09d37b4cc6f 100644 --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -2122,9 +2122,6 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status, 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 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status, } 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; diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c index ebcd5b0a70f..76a0cfc45fb 100644 --- a/gdb/python/py-inferior.c +++ b/gdb/python/py-inferior.c @@ -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 /* exit_code */, + bool /* silent */) { struct threadlist_entry **entry, *tmp; diff --git a/gdb/thread.c b/gdb/thread.c index 9aa2189160e..f8c8b9bc6c0 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -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 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 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) + : ""), + 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 * diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 9705fa33cc2..8371de01013 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -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 (), -- 2.36.0