From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15869 invoked by alias); 18 Feb 2020 00:02:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 15855 invoked by uid 89); 18 Feb 2020 00:02:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=UD:process-dies-while-handling-bp.exp X-HELO: mail.efficios.com Received: from mail.efficios.com (HELO mail.efficios.com) (167.114.26.124) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Feb 2020 00:01:59 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 9877424FBD7 for ; Mon, 17 Feb 2020 19:01:57 -0500 (EST) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id N9zwhtdACxdh for ; Mon, 17 Feb 2020 19:01:57 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 391D024FBD6 for ; Mon, 17 Feb 2020 19:01:57 -0500 (EST) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 391D024FBD6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficios.com; s=default; t=1581984117; bh=7M6e7SApNCumiYDuL+/ry/QblIHJSFY6lwXaoecBMsU=; h=MIME-Version:From:Date:Message-ID:To; b=eSlwl6IVKZXi7szGC3fL8YnmmHuFILP67r+Mtvh8Qh/KLsCS2mm47HvA/oYzmGHXC YJQUZtdQ/WAbJJ+Xg7N6d2he8y9C4Ybk7l1KwgPoZKid75KiIIxt7IqOYG/1rQilES wwlsNf4MG2nyauhQdRvFTCU/9njWlrkfvBGBVWUddr/m6cWPuvpBoOF/nLyyQhPWrJ fO1zR0eYrKnE/sMphuj+c+INgNzM+njreKfreQBb5yiccMHtKvEOXNG5qwGTygEZsU bwT6cu+pwT6NWvYUnE9/1jSU/olEtbVzMuNiPqmiXMXnEChyhm5g8yVx423x2wG5Dj mEjiIBpdkaFTQ== Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id dJUKK0BlYKYw for ; Mon, 17 Feb 2020 19:01:57 -0500 (EST) Received: from mail-lj1-f169.google.com (mail-lj1-f169.google.com [209.85.208.169]) by mail.efficios.com (Postfix) with ESMTPSA id D66DE24FE94 for ; Mon, 17 Feb 2020 19:01:56 -0500 (EST) Received: by mail-lj1-f169.google.com with SMTP id x7so20800601ljc.1 for ; Mon, 17 Feb 2020 16:01:56 -0800 (PST) MIME-Version: 1.0 References: <20200217223021.38030-1-jeremie.galarneau@efficios.com> <86170965-88fc-a62a-ea65-8ea53d2d8643@linaro.org> In-Reply-To: <86170965-88fc-a62a-ea65-8ea53d2d8643@linaro.org> From: =?UTF-8?Q?J=C3=A9r=C3=A9mie_Galarneau?= Date: Tue, 18 Feb 2020 00:02:00 -0000 Message-ID: Subject: Re: [PATCH] gdb: print thread names in thread apply command output To: Luis Machado Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2020-02/txt/msg00697.txt.bz2 On Mon, 17 Feb 2020 at 18:17, Luis Machado wrote: > > Hi, > > Thanks for the patch. Some comments below. > > On 2/17/20 7:30 PM, J=C3=A9r=C3=A9mie Galarneau wrote: > > This makes the thread apply command print the thread's name. The use > > of target_pid_to_str is replaced by thread_target_id_str, which > > provides the same output as "info threads". > > Is this a cosmetic change to the way GDB outputs things? If so, I wonder > if the current tests expect the old format and thus will start to fail > with the new output? Have you exercised the testsuite? Hi, It does indeed introduce a cosmetic change. I have exercised the test suite with and without my patch applied. The only difference I see is "gdb.threads/process-dies-while-handling-bp.exp" which sometimes passes/fails, but I think this test is known to be flaky. > > > > > Before: > > (gdb) thread apply 2 bt > > > > Thread 2 (Thread 0x7fd245602700 (LWP 3837)): > > [...] > > > > After: > > (gdb) thread apply 2 bt > > > > Thread 2 (Thread 0x7fd245602700 (LWP 3837) "HT cleanup"): > > [...] > > > > The thread's description header is pre-computed before running the > > command since the command may change the selected inferior. This is > > not permitted by thread_target_id_str as target_thread_name asserts > > that `info->inf =3D=3D current_inferior ()`. > > > > This situation arises in the `gdb.threads/threadapply.exp` test which > > kills and removes the inferior as part of a "thread apply" command. > > > > gdb/ChangeLog: > > > > * thread.c (thr_try_catch_cmd): Print thread name. > > --- > > gdb/ChangeLog | 4 ++++ > > gdb/thread.c | 16 ++++++++++------ > > 2 files changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > > index dafd90ec37..4fce196187 100644 > > --- a/gdb/ChangeLog > > +++ b/gdb/ChangeLog > > @@ -1,3 +1,7 @@ > > +2020-02-17 J=C3=A9r=C3=A9mie Galarneau > > + > > + * thread.c (thr_try_catch_cmd): Print thread name. > > + > > 2020-02-14 Simon Marchi > > > > * aarch64-tdep.c (aarch64_displaced_step_copy_insn): Use > > diff --git a/gdb/thread.c b/gdb/thread.c > > index 54b59e2244..0c1611bdc1 100644 > > --- a/gdb/thread.c > > +++ b/gdb/thread.c > > @@ -1563,6 +1563,14 @@ thr_try_catch_cmd (thread_info *thr, const char = *cmd, int from_tty, > > const qcs_flags &flags) > > { > > switch_to_thread (thr); > > + > > + /* The thread header is computed before running the command since > > + * the command can change the inferior, which is not permitted > > + * by thread_target_id_str. */ > > The formatting of the comment block is not right. We don't start > intermediate lines with *. You can check existing blocks for the format > that we use. Right! Sorry about that. > > > + std::string thr_header =3D string_printf(_("\nThread %s (%s):\n"), > > + print_thread_id (thr), > > + thread_target_id_str (thr).c_str= ()); > > Maybe write it as: > > std::string thr_header =3D > string_printf(_("\nThread %s (%s):\n"), print_thread_id (thr), > thread_target_id_str (thr).c_str ()); > > I think it gives it a better alignment. Just a suggestion. Alright, I'll send a v2 with those comments addressed. Thanks for the review! J=C3=A9r=C3=A9mie > > > + > > try > > { > > std::string cmd_result =3D execute_command_to_string > > @@ -1570,9 +1578,7 @@ thr_try_catch_cmd (thread_info *thr, const char *= cmd, int from_tty, > > if (!flags.silent || cmd_result.length () > 0) > > { > > if (!flags.quiet) > > - printf_filtered (_("\nThread %s (%s):\n"), > > - print_thread_id (thr), > > - target_pid_to_str (inferior_ptid).c_str ()); > > + printf_filtered ("%s", thr_header.c_str ()); > > printf_filtered ("%s", cmd_result.c_str ()); > > } > > } > > @@ -1581,9 +1587,7 @@ thr_try_catch_cmd (thread_info *thr, const char *= cmd, int from_tty, > > if (!flags.silent) > > { > > if (!flags.quiet) > > - printf_filtered (_("\nThread %s (%s):\n"), > > - print_thread_id (thr), > > - target_pid_to_str (inferior_ptid).c_str ()); > > + printf_filtered ("%s", thr_header.c_str ()); > > if (flags.cont) > > printf_filtered ("%s\n", ex.what ()); > > else > > --=20 J=C3=A9r=C3=A9mie Galarneau EfficiOS Inc. http://www.efficios.com