From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59880 invoked by alias); 14 Sep 2016 18:30:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 59841 invoked by uid 89); 14 Sep 2016 18:30:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Sep 2016 18:30:14 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C1039C05AA40; Wed, 14 Sep 2016 18:30:12 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8EIUB3D029358; Wed, 14 Sep 2016 14:30:11 -0400 Subject: Re: [PATCH master+7.12 v2 2/3] Emit inferior, thread and frame selection events to all UIs To: Simon Marchi , gdb-patches@sourceware.org References: <20160914174548.5873-1-simon.marchi@ericsson.com> <20160914174548.5873-3-simon.marchi@ericsson.com> Cc: Antoine Tremblay From: Pedro Alves Message-ID: <8f1b10b3-131d-df38-573f-dedf0cc0103d@redhat.com> Date: Wed, 14 Sep 2016 18:30:00 -0000 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: <20160914174548.5873-3-simon.marchi@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-09/txt/msg00120.txt.bz2 Hi Simon, On 09/14/2016 06:45 PM, Simon Marchi wrote: > From: Antoine Tremblay > > With this patch, when an inferior, thread or frame is explicitly > selected by the user, notifications will appear on all CLI and MI UIs. > When a GDB console is integrated in a front-end, this allows the > front-end to follow a selection made by the user ont he CLI, and it > informs the user about selection changes made behind the scenes by the > front-end. > > This patch fixes PR gdb/20487. > > In order to communicate frame changes to the front-end, this patch adds > a new field to the =thread-selected event for the selected frame. The > idea is that since inferior/thread/frame can be seen as a composition, > it makes sense to send them together in the same event. The vision > would be to eventually send the inferior information as well, if we find > that it's needed, although the "=thread-selected" event would be > ill-named for that job. Given per-inferior thread numbers, we could also say that we switch to thread 0 of inferior INF (i.e., "INF.0"). Then it wouldn't sound that strange, maybe. The problem is that MI talks in terms of the global thread id, not the per-inferior id. Anyway, since is for machine consumption, odd naming should not be a big deal, IMO. > frame > ----- > > 1. CLI command: > > frame 1 > > MI event: > > =thread-selected,id="3",frame={level="1",...} > > 2. MI command: > > -stack-select-frame 1 > > CLI event: > > #1 0x00000000004007f0 in child_function... > I think it's likely that experience will show that will want to tweak what we print in the CLI in the future, along with whether we print at all, but that's fine for now. Making all user-selection change handling be consistent makes sense. > 3. MI command (CLI-in-MI): > > frame 1 > > MI event/reply: > > &"frame 1\n" > ~"#1 0x00000000004007f9 in ..." > =thread-selected,id="3",frame={level="1"...} > ^done > > inferior > -------- > > Inferior selection events only go from the console to MI, since there's > no way to select the inferior in pure MI. > > 1. CLI command: > > inferior 2 > > MI event: > > =thread-selected,id="3" > > Note that if the user selects an inferior that is not started or exited, > the MI doesn't receive a notification. Since there is no threads to > select, the =thread-selected event does not apply... We could solve that by adding the thread group id (inferior id) to the notification, I think: =thread-selected,id="3",thread-group="i1",frame="..." =thread-selected,id="0",thread-group="i2",frame="..." ... If you select an inferior that is not running yet, thread 0 is what you effectively get: (gdb) p $_inferior $1 = 1 (gdb) p $_thread $2 = 1 (gdb) p $_gthread $3 = 1 (gdb) add-inferior Added inferior 2 (gdb) inferior 2 [Switching to inferior 2 [] ()] (gdb) p $_inferior $4 = 2 (gdb) p $_thread $5 = 0 (gdb) p $_gthread $6 = 0 (gdb) > @@ -2124,9 +2137,16 @@ mi_execute_command (const char *cmd, int from_tty) > if (command != NULL) > { > ptid_t previous_ptid = inferior_ptid; > + struct cleanup *cleanup = make_cleanup (null_cleanup, NULL); > > command->token = token; > > + if (command->cmd != NULL && command->cmd->suppress_notification != NULL) > + { > + make_cleanup_restore_integer (command->cmd->suppress_notification); > + *command->cmd->suppress_notification = 1; > + } > + > if (do_timings) > { > command->cmd_start = XNEW (struct mi_timestamp); > @@ -2161,10 +2181,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. */ I'm still uneasy about this. Do we now emit a =thread-selected event when the frontend uses -thread-select? Is that a deliberate change? > - && strcmp (command->command, "thread-select") != 0) > + /* For CLI commands "thread" and "inferior", the event is already sent > + by the command, so 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 > + && strcmp (command->command, "interpreter-exec") == 0 > + && strncmp (command->argv[1], "thread", 6) != 0 > + && strncmp (command->argv[1], "inferior", 8) != 0))) These "strncmp" calls return 0 when the command is "threadfoo" or "inferiorfoo" I think we need to check the next character too, somehow? I think it doesn't make a difference for any of the current "thread" subcommands ("thread apply", etc.), so probably not a big deal. (though it'd be nice to clean it up sooner than later to avoid this getting forgotten and breaking in the future.) But, I suspect that we end up suppressing this case: (gdb) define thread-foo >thread $arg0 >end (gdb) thread-foo 2 Contrived, but certainly not hard to imagine user-commands doing something useful along with changing the selected thread. What happens in this case? Thanks, Pedro Alves