From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM10-MW2-obe.outbound.protection.outlook.com (mail-mw2nam10on2078.outbound.protection.outlook.com [40.107.94.78]) by sourceware.org (Postfix) with ESMTPS id CE4033857405 for ; Mon, 14 Mar 2022 11:58:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CE4033857405 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XQzfQBh6Vp/cIiC1AHzDwn68NiydBSFDH8/jfaylDjbfxxnXBik0jlYlKDhSwL7sDZAck6XdDI/4h8e98R5HEfb4dUbMoxoqSi9mKkgUICVYuCioJwaVdZzEPXRFlFluJTlZR/42G7NVdpXSU3QACJCz6e2ZW1+4NhNnOrWFdM4bCekGVQkR9G3KDlK0jPOzI0mV52x86I+1KaJbzDkC4JxRPSlnaL4HozDIvW8ZP5tUQnrekEXqB/APVimlpaXfv1+y153fCkTiH7GtZ4Y+RUNbPWYWRvPAoOQfFQYX8ZaG9g1ODcfdh17T0OctEzqyo5oY7/5Yomy6ctWC9nEkKA== 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=rgL3kuSNWqkVaszfSuen/n0SACvy09HHDIvJDQDwh7c=; b=DSJA/9+Qz+ihgLUN6m1CmVNQnpB+fbyXpjrGTRt1g47o7my1cD4iGDeF5y8OJ22lXp7NGURZSTnpS/FN72u2Z/8HIKCqUjTWOjv6HJB30/wtIYzcrK9c+Gkh6q8JwF2dpOXg9O277oE812C0HSb3kNBs6RruavQl//ZFdEHAelKmfEDGYSdRN85XNN+vky2oZZEn/R1+uF/BWchP61pir7Sd5VtJmmXDhCgIm8NPZyz2lp4fTZypK/WzIQsyjO01qQLhGch5fhawJnNbr5HMok2P0ZOc5hpQQnNRHZTZVi3g3lH5ShMc/hn9Md60UvwAuO1j6t1JkvRsPqFNyxMG1A== 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 SA0PR17MB4160.namprd17.prod.outlook.com (2603:10b6:806:8b::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5061.28; Mon, 14 Mar 2022 11:58:02 +0000 Received: from DM6PR17MB3113.namprd17.prod.outlook.com ([fe80::b4e5:7f52:3fb8:ef7b]) by DM6PR17MB3113.namprd17.prod.outlook.com ([fe80::b4e5:7f52:3fb8:ef7b%7]) with mapi id 15.20.5061.028; Mon, 14 Mar 2022 11:58:02 +0000 From: Jan Vrany To: gdb-patches@sourceware.org Cc: Jan Vrany , aburgess@redhat.com Subject: [RFC PATCH] gdb/mi: consistently notify user when GDB/MI client uses -thread-select Date: Mon, 14 Mar 2022 11:57:41 +0000 Message-Id: <20220314115741.574709-1-jan.vrany@labware.com> X-Mailer: git-send-email 2.35.1 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: LO4P123CA0104.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:191::19) 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: 92bbbbd1-506e-4275-604d-08da05b1e41b X-MS-TrafficTypeDiagnostic: SA0PR17MB4160: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: w5j7/bAnvsmfPTYoWG8aSkLa6fIFAqawUQkr6Zpwd7NCMsrIvQBJ2guRkc3TPA5eEQ5Ta8+H+uiMJbJGoWqlglsJDBNbys5/kVuSaYtvR5Vk5b2u8OzJ1wz0VXHRjaqK7kqOwMor3zBf3OLUv9+hBzyhfLsjKhaCrz0XX9p4n9sHVzN+31z3uM0rNall/FuE6m0UYpuh+a1Nx1Z64TKj5iz8RRrET6rJMrZysPgs2aDzilVqsSXQuJhss4fAFP02+7SInPtD7XcZgzqcOzaDheNDEW8zmmiTIIrce/I0PVITdaSWe7KWlEB9/mMWG9e0v8T5bjC+jcNA5VapEdT2AQM6Pa7/T/8Y1mWuvJ6bHHdIWgiHJsoms2biwVG0P12vPlQgG7xEWwHyv5G1oPgi5i58Y/2vUMlYX0si4JqVlSeqz/ehQMpBj4sZz4YMzPt9kGTSh4pKrTuldhFGW6hOUlUMDNz/yzmhq2xxhpWsVVNiCQhMylChC6hq0SfW8jEM+oqNht8DPkeo9Zn+ONNbvi3E/km/ohRk9C0qGR1UO9StSfYACBNVZHEgKUUBFz4kY3lZq8Qwk5R4NN3ZT9BCm0DjTvKw58gygov867TYkzx6O+0rnc99DnfQkyXpmOzVvO7rKQtKcRX+4bUFyuXot7hQDKxZKIoSHB+WcDIy36L0uwKDp95bVBfBnVxQYcWO9KQYDwivSmk0ZIvlp0zL2mUUhzyr7BOHvuvz6wpi/wRs1h80gW11yPLdW6JKl2wQBlRQT7ADvU8y8fBZToZNw24vIa1TIk2KLWpo9YnxeHzEUT+MRLWQyCCs8XwAC43KcGQkOiIzPQQao0Y6DmdEUw== 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)(316002)(966005)(508600001)(6486002)(36756003)(8676002)(4326008)(66476007)(66556008)(66946007)(2906002)(86362001)(5660300002)(83380400001)(44832011)(26005)(8936002)(52116002)(186003)(1076003)(6506007)(6916009)(6666004)(38350700002)(6512007)(2616005)(38100700002)(403724002)(2004002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?pkXdRjNTX2+Xnnh8A8v+MZXGrrS32mj6rqQJpGkHOQXOR0WG8vjjwrmASpDI?= =?us-ascii?Q?X2l+8IitQlFN5Tx+ynO4yp7cJZK0y3J4eZY7LKg39fYUVBl6ylsg48/L9ius?= =?us-ascii?Q?n2DW9SDRu9Ms1gtd0xfmlIUBhA6iH14G+EXGoaBbK1o07mUgPJtesWnR9GWd?= =?us-ascii?Q?ntFtuIwTuKAR8LPtXV7JBo2qoUx6lEvelKQQya9j1A3+3Mz0tWErI35gpWwv?= =?us-ascii?Q?BSkQHATemriwKmF6sfQ/XurKiRQr9H0/dxObRxIxUGh7V0AyuoXAFxeyIqZ8?= =?us-ascii?Q?vSrJJWdeA29cA/jH8p/GBfH/lyecMwlwhWndTr4tNodhuY3gOHK47fHS00km?= =?us-ascii?Q?8n8J2KfQMNfEXREjQ6huXY7+OoY2FWk87yIy5xQOU0KrJnqXf1ZsUjzvUsPn?= =?us-ascii?Q?KqWihNSd0svyHwxkrzE4UzzAi8o3guMhWjcTYDYgC+2w3iwTy7mmujNmiLmx?= =?us-ascii?Q?Dd82mg5hBI2myKiJ0jLPkFeDWcoDGKGy30f980h7yslibYxCCbZOCKctBhev?= =?us-ascii?Q?n6rdTfTAcCA6YYKBf1IeRVvo2J1weLYRcK4NNLLKuMUZWOB0V6eOtm2hwd7W?= =?us-ascii?Q?cBRAU3nPtWKTPPWo6eyz9KZnQTg/Z3xZGWO5xyyAe0so/65DFhUY+jUfDfph?= =?us-ascii?Q?CXqr4wXqXCLG/aDTpBBitDqXXMPlFkU8gaxnM0ZwSnyzQsEcBtyhYDkM/bo+?= =?us-ascii?Q?Yuc/ae+Hvw6P3Vrka4uqRl/nZeTPOElPRJRTb9/U7RBfgwM9jZTbT8P4OiIw?= =?us-ascii?Q?zWh/2tuZnHmrWr89j1pvCdXobODxUTj73NItndvDHwGjGQ4W89beV7c2jTZa?= =?us-ascii?Q?GTgyqZbvqpqWNvzwUqmE3i1ctkfMCSDzrtZlzVJxx3z42TBM73V13gEMrm4q?= =?us-ascii?Q?v1uo457y2wIw5KwShyQY1JEYxe2uKgzJYV0xMgdWbRxE9K4lxqpYjLYBz12K?= =?us-ascii?Q?wrcxv039sOVm7wE+5bsnNzfjikLgwdICK2VDlwEuJecnDDcKGPEtQ/iZP7Vk?= =?us-ascii?Q?9AMuIUWhLnBnTNBgobwmkmLbNSFMFXl/1TT/+SK/DQyaUwZREH7Ck6uX2rM+?= =?us-ascii?Q?/1ozSVUQ/q5HPBnoK62emac60p+boHlc0u3d7uxOHxb9X71mCJ1kr1puw0P6?= =?us-ascii?Q?iLqHy6oMrFxaDTalzpSb4wwwuxOhgAFXu6RPVnJocg5PK7+VZ8i+bv2QB5yA?= =?us-ascii?Q?Smxu7XLnUXH/1+JVizwFxIi10gYghAqUm4WAJRTsjYgOsXQUFvWFS2agG6QB?= =?us-ascii?Q?UUbz9wgXsAEHwlfxsztGDyHEy36yCY9beWqY9We9hXOys0jHf57hMYKY+Qac?= =?us-ascii?Q?MFEgHxaoy5lTp7a/wy6EYRG2KHy1vpQMXyZzzzmNLEXMCY0ge1fFhnxrUfQw?= =?us-ascii?Q?PLEiTBN3w4yn2ESKqMpx/bh6lXnCguWf0G3XXnuRAP2JHmMkCL8l9eUOdIb4?= =?us-ascii?Q?C+Ozp4MZZtNGWmaAUh2/7mk6hZjL9ML4?= X-OriginatorOrg: labware.com X-MS-Exchange-CrossTenant-Network-Message-Id: 92bbbbd1-506e-4275-604d-08da05b1e41b X-MS-Exchange-CrossTenant-AuthSource: DM6PR17MB3113.namprd17.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2022 11:58:02.2727 (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: 0WFHuENrueuQZVDJPkh9UBz/S8lEhd7qt5fDOoKpQgH1YhhyiiGcQGMKC0GtOkMP8T2KekYHah/qyxK/bj+NxA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR17MB4160 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, 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: Mon, 14 Mar 2022 11:58:09 -0000 Hi Andrew, this is a follow-up patch, trying to address inconsistency in GDB/MI notifications you pointed out the other day [1]. The below patch seems to fix it when I manually test it, but I did not manage to write a test. I tried several things, but always get timeouts. See my (failed) attempt in below patch (note, that the expected output is wrong by purpose to ensure the test actually works). At this point I'd like to take you offer of help - if you have an idea how to write the test, I'd be great. [1]: https://sourceware.org/pipermail/gdb-patches/2022-February/185989.html -- >8 -- GDB notifies users about user selected thread changes somewhat inconsistently as mentioned on gdb-patches mailing list here: https://sourceware.org/pipermail/gdb-patches/2022-February/185989.html Consider GDB debugging a multi-threaded inferior with both CLI and GDB/MI interfaces connected to separate terminals. Assuming inferior is stopped and thread 1 is selected, when a thread 2 is selected using '-thread-select 2' command on GDB/MI terminal: -thread-select 2 ^done,new-thread-id="2",frame={level="0",addr="0x00005555555551cd",func="child_sub_function",args=[],file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",fullname="/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c",line="30",arch="i386:x86-64"} (gdb) and on CLI terminal we get the notification (as expected): [Switching to thread 2 (Thread 0x7ffff7daa640 (LWP 389659))] #0 child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30 30 volatile int dummy = 0; However, now that thread 2 is selected, if thread 1 is selected using 'thread-select --thread 1 1' command on GDB/MI terminal terminal: -thread-select --thread 1 1 ^done,new-thread-id="1",frame={level="0",addr="0x0000555555555294",func="main",args=[],file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",line="66",arch="i386:x86-64"} (gdb) but no notification is printed on CLI terminal, despite the fact that user selected thread has changed. The problem is that when `-thread-select --thread 1 1` is executed then thread is switched to thread 1 before mi_cmd_thread_select () is called, therefore the condition "inferior_ptid != previous_ptid" there does not hold. To address this problem, we have to move notification 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: * fix 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 +++++--- ...mi-user-selected-context-notifications.exp | 78 +++++++++++++++++++ 5 files changed, 109 insertions(+), 29 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-user-selected-context-notifications.exp 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 == 0 || argc > 1) error (_("-stack-select-frame: Usage: FRAME_SPEC")); - - ptid_t previous_ptid = inferior_ptid; - select_frame_for_mi (parse_frame_specification (argv[0])); - - /* Notify if the thread has effectively changed. */ - if (inferior_ptid != previous_ptid) - { - gdb::observers::user_selected_context_changed.notify - (USER_SELECTED_THREAD | USER_SELECTED_FRAME); - } } 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 *suppress_notification) void mi_command::invoke (struct mi_parse *parse) const { - gdb::optional> restore - = do_suppress_notification (); this->do_invoke (parse); } 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 != &mi_suppress_notification.user_selected_context; } -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 = 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 () const; +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 = 0; + +private: + /* The name of the command. */ const char *m_name; diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 73380f5e668..a5b847c7039 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 **argv, int argc) if (thr == NULL) error (_("Thread ID %d not known."), num); - ptid_t previous_ptid = inferior_ptid; - thread_select (argv[0], thr); print_selected_thread_frame (current_uiout, USER_SELECTED_THREAD | USER_SELECTED_FRAME); - - /* Notify if the thread has effectively changed. */ - if (inferior_ptid != previous_ptid) - { - gdb::observers::user_selected_context_changed.notify - (USER_SELECTED_THREAD | USER_SELECTED_FRAME); - } } void @@ -1975,6 +1966,16 @@ mi_execute_command (const char *cmd, int from_tty) } } +/* Determine whether the thread has changed. */ + +static bool +command_changed_user_selected_thread (ptid_t previous_ptid, ptid_t current_ptid) +{ + return (previous_ptid != null_ptid + && current_ptid != previous_ptid + && current_ptid != null_ptid); +} + static void mi_cmd_execute (struct mi_parse *parse) { @@ -2015,6 +2016,8 @@ mi_cmd_execute (struct mi_parse *parse) set_current_program_space (inf->pspace); } + ptid_t previous_ptid = inferior_ptid; + gdb::optional thread_saver; if (parse->thread != -1) { @@ -2060,7 +2063,18 @@ mi_cmd_execute (struct mi_parse *parse) current_context = parse; gdb_assert (parse->cmd != nullptr); + + gdb::optional> restore_suppress_notification + = parse->cmd->do_suppress_notification (); + parse->cmd->invoke (parse); + + if (!parse->cmd->preserve_user_selected_context () + && command_changed_user_selected_thread (previous_ptid, inferior_ptid)) + { + gdb::observers::user_selected_context_changed.notify + (USER_SELECTED_THREAD | USER_SELECTED_FRAME); + } } /* See mi-main.h. */ diff --git a/gdb/testsuite/gdb.mi/mi-user-selected-context-notifications.exp b/gdb/testsuite/gdb.mi/mi-user-selected-context-notifications.exp new file mode 100644 index 00000000000..cf9c02e54a4 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-user-selected-context-notifications.exp @@ -0,0 +1,78 @@ +# Copyright 2022-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 user is propery notified on CLI channel when. +# selected thread and/or frame is changed using GDB/MI. +# +# See https://sourceware.org/pipermail/gdb-patches/2022-February/185989.html + +load_lib mi-support.exp + +standard_testfile user-selected-context-sync.c + +# Multiple threads are needed, therefore only native gdb and extended +# gdbserver modes are supported. +if [use_gdb_stub] { + untested "using gdb stub" + return +} + +if {[build_executable $testfile.exp $testfile ${srcfile} "debug pthreads"] == -1} { + untested "failed to compile" + return -1 +} + +if [eval mi_gdb_start { "separate-mi-tty" }] { + return +} + +# Useful for debugging: +verbose -log "Channels:" +verbose -log " inferior_spawn_id=$inferior_spawn_id" +verbose -log " gdb_spawn_id=$gdb_spawn_id" +verbose -log " gdb_main_spawn_id=$gdb_main_spawn_id" +verbose -log " mi_spawn_id=$mi_spawn_id" + +mi_gdb_load $binfile +if { [mi_runto_main] < 0 } { + return +} + +switch_gdb_spawn_id $gdb_main_spawn_id + +gdb_test "break $srcfile:child_function" \ + "Breakpoint.*at.* file .*$srcfile, line.*" \ + "breakpoint function in file" + +gdb_test "continue" \ + ".*Thread 2.*hit Breakpoint 2.*" \ + "continue to child_function" + +switch_gdb_spawn_id $mi_spawn_id + +send_gdb "111-thread-select 1" +gdb_expect { + -i "$mi_spawn_id" + -re "\\^done,new-thread-id=\"2\".*" { + fail "111-thread-select 1 (MI output)" + } + -i "$gdb_main_spawn_id" + -re "\\\[Switching to thread 1.*" { + fail "111-thread-select 1 (CLI output)" + } + timeout { + fail "111-thread-select 1 (timeout)" + } +} -- 2.35.1