From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 4DE6A385041C for ; Wed, 26 Jan 2022 15:01:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4DE6A385041C Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 20QF1Qq4019600 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 26 Jan 2022 10:01:31 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 20QF1Qq4019600 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 559621EA69; Wed, 26 Jan 2022 10:01:26 -0500 (EST) Message-ID: <74e5b388-9f7a-dd0a-befe-3b069fba54b7@polymtl.ca> Date: Wed, 26 Jan 2022 10:01:25 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 1/1] gdb/mi: PR20684, preserve user selected thread and frame when invoking MI commands Content-Language: en-US To: Jan Vrany , gdb-patches@sourceware.org References: <20220126132112.3626367-2-jan.vrany@labware.com> From: Simon Marchi In-Reply-To: <20220126132112.3626367-2-jan.vrany@labware.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 26 Jan 2022 15:01:26 +0000 X-Spam-Status: No, score=-3038.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 26 Jan 2022 15:01:39 -0000 Thanks, this is OK. 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://sourceware.org/bugzilla/show_bug.cgi?id=20684 > --- > 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 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 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 . > + > +# 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"