From patchwork Fri Sep 9 18:51:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 15470 Received: (qmail 13329 invoked by alias); 9 Sep 2016 18:58:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 13319 invoked by uid 89); 9 Sep 2016 18:58:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.1 required=5.0 tests=BAYES_00, SPF_PASS, TBC autolearn=no version=3.3.2 spammy=intentions, shares, stopping, sk:build_e X-HELO: sesbmg23.ericsson.net Received: from sesbmg23.ericsson.net (HELO sesbmg23.ericsson.net) (193.180.251.37) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Sep 2016 18:58:44 +0000 Received: from ESESSHC018.ericsson.se (Unknown_Domain [153.88.183.72]) by (Symantec Mail Security) with SMTP id 67.81.04209.06603D75; Fri, 9 Sep 2016 20:58:41 +0200 (CEST) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.72) with Microsoft SMTP Server (TLS) id 14.3.301.0; Fri, 9 Sep 2016 20:52:12 +0200 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [142.133.110.144] (192.75.88.130) by AMSPR07MB390.eurprd07.prod.outlook.com (10.242.22.13) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.1.557.21; Fri, 9 Sep 2016 18:52:09 +0000 From: Simon Marchi Subject: Re: [PATCH master+7.12] Send thread, frame and inferior selection events to all uis To: Pedro Alves , Antoine Tremblay , References: <20160831135148.14381-1-antoine.tremblay@ericsson.com> <371bf10f-1126-4d45-24ee-3e7c546eb84d@redhat.com> Message-ID: Date: Fri, 9 Sep 2016 14:51:48 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <371bf10f-1126-4d45-24ee-3e7c546eb84d@redhat.com> X-ClientProxiedBy: SN2PR18CA0005.namprd18.prod.outlook.com (10.169.189.15) To AMSPR07MB390.eurprd07.prod.outlook.com (10.242.22.13) X-MS-Office365-Filtering-Correlation-Id: 61876350-f2e7-4231-57c3-08d3d8e267f5 X-Microsoft-Exchange-Diagnostics: 1; AMSPR07MB390; 2:yndm2tn+9521Sa9rF/N/8ozd/GpToNTwXfv10M6YTCV11Q0fXG1GjP5V3gXCb/Ald4gc83TTtUd/2K2c2oxwalG4Gp5LcDMvMPCeAAwfPOASsxT9VK8NuNoJpErC0mTSCiIRMI+KhrqIGFSVzM7OzatlZ1feDEWs4VTSkgwzUVswUBKqyf868pXHICi0/Bkc; 3:hK65L1/3cWPd5tECZOcLUjl/K5bTLOp/tMHzyLmVtgX0UFZEefJsrb2oaOcAU+vo9K23njtpex/YG+VZ/k/BPPMve1qCQRsaPIspC/VG8JSNFY+qX2EP88fle5znrliW X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AMSPR07MB390; X-Microsoft-Exchange-Diagnostics: 1; AMSPR07MB390; 25:2cK+aIl0xe5O/c1qWrNpPUw86UwkzWd/uE2ajo1wyDfFez2565lWLG5i2FcjElkpg5XQp9TawbsyBla75pXOmpwa8SUXmoj/ymMnaUGhsbMO3zLrgX0UZMYHcSFMZ2CURpFgj2vX+iX3ejsd5G3hqrQsSrtTNrKGj3ku64MbKAFMELp7laT3yPnprlCyAVsyJV8hlwW8z11VtdAdysZN2rp46wP3nfXPn+LKq9HCF+qepx+pof8UvvDaCNmNafFTM157QjSMS4NUzCbFVR8xfxq8Jv7MyKdisWg0ahPwh0fKTxw7ejruziqBrcTTj0SZYtaOEFUgdatBIoafab3uAQ7G3fLvWCHJjeRBmeVxr0H9+RKIzqOKXriFYatqP+Je80zGHY836jTcN7+HW6TgbpY938iNMxsOl4Fi2zomBSHqI4+zyS0E1JOxdNI7csasmsfweW8ZPIuErKPnZOv4kIZb/EapqysiUznSm8xJQUfBt3ncKvDmVj1F9bJcKxAoSexpsDkz79O4kNglKkToYft76ZEkbav+gqBAq47ggep7fxlH2eS500viZhUtcp/xDQ/kYtDXyZ+rBrFsW6KhXD4FUG5V2mFvIVHFIiwUYOMWLnIfyfe8zGq06m2/QR2YnEtKqwB+fMr/6g87eM8notCjMbOpmabg9MPEAVC+J9UbdmMYJr9kegn4aZASyGZsCk4gXOH2TFQFBr+LlfmJIhud/Up8n+Jw6WhUcwkXGgFTl0kgDMkcvCnwIY4LfVVTayQw0QqT+M5jaIjUQRBf6jureRO6QgQY33DicXyR9BBpskKZx2+XugtidWQs4sF92Egz5zQ6E3QbZAqNwNYvNuvpKqN+x7OrWZ3XqpZ1L/c= X-Microsoft-Exchange-Diagnostics: 1; AMSPR07MB390; 31:NMD8EzniPG/k6JMl79ogVEqsOCtNiz8aBo0h9SSYa8W41PebyI8IymRHeQ9uwc/3c5m4pmuyurLihFrc6wDW6Dk6bufFKPOsA9nUUGpZWk5Fp1JYqttNJyHD2Can3nJVS6EY/EZV+tPn+nXMxpoT3ZfQ9tFg2TEFXX+OrZLPYDMivjhwVIH6OVCq4HwXoN+bS8udYS6q9wc9Cc3H88lE+/o3/8jjpLmkqV1+c3iwm3c=; 20:ChGpBWTVuOY414l+raF3PTtrz0obhg4TK41Yszacf1vj3t+sQoRTm7zOj+1slqotIZFfiEWux97WMd5bYPYbOND7OTCZBIcpznktkuSyEuH1FvETd9h+Ph7Gex6I8frvMuEWURgGf7r5fvg85UsnAdHwSRUDPKbTDRfQihT+NxFMAazO/6tTJK5L9BdVdRdeyjSBZOkSXZvbKZ2YonAiVw+TT4/t469njrDZydgwcA8hViRNXngbbnD/p64erxcZQZE39Sc71tU5+eVwCI1d8s5rvRptrIjhT0QQWLY9CqrhTlHNDBcWnVZ/TyGajAyJIIByPutq8r2s3FytbzPIps6XGZ9xmFTThoDeQA5+KdVK1lfkBDj9fQipIVQZyuDThu8e07P9USCD7Jl+YSO7ZJk4skKiDJOI+Cy5MCxUXwlI18bKVFZtSaoTac/1a3P4dyNTVhkut9QAoAEr132ZabOIZB3uRxNrn8/YtHakMRU2MHAsZmSjxn3hf+Bv8YvH X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(250305191791016)(22074186197030)(21532816269658); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046); SRVR:AMSPR07MB390; BCL:0; PCL:0; RULEID:; SRVR:AMSPR07MB390; X-Microsoft-Exchange-Diagnostics: 1; AMSPR07MB390; 4:KZIZQabVFGw6rihTfuBIs0YjbYooywdQxHBQrXPY32VkqEm7Js5oLZwQn07BQ4HIN0OUq8tTMifB329ioSWPv3J2CEo/puZ1h6/XafDskyTJxpR1L/iVcoJfGqzFoUHJszUQZFhYokjYyBihTDb5s7Oxs9nVAFcqyPJy3TpUkbTc2ja0SNvR9U53Jkc3xsnfbHI0kjGiBKsD4pV0vcXNgoHO9NJ4bv595qiNBApl9yfwNGYwqEH3kFUDoB7U1NBPDk6N71n5H7VZEF68Balyp94ISOEwIo8BPAWrpDyPHyZRk7qJYYUvLFHSoV4HwGsx3GLqZEzvy7CYN6qumHLUMdBKu2L6ksHYvnrwlNkjP7aSL9zsXNWF8XLONLAlDslL8qmam3Sd4eM28SYtbotejVS0rrJWbeli6jAtrUL/DQit6A0bTZ8E/NVIJO8RZmdKGFmOEHQC0rg+x2cBDrEpUpneVXUk5oMYU4lDq+F7xfuPLVUfjoBLrWRxSfPsnEUs X-Forefront-PRVS: 00603B7EEF X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6009001)(6049001)(7916002)(377424004)(189002)(199003)(377454003)(24454002)(51444003)(43544003)(101416001)(77096005)(31696002)(23746002)(4001350100001)(42186005)(31686004)(54356999)(15975445007)(65806001)(50466002)(65956001)(586003)(66066001)(76176999)(230700001)(2950100001)(92566002)(47776003)(81156014)(2906002)(19580395003)(50986999)(36756003)(64126003)(7736002)(8676002)(33646002)(86362001)(5001770100001)(5660300001)(7846002)(65826007)(6116002)(189998001)(3846002)(107886002)(106356001)(81166006)(105586002)(83506001)(97736004)(575784001)(305945005)(68736007)(403724002)(2004002)(579004); DIR:OUT; SFP:1101; SCL:1; SRVR:AMSPR07MB390; H:[142.133.110.144]; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1; AMSPR07MB390; 23:TlRvbEj1hBaEdeYjWIz5i/pf78fYKXIfwq3a54?= =?Windows-1252?Q?/VWL+J/Dv98epHcE/3LgwOoVbUxkfMe4zw4BwigPZm/ajYWTqNAA1z3q?= =?Windows-1252?Q?jS+mzoyGSJoz8zE8YskNwRKJXg7Ln+SoXIOPQeAD9OOhC66Ilg40f2Tw?= =?Windows-1252?Q?KpLn9jhL5vZJeFzfpe532r6fkn+FTM5Kt0ly2szhMEjvXEwf3WgjjdtB?= =?Windows-1252?Q?hQne//ZFm9Ye8631ddUz8uD8fHnPmVoSeo8/4AXFWZez7f1E6Z0nKcX8?= =?Windows-1252?Q?D3NkoQjXN9E78E+KpOa5VuxTvL/IsY9Jbn76A6AUkmih0lHKXRjEb/51?= =?Windows-1252?Q?9rerG7SDIu8OQ9MasEllBpnA0GbWHOSh6gKXwXXMC06jjZ5E6RtENMV7?= =?Windows-1252?Q?WsEICBM8PbQReaXvwHf9hCqgiWUUlTwwwfdiKmi0hJGgS4tJGWmSLBnZ?= =?Windows-1252?Q?CvPMQOpeaAj8HyCQ2w6GCHbApiBRlFag5YqsMocNrbshx5PmShvRuV1h?= =?Windows-1252?Q?03MHdnGi/6y13q1rUBXp4wOPm+wFI7dAUb9HoiQTB0EvM1TP9Vac2+BR?= =?Windows-1252?Q?aL+/ezasa9WK7OIRSzk66LDamW+90/+NPWrE6QychaazrgigK1W1fUYW?= =?Windows-1252?Q?wxSc3/j4Hw5qyTnZghkjcHSCZ28nS90ifQYt3eu3ppJ4NGCBMCamwR4Q?= =?Windows-1252?Q?A9NpB0Tjb4vjxxXKMWjg57VTVNjABVb7PdXz0atZIxc7EBtnDNL8sop+?= =?Windows-1252?Q?UqajgIepR7U5yz979rjDG5yD6zha7BS0OA9ry4w7ejLtOjj65A7UxMHI?= =?Windows-1252?Q?uXNMzdqj/9DN0jRHjJBECMyhU9kjP0skTG12ZevqUJPqvm6IPb4g+ufh?= =?Windows-1252?Q?tLr8KJkxj+2x5ld0yv/qEeXzQKio90/iyhVBQ3jMfqJqBnRZw7oG2oD+?= =?Windows-1252?Q?Qu4Rq3Ez9chG7bdYoHdK4WpCNwoLV1zWDSzs5HzgiKftPAI7nlZnEII7?= =?Windows-1252?Q?4Bzl+8Z8Xi7mWuhLswMJxVzFYOIChjDXbAlSdjwnENy0gxxYUoixHyQX?= =?Windows-1252?Q?eSyjEskIQ9y2xF6+OpUsDcg8uN1QUOQGPNX86J6O0pTo6HWNGR94vm15?= =?Windows-1252?Q?4jw5aYZj6iPC6j063Ceb4e8Zw23L3W3TteiB9MCtJs72BUqmL92bgE6W?= =?Windows-1252?Q?nd2dHFwpYj1EGzl4yhu3nf92iVCoSMdHRPj1vJ8dRlg89czjhUIG3h93?= =?Windows-1252?Q?KJ6peAtRTfN30DJ8EqUMTMJ1eDV6Q5SrOIN3I+ylOTQ+D7GsP6/70+1x?= =?Windows-1252?Q?vj0eh8RrYXcXcpRijVP9Vjf/NKshWHylNBZJoX6/ag5xqVut98nMNjyo?= =?Windows-1252?Q?yD91WYkHV93C85uwVu69ESL8PItt0+SFvQ7khx8GgBJ6AfX0Us5kynBv?= =?Windows-1252?Q?yh9Q7gDr9/M/m8YeMnCLr9fxQDOA8CEB6NX+VuaS5hb5HLjgkcL0fgeH?= =?Windows-1252?Q?UnnqbV8GXwjONvKaIWFc1xoEkVlSXKUXDxYBii/mRojj/DroQy/zQCBv?= =?Windows-1252?Q?4PVP1YV4eP3TuOmxWD8x2MaHsRcdAGr5xT?= X-Microsoft-Exchange-Diagnostics: 1; AMSPR07MB390; 6:1WciIPvrgPbIcACbZS56AGHtJGnG87ZLodKAvgTicQK0LB/QIqCPL+ELj3yoHGk/tZELH2wBwVYrNfAy6j6jcin5ZQJQVrCm8kIDR6ZY6bDyI5voTcP2oYqC+CMhVWcDzXpnUSTEMYawNUpPgmJwTLkOlIeRnR8LptohZ3WQCCG7CJJ+op7Vs8+8ZdSlajnvg9FPYvjWZR6jxGV5F1bZo3GNQwDX8OnRCjGLKRhotiwf/eZSriSMIiUxk6nf0mPDBqiSG8yf/oBUgIkVD5BZXR9oQW0icC9ozdPr7HBZkhU=; 5:wf3zwNQ9je6JNjd9lqOtm+p81D/HPogvhuDENnkF09NwWRUywYaOFGFqy5c5gWC+CoxBP2UWI8qKkkLDYKFbP7+unVv7/ftgopVn0Dmk0NW+VZKkon2aS08CmNpKyyGJ5wA4HAlL4JWo435gvKZs9Q==; 24:JrruxHJvRnpHtfLqpdxBir+NqP0TO6vYc5YUc+wbokxvzSjtFRRv9jvJFKoNc7WrVyfw4zGzzgld1al6nN2Wvm4tZigENZJiw/x5+lg9NeQ=; 7:08x3kFomg8IAF4RCjoJ4rH/r9TycUqsnRdN8Z32WfITbFKdhM9w+33tB52Yz4Q5ysSOoVp8VXu36rcctq88vvLz5e3CkSNRFzW8KTCUHKB/GCLiYeU4OKz8rlHO7GBkzkC0FZJuoEeG++57Y4nkOG4TgXQhsQUfmuuw/oXLy4a8ISTOsJK0LCa1no7RljA1hgUIy1t1WEcJi7ulhlRXBwo2A1KpzOpf/KIWdjzTSprzB88n/6Qr+S8NALdzEsZhX SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Sep 2016 18:52:09.8962 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AMSPR07MB390 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes On 16-09-05 10:44 AM, Pedro Alves wrote: > Hi Antoine, Hi Pedro, I am taking over for Antoine, as he went to take care of another kind of child process. Thanks, for the thorough review. I'll try to send a v2 hopefully soon, in the mean time there are a few things you can look at in this message, such as proposed procedure documentation in the test. >> From mi: thread 1.3 >> Will generate the following mi: >> &"thread 1.3\n" >> ~"#0 child_sub_function () ... >> =thread-selected,id="3",frame={level="0",...} >> ^done > > Does the later generate the event in the console too? I assume so. Yes: [Switching to thread 3 (Thread 0x7ffff6ff5700 (LWP 8972))] #0 0x00007ffff78b7dfd in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6 >> Note also that this patch makes it possible to suppress notifications >> caused by a cli command, like was done in mi-interp previously. >> >> This means that it's now possible to use the add_com_suppress_notification >> function to register a command with some event suppressed like is done >> with the select-frame command in this patch. > > I should note that this suppression mechanism is broken when you > have multiple MI uis or multiple consoles. We end up suppressing > the event on all uis that happen run the same top level interpreter... > I.e., with two CLIs, do "thread" on one UI, and notice how the > event is suppressed on the other UI. Likewise with two MIs, etc. > > TBC, it's fine with me to not address this for now. That's right. > This needs a manual update: > > @subsubsection Threads and Frames > > [...] > it is reasonable to switch to the thread where breakpoint is hit. For > another example, if the user issues the CLI @samp{thread} command via > the frontend, it is desirable to change the frontend's selected thread to the > one specified by user. @value{GDBN} communicates the suggestion to > change current thread using the @samp{=thread-selected} notification. > No such notification is available for the selected frame at the moment. > [...] > @item =thread-selected,id="@var{id}" > Informs that the selected thread was changed as result of the last > command. > [...] Suggested changes: >> diff --git a/gdb/NEWS b/gdb/NEWS >> index 6f5feb1..b427a7e 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -11,6 +11,10 @@ >> >> *** Changes in GDB 7.12 >> >> +* MI =thread-selected event now includes the frame field. For example: >> + >> + =thread-selected,id="3",frame={level="0",addr="0x00000000004007c0"} >> + > > > Please put this next to the other MI change: > > * MI async record =record-started now includes the method and format used for > recording. For example: > > =record-started,thread-group="i1",method="btrace",format="bts" > > and I guess use "async record" too for consistency. Done. >> +/* Suppress notification struct. */ >> +struct cli_suppress_notification cli_suppress_notification = >> + { >> + 0 /* user_selected_inf_thread_frame */ > > Maybe rename this to user_selected_context? Using "inf_thread_frame" > will get weird once we add more "what"s. Makes sense. I changed all the occurrences. I named the observer "user_selected_context_changed", to be more in line with the other names. >> +{ >> + struct switch_thru_all_uis state; >> + struct thread_info *tp = find_thread_ptid (inferior_ptid); >> + >> + /* This event is suppressed. */ >> + if (cli_suppress_notification.user_selected_inf_thread_frame) >> + return; > > Move the find_thread_ptid call after this, to avoid an > unnecessary lookup? Done. >> diff --git a/gdb/defs.h b/gdb/defs.h >> index fee5f41..cb180a9 100644 >> --- a/gdb/defs.h >> +++ b/gdb/defs.h >> @@ -750,6 +750,21 @@ enum block_enum >> FIRST_LOCAL_BLOCK = 2 >> }; >> >> +/* User selection used in observer.h and multiple print functions. */ >> + >> +enum user_selected_what_flag >> + { >> + /* Inferior selected. */ >> + USER_SELECTED_INFERIOR = 1 << 1, >> + >> + /* Thread selected. */ >> + USER_SELECTED_THREAD = 1 << 2, >> + >> + /* Frame selected. */ >> + USER_SELECTED_FRAME = 1 << 3 >> + }; >> +DEF_ENUM_FLAGS_TYPE (enum user_selected_what_flag, user_selected_what); >> + > > This needs to #include "common/enum-flags.h". > > If this didn't cause you a compile error, then it must be some other > header is already pulling the first one in. I got curious and checked > what it was. It's this sequence: > > In file included from /home/pedro/gdb/mygit/src/gdb/symtab.h:26:0, > from /home/pedro/gdb/mygit/src/gdb/language.h:26, > from /home/pedro/gdb/mygit/src/gdb/frame.h:72, > from /home/pedro/gdb/mygit/src/gdb/gdbarch.h:38, > from /home/pedro/gdb/mygit/src/gdb/defs.h:652, > from /home/pedro/gdb/mygit/src/gdb/gdb.c:19: > /home/pedro/gdb/mygit/src/gdb/common/enum-flags.h:1:2: error: #error "enum-flags.h" included > > In any case, we should #include what we depend on directly > instead of relying on some accidental indirect dependency. Agreed, done. >> diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi >> index fc7aac4..4f8f0bb 100644 >> --- a/gdb/doc/observer.texi >> +++ b/gdb/doc/observer.texi >> @@ -307,3 +307,7 @@ This observer is used for internal testing. Do not use. >> See testsuite/gdb.gdb/observer.exp. >> @end deftypefun >> >> +@deftypefun void user_selected_inf_thread_frame (user_selected_what @var{selection}) >> +The user-selected inferior,thread and/or frame has changed. The user_select_what >> +flag specifies if the inferior, thread or frame has changed. > > Missing space in "inferior,thread". Done. >> +/* See inferior.h. */ >> + >> +void >> +print_selected_inferior (struct ui_out *uiout) >> +{ >> + char buf[PATH_MAX+256]; > > Missing spaces. Done. >> +static void >> +mi_user_selected_inf_thread_frame (user_selected_what selection) >> +{ >> + struct switch_thru_all_uis state; >> + struct thread_info *tp = find_thread_ptid (inferior_ptid); >> + >> + /* Don't send an event if we're responding to an MI command. */ >> + if (mi_suppress_notification.user_selected_inf_thread_frame) >> + return; > > Same comment about moving find_thread_ptid. Done. >> + >> + SWITCH_THRU_ALL_UIS (state) >> + { >> + struct mi_interp *mi = as_mi_interp (top_level_interpreter ()); >> + struct ui_out *mi_uiout; >> + struct cleanup *old_chain; >> + >> + if (mi == NULL) >> + continue; >> + >> + mi_uiout = interp_ui_out (top_level_interpreter ()); >> + >> + ui_out_redirect (mi_uiout, mi->event_channel); >> + >> + old_chain = make_cleanup_restore_target_terminal (); >> + target_terminal_ours_for_output (); >> + >> + if (selection & USER_SELECTED_INFERIOR) >> + print_selected_inferior (mi->cli_uiout); >> + >> + if (tp != NULL >> + && ((selection & (USER_SELECTED_THREAD | USER_SELECTED_FRAME)))) > > One level of parens too many? Done. >> + { >> + print_selected_thread_frame (mi->cli_uiout, selection); >> + >> + fprintf_unfiltered (mi->event_channel, >> + "thread-selected,id=\"%d\"", >> + tp->global_num); >> + >> + if (tp->state != THREAD_RUNNING) >> + { >> + if (has_stack_frames ()) >> + print_stack_frame_to_uiout (mi_uiout, get_selected_frame (NULL), >> + 1, SRC_AND_LOC, 1); >> + } >> + } >> + >> + ui_out_redirect (mi_uiout, NULL); > > Use make_cleanup_ui_out_redirect_pop instead. Like this? @@ -1360,14 +1362,16 @@ mi_user_selected_inf_thread_frame (user_selected_what selection) ui_out_redirect (mi_uiout, mi->event_channel); - old_chain = make_cleanup_restore_target_terminal (); + old_chain = make_cleanup_ui_out_redirect_pop (mi_uiout); + + make_cleanup_restore_target_terminal (); target_terminal_ours_for_output (); if (selection & USER_SELECTED_INFERIOR) print_selected_inferior (mi->cli_uiout); >> @@ -2102,6 +2106,7 @@ mi_execute_command (const char *cmd, int from_tty) >> { >> char *token; >> struct mi_parse *command = NULL; >> + struct cleanup *cleanup = NULL; > > Move this to the scope that needs it. Done. >> >> /* This is to handle EOF (^D). We just quit gdb. */ >> /* FIXME: we should call some API function here. */ >> @@ -2161,10 +2166,15 @@ mi_execute_command (const char *cmd, int from_tty) >> /* Don't try report anything if there are no threads -- >> the program is dead. */ >> && thread_count () != 0 >> - /* -thread-select explicitly changes thread. If frontend uses that >> - internally, we don't want to emit =thread-selected, since >> - =thread-selected is supposed to indicate user's intentions. */ >> - && strcmp (command->command, "thread-select") != 0) >> + /* For the cli commands thread and inferior, the event is already sent >> + by the command, don't send it again. */ >> + && ((command->op == CLI_COMMAND >> + && strncmp (command->command, "thread", 6) != 0 >> + && strncmp (command->command, "inferior", 8) != 0) >> + || (command->op == MI_COMMAND && command->argc <= 1) >> + || (command->op == MI_COMMAND && command->argc > 1 >> + && strncmp (command->argv[1], "thread", 6) != 0 >> + && strncmp (command->argv[1], "inferior", 8) != 0))) > > Urgh. :-/ I wish we'd find some better way... > > - This is matching "threadfoo", "inferiorfoo" as well. > > - This is matching the "thread" subcommands too. Probably OK. > > - Does this match command aliases? (alias foo=thread) > > - The suppression used to be for "thread-select" only, now > it's done for all MI commands. Not sure whether this was intended > or not. IOW, this needs an explicit rationale/comments. Are you talking about this line? (command->op == MI_COMMAND && command->argc <= 1) I am not sure what it's for... > - Is the argc checking meant to handle "-interpreter-exec"? > Shouldn't there be an explicit check for that? It shouldn't be too hard to do. > I guess Simon's work on decoupling user-selected state a > little better might help clean this up. > But meanwhile, we need to make sure to err on the > "emit notification" side and the latter two points above worry me. Agreed, I think it's going to help clean up a few places. Until then, I don't see a better solution than what Antoine did. >> { >> struct mi_interp *mi >> = (struct mi_interp *) top_level_interpreter_data (); >> @@ -2185,18 +2195,21 @@ mi_execute_command (const char *cmd, int from_tty) >> >> if (report_change) >> { >> - struct thread_info *ti = inferior_thread (); >> - struct cleanup *old_chain; >> - >> - old_chain = make_cleanup_restore_target_terminal (); >> - target_terminal_ours_for_output (); >> - >> - fprintf_unfiltered (mi->event_channel, >> - "thread-selected,id=\"%d\"", >> - ti->global_num); >> - gdb_flush (mi->event_channel); >> - >> - do_cleanups (old_chain); >> + /* Make sure we still keep event suppression. This is >> + handled in mi_cmd_execute so at this point this has been >> + reset. We still need it here however. */ > > This raises the question of why is the event suppression done in > mi_cmd_execute instead of in this function, before calling > mi_cmd_execute? I moved the handling of event suppression to this function, it seems to work fine. >> + if (command->cmd->suppress_notification != NULL) >> + { >> + cleanup = make_cleanup_restore_integer >> + (command->cmd->suppress_notification); >> + *command->cmd->suppress_notification = 1; >> + } >> + >> + observer_notify_user_selected_inf_thread_frame >> + (USER_SELECTED_THREAD | USER_SELECTED_FRAME); >> + >> + if (cleanup != NULL) >> + do_cleanups (cleanup); >> } >> } >> >> diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h >> index 18000cf..f5b6bcf 100644 >> --- a/gdb/mi/mi-main.h >> +++ b/gdb/mi/mi-main.h >> @@ -49,6 +49,8 @@ struct mi_suppress_notification >> int traceframe; >> /* Memory changed notification suppressed? */ >> int memory; >> + /* Inferior, thread, frame selected notification suppressed? */ >> + int user_selected_inf_thread_frame; >> }; >> extern struct mi_suppress_notification mi_suppress_notification; >> >> diff --git a/gdb/stack.c b/gdb/stack.c >> index 417e887..80ce00b 100644 >> --- a/gdb/stack.c >> +++ b/gdb/stack.c >> @@ -51,6 +51,7 @@ >> #include "safe-ctype.h" >> #include "symfile.h" >> #include "extension.h" >> +#include "observer.h" >> >> /* The possible choices of "set print frame-arguments", and the value >> of this setting. */ >> @@ -141,6 +142,22 @@ frame_show_address (struct frame_info *frame, >> return get_frame_pc (frame) != sal.pc; >> } >> >> +/* See frame.h */ >> + >> +void >> +print_stack_frame_to_uiout (struct ui_out *uiout, struct frame_info *frame, >> + int print_level, enum print_what print_what, >> + int set_current_sal) >> +{ >> + struct ui_out *saved_uiout; >> + saved_uiout = current_uiout; >> + current_uiout = uiout; >> + >> + print_stack_frame (frame, print_level, print_what, set_current_sal); >> + >> + current_uiout = saved_uiout; > > Use a cleanup. Done, I factored it out from infrun.c:print_stop_event and will post it in a preparatory patch. >> diff --git a/gdb/testsuite/gdb.mi/mi-pthreads.exp b/gdb/testsuite/gdb.mi/mi-pthreads.exp >> index 88a600a..a49856e 100644 >> --- a/gdb/testsuite/gdb.mi/mi-pthreads.exp >> +++ b/gdb/testsuite/gdb.mi/mi-pthreads.exp >> @@ -53,7 +53,7 @@ proc check_mi_thread_command_set {} { >> >> foreach thread $thread_list { >> mi_gdb_test "-interpreter-exec console \"thread $thread\"" \ >> - ".*\\^done\r\n=thread-selected,id=\"$thread\"" \ >> + ".*=thread-selected,id=\"$thread\".*\[r\n\]+\\^done\[\r\n\]+" \ >> "check =thread-selected: thread $thread" > > Not sure I'm reading this right, but I think this just means that > the notification is now sent before the ^done. Is that right? > Probably not a problem, just checking. That would make sense. The =thread-selected event used to be emitted after the command complete, when GDB realized that currently selected thread != previously selected thread. Now, it's done during the execution of the command, so before the ^done. >> } >> } >> diff --git a/gdb/testsuite/gdb.mi/thread-selected-sync.c b/gdb/testsuite/gdb.mi/thread-selected-sync.c >> new file mode 100644 >> index 0000000..4113d26 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/thread-selected-sync.c > > Maybe rename this to user-selected-context-sync.c ? Makes sense, done. Should it be mi-user-selected-context-sync.{c,exp} though, since all the test cases in gdb.mi start with mi-? By the way, why do they all start with mi-, when they are already in the "gdb.mi" directory? >> @@ -0,0 +1,64 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2016 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 . >> + >> +*/ >> + >> +/* Note that this test is not expected to exit cleanly. All threads will >> + block at the barrier and they won't be waken up. */ > > Please add an alarm call then: > > https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Don.27t_write_tests_that_run_forever Instead, I made the main thread call sleep (30), and then call pthread_barrier_wait, which unlocks the other threads and allows the program to end. >> +void >> +child_sub_function () > > (void) Done. >> +# It also considers the case were the console commands are sent directly in >> +# the mi channel as described in PR 20487. > > s/were/where/ Done. >> +# >> +# It does so by starting 2 inferiors with 3 threads each. >> +# Thread 1 is the main thread starting the others. >> +# Thread 2 is stopped in non-stop mode. >> +# Thread 3 is the thread used for testing, in non-stop mode this thread is running. >> + >> +load_lib mi-support.exp >> + >> +standard_testfile >> + >> +# Multiple inferiors are needed, therefore only native gdb and extended >> +# gdbserver modes are supported. > > Could we restrict this to the specific tests that need multiple inferiors? > I think you'd just need to guard the creation of the second inferior > in test_setup, and then make test_*_select_inferior bail out with > unsupported or some such. Probably. I just tried to do that, and here are a few things to consider: - With a single inferior, thread numbers are only TH, instead of INF.TH, so many functions need to take that into account. - By skipping some *_inferior tests, the user selection is left a different state for the next procedure. For example: * thread 1.3 is currently selected * test_inferior switches inferior, making thread 2.1 the selected one * test_thread switches to thread 1.3 and expects an event If we skip test_inferior, there won't be an event when test_thread tries to switch to thread 1.3, as it will already be selected. That kind of dependency between test procedures should be avoided as much as possible I think, as it will make the test very hard to modify (as we see now). I think that we could add some kind of "reset_selection" procedure that takes care of restoring a known, stable selection at the beginning of each test procedure. >> +if [use_gdb_stub] { >> + untested ${testfile}.exp >> + return >> +} >> + >> +set compile_options "debug pthreads" >> +if {[build_executable $testfile.exp $testfile ${srcfile} ${compile_options}] == -1} { >> + untested "failed to compile $testfile" >> + return -1 >> +} >> + >> +set bp_lineno [gdb_get_line_number "set break here"] >> +set caller_lineno [gdb_get_line_number "caller"] >> + >> +# Make a regular expression for cli. > > It'd be nice to extend this comment. What does the > regular expression match? What are the parameters for? What about: # Make a regular expression to match the "thread selected" event for CLI. # # MODE can be either "all-stop" or "non-stop", indicating which one is currently # in use. # INF is the inferior number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce an inferior switch. # THREAD is the thread number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce a thread switch. # FRAME is the frame number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce a frame switch. See the FRAME_RE variable for # details. # If LINEEND is 1, expect and end of line at the end of the regular expression. >> + >> +proc make_cli_re {mode inf thread frame lineend} { >> + global srcfile bp_lineno caller_lineno >> + >> + set inf_re "\\\[Switching to inferior $inf.*\\\]" >> + set all_stop_thread_re "\\\[Switching to thread $thread.*\\\]" >> + set non_stop_thread_re "$all_stop_thread_re\\\(running\\\)" >> + set frame_re(0) "#0.*child_sub_function.*$srcfile:$bp_lineno\[\r\n\]+.*set break here \\\*/" >> + set frame_re(1) "#1.*child_function \\\(args=0x0\\\) at .*$srcfile:$caller_lineno\[\r\n\]+$caller_lineno.*caller \\\*/" >> + # Special frame for main threads. >> + set frame_re(2) "#0.*" >> + set end "\[\r\n\]" >> + >> + set cli_re "" >> + >> + if {$inf != -1} { >> + append cli_re $inf_re >> + } >> + >> + if {$thread != "-1"} { >> + if {$inf != -1} { >> + append cli_re "\[\r\n\]+" >> + } >> + set thread_re "" >> + if {$mode == "all-stop"} { >> + set thread_re $all_stop_thread_re >> + } elseif {$mode == "non-stop"} { >> + set thread_re $non_stop_thread_re >> + } >> + append cli_re $thread_re >> + } >> + >> + if {$mode == "all-stop" && $frame != -1} { >> + if {$thread != -1} { >> + append cli_re "\[\r\n\]+" >> + } >> + append cli_re $frame_re($frame) >> + } >> + >> + if {$lineend == 1} { >> + append cli_re $end >> + } >> + >> + return $cli_re >> +} >> + >> +# Make a regular expression for mi. >> + > > Ditto. Essentially the same, but without the INF parameter: # Make a regular expression to match the "thread selected" event for MI. # # MODE can be either "all-stop" or "non-stop", indicating which one is currently # in use. # THREAD is the thread number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce a thread switch. # FRAME is the frame number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce a frame switch. See the FRAME_RE variable for # details. # If LINEEND is 1, expect and end of line at the end of the regular expression. >> +proc make_mi_re {mode thread event frame lineend} { >> + global srcfile bp_lineno caller_lineno hex >> + >> + set mi_re "" >> + set thread_event_re "=thread-selected,id=\"$thread\"" >> + set thread_answer_re ".*\\^done,new-thread-id=\"$thread\"" >> + set frame_re(0) ",frame=\{level=\"0\",addr=\"$hex\",func=\"child_sub_function\",args=\\\[\\\],file=\".*$srcfile\",.*line=\"$bp_lineno\"\}" >> + set frame_re(1) ",frame=\{level=\"1\",addr=\"$hex\",func=\"child_function\",args=\\\[\{name=\"args\",value=\"0x0\"\}\\\],file=\".*$srcfile\",.*line=\"$caller_lineno\"\}" >> + #Special frame for main thread. >> + set frame_re(2) ",frame=\{level=\"0\",addr=\"$hex\",func=\".*\",args=\\\[\\\],from=\".*\"\}" >> + set end "\[\r\n\]" >> + >> + if {$thread != "-1"} { >> + if {$event == 1} { >> + append mi_re $thread_event_re >> + } elseif {$event == 0} { >> + append mi_re $thread_answer_re >> + } >> + } >> + >> + if {$mode == "all-stop" && $frame != -1} { >> + append mi_re $frame_re($frame) >> + } >> + >> + if {$lineend == 1} { >> + append mi_re $end >> + } >> + >> + return $mi_re >> +} >> + >> +# Make a regular expression for cli in mi. And here too: # Make a regular expression to match the "thread selected" event for CLI-in-MI. # # COMMAND is the CLI command that was sent to GDB, which will be output in the # console output stream. # MODE can be either "all-stop" or "non-stop", indicating which one is currently # in use. # INF is the inferior number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce an inferior switch. # CLI_THREAD is the thread number as seen in the CLI (inferior-qualified) we are # expecting GDB to switch to, or -1 if we are not expecting GDB to announce a # thread switch. # MI_THREAD is the thread number as seen in the MI (global number) we are # expecting GDB to switch to, or -1 if we are not expecting GDB to announce a # thread switch. # FRAME is the frame number we are expecting GDB to switch to, or -1 if we are # not expecting GDB to announce a frame switch. See the FRAME_RE variable for # details. # If LINEEND is 1, expect and end of line at the end of the regular expression. >> +proc make_cli_in_mi_re {command mode inf cli_thread mi_thread event frame lineend} { ... >> +# Continue to the breakpoint indicating the start position of the threads. >> + >> +proc test_continue_to_start {mode inf} { >> + global gdb_prompt mi_spawn_id >> + >> + if {$mode == "all-stop"} { >> + for {set i 1} { $i <= 2 } { incr i } { >> + gdb_continue_to_breakpoint "barrier breakpoint" >> + } >> + } else { >> + set test "continue&" >> + >> + gdb_test_multiple $test $test { >> + -re "Continuing\\.\[\r\n\]+$gdb_prompt " { >> + pass $test >> + } >> + } >> + >> + # Wait until we've hit the breakpoint for the 2 threads. >> + for {set i 1} { $i <= 2 } { incr i } { >> + set test "thread $i started" >> + gdb_test_multiple "" $test { >> + -re "hit Breakpoint" { >> + # The prompt was already matched in the "continue &" >> + # test above. We're now consuming asynchronous output >> + # that comes after the prompt. >> + pass $test >> + } >> + } >> + } >> + # Switch to the test thread. >> + if {$inf == 1} { >> + gdb_test "thread 3" [make_cli_re "all-stop" -1 "3" 0 -1] "Switch to test thread" >> + } else { >> + gdb_test "thread 2.3" [make_cli_re "all-stop" -1 "2\\.3" 0 -1] "Switch to test thread" > > How came these pass "all-stop", when you're in the "non-stop" if/then branch? That's odd. When I switch them to non-stop it fails. I'll look into it. >> +proc test_setup {mode} { >> + global srcfile srcdir subdir testfile >> + global gdb_main_spawn_id mi_spawn_id >> + global decimal binfile bp_lineno >> + global GDBFLAGS >> + >> + mi_gdb_exit >> + >> + set saved_gdbflags $GDBFLAGS >> + >> + if {$mode == "non-stop"} { >> + set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop 1\""] >> + } >> + >> + if {[mi_gdb_start "separate-mi-tty"] != 0} { >> + return >> + } >> + >> + mi_delete_breakpoints >> + mi_gdb_reinitialize_dir $srcdir/$subdir >> + mi_gdb_load $binfile >> + >> + if {[mi_runto main] < 0} { >> + fail "Can't run to main" >> + return >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + >> + gdb_test "break $srcfile:$bp_lineno" \ >> + "Breakpoint $decimal .*$srcfile, line $bp_lineno\\\." \ >> + "set breakpoint" >> + >> + test_continue_to_start $mode 1 >> + >> + # Add a second inferior to test inferior selection. >> + gdb_test "add-inferior" "Added inferior 2" "Add inferior 2" >> + gdb_test "inferior 2" [make_cli_re $mode 2 "-1" -1 -1] >> + gdb_load ${binfile} >> + gdb_test "start" "Temporary breakpoint.*Starting program.*" >> + test_continue_to_start $mode 2 >> + gdb_test "inferior 1" [make_cli_re $mode 1 "1\\.1" 2 -1] >> + } >> + >> + set GDBFLAGS $saved_gdbflags > > There are early returns before you reach here. > Use 'save_vars { GDBFLAGS } { ... }' around the mi_gdb_start > call instead. Ok. if there is a return in the save_vars, will it return from the test setup as we expect? For example: save_vars { GDBFLAGS } { if {$mode == "non-stop"} { set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop 1\""] } if {[mi_gdb_start "separate-mi-tty"] != 0} { return } } Otherwise, when using MI, couldn't we just set non-stop after having started GDB? mi_gdb_start only starts GDB, but doesn't connect to anything, so it wouldn't be too late to send a "set non-stop 1" after. >> +proc test_cli_select_thread_twice {mode} { >> + global gdb_main_spawn_id mi_spawn_id >> + >> + set mi_re [make_mi_re $mode "3" 1 0 1] >> + set cli_re [make_cli_re $mode -1 "1\\.3" 0 0] >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "thread 1.3" $cli_re "cli select thread twice, first call" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi select thread twice, first call" >> + gdb_test_multiple "" $test { >> + -re $mi_re { >> + pass $test >> + } >> + } >> + } >> + >> + # No event for that one. >> + set mi_re "" >> + set cli_re [make_cli_re $mode -1 "1\\.3" 0 0] >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "thread 1.3" $cli_re "cli select thread twice, second call" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi select thread twice, second call" >> + gdb_test_multiple "" $test { >> + -re $mi_re { > > "$mi_re" is empty here, so this doesn't look it's actually > testing anything? Several other instances in other tests. > > See ensure_no_output in new-ui.exp for an idea of how to > check that no output has been sent. > > I don't think we should be using gdb_test_multiple on MI. > It's mostly useless, compared to gdb_expect, since > its internal matchings expect CLI's "$gdb_prompt $", which > won't match in MI. Ideally we'd refactor out something > similar to gdb_test_multiple out of mi_gdb_test (and then > even try to factor out common bits between the result and > gdb_test_multiple), but nobody's ever bothered. So you > get to use gdb_expect directly. I'll look into it, along with the other comments about the test. >> + pass $test >> + } >> + } >> + } >> + >> +} >> + >> +# Test frame selection from cli. >> + >> +proc test_cli_select_frame {mode frame} { >> + global gdb_main_spawn_id mi_spawn_id >> + >> + if {$mode == "all-stop"} { >> + set mi_re [make_mi_re $mode "3" 1 $frame 1] >> + set cli_re [make_cli_re $mode -1 "-1" $frame 0] >> + } elseif {$mode == "non-stop"} { >> + set cli_re "Selected thread is running\\." >> + # No output >> + set mi_re "" >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "frame $frame" $cli_re "cli select frame" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi select frame" >> + gdb_test_multiple "" $test { >> + -re $mi_re { >> + pass $test >> + } >> + } >> + } >> +} >> + >> +# Test frame selection from cli with the select-frame command. >> + >> +proc test_cli_select_select_frame {mode frame} { >> + global gdb_main_spawn_id mi_spawn_id >> + >> + if {$mode == "all-stop"} { >> + set cli_re "" >> + set mi_re [make_mi_re $mode "3" 1 $frame 1] >> + } elseif {$mode == "non-stop"} { >> + set cli_re "Selected thread is running\\." >> + # No output >> + set mi_re "" >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + if {$cli_re != ""} { >> + gdb_test "select-frame $frame" $cli_re "cli select frame with select-frame" >> + } else { >> + gdb_test_no_output "select-frame $frame" $cli_re "cli select frame with select-frame" >> + } >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi select frame with select-frame" >> + gdb_test_multiple "" $test { >> + -re $mi_re { >> + pass $test >> + } >> + } >> + } >> +} >> + >> +# Test doing and up and then down command from cli. > > "an up", I guess. Done. >> + >> +proc test_cli_up_down {mode} { >> + global gdb_main_spawn_id mi_spawn_id >> + >> + if {$mode == "all-stop"} { >> + set cli_up_re [make_cli_re $mode -1 "-1" 1 0] >> + set mi_up_re [make_mi_re $mode "3" 1 1 1] >> + set cli_down_re [make_cli_re $mode -1 "-1" 0 0] >> + set mi_down_re [make_mi_re $mode "3" 1 0 1] >> + } elseif {$mode == "non-stop"} { >> + set cli_up_re "No stack\\." >> + set mi_up_re "" >> + set cli_down_re "No stack\\." >> + set mi_down_re "" >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "up" $cli_up_re "cli up" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi up" >> + gdb_test_multiple "" $test { >> + -re $mi_up_re { >> + pass $test >> + } >> + } >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "down" $cli_down_re "cli down" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi down" >> + gdb_test_multiple "" $test { >> + -re $mi_down_re { >> + pass $test >> + } >> + } >> + } >> +} >> + >> +# Test same frame selection from cli. >> + >> +proc test_cli_select_frame_twice {mode frame} { >> + global gdb_main_spawn_id mi_spawn_id >> + >> + if {$mode == "all-stop"} { >> + set mi_re [make_mi_re $mode "3" 1 $frame 1] >> + set cli_re [make_cli_re $mode -1 "-1" $frame 0] >> + } elseif {$mode == "non-stop"} { >> + set cli_re "Selected thread is running\\." >> + # No output >> + set mi_re "" >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "frame $frame" $cli_re "cli select frame twice, first call" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi select frame twice, first call" >> + gdb_test_multiple "" $test { >> + -re $mi_re { >> + pass $test >> + } >> + } >> + } >> + >> + if {$mode == "all-stop"} { >> + #No output thread has not changed. > > Did you mean to put a period or something else after "No output"? > Reads strange as is. Changed to: # No output, thread has not changed. >> + set mi_re "" >> + set cli_re [make_cli_re $mode -1 "-1" $frame 0] >> + } elseif {$mode == "non-stop"} { >> + set cli_re "Selected thread is running\\." >> + # No output >> + set mi_re "" >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + gdb_test "frame $frame" $cli_re "cli select frame twice, second call" >> + } >> + >> + with_spawn_id $mi_spawn_id { >> + set test "mi select frame, second call" >> + gdb_test_multiple "" $test { >> + -re $mi_re { >> + pass $test >> + } >> + } >> + } >> +} >> + >> +# Test thread command without any arguments fromt the cli. > > "from". Done. >> + with_spawn_id $mi_spawn_id { >> + mi_gdb_test "-stack-select-frame $frame" $mi_re "mi select frame twice, frist call" > > > "first" Done. >> + } >> + >> + with_spawn_id $gdb_main_spawn_id { >> + set test "cli select frame twice, frist call" > > > "first" Done. >> + /* Print if the thread has not changed, otherwise an event will be sent. */ >> + if (ptid_equal (inferior_ptid, previous_ptid)) >> + { >> + print_selected_thread_frame (current_uiout, USER_SELECTED_THREAD >> + | USER_SELECTED_FRAME); > > The "|" should be aligned under "USER_", and you'd need an extra set of > parens to make emacs happy: > > print_selected_thread_frame (current_uiout, (USER_SELECTED_THREAD > | USER_SELECTED_FRAME)); > > but in this case it's more readable IMO to put the USER_SELECTED_THREAD > on the next line too: > > print_selected_thread_frame (current_uiout, > USER_SELECTED_THREAD | USER_SELECTED_FRAME); Done. >> +/* Observer for the user_selected_thread_frame notification. */ >> + >> +static void >> +tui_on_user_selected_inf_thread_frame (user_selected_what selection) >> +{ >> + struct switch_thru_all_uis state; >> + struct thread_info *tp = find_thread_ptid (inferior_ptid); > > Same comment as in CLI/MI. Done. >> + >> + /* This event is suppressed. */ >> + if (cli_suppress_notification.user_selected_inf_thread_frame) >> + return; >> + > > I tried running the new test here, and got: > > Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/thread-selected-sync.exp ... > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli: mi select inferior (timeout) > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli: mi select inferior (timeout) > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from mi: mi select inferior (timeout) > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from mi: mi select inferior (timeout) > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli in mi: mi select inferior > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli in mi: mi select inferior > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: interpreter-exec: mi select inferior > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: interpreter-exec: mi select inferior > > gdb.log shows, for the first FAIL above: > > [...] > ~"[Switching to inferior 2 [process 10838] (/home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.mi/thread-selected-sync/thread-selected-sync)]\n" > ~"[Switching to thread 2.1 (Thread 0x7ffff7fc7700 (LWP 10838))]\n" > ~"#0 0x00007ffff7bc66bd in pthread_join (threadid=140737342580480, thread_return=0x0) at pthread_join.c:90\n" > ~"90\t lll_wait_tid (pd->tid);\n" > =thread-selected,id="4",frame={level="0",addr="0x00007ffff7bc66bd",func="pthread_join",args=[{name="threadid",value="140737342580480"},{name="thread_return",value="0x0"}],file="pthread_join.c",fullname="/usr/src/debug/glibc-2.22/nptl/pthread_join.c",line="90"} > FAIL: gdb.mi/thread-selected-sync.exp: all-stop: from cli: mi select inferior (timeout) > [...] > > > Try to check with "make check-read1" too. I'll look into it. > I also notice that the test has many duplicated test messages: > https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique I'll look into it. Thanks, Simon diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index d1a5e7c..0c9c866 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -25807,13 +25807,13 @@ identifier for thread and frame to operate on. Usually, each top-level window in a frontend allows the user to select a thread and a frame, and remembers the user selection for further operations. However, in some cases @value{GDBN} may suggest that the -current thread be changed. For example, when stopping on a breakpoint -it is reasonable to switch to the thread where breakpoint is hit. For -another example, if the user issues the CLI @samp{thread} command via -the frontend, it is desirable to change the frontend's selected thread to the -one specified by user. @value{GDBN} communicates the suggestion to -change current thread using the @samp{=thread-selected} notification. -No such notification is available for the selected frame at the moment. +current thread or frame be changed. For example, when stopping on a +breakpoint it is reasonable to switch to the thread where breakpoint is +hit. For another example, if the user issues the CLI @samp{thread} or +@samp{frame} commands via the frontend, it is desirable to change the +frontend's selection to the one specified by user. @value{GDBN} +communicates the suggestion to change current thread and frame using the +@samp{=thread-selected} notification. Note that historically, MI shares the selected thread with CLI, so frontends used the @code{-thread-select} to execute commands in the @@ -26459,13 +26459,18 @@ A thread either was created, or has exited. The @var{id} field contains the global @value{GDBN} identifier of the thread. The @var{gid} field identifies the thread group this thread belongs to. -@item =thread-selected,id="@var{id}" -Informs that the selected thread was changed as result of the last -command. This notification is not emitted as result of @code{-thread-select} -command but is emitted whenever an MI command that is not documented -to change the selected thread actually changes it. In particular, -invoking, directly or indirectly (via user-defined command), the CLI -@code{thread} command, will generate this notification. +@item =thread-selected,id="@var{id}"[,frame="@var{frame}"] +Informs that the selected thread or frame were changed. This notification +is not emitted as result of the @code{-thread-select} or +@code{-stack-select-frame} commands, but is emitted whenever an MI command +that is not documented to change the selected thread and frame actually +changes them. In particular, invoking, directly or indirectly +(via user-defined command), the CLI @code{thread} or @code{frame} commands, +will generate this notification. Changing the thread of frame from another +user interface (@xref{Interpreters}) will also generate this notification. + +The @var{frame} field is only present if the newly selected thread is +stopped. See @ref{GDB/MI Frame Information} for the format of its value. We suggest that in response to this notification, front ends highlight the selected thread and cause subsequent commands to apply to