From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2062.outbound.protection.outlook.com [40.107.236.62]) by sourceware.org (Postfix) with ESMTPS id BE9B03858D20 for ; Tue, 1 Mar 2022 11:59:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BE9B03858D20 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Gl6ndWKvx/FSYzjIuLwZr5NUTilmOcpHF1CRWvc9GR37WsTxu1tkGeR3eRsVR7ylkpFuZb318h99u+W1FuawETNGFcpBotbHAmE2RnhR+4Yf6xqfvec3dSrqbdpeQFgfoxrk1+E5eOJw9FwdBxGwkJyerHM8kbTUHSGqwcmA0Oc8qyE7kCgDoq/lCwa7H8xn/fhK5EzFlNuI1pqYZ0sTiQlpw9MziDhQrmN+gjqsM4kBx6UHlOBOgF6waA5na+x+Jl6hfT0n7oV1ftlLpyKtdvBOR19bMhF5wyT2HuDz1LmLgLPfARhLoyTz6XbaKmX8oYiGkVMbFD+0pnPeE22V2w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=o6Nayt+hri/xDSDcu4I0Aw2xpIuGHokROu8elez3hDo=; b=XzndLDq3CFDdN7slqZkwmQxyJfPrCn4wEhqVgtZoshmLEuTfYxdcOUvz56x3KLFT47WOhIL0pQ/x7GqmE5Qmkhw9oopjwmZK12e+5QAYDNa3XFwEqg9RJ3htHfBML1DkEmSd/SBxQGs5W3ztkYNtDyZwwo4NyMoAmb9wT/ralVz7qYvWAE/9qPqe0ZhAnOhOvFD1YJRdbRheJ5RTiu9vci59jJw8kIerZUB8KaLYmJpF+kiwYjx4xf0aZ6rzAaJVDN4Krx8EGv5J2wFVpc3nl+m0VXHZgmgmE4ObeVqtju5OPRR5YSfl/C49EYNEWGbjn77goqkjXbzP2ViWlVhpNQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=labware.com; dmarc=pass action=none header.from=labware.com; dkim=pass header.d=labware.com; arc=none Received: from DM6PR17MB3113.namprd17.prod.outlook.com (2603:10b6:5:6::10) by BY5PR17MB3191.namprd17.prod.outlook.com (2603:10b6:a03:190::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5017.26; Tue, 1 Mar 2022 11:59:47 +0000 Received: from DM6PR17MB3113.namprd17.prod.outlook.com ([fe80::78f9:3a56:7d30:e2c9]) by DM6PR17MB3113.namprd17.prod.outlook.com ([fe80::78f9:3a56:7d30:e2c9%6]) with mapi id 15.20.5017.027; Tue, 1 Mar 2022 11:59:47 +0000 Message-ID: Subject: [PING] Re: [PATCH v3 1/1] gdb/mi: PR20684, preserve user selected thread and frame when invoking MI commands From: Jan Vrany To: Andrew Burgess Cc: GDB Patches Date: Tue, 01 Mar 2022 11:59:42 +0000 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> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.43.2-2 X-ClientProxiedBy: LO4P123CA0221.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:1a6::10) To DM6PR17MB3113.namprd17.prod.outlook.com (2603:10b6:5:6::10) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 5bcc6a50-b60b-455e-7ffe-08d9fb7afb22 X-MS-TrafficTypeDiagnostic: BY5PR17MB3191:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 0dvLdGRpXJC31bb28U12LVX9pDfw8k2J3qnuryccLl76RCAc7J4V3u7uf8boPpKQxxG8Oh3eJGTwqdRbl4jrXCDOgmRMRy5TwxW2Nrh4nBab/KY2sDQi/w5UZ5DXPE/aaHGrIgfHccr72slF1+2A+rohuZroUb+g2yYnMOBpVJ5e4D5JCzE1AVIMl1AdbRA3QqKUshiPEvm2o5VSER4YGvtWj0bG6ipWt8xWKl3Zb0RIxLvAiJYD0MvUjR3SOhH1cTfDgfw6xz39G6De4tjbwCCjX8EymKUE3f/qqOGMSOxaA3UWt1xON7Wau+JEUWJeu/5WkPPZwRqSfarGdx6r7rWRjr47me39iEbp5YdJGULzME9RSIsELZm50DNEE3eicA+e9T9OUAfB1A5QLsDQCqUqPvmPx4u9Y3syo10L6KETrdIHZ97hBGQ5ZL/nEFYmsPB/KBOGA+EwCB0dknBi2ao4NODfQQm3DVU5PVRy3SY6pVK1qi05+GDK2GivxbN+AIiaeL+he4t3yNqmP5dX6SR9Cmyos6YGW8IqjCAuh+LTV4FJqqZ08WKyyUJswfgkmDbrnWt/1jyqGjeGcvOVmIndjv4++pfBwhb2IPj0o+RRufLKoTThBEWsQGfNwmAk48vM2XdGLjda/ZYJ2gEhwNTLGDsS2uKYFASy7LV1Aj2ch48MWVSLxTdVLVMK4Ok4TIeaYlMwJ21VwveTJBdI5ggLrgNaAsqQR0m5pl7J/n4oxJZiDMm7I3S6Fdosrqn4up3O88rvM9kJNwb50oVbKXHSRKgojmFyD1611IoSk3Q= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR17MB3113.namprd17.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(366004)(4326008)(186003)(8676002)(86362001)(6506007)(36756003)(6666004)(26005)(2906002)(508600001)(38100700002)(38350700002)(84970400001)(44832011)(5660300002)(6916009)(53546011)(52116002)(30864003)(6486002)(966005)(2616005)(316002)(8936002)(83380400001)(6512007)(66556008)(66946007)(66476007)(2004002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RUEwN015YW5sMDE5VlpVc2JYZlpOd0tLMTgvWHVhTjF5VnB2UEg3bE1IaExh?= =?utf-8?B?VGRzRzNqQ0NFcnR1aENScDRJWElXRE5YSVJBVGFzMGtieWgrcTlTQVlib09x?= =?utf-8?B?YnlWbWE5d29MSjdpa243R0hDQTczYkcxOGhxSEcrV2lkYWgzMmFIbVI5d0VL?= =?utf-8?B?SHJqNmlJRk5zQWUxcjB3M05YMmFWaW05ZnF4SmQxeStrUGlpL3ZoaHpnZmg4?= =?utf-8?B?NzJLYmpNNGZDa2JxUzR1M0FLZktiWlNGL29RNjhIY2c1Y25pV0xIb2xQWHZl?= =?utf-8?B?ejlqWDlQT1lDTkk3eXk0bXM4WTV3UC9oeTNreU9kQWRUeGp6VTJVRTRlMkZy?= =?utf-8?B?Tlh0K0FzK0Z6Uko0SjVlYTBWN0txTjljVFNKNFVhRExSWEQ5RlBuY2NZenc5?= =?utf-8?B?MWdnU0JPYjJFWnpuSU1tVEV5eHQwRjlWcis5dmIwbmp0UkxtYU0xSjBHakhn?= =?utf-8?B?SURuNjFjUEJaQTlUazAyWVpjb2htdzgyK3RibXdTajAwbDJ0TCtmZVpsL3Bi?= =?utf-8?B?aE1yNXkxS3h1NXduZCtMeis4bWFTTEFTanREZ2RwVUhPY2I0YWRYSUhDTlkv?= =?utf-8?B?RmJFYlZTRlJXemJxZmVDeGRqSWRxblhuWVA4TWJjTCsxZE5UYXNEdnVTSENv?= =?utf-8?B?QlIrVFRsVkJBUDlPSVNQcWFCRUpYaUN3dHBxT1J2MktqNGZ3ZU1SRnh4L2V0?= =?utf-8?B?S1RHRlV4UUhJQ1pBQWYzVXJaaGR2UVlRY05sSG56aCt3UlpXT3lOQUJJVWRz?= =?utf-8?B?L3VtVzltU240YXBXSm5DVWxmb1FtWG9OYVR1U1FXVE0xakpSTXlQU2lsODFl?= =?utf-8?B?R1BmdWIzcmYybUJxdUk0WTFuTVR1S1BkZFFiR1ZOYzFIdEpiZkNCd0R3WW9C?= =?utf-8?B?eVVvcUZ0MWErdzdDd09aTzd6Y0VWdG1URW5JZGJsY0dKbVV6SDlFUWUrRUw4?= =?utf-8?B?VWVGWkZDRnpvd1hIT2VkaUlHdFlPdjZERGkwL0c2dm03OUxIMnJCRjZQSGpy?= =?utf-8?B?V3c2WWU4dVp1djlVVFVNNGR3VG15WHBLalkza0JUanhJeXNQa1FFU2JmNkhx?= =?utf-8?B?ZjhaekZaaWZBa2FxNzByYmZTWStYS3ZsQ0V2anJPdWkwd1MxcE1IaEMrL1Z0?= =?utf-8?B?ZXZmUS90YVdLMm04cHQxYitJeXdDUWdsS0ltZkRFMzFlOGR6WXdLT2dTYjdV?= =?utf-8?B?eVluYTF6YnBiUnJYdDIxTnBMeWVkdUVYMzRpYWxoOW1rRVExMnRaOGxqa0FB?= =?utf-8?B?QXFOK3NrRzdHdEgzSDE5Skdvc2dUa1o0SlQvajFiODRqSHNCbXErckU5RWZp?= =?utf-8?B?eTlBbjYwNURXUmluZThzUDlVeDZ5MlpyTFNhV1RlVmJudEFuay9NVTd2M0U3?= =?utf-8?B?WVZZZ0VLanRXbm50VnpNYXJsWTBBckhvSGV1YjBZSHJQNWtzaXV4bWF5VmxH?= =?utf-8?B?ZVBwZTdNSTYwYWJEai9PQm4xcDJrWWJKVXRoVnNmN0MrNXJpMHUzc3RESGly?= =?utf-8?B?azJ3a2wzZXY3QW0vTHhOeldXM2ZCM1JLOENOYXFRSmZmaW11YlJwaWJYNWUx?= =?utf-8?B?TVZIeTVGeVlCYm5xOS81UHYybjFOd3ZzQ1lON1hyZVNteUR5Wml5S05UeWor?= =?utf-8?B?aEN6TFp2dTl5QTdEYis0UU1qbVdCUEpaMktaUjh5U256Z1JFZ29qdmV3ejRZ?= =?utf-8?B?a29XWkNxOXkwaTZRWDJ2elh6Ymh6ZTkySmxTczVmK0VFUzU2bmJSa0NXcWVn?= =?utf-8?B?Mkc1RFZIZld6T2E1eHhJRy9yalZoZkJ6NlVURVdkRHRCVitGYWVKbitKK3dp?= =?utf-8?B?bUVPK3BRYWJ0Zm5sQmFsa2tFd2RDTkswdTNQM0s3WGx0cVMvY01XbWovUXNH?= =?utf-8?B?NlUrYkJtbTNTbDIrMHQ1WVhmSVBiUEdpa1lBOU5qRG1LWUZPYWpXcmNpS0Q0?= =?utf-8?B?L3BmMS9DZWVQMWc0bC9GYXRTUGN4R3owTHlVQUVZQlYwRHRkME5qa01KTlJK?= =?utf-8?B?K09zWElrVUNtdnNXc0kxWmVUcmNhYTV2ZGt1UUVSNFh3TEVObnYyNlB2UkJy?= =?utf-8?B?V1Iyd2F2UUEyVHZJOUdrcldHNUJ1ZGw2ZXlZR1FycUg5RURzMnpMa2crUGJR?= =?utf-8?B?SDlIQTZwd292T1QrRWl1MVZnbVJrcDJhcDQ5bXRIRWh6T2hYTWtGcHdlMjcr?= =?utf-8?Q?Mu/U3LJbUeWmCa7sqILvZts=3D?= X-OriginatorOrg: labware.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5bcc6a50-b60b-455e-7ffe-08d9fb7afb22 X-MS-Exchange-CrossTenant-AuthSource: DM6PR17MB3113.namprd17.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Mar 2022 11:59:47.0840 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: b5db0322-1aa0-4c0a-859c-ad0f96966f4c X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: MrffMEcxOZaXWJM52K/7CLpY2JIVhO2sdeTXoSYAJ2UKillc6WjRlTRYxhbRVNtrkdWsOmd+CQ0/23nXTDXtPw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR17MB3191 X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, 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: Tue, 01 Mar 2022 11:59:57 -0000 On Fri, 2022-02-18 at 17:45 +0000, Jan Vrany wrote: > 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\" chil= d_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-conte= xt-sync.c:30\n" > > > ~" 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" chil= d_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-conte= xt-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.= mi/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\" chil= d_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-conte= xt-sync.c:30\n" > > > ~"* 3 Thread 0x7ffff742e700 (LWP 19307) \"user-selected-c\" chil= d_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-conte= xt-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/= testsuite/gdb.mi/user-selected-context-sync.c:30); saved rip =3D 0x55555555= 5188\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 abo= ve). > > >=20 > > > Bug: https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__sourcewar= e.org_bugzilla_show-5Fbug.cgi-3Fid-3D20684&d=3DDwIBAg&c=3DsPZ6DeHLiehUHQWKI= rsNwWp3t7snrE-az24ztT0w7Jc&r=3DWpFFGgYa98Yp-c29WHTCwU1wAGFBvszA6a4RzgpMSqc&= m=3DsoXzjKh4zT-FpxAe1wtdv7ka2r_jXNbrkEUR4XdUcOY&s=3DQkEM5YHXj7mpWI1y1z1AxRb= osKxMhhfvfsMXmRnFoQc&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 (th= read > > > + and frame). */ > > > + bool preserve_user_selected_context () const > > > + { > > > + /* Here we exploit the fact that if MI command is supposed to ch= ange > > > + 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 !=3D &mi_suppress_notification.us= er_selected_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..e112707e4d1 100644 > > > --- a/gdb/mi/mi-main.c > > > +++ b/gdb/mi/mi-main.c > > > @@ -1907,6 +1907,16 @@ command_notifies_uscc_observer (struct mi_pars= e *command) > > > } > > > } > > > =20 > > > +/* Determine whether the thread has changed. */ > > > + > > > +static bool > > > +command_changed_user_selected_thread (ptid_t previous_ptid, ptid_t c= urrent_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 >=20 > Thanks!=20 >=20 > > > +} > > > + > > > void > > > mi_execute_command (const char *cmd, int from_tty) > > > { > > > @@ -1971,28 +1981,17 @@ mi_execute_command (const char *cmd, int from= _tty) > > > && any_thread_p () > > > /* If the command already reports the thread change, no need to d= o it > > > again. */ > > > - && !command_notifies_uscc_observer (command.get ())) > > > + && !command_notifies_uscc_observer (command.get ()) > > > + /* Don't report anything if --thread was specified -- the user se= lected > > > + thread is preserved by mi_execute_command and therefore cannot > > > + change. */ > > > + && 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, >=20 > Yes, this looks like a bug indeed. And I think we just opened=20 > can of worms... >=20 > > and it's caused by the 'command->thread =3D=3D -1' check above. >=20 > 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 > >=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. >=20 > 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 >=20 > But: >=20 > 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? >=20 > 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 >=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 >=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). >=20 > 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 >=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 >=20 > Makes sense?=20 >=20 > >=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-sp= ace > >=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 > > > + /* Don't report anything if the selected thread actually did not = change > > > + (compared to selected thread before execution the command). *= / > > > + && command_changed_user_selected_thread (previous_ptid, inferior_= ptid)) > > > { > > > - int report_change =3D 0; > > > - > > > - if (command->thread =3D=3D -1) > > > - { > > > - report_change =3D (previous_ptid !=3D null_ptid > > > - && inferior_ptid !=3D previous_ptid > > > - && inferior_ptid !=3D null_ptid); > > > - } > > > - else if (inferior_ptid !=3D null_ptid) > > > - { > > > - struct thread_info *ti =3D inferior_thread (); > > > - > > > - report_change =3D (ti->global_num !=3D command->thread); > > > - } > > > - > > > - if (report_change) > > > - { > > > - gdb::observers::user_selected_context_changed.notify > > > - (USER_SELECTED_THREAD | USER_SELECTED_FRAME); > > > - } > > > + gdb::observers::user_selected_context_changed.notify > > > + (USER_SELECTED_THREAD | USER_SELECTED_FRAME); > > > } > > > } > > > } > > > @@ -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) > > > error (_("Thread id: %d has terminated"), parse->thread); > > > =20 > > > + if (parse->cmd->preserve_user_selected_context ()) > > > + thread_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) > > > - /* 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/tests= uite/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 modi= fy > > > +# 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 pthr= eads"] =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 = \ > > > + { "" "disp=3D\"keep\"" } "run to breakpoint in main" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 1.*" \ > > > + "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" \ > > > + "\\^done,depth=3D.*" \ > > > + "-stack-info-depth --thread 3" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 1.*" \ > > > + "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" \ > > > + "\\^done,.*" \ > > > + "-thread-select 3" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 3.*" \ > > > + "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" \ > > > + "\\^done,.*" \ > > > + "-thread-select --thread 2 1" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 1.*" \ > > > + "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" \ > > > + "\\^done,.*" \ > > > + "-thread-select --thread 2 2" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 2.*" \ > > > + "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" \ > > > + ".*#0 0x.*" \ > > > + "frame 1" > > > + > > > +mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \ > > > + "\\^done,frame=3D\{level=3D\"1\".*" \ > > > + "-stack-info-frame 1" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 2.*" \ > > > + "info thread 6" > > > + > > > +mi_gdb_test "frame" \ > > > + ".*#0 0x.*" \ > > > + "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" \ > > > + "\\^done,frame=3D\{level=3D\"1\".*" \ > > > + "-stack-info-frame 2" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 2.*" \ > > > + "info thread 7" > > > + > > > +mi_gdb_test "frame" \ > > > + ".*#0 0x.*" \ > > > + "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" \ > > > + "\\^done" \ > > > + "--stack-select-frame 1" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 2.*" \ > > > + "info thread 8" > > > + > > > +mi_gdb_test "frame" \ > > > + ".*#1 0x.*" \ > > > + "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" \ > > > + "\\^done" \ > > > + "--stack-select-frame 2" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 2.*" \ > > > + "info thread 9" > > > + > > > +mi_gdb_test "frame" \ > > > + ".*#2 0x.*" \ > > > + "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" \ > > > + "\\^done" \ > > > + "--stack-select-frame 3" > > > + > > > +mi_gdb_test "thread" \ > > > + ".*Current thread is 1.*" \ > > > + "info thread 10" > > > + > > > +mi_gdb_test "frame" \ > > > + ".*#0 main.*" \ > > > + "frame 6" > > > --=20 > > > 2.30.2 > > >=20 > >=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 >=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_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) >=20 > This was problematic for frontends that provide access to CLI because UI > may silently change the context for CLI commands (as demonstrated above). >=20 > 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 >=20 > 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 > + (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 ") > - || startswith (command->command, "inferior ")); > - } > - else /* MI_COMMAND */ > - { > - if (strcmp (command->command, "interpreter-exec") =3D=3D 0 > - && command->argc > 1) > - { > - /* "thread" and "inferior" again, but through -interpreter-exec. */ > - return (startswith (command->argv[1], "thread ") > - || startswith (command->argv[1], "inferior ")); > - } > - > - else > - /* -thread-select already sends it. */ > - return 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 > - interpreter (specified on the command line) is MI. */ > - top_level_interpreter ()->interp_ui_out ()->is_mi_like_p () > - /* Don't try report anything if there are no threads -- > - the program is dead. */ > - && any_thread_p () > - /* If the command already reports the thread change, no need to do it > - again. */ > - && !command_notifies_uscc_observer (command.get ())) > - { > - int report_change =3D 0; > - > - if (command->thread =3D=3D -1) > - { > - report_change =3D (previous_ptid !=3D null_ptid > - && inferior_ptid !=3D previous_ptid > - && inferior_ptid !=3D null_ptid); > - } > - else if (inferior_ptid !=3D null_ptid) > - { > - struct thread_info *ti =3D inferior_thread (); > - > - report_change =3D (ti->global_num !=3D command->thread); > - } > - > - if (report_change) > - { > - gdb::observers::user_selected_context_changed.notify > - (USER_SELECTED_THREAD | USER_SELECTED_FRAME); > - } > - } > } > } > =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) > error (_("Thread id: %d has terminated"), parse->thread); > =20 > + if (parse->cmd->preserve_user_selected_context ()) > + thread_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) > - /* 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= "] =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 \ > + { "" "disp=3D\"keep\"" } "run to breakpoint in main" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 1.*" \ > + "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" \ > + "\\^done,depth=3D.*" \ > + "-stack-info-depth --thread 3" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 1.*" \ > + "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" \ > + "\\^done,.*" \ > + "-thread-select 3" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 3.*" \ > + "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" \ > + "\\^done,.*" \ > + "-thread-select --thread 2 1" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 1.*" \ > + "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" \ > + "\\^done,.*" \ > + "-thread-select --thread 2 2" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 2.*" \ > + "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" \ > + ".*#0 0x.*" \ > + "frame 1" > + > +mi_gdb_test "-stack-info-frame --thread 2 --frame 1" \ > + "\\^done,frame=3D\{level=3D\"1\".*" \ > + "-stack-info-frame 1" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 2.*" \ > + "info thread 6" > + > +mi_gdb_test "frame" \ > + ".*#0 0x.*" \ > + "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" \ > + "\\^done,frame=3D\{level=3D\"1\".*" \ > + "-stack-info-frame 2" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 2.*" \ > + "info thread 7" > + > +mi_gdb_test "frame" \ > + ".*#0 0x.*" \ > + "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" \ > + "\\^done" \ > + "--stack-select-frame 1" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 2.*" \ > + "info thread 8" > + > +mi_gdb_test "frame" \ > + ".*#1 0x.*" \ > + "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" \ > + "\\^done" \ > + "--stack-select-frame 2" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 2.*" \ > + "info thread 9" > + > +mi_gdb_test "frame" \ > + ".*#2 0x.*" \ > + "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" \ > + "\\^done" \ > + "--stack-select-frame 3" > + > +mi_gdb_test "thread" \ > + ".*Current thread is 1.*" \ > + "info thread 10" > + > +mi_gdb_test "frame" \ > + ".*#0 main.*" \ > + "frame 6" > --=20 > 2.34.1 >=20 >=20 > 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 >=20 > 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. >=20 > Assuming thread 1 is selected, then, from the mi terminal I do: >=20 > -thread-select 2 >=20 > 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. >=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 seems to be a bug. >=20 > 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. >=20 > 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. >=20 > 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 (). >=20 > With this change, all gdb.mi tests pass, tested on x86_64-linux. >=20 > 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(-) >=20 > 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 > - (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, > 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 > - (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 > + && current_ptid !=3D previous_ptid > + && 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. */