From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 7EB643858D37 for ; Wed, 2 Mar 2022 10:00:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7EB643858D37 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-199-Hp0bxDfuNHCVolBb-diVdw-1; Wed, 02 Mar 2022 05:00:22 -0500 X-MC-Unique: Hp0bxDfuNHCVolBb-diVdw-1 Received: by mail-wr1-f69.google.com with SMTP id f14-20020adfc98e000000b001e8593b40b0so433061wrh.14 for ; Wed, 02 Mar 2022 02:00:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=BCo7kXxHs9ZP6P3rhUvWD/dEpidaPkqsJAUA262SdOk=; b=3lZTcyShurX81zwKqMc0vltIHxSUpsYsVy3S97IPZc+8UwvFwQWu5t3WRFMzwMARPR tSaJYLL1bh9b34iJO9UcKCfxvoP8jpDJkBEAYwrpNc79G0+7zG1lZCShbUgBuzPH8LJQ pJ7vDX9EiKN3v3c9SfxFp9wGZxRHOemcwNw3xyoJfIugxxCX/o6R7iJY6iV2DRCwCnqE u4vuro3hXW1/RoDFPbJmsCQtphnJH5cw2d/F2AyGCX8ADzaKbkdwnEeuATDT9UpYGWOy KKH/IB9eTYb4bw03sEnMVy0pgkMZJzxyjjHZ2Ho4TOvVC08f4LKcDYzUhITpqYV86Ouy Jong== X-Gm-Message-State: AOAM530iOU+JLc3Ff7y2G4N5owoctLlA94hgLYnZcbHYYsIzoOHtB3lp QlPCZJUrYs+EtYwLDpWLdtc/vun2vj+6fE6B4dcIHFKMQeIquZ1zwQk0Xo49OTDTYEfatz0JYxs lhISaY/26ihMLzQUa9lpmHA== X-Received: by 2002:a1c:e90a:0:b0:381:504e:b57d with SMTP id q10-20020a1ce90a000000b00381504eb57dmr13914753wmc.177.1646215219941; Wed, 02 Mar 2022 02:00:19 -0800 (PST) X-Google-Smtp-Source: ABdhPJzvW+z9tNb4mGAY2gqFupAVxK7JKhhSsoTg/+bTIelk0h/u70WVAnZjsxbwIh6rgTrkTsDnyQ== X-Received: by 2002:a1c:e90a:0:b0:381:504e:b57d with SMTP id q10-20020a1ce90a000000b00381504eb57dmr13914724wmc.177.1646215219474; Wed, 02 Mar 2022 02:00:19 -0800 (PST) Received: from localhost (host86-169-131-29.range86-169.btcentralplus.com. [86.169.131.29]) by smtp.gmail.com with ESMTPSA id w9-20020a5d6089000000b001f0256761b9sm3092696wrt.45.2022.03.02.02.00.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Mar 2022 02:00:18 -0800 (PST) From: Andrew Burgess To: Jan Vrany Cc: GDB Patches Subject: Re: [PATCH v3 1/1] gdb/mi: PR20684, preserve user selected thread and frame when invoking MI commands In-Reply-To: <4c975d42f6a2a165328ff0132f7be3feccae4828.camel@labware.com> References: <74e5b388-9f7a-dd0a-befe-3b069fba54b7@polymtl.ca> <20220127145011.1153142-2-jan.vrany@labware.com> <20220217222424.GQ2571@redhat.com> <4c975d42f6a2a165328ff0132f7be3feccae4828.camel@labware.com> Date: Wed, 02 Mar 2022 10:00:17 +0000 Message-ID: <87a6e8znbi.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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, 02 Mar 2022 10:00:32 -0000 Jan Vrany via Gdb-patches writes: > On Thu, 2022-02-17 at 22:24 +0000, Andrew Burgess wrote: >> * Jan Vrany via Gdb-patches [2022-01-27 14:= 50:11 +0000]: >>=20 >> > When invoking MI commands with --thread and/or --frame, user selected >> > thread and frame was not preserved: >> >=20 >> > (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-contex= t-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-contex= t-sync.c:30\n" >> > ^done >> > (gdb) >> > info frame >> > &"info frame\n" >> > ~"Stack level 0, frame at 0x7fffffffdf90:\n" >> > ~" rip =3D 0x555555555207 in main (/home/uuu/gdb/gdb/testsuite/gdb.m= i/user-selected-context-sync.c:60); saved rip =3D 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=3D"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-contex= t-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-contex= t-sync.c:30\n" >> > ^done >> > (gdb) >> > info frame >> > &"info frame\n" >> > ~"Stack level 0, frame at 0x7ffff742dee0:\n" >> > ~" rip =3D 0x555555555169 in child_sub_function (/home/uuu/gdb/gdb/t= estsuite/gdb.mi/user-selected-context-sync.c:30); saved rip =3D 0x555555555= 188\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) >> >=20 >> > This was problematic for frontends that provide access to CLI because = UI >> > may silently change the context for CLI commands (as demonstrated abov= e). >> >=20 >> > Bug: https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__sourceware= .org_bugzilla_show-5Fbug.cgi-3Fid-3D20684&d=3DDwIBAg&c=3DsPZ6DeHLiehUHQWKIr= sNwWp3t7snrE-az24ztT0w7Jc&r=3DWpFFGgYa98Yp-c29WHTCwU1wAGFBvszA6a4RzgpMSqc&m= =3DsoXzjKh4zT-FpxAe1wtdv7ka2r_jXNbrkEUR4XdUcOY&s=3DQkEM5YHXj7mpWI1y1z1AxRbo= sKxMhhfvfsMXmRnFoQc&e=3D=20 >> > --- >> > gdb/mi/mi-cmds.h | 12 ++ >> > gdb/mi/mi-main.c | 54 ++++--- >> > gdb/testsuite/gdb.mi/mi-cmd-user-context.exp | 155 ++++++++++++++++++= + >> > 3 files changed, 198 insertions(+), 23 deletions(-) >> > create mode 100644 gdb/testsuite/gdb.mi/mi-cmd-user-context.exp >> >=20 >> > 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 >> > =20 >> > #include "gdbsupport/gdb_optional.h" >> > +#include "mi/mi-main.h" >> > =20 >> > enum print_values { >> > PRINT_NO_VALUES, >> > @@ -163,6 +164,17 @@ struct mi_command >> > wrong. */ >> > void invoke (struct mi_parse *parse) const; >> > =20 >> > + /* Return whether this command preserves user selected context (thr= ead >> > + and frame). */ >> > + bool preserve_user_selected_context () const >> > + { >> > + /* Here we exploit the fact that if MI command is supposed to cha= nge >> > + user context, then it should not emit change notifications. T= herefore if >> > + command does not suppress user context change notifications, t= hen it should >> > + preserve the context. */ >> > + return m_suppress_notification !=3D &mi_suppress_notification.use= r_selected_context; >> > + } >> > + >> > protected: >> > =20 >> > /* The core of command invocation, this needs to be overridden in e= ach >> > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c >> > index 4860da7536a..e112707e4d1 100644 >> > --- a/gdb/mi/mi-main.c >> > +++ b/gdb/mi/mi-main.c >> > @@ -1907,6 +1907,16 @@ command_notifies_uscc_observer (struct mi_parse= *command) >> > } >> > } >> > =20 >> > +/* Determine whether the thread has changed. */ >> > + >> > +static bool >> > +command_changed_user_selected_thread (ptid_t previous_ptid, ptid_t cu= rrent_ptid) >> > +{ >> > + return previous_ptid !=3D null_ptid >> > + && current_ptid !=3D previous_ptid >> > + && current_ptid !=3D null_ptid; >>=20 >> You need to wrap multi-line conditions like this inside parenthesis. >>=20 > > Thanks!=20 > >> > +} >> > + >> > void >> > mi_execute_command (const char *cmd, int from_tty) >> > { >> > @@ -1971,28 +1981,17 @@ mi_execute_command (const char *cmd, int from_= tty) >> > =09 && any_thread_p () >> > =09 /* If the command already reports the thread change, no need to = do it >> > =09 again. */ >> > -=09 && !command_notifies_uscc_observer (command.get ())) >> > +=09 && !command_notifies_uscc_observer (command.get ()) >> > +=09 /* Don't report anything if --thread was specified -- the user s= elected >> > +=09 thread is preserved by mi_execute_command and therefore canno= t >> > +=09 change. */ >> > +=09 && command->thread =3D=3D -1 >>=20 >> I think including this check is a bug. Not a bug of your making I >> think, as I believe the bug already existed. I'll explain... >>=20 >> If I start GDB, and then spawn a new-ui for the mi interpreter. Start >> a multi-threaded inferior, and then stop at a gdb prompt. >>=20 >> Assuming thread 1 is selected, then, from the mi terminal I do: >>=20 >> -thread-select 2 >>=20 >> I will get the '^done,new-thread-id=3D"2"....' result on the mi >> terminal, but on the cli terminal I'll also get some output telling me >> that the thread changed. I think this is what you'd want, right? The >> cli thread _did_ just change. >>=20 >> However, now that thread 2 is selected, if I do this from the mi >> terminal: >>=20 >> -thread-select --thread 1 1 >>=20 >> then I get nothing on the cli terminal. This feels like a bug to me, > > Yes, this looks like a bug indeed. And I think we just opened=20 > can of worms... > >> and it's caused by the 'command->thread =3D=3D -1' check above. > > I do not think so. When the command is -thread-select, then > command_notifies_uscc_observer() return 1, therefore the whole > condition won't hold anyway, right?=20 > >>=20 >> I think that check should just be dropped completely. Your scoped >> thread/frame restore that you've added will ensure that, when >> appropriate, the thread doesn't change. If it does change, then you >> want to notify I think, regardless of whether --thread or --frame was >> used. > > If I do just drop "&& command->thread =3D=3D -1" from the condition, it b= reaks > existing test "--stack-select-frame 3" in mi-cmd-user-context.exp since i= t > produces change notification the test does not expect. Clearly it should = not > produce MI notification, whether it should "Switching to..." CLI output o= ver MI > channel I'm not sure.=20 > > But: > > Now I think we can get rid of the whole "if" and with that command_notifi= es_uscc_observer()=C2=A0 > and command_changed_user_selected_thread(). There are only two MI command= s=C2=A0 > that can change context are -thread-select and -stack-select-frame, right= ?=20 > -thread-select implementation ( in mi_cmd_thread_select () ) does notify = observers > on its own and if we add similar logic to -stack-select-frame implementat= ion > ( in mi_cmd_stack_select_frame () ) we're fine.=C2=A0Makes sense? > > I tried that (see the first patch below) and it seems to pass all MI test= s > and the code seems to me much simpler.=20 > > Still, this change itself does not solve your original bug. That's becaus= e=20 > thread is already switched when we enter mi_cmd_thread_select() so the=20 > condition "inferior_ptid !=3D previous_ptid" won't hold and so no notific= ation > is printed.=20 > > I went ahead and tried to address this by lifting the logic up to mi_cmd_= execute () > where --thread option is handled - see the second patch below. With that = patch, > all gdb.mi tests pass and your bug seems to be fixed. Still, the patch ne= eds more > work (test + check it works with -stack-frame-select). > > That being said, I'd be cautious. It took me quite a bit to make heads an= d tails > of this and still not sure I got it. My preference would be to fix the or= iginal=C2=A0 > problem *WITHOUT* fixing "your" bug and once done, submit another patch f= ixing=C2=A0 > "your" bug.=20 > > So, if you agree and the first part of my response about dropping the=C2= =A0 > whole "if" makes any sense, I'll properly resubmit it as v4.=20 > > Makes sense? Sorry for missing this reply. Yes, what you said makes sense. If you follow up with a v4 based on your original patch plus minor fixes I suggested that will probably be fine I think. Thanks, Andrew > >>=20 >> It would be great if we could have a test for this added too. You'll >> need to start gdb with a separate mi tty. You can look at >> gdb.mi/mi-exec-run.exp for an example, look for passing the >> 'separate-mi-tty' string to mi_gdb_start, and then later for using >> switch_gdb_spawn_id to switch between the main cli tty and the extra >> mi tty. If you have any problem, I'll be happy to help. >>=20 >> You still have some white space bugs (tabs vs spaces at the start of >> lines), I have this in my .gitconfig: >>=20 >> [core] >> whitespace =3D space-before-tab,indent-with-non-tab,trailing-spa= ce >>=20 >> which will have git highlight anywhere I've failed to indent with a >> tab when I should have done. >>=20 >> Additionally, some of your lines are over 80 characters, so need to be >> wrapped. >>=20 >> Thanks, >> Andrew >>=20 >>=20 >> > +=09 /* Don't report anything if the selected thread actually did not= change >> > +=09 (compared to selected thread before execution the command). = */ >> > +=09 && command_changed_user_selected_thread (previous_ptid, inferior= _ptid)) >> > =09{ >> > -=09 int report_change =3D 0; >> > - >> > -=09 if (command->thread =3D=3D -1) >> > -=09 { >> > -=09 report_change =3D (previous_ptid !=3D null_ptid >> > -=09=09=09 && inferior_ptid !=3D previous_ptid >> > -=09=09=09 && inferior_ptid !=3D null_ptid); >> > -=09 } >> > -=09 else if (inferior_ptid !=3D null_ptid) >> > -=09 { >> > -=09 struct thread_info *ti =3D inferior_thread (); >> > - >> > -=09 report_change =3D (ti->global_num !=3D command->thread); >> > -=09 } >> > - >> > -=09 if (report_change) >> > -=09 { >> > -=09 gdb::observers::user_selected_context_changed.notify >> > -=09=09(USER_SELECTED_THREAD | USER_SELECTED_FRAME); >> > -=09 } >> > +=09 gdb::observers::user_selected_context_changed.notify >> > +=09=09=09(USER_SELECTED_THREAD | USER_SELECTED_FRAME); >> > =09} >> > } >> > } >> > @@ -2037,6 +2036,7 @@ mi_cmd_execute (struct mi_parse *parse) >> > set_current_program_space (inf->pspace); >> > } >> > =20 >> > + gdb::optional thread_saver; >> > if (parse->thread !=3D -1) >> > { >> > thread_info *tp =3D find_thread_global_id (parse->thread); >> > @@ -2047,9 +2047,13 @@ mi_cmd_execute (struct mi_parse *parse) >> > if (tp->state =3D=3D THREAD_EXITED) >> > =09error (_("Thread id: %d has terminated"), parse->thread); >> > =20 >> > + if (parse->cmd->preserve_user_selected_context ()) >> > +=09thread_saver.emplace (); >> > + >> > switch_to_thread (tp); >> > } >> > =20 >> > + gdb::optional frame_saver; >> > if (parse->frame !=3D -1) >> > { >> > struct frame_info *fid; >> > @@ -2057,8 +2061,12 @@ mi_cmd_execute (struct mi_parse *parse) >> > =20 >> > fid =3D find_relative_frame (get_current_frame (), &frame); >> > if (frame =3D=3D 0) >> > -=09/* find_relative_frame was successful */ >> > -=09select_frame (fid); >> > + { >> > +=09 if (parse->cmd->preserve_user_selected_context ()) >> > +=09 frame_saver.emplace (); >> > + >> > + select_frame (fid); >> > + } >> > else >> > =09error (_("Invalid frame id: %d"), frame); >> > } >> > diff --git a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp b/gdb/testsu= ite/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 modif= y >> > +# it under the terms of the GNU General Public License as published b= y >> > +# 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 pthre= ads"] =3D=3D -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 m= ain" >> > +mi_run_cmd >> > +mi_expect_stop "breakpoint-hit" "main" "" $srcfile $main_break_line \ >> > +=09 { "" "disp=3D\"keep\"" } "run to breakpoint in main" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 1.*" \ >> > +=09"info thread 1" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-stack-info-depth --thread 3" \ >> > +=09"\\^done,depth=3D.*" \ >> > +=09"-stack-info-depth --thread 3" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 1.*" \ >> > +=09"info thread 2" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-thread-select 3" \ >> > +=09"\\^done,.*" \ >> > +=09"-thread-select 3" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 3.*" \ >> > +=09"info thread 3" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-thread-select --thread 2 1" \ >> > +=09"\\^done,.*" \ >> > +=09"-thread-select --thread 2 1" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 1.*" \ >> > +=09"info thread 4" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-thread-select --thread 2 2" \ >> > +=09"\\^done,.*" \ >> > +=09"-thread-select --thread 2 2" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 2.*" \ >> > +=09"info thread 5" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "frame" \ >> > +=09".*#0 0x.*" \ >> > +=09"frame 1" >> > + >> > +mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \ >> > +=09"\\^done,frame=3D\{level=3D\"1\".*" \ >> > +=09"-stack-info-frame 1" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 2.*" \ >> > +=09"info thread 6" >> > + >> > +mi_gdb_test "frame" \ >> > +=09".*#0 0x.*" \ >> > +=09"frame 2" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-stack-info-frame --thread 3 --frame 1" \ >> > +=09"\\^done,frame=3D\{level=3D\"1\".*" \ >> > +=09"-stack-info-frame 2" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 2.*" \ >> > +=09"info thread 7" >> > + >> > +mi_gdb_test "frame" \ >> > +=09".*#0 0x.*" \ >> > +=09"frame 3" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-stack-select-frame --thread 2 --frame 0 1" \ >> > +=09"\\^done" \ >> > +=09"--stack-select-frame 1" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 2.*" \ >> > +=09"info thread 8" >> > + >> > +mi_gdb_test "frame" \ >> > +=09".*#1 0x.*" \ >> > +=09"frame 4" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-stack-select-frame --thread 2 --frame 2 2" \ >> > +=09"\\^done" \ >> > +=09"--stack-select-frame 2" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 2.*" \ >> > +=09"info thread 9" >> > + >> > +mi_gdb_test "frame" \ >> > +=09".*#2 0x.*" \ >> > +=09"frame 5" >> > + >> > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> > + >> > +mi_gdb_test "-stack-select-frame --thread 1 --frame 0 0" \ >> > +=09"\\^done" \ >> > +=09"--stack-select-frame 3" >> > + >> > +mi_gdb_test "thread" \ >> > +=09".*Current thread is 1.*" \ >> > +=09"info thread 10" >> > + >> > +mi_gdb_test "frame" \ >> > +=09".*#0 main.*" \ >> > +=09"frame 6" >> > --=20 >> > 2.30.2 >> >=20 >>=20 > > From db229be447707d57980e4c21f394cdba1ec86b4c Mon Sep 17 00:00:00 2001 > From: Jan Vrany > Date: Thu, 27 Jan 2022 13:45:09 +0000 > Subject: [PATCH 1/2] gdb/mi: PR20684, preserve user selected thread and f= rame > when invoking MI commands > > 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_su= b_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-s= ync.c:30\n" > ~" 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_su= b_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-s= ync.c:30\n" > ^done > (gdb) > info frame > &"info frame\n" > ~"Stack level 0, frame at 0x7fffffffdf90:\n" > ~" rip =3D 0x555555555207 in main (/home/uuu/gdb/gdb/testsuite/gdb.mi/u= ser-selected-context-sync.c:60); saved rip =3D 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=3D"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_su= b_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-s= ync.c:30\n" > ~"* 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" child_su= b_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-s= ync.c:30\n" > ^done > (gdb) > info frame > &"info frame\n" > ~"Stack level 0, frame at 0x7ffff742dee0:\n" > ~" rip =3D 0x555555555169 in child_sub_function (/home/uuu/gdb/gdb/test= suite/gdb.mi/user-selected-context-sync.c:30); saved rip =3D 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=3D20684 > --- > gdb/mi/mi-cmd-stack.c | 11 ++ > gdb/mi/mi-cmds.h | 12 ++ > gdb/mi/mi-main.c | 74 ++------- > gdb/testsuite/gdb.mi/mi-cmd-user-context.exp | 155 +++++++++++++++++++ > 4 files changed, 189 insertions(+), 63 deletions(-) > create mode 100644 gdb/testsuite/gdb.mi/mi-cmd-user-context.exp > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > index e63f1706149..1be8aa81c3d 100644 > --- a/gdb/mi/mi-cmd-stack.c > +++ b/gdb/mi/mi-cmd-stack.c > @@ -36,6 +36,8 @@ > #include "mi-parse.h" > #include "gdbsupport/gdb_optional.h" > #include "safe-ctype.h" > +#include "inferior.h" > +#include "observable.h" > =20 > enum what_to_list { locals, arguments, all }; > =20 > @@ -756,7 +758,16 @@ mi_cmd_stack_select_frame (const char *command, char= **argv, int argc) > if (argc =3D=3D 0 || argc > 1) > error (_("-stack-select-frame: Usage: FRAME_SPEC")); > =20 > + ptid_t previous_ptid =3D inferior_ptid; > + > select_frame_for_mi (parse_frame_specification (argv[0])); > + > + /* Notify if the thread has effectively changed. */ > + if (inferior_ptid !=3D previous_ptid) > + { > + gdb::observers::user_selected_context_changed.notify > +=09(USER_SELECTED_THREAD | USER_SELECTED_FRAME); > + } > } > =20 > void > 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 > =20 > #include "gdbsupport/gdb_optional.h" > +#include "mi/mi-main.h" > =20 > enum print_values { > PRINT_NO_VALUES, > @@ -163,6 +164,17 @@ struct mi_command > wrong. */ > void invoke (struct mi_parse *parse) const; > =20 > + /* 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. Ther= efore if > + command does not suppress user context change notifications, then= it should > + preserve the context. */ > + return m_suppress_notification !=3D &mi_suppress_notification.user_s= elected_context; > + } > + > protected: > =20 > /* 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..23e2373dcec 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -1879,34 +1879,6 @@ mi_print_exception (const char *token, const struc= t gdb_exception &exception) > fputs_unfiltered ("\n", mi->raw_stdout); > } > =20 > -/* Determine whether the parsed command already notifies the > - user_selected_context_changed observer. */ > - > -static int > -command_notifies_uscc_observer (struct mi_parse *command) > -{ > - if (command->op =3D=3D CLI_COMMAND) > - { > - /* CLI commands "thread" and "inferior" already send it. */ > - return (startswith (command->command, "thread ") > -=09 || startswith (command->command, "inferior ")); > - } > - else /* MI_COMMAND */ > - { > - if (strcmp (command->command, "interpreter-exec") =3D=3D 0 > -=09 && command->argc > 1) > -=09{ > -=09 /* "thread" and "inferior" again, but through -interpreter-exec. *= / > -=09 return (startswith (command->argv[1], "thread ") > -=09=09 || startswith (command->argv[1], "inferior ")); > -=09} > - > - else > -=09/* -thread-select already sends it. */ > -=09return strcmp (command->command, "thread-select") =3D=3D 0; > - } > -} > - > void > mi_execute_command (const char *cmd, int from_tty) > { > @@ -1932,8 +1904,6 @@ mi_execute_command (const char *cmd, int from_tty) > =20 > if (command !=3D NULL) > { > - ptid_t previous_ptid =3D inferior_ptid; > - > command->token =3D token; > =20 > if (do_timings) > @@ -1963,37 +1933,6 @@ mi_execute_command (const char *cmd, int from_tty) > =20 > bpstat_do_actions (); > =20 > - if (/* The notifications are only output when the top-level > -=09 interpreter (specified on the command line) is MI. */ > -=09 top_level_interpreter ()->interp_ui_out ()->is_mi_like_p () > -=09 /* Don't try report anything if there are no threads -- > -=09 the program is dead. */ > -=09 && any_thread_p () > -=09 /* If the command already reports the thread change, no need to do = it > -=09 again. */ > -=09 && !command_notifies_uscc_observer (command.get ())) > -=09{ > -=09 int report_change =3D 0; > - > -=09 if (command->thread =3D=3D -1) > -=09 { > -=09 report_change =3D (previous_ptid !=3D null_ptid > -=09=09=09 && inferior_ptid !=3D previous_ptid > -=09=09=09 && inferior_ptid !=3D null_ptid); > -=09 } > -=09 else if (inferior_ptid !=3D null_ptid) > -=09 { > -=09 struct thread_info *ti =3D inferior_thread (); > - > -=09 report_change =3D (ti->global_num !=3D command->thread); > -=09 } > - > -=09 if (report_change) > -=09 { > -=09 gdb::observers::user_selected_context_changed.notify > -=09=09(USER_SELECTED_THREAD | USER_SELECTED_FRAME); > -=09 } > -=09} > } > } > =20 > @@ -2037,6 +1976,7 @@ mi_cmd_execute (struct mi_parse *parse) > set_current_program_space (inf->pspace); > } > =20 > + gdb::optional thread_saver; > if (parse->thread !=3D -1) > { > thread_info *tp =3D find_thread_global_id (parse->thread); > @@ -2047,9 +1987,13 @@ mi_cmd_execute (struct mi_parse *parse) > if (tp->state =3D=3D THREAD_EXITED) > =09error (_("Thread id: %d has terminated"), parse->thread); > =20 > + if (parse->cmd->preserve_user_selected_context ()) > +=09thread_saver.emplace (); > + > switch_to_thread (tp); > } > =20 > + gdb::optional frame_saver; > if (parse->frame !=3D -1) > { > struct frame_info *fid; > @@ -2057,8 +2001,12 @@ mi_cmd_execute (struct mi_parse *parse) > =20 > fid =3D find_relative_frame (get_current_frame (), &frame); > if (frame =3D=3D 0) > -=09/* find_relative_frame was successful */ > -=09select_frame (fid); > +=09{ > +=09 if (parse->cmd->preserve_user_selected_context ()) > +=09 frame_saver.emplace (); > + > +=09 select_frame (fid); > +=09} > else > =09error (_("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= "] =3D=3D -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 \ > +=09 { "" "disp=3D\"keep\"" } "run to breakpoint in main" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 1.*" \ > +=09"info thread 1" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-stack-info-depth --thread 3" \ > +=09"\\^done,depth=3D.*" \ > +=09"-stack-info-depth --thread 3" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 1.*" \ > +=09"info thread 2" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-thread-select 3" \ > +=09"\\^done,.*" \ > +=09"-thread-select 3" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 3.*" \ > +=09"info thread 3" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-thread-select --thread 2 1" \ > +=09"\\^done,.*" \ > +=09"-thread-select --thread 2 1" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 1.*" \ > +=09"info thread 4" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-thread-select --thread 2 2" \ > +=09"\\^done,.*" \ > +=09"-thread-select --thread 2 2" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 2.*" \ > +=09"info thread 5" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "frame" \ > +=09".*#0 0x.*" \ > +=09"frame 1" > + > +mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \ > +=09"\\^done,frame=3D\{level=3D\"1\".*" \ > +=09"-stack-info-frame 1" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 2.*" \ > +=09"info thread 6" > + > +mi_gdb_test "frame" \ > +=09".*#0 0x.*" \ > +=09"frame 2" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-stack-info-frame --thread 3 --frame 1" \ > +=09"\\^done,frame=3D\{level=3D\"1\".*" \ > +=09"-stack-info-frame 2" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 2.*" \ > +=09"info thread 7" > + > +mi_gdb_test "frame" \ > +=09".*#0 0x.*" \ > +=09"frame 3" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-stack-select-frame --thread 2 --frame 0 1" \ > +=09"\\^done" \ > +=09"--stack-select-frame 1" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 2.*" \ > +=09"info thread 8" > + > +mi_gdb_test "frame" \ > +=09".*#1 0x.*" \ > +=09"frame 4" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-stack-select-frame --thread 2 --frame 2 2" \ > +=09"\\^done" \ > +=09"--stack-select-frame 2" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 2.*" \ > +=09"info thread 9" > + > +mi_gdb_test "frame" \ > +=09".*#2 0x.*" \ > +=09"frame 5" > + > +#=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D > + > +mi_gdb_test "-stack-select-frame --thread 1 --frame 0 0" \ > +=09"\\^done" \ > +=09"--stack-select-frame 3" > + > +mi_gdb_test "thread" \ > +=09".*Current thread is 1.*" \ > +=09"info thread 10" > + > +mi_gdb_test "frame" \ > +=09".*#0 main.*" \ > +=09"frame 6" > --=20 > 2.34.1 > > > From 18d8dc95fe1a834ecbde319dd6e5960fd4d82a3f Mon Sep 17 00:00:00 2001 > From: Jan Vrany > Date: Fri, 18 Feb 2022 17:04:13 +0000 > Subject: [PATCH 2/2] [WIP] gdb/mi: properly notify user when GDB/MI clien= t > uses -thread-select > > If we start GDB, and then spawn a new-ui for the mi interpreter. Start > a multi-threaded inferior, and then stop at a gdb prompt. > > Assuming thread 1 is selected, then, from the mi terminal I do: > > -thread-select 2 > > We will get the '^done,new-thread-id=3D"2"....' result on the mi > terminal, but on the cli terminal I'll also get some output telling me > that the thread changed. > > However, now that thread 2 is selected, if I do this from the mi > terminal: > > -thread-select --thread 1 1 > > then I get nothing on the CLI terminal. This seems to be a bug. > > The problem was that when `-thread-select --thread 1 1` then thread > is switched to thread 1 before mi_cmd_thread_select () is called, > therefore the condition "inferior_ptid !=3D previous_ptid" there does > not hold. > > To address this problem, we have to move this logic up to > mi_cmd_execute () where --thread option is processed and notify > user selected contents observers there if context changes. > > However, this in itself breaks GDB/MI because it would cause context > notification to be sent on MI channel. This is because by the time > we notify, MI notification suppression is already restored (done in > mi_command::invoke(). Therefore we had to lift notification suppression > logic also up to mi_cmd_execute (). > > With this change, all gdb.mi tests pass, tested on x86_64-linux. > > TODO: > * add test > --- > gdb/mi/mi-cmd-stack.c | 10 ---------- > gdb/mi/mi-cmds.c | 2 -- > gdb/mi/mi-cmds.h | 16 ++++++++-------- > gdb/mi/mi-main.c | 32 +++++++++++++++++++++++--------- > 4 files changed, 31 insertions(+), 29 deletions(-) > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > index 1be8aa81c3d..afe584b7fca 100644 > --- a/gdb/mi/mi-cmd-stack.c > +++ b/gdb/mi/mi-cmd-stack.c > @@ -757,17 +757,7 @@ mi_cmd_stack_select_frame (const char *command, char= **argv, int argc) > { > if (argc =3D=3D 0 || argc > 1) > error (_("-stack-select-frame: Usage: FRAME_SPEC")); > - > - ptid_t previous_ptid =3D inferior_ptid; > - > select_frame_for_mi (parse_frame_specification (argv[0])); > - > - /* Notify if the thread has effectively changed. */ > - if (inferior_ptid !=3D previous_ptid) > - { > - gdb::observers::user_selected_context_changed.notify > -=09(USER_SELECTED_THREAD | USER_SELECTED_FRAME); > - } > } > =20 > void > diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c > index cd7cabdda9b..957dfb4e03d 100644 > --- a/gdb/mi/mi-cmds.c > +++ b/gdb/mi/mi-cmds.c > @@ -171,8 +171,6 @@ mi_command::mi_command (const char *name, int *suppre= ss_notification) > void > mi_command::invoke (struct mi_parse *parse) const > { > - gdb::optional> restore > - =3D do_suppress_notification (); > this->do_invoke (parse); > } > =20 > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > index 785652ee1c9..da1598a3f32 100644 > --- a/gdb/mi/mi-cmds.h > +++ b/gdb/mi/mi-cmds.h > @@ -175,14 +175,6 @@ struct mi_command > return m_suppress_notification !=3D &mi_suppress_notification.user_s= elected_context; > } > =20 > -protected: > - > - /* The core of command invocation, this needs to be overridden in each > - base class. PARSE is the parsed command line from the user. */ > - virtual void do_invoke (struct mi_parse *parse) const =3D 0; > - > -private: > - > /* If this command was created with a suppress notifications pointer, > then this function will set the suppress flag and return a > gdb::optional with its value set to an object that will restore the > @@ -192,6 +184,14 @@ struct mi_command > then this function returns an empty gdb::optional. */ > gdb::optional> do_suppress_notification () co= nst; > =20 > +protected: > + > + /* The core of command invocation, this needs to be overridden in each > + base class. PARSE is the parsed command line from the user. */ > + virtual void do_invoke (struct mi_parse *parse) const =3D 0; > + > +private: > + > /* The name of the command. */ > const char *m_name; > =20 > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 23e2373dcec..608b226f496 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -556,19 +556,10 @@ mi_cmd_thread_select (const char *command, char **a= rgv, int argc) > if (thr =3D=3D NULL) > error (_("Thread ID %d not known."), num); > =20 > - ptid_t previous_ptid =3D inferior_ptid; > - > thread_select (argv[0], thr); > =20 > print_selected_thread_frame (current_uiout, > =09=09=09 USER_SELECTED_THREAD | USER_SELECTED_FRAME); > - > - /* Notify if the thread has effectively changed. */ > - if (inferior_ptid !=3D previous_ptid) > - { > - gdb::observers::user_selected_context_changed.notify > -=09(USER_SELECTED_THREAD | USER_SELECTED_FRAME); > - } > } > =20 > void > @@ -1936,6 +1927,16 @@ mi_execute_command (const char *cmd, int from_tty) > } > } > =20 > +/* Determine whether the thread has changed. */ > + > +static bool > +command_changed_user_selected_thread (ptid_t previous_ptid, ptid_t curre= nt_ptid) > +{ > + return (previous_ptid !=3D null_ptid > +=09 && current_ptid !=3D previous_ptid > +=09 && current_ptid !=3D null_ptid); > +} =20 > + > static void > mi_cmd_execute (struct mi_parse *parse) > { > @@ -1976,6 +1977,8 @@ mi_cmd_execute (struct mi_parse *parse) > set_current_program_space (inf->pspace); > } > =20 > + ptid_t previous_ptid =3D inferior_ptid; > + > gdb::optional thread_saver; > if (parse->thread !=3D -1) > { > @@ -2021,7 +2024,18 @@ mi_cmd_execute (struct mi_parse *parse) > current_context =3D parse; > =20 > gdb_assert (parse->cmd !=3D nullptr); > + =20 > + gdb::optional> restore_suppress_notification > + =3D parse->cmd->do_suppress_notification (); > + > parse->cmd->invoke (parse); > + > + if (!parse->cmd->preserve_user_selected_context () > + && command_changed_user_selected_thread (previous_ptid, inferior_p= tid)) > + { > + gdb::observers::user_selected_context_changed.notify > + (USER_SELECTED_THREAD | USER_SELECTED_FRAME); > + } > } > =20 > /* See mi-main.h. */ > --=20 > 2.34.1