From: Jan Vrany <jan.vrany@labware.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH v2 1/1] gdb/mi: PR20684, preserve user selected thread and frame when invoking MI commands
Date: Wed, 26 Jan 2022 17:08:38 +0000 [thread overview]
Message-ID: <083e4514af3e0707035fa8517e107adc58da051e.camel@labware.com> (raw)
In-Reply-To: <74e5b388-9f7a-dd0a-befe-3b069fba54b7@polymtl.ca>
Hi,
On Wed, 2022-01-26 at 10:01 -0500, Simon Marchi wrote:
> Thanks, this is OK.
Unfortunately, it is not. I have found a regression when doing final
pre-push testing. I think I have a tentative fix, but needs more
testing. I'll post v3 of this patch tomorrow.
Sorry about that!
Jan
>
> Simon
>
> On 2022-01-26 08:21, Jan Vrany via Gdb-patches wrote:
> > When invoking MI commands with --thread and/or --frame, user selected
> > thread and frame was not preserved:
> >
> > (gdb)
> > info thread
> > &"info thread\n"
> > ~" Id Target Id Frame \n"
> > ~"* 1 Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60\n"
> > ~" 2 Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
> > ~" 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
> > ^done
> > (gdb)
> > info frame
> > &"info frame\n"
> > ~"Stack level 0, frame at 0x7fffffffdf90:\n"
> > ~" rip = 0x555555555207 in main (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60); saved rip = 0x7ffff7c5709b\n"
> > ~" source language c.\n"
> > ~" Arglist at 0x7fffffffdf80, args: \n"
> > ~" Locals at 0x7fffffffdf80, Previous frame's sp is 0x7fffffffdf90\n"
> > ~" Saved registers:\n "
> > ~" rbp at 0x7fffffffdf80, rip at 0x7fffffffdf88\n"
> > ^done
> > (gdb)
> > -stack-info-depth --thread 3
> > ^done,depth="4"
> > (gdb)
> > info thread
> > &"info thread\n"
> > ~" Id Target Id Frame \n"
> > ~" 1 Thread 0x7ffff7c30740 (LWP 19302) \"user-selected-c\" main () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:60\n"
> > ~" 2 Thread 0x7ffff7c2f700 (LWP 19306) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
> > ~"* 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30\n"
> > ^done
> > (gdb)
> > info frame
> > &"info frame\n"
> > ~"Stack level 0, frame at 0x7ffff742dee0:\n"
> > ~" rip = 0x555555555169 in child_sub_function (/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30); saved rip = 0x555555555188\n"
> > ~" called by frame at 0x7ffff742df00\n"
> > ~" source language c.\n"
> > ~" Arglist at 0x7ffff742ded0, args: \n"
> > ~" Locals at 0x7ffff742ded0, Previous frame's sp is 0x7ffff742dee0\n"
> > ~" Saved registers:\n "
> > ~" rbp at 0x7ffff742ded0, rip at 0x7ffff742ded8\n"
> > ^done
> > (gdb)
> >
> > This was problematic for frontends that provide access to CLI because UI
> > may silently change the context for CLI commands (as demonstrated above).
> >
> > Bug: https://urldefense.proofpoint.com/v2/url?u=https-3A__sourceware.org_bugzilla_show-5Fbug.cgi-3Fid-3D20684&d=DwICaQ&c=sPZ6DeHLiehUHQWKIrsNwWp3t7snrE-az24ztT0w7Jc&r=WpFFGgYa98Yp-c29WHTCwU1wAGFBvszA6a4RzgpMSqc&m=YXXmP3UHATiIQ8jPJrNxkE33ypHZ5NAYKMAvSdtuERM&s=wWlBE5Q8xGagnhvV57p8mRde5PHtg__CrplJVjeVGIU&e=
> > ---
> > gdb/mi/mi-cmds.h | 12 ++
> > gdb/mi/mi-main.c | 13 +-
> > gdb/testsuite/gdb.mi/mi-cmd-user-context.exp | 155 +++++++++++++++++++
> > 3 files changed, 178 insertions(+), 2 deletions(-)
> > create mode 100644 gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
> >
> > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> > index 2a93a9f5476..785652ee1c9 100644
> > --- a/gdb/mi/mi-cmds.h
> > +++ b/gdb/mi/mi-cmds.h
> > @@ -23,6 +23,7 @@
> > #define MI_MI_CMDS_H
> >
> >
> >
> >
> > #include "gdbsupport/gdb_optional.h"
> > +#include "mi/mi-main.h"
> >
> >
> >
> >
> > enum print_values {
> > PRINT_NO_VALUES,
> > @@ -163,6 +164,17 @@ struct mi_command
> > wrong. */
> > void invoke (struct mi_parse *parse) const;
> >
> >
> >
> >
> > + /* Return whether this command preserves user selected context (thread
> > + and frame). */
> > + bool preserve_user_selected_context () const
> > + {
> > + /* Here we exploit the fact that if MI command is supposed to change
> > + user context, then it should not emit change notifications. Therefore if
> > + command does not suppress user context change notifications, then it should
> > + preserve the context. */
> > + return m_suppress_notification != &mi_suppress_notification.user_selected_context;
> > + }
> > +
> > protected:
> >
> >
> >
> >
> > /* The core of command invocation, this needs to be overridden in each
> > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > index 4860da7536a..b25e87a1476 100644
> > --- a/gdb/mi/mi-main.c
> > +++ b/gdb/mi/mi-main.c
> > @@ -2037,6 +2037,7 @@ mi_cmd_execute (struct mi_parse *parse)
> > set_current_program_space (inf->pspace);
> > }
> >
> >
> >
> >
> > + gdb::optional<scoped_restore_current_thread> thread_saver;
> > if (parse->thread != -1)
> > {
> > thread_info *tp = find_thread_global_id (parse->thread);
> > @@ -2047,9 +2048,13 @@ mi_cmd_execute (struct mi_parse *parse)
> > if (tp->state == THREAD_EXITED)
> > error (_("Thread id: %d has terminated"), parse->thread);
> >
> >
> >
> >
> > + if (parse->cmd->preserve_user_selected_context ())
> > + thread_saver.emplace ();
> > +
> > switch_to_thread (tp);
> > }
> >
> >
> >
> >
> > + gdb::optional<scoped_restore_selected_frame> frame_saver;
> > if (parse->frame != -1)
> > {
> > struct frame_info *fid;
> > @@ -2057,8 +2062,12 @@ mi_cmd_execute (struct mi_parse *parse)
> >
> >
> >
> >
> > fid = find_relative_frame (get_current_frame (), &frame);
> > if (frame == 0)
> > - /* find_relative_frame was successful */
> > - select_frame (fid);
> > + {
> > + if (parse->cmd->preserve_user_selected_context ())
> > + frame_saver.emplace ();
> > +
> > + select_frame (fid);
> > + }
> > else
> > error (_("Invalid frame id: %d"), frame);
> > }
> > diff --git a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
> > new file mode 100644
> > index 00000000000..a373dd3a4b0
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
> > @@ -0,0 +1,155 @@
> > +# Copyright 2022 Free Software Foundation, Inc.
> > +
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program. If not, see <https://urldefense.proofpoint.com/v2/url?u=http-3A__www.gnu.org_licenses_&d=DwICaQ&c=sPZ6DeHLiehUHQWKIrsNwWp3t7snrE-az24ztT0w7Jc&r=WpFFGgYa98Yp-c29WHTCwU1wAGFBvszA6a4RzgpMSqc&m=YXXmP3UHATiIQ8jPJrNxkE33ypHZ5NAYKMAvSdtuERM&s=e40OHXPdnSaDFlYkl3p8U6MtVrs_pIOj7_bbg84qYT4&e= >.
> > +
> > +# Test that GDB/MI commands preserve user selected context when
> > +# passed --thread and/or --frame.
> > +
> > +load_lib mi-support.exp
> > +
> > +standard_testfile user-selected-context-sync.c
> > +
> > +if {[build_executable $testfile.exp $testfile ${srcfile} "debug pthreads"] == -1} {
> > + untested "failed to compile"
> > + return -1
> > +}
> > +
> > +set main_break_line [gdb_get_line_number "main break line"]
> > +
> > +set any "\[^\r\n\]*"
> > +set nl "\[\r\n\]"
> > +
> > +mi_clean_restart $binfile
> > +mi_create_breakpoint "$srcfile:$main_break_line" "set breakpoint in main"
> > +mi_run_cmd
> > +mi_expect_stop "breakpoint-hit" "main" "" $srcfile $main_break_line \
> > + { "" "disp=\"keep\"" } "run to breakpoint in main"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 1.*" \
> > + "info thread 1"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-stack-info-depth --thread 3" \
> > + "\\^done,depth=.*" \
> > + "-stack-info-depth --thread 3"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 1.*" \
> > + "info thread 2"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-thread-select 3" \
> > + "\\^done,.*" \
> > + "-thread-select 3"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 3.*" \
> > + "info thread 3"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-thread-select --thread 2 1" \
> > + "\\^done,.*" \
> > + "-thread-select --thread 2 1"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 1.*" \
> > + "info thread 4"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-thread-select --thread 2 2" \
> > + "\\^done,.*" \
> > + "-thread-select --thread 2 2"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 2.*" \
> > + "info thread 5"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "frame" \
> > + ".*#0 0x.*" \
> > + "frame 1"
> > +
> > +mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \
> > + "\\^done,frame=\{level=\"1\".*" \
> > + "-stack-info-frame 1"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 2.*" \
> > + "info thread 6"
> > +
> > +mi_gdb_test "frame" \
> > + ".*#0 0x.*" \
> > + "frame 2"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-stack-info-frame --thread 3 --frame 1" \
> > + "\\^done,frame=\{level=\"1\".*" \
> > + "-stack-info-frame 2"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 2.*" \
> > + "info thread 7"
> > +
> > +mi_gdb_test "frame" \
> > + ".*#0 0x.*" \
> > + "frame 3"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-stack-select-frame --thread 2 --frame 0 1" \
> > + "\\^done" \
> > + "--stack-select-frame 1"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 2.*" \
> > + "info thread 8"
> > +
> > +mi_gdb_test "frame" \
> > + ".*#1 0x.*" \
> > + "frame 4"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-stack-select-frame --thread 2 --frame 2 2" \
> > + "\\^done" \
> > + "--stack-select-frame 2"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 2.*" \
> > + "info thread 9"
> > +
> > +mi_gdb_test "frame" \
> > + ".*#2 0x.*" \
> > + "frame 5"
> > +
> > +#=========================
> > +
> > +mi_gdb_test "-stack-select-frame --thread 1 --frame 0 0" \
> > + "\\^done" \
> > + "--stack-select-frame 3"
> > +
> > +mi_gdb_test "thread" \
> > + ".*Current thread is 1.*" \
> > + "info thread 10"
> > +
> > +mi_gdb_test "frame" \
> > + ".*#0 main.*" \
> > + "frame 6"
next prev parent reply other threads:[~2022-01-26 17:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-25 14:51 [PATCH 0/1] PR20684, preserve user selected context " Jan Vrany
2022-01-25 14:51 ` [PATCH 1/1] gdb/mi: PR20684, preserve user selected thread and frame " Jan Vrany
2022-01-25 18:59 ` Simon Marchi
2022-01-26 13:21 ` [PATCH v2 0/1] PR20684, preserve user selected context " Jan Vrany
2022-01-26 13:21 ` [PATCH v2 1/1] gdb/mi: PR20684, preserve user selected thread and frame " Jan Vrany
2022-01-26 15:01 ` Simon Marchi
2022-01-26 17:08 ` Jan Vrany [this message]
2022-01-27 14:50 ` [PATCH v3 0/1] PR20684, preserve user selected context " Jan Vrany
2022-02-07 11:56 ` Jan Vrany
2022-02-17 16:07 ` [PING] " Jan Vrany
2022-01-27 14:50 ` [PATCH v3 1/1] gdb/mi: PR20684, preserve user selected thread and frame " Jan Vrany
2022-02-17 22:24 ` Andrew Burgess
2022-02-18 17:45 ` Jan Vrany
2022-03-01 11:59 ` [PING] " Jan Vrany
2022-03-02 10:00 ` Andrew Burgess
2022-03-02 13:23 ` [PATCH v4] " Jan Vrany
2022-03-08 16:58 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=083e4514af3e0707035fa8517e107adc58da051e.camel@labware.com \
--to=jan.vrany@labware.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).