From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128304 invoked by alias); 28 Jul 2016 05:32:19 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 128292 invoked by uid 89); 28 Jul 2016 05:32:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=ideal, clients X-HELO: mail-vk0-f49.google.com Received: from mail-vk0-f49.google.com (HELO mail-vk0-f49.google.com) (209.85.213.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 28 Jul 2016 05:32:07 +0000 Received: by mail-vk0-f49.google.com with SMTP id s189so24588170vkh.1 for ; Wed, 27 Jul 2016 22:32:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=re0bxAQPde4gLfGQoHJILlOctV0kB/gDG5OrzF7caDE=; b=Ua+yeiLT9AJmx6ID+c4+bGbxoxaMVcl1BDIS/6dILzqUBNz+o+4nD7Rlkf6ruNqbZt Ja269Pw33qwVN6K+ZAJg4EbUyYuixBNAI/S0dG9+e5nH1Pf3F9la38s5eq7F+Fcx0LLN 3NAsEBwcYGRh+rRFOZvFbieYwo89Gl/dw/OWgBrquQRRxGG17gABP/1rVxRO6Me6IH+v aJdvqFGdZjiKLverj8r6ITORbTEn0W2mo2EMGy8dc2gMxKmiK9Z5TTLynraH3L5MQw2C B+6WQMDgyvcbHudHPL7hrOYqmaqQg3xlrLOsQwZXqbC8BFbu/+4RHBKsb3Tcf7XxSGh4 pOXw== X-Gm-Message-State: AEkooutyGv0vZjF2sgSTWufhEVLWrNHaBK/JWafk0CBXvW+3CkMoXWbU8W9hGFTmAd7pgOFlteMtkpvkcGbbUQ== X-Received: by 10.31.194.19 with SMTP id s19mr12324636vkf.126.1469683924915; Wed, 27 Jul 2016 22:32:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.153.4 with HTTP; Wed, 27 Jul 2016 22:32:04 -0700 (PDT) In-Reply-To: <20160726232223.GF4517@embecosm.com> References: <20160726080900.GE4517@embecosm.com> <20160726232223.GF4517@embecosm.com> From: Paramjot Oberoi Date: Thu, 28 Jul 2016 05:32:00 -0000 Message-ID: Subject: Re: How to Read Program Architecture from GDB/MI? To: Andrew Burgess Cc: gdb@sourceware.org Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2016-07/txt/msg00036.txt.bz2 Andrew, I agree with what you've said regarding moving to tuples. One small suggestion I would like to make is to have the tuple include the original parameter to show as well. (gdb) interpreter-exec mi "1234-gdb-show architecture" ^done,show={type="architecture",value="auto", current="i386"} This would allow a frontend developer to parse out different different fields in the tuple depending on the type of of the data. As it is now I don't believe there's a way to determine which show command was requested just by looking at the results. On Tue, Jul 26, 2016 at 7:22 PM, Andrew Burgess wrote: > * Paramjot Oberoi [2016-07-26 14:36:35 -0400]: > >> Andrew, >> >> Thank you for your patch, it's a huge improvement over the hack I was >> considering. For the time being I will use "interpreter-exec mi "show >> architecture."", and hook the console output. This has some drawbacks, >> but it will save me from having to distribute a custom GDB patch if I >> want to distribute my code. >> >> I'm still familiarizing myself with the code base, but long term I'm >> divided on whether it makes sense to use tuples here for the output. >> Wouldn't that break existing compatibility with all GDB/MI frontends >> that use -gdb-show? >> >> "show architecture" currently returns two pieces of information, >> whether the architecture is set to "auto" ("target architecture is set >> automatically") and what the currently executing architecture is >> ("currently i386"). Let's simply add another show command that always >> returns the currently executing architecture. "show >> current-architecture". Now you can query if the architecture is set to >> auto (not particularly useful to me) or if you don't care, you can >> simply query the currently executing architecture directly. > > I don't think that's a great idea. The problem is you're focusing too > much on the architecture example that you've hit, when in reality this > problem extends to all -gdb-show VAR cases (or could do). > > Switching to tuples could be a problem, but given the current pretty > poor state of the MI show support I think that a case could be made to > fix this properly once, causing a little pain now, but resulting in a > much better situation moving forward. > > If we took your suggestion, adding a new variable > 'current-architecture' then things start to get pretty messy. I guess > you're thinking that this variable would never be set. Would it make > sense to be able to view this variable from the CLI? I guess it > doesn't hurt, but then you have the confusion of having the same > information (apparently) stored in two variables.... > > Not to mention that that are lots of variables that follow the auto > model, so you'd want to add current-* versions for all of these too. > > If backwards compatibility was a real issue then we could make the MI > return the current non-tuple style answer by default, and have a > config switch that changes the output to the tuple style fuller > answer. Clients that expect or want this behaviour can then select it > ... or maybe we just add a new MI command -gdb-show-full VAR that > returns the tuple style output. > > In summary I still think tuples are the way to go, there are ways to > solve the backwards compatibility issues, and adding a new variable > would not, I think, scale well over all of the "auto" variables. > > Hope that helps, > > Thanks, > Andrew > > > > > >> >> -gdb-show architecture -> maps directly to the set architecture >> command. Returns null if the architecture hasn't been set (which means >> it's auto), "auto" if the user set it to auto, or the architecture >> type that the user set. >> -gdb-show current-architecture -> always returns the current executing >> architecture. >> >> Let me know what your thoughts are. Thanks in advance. >> >> >> On Tue, Jul 26, 2016 at 4:09 AM, Andrew Burgess >> wrote: >> > * Paramjot Oberoi [2016-07-26 02:27:31 -0400]: >> > >> >> Thank you for the quick response, it got me down the right track. >> >> Unfortunately simply commenting out that if() won't work for me as the >> >> output will no longer have the GDB/MI request IDs. It would be the >> >> same as if I did "interpreter-exec mi "show architecture." I believe >> >> the correct fix will require some proper planning. I've spent a few >> >> hours looking at GDB's source code and I can't think of a clean way to >> >> implement it. >> >> >> >> The core issue is the output from "show architecture" and >> >> "interpreter-exec mi "-gdb-show architecture"" do not match in the >> >> case that the architecture is not set or is set to auto. If you >> >> manually set the architecture ("set architecture i386") the GDB/MI >> >> output is correct. As you mentioned the c->show_value_func >> >> (show_architecture) only gets called for the console output case, and >> >> not for the GDB/MI. It is the source of the difference. >> >> >> >> Console output: >> >> (gdb) show architecture >> >> The target architecture is set automatically (currently i386) ---> >> >> set_architecture_string is NULL, but output correctly says the target >> >> architecture is auto, and prints out the current architecture >> >> (gdb) set architecture auto >> >> The target architecture is set automatically (currently i386) >> >> (gdb) show architecture >> >> The target architecture is set automatically (currently i386) ---> >> >> set_architecture_string is "auto", and prints out the current >> >> architecture >> >> >> >> GDB/MI output: >> >> (gdb) interpreter-exec mi "-gdb-show architecture" >> >> ^done ---> set_architecture_string is NULL so nothing is output, does >> >> not print out the current architecture >> >> (gdb) set architecture auto >> >> The target architecture is set automatically (currently i386) >> >> (gdb) interpreter-exec mi "-gdb-show architecture" >> >> ^done,value="auto" ---> set_architecture_string is "auto", does not >> >> print out the current architecture >> >> >> >> c->show_value_func (show_architecture) is what handles the special >> >> logic for having "auto" or uninitialized architectures: >> >> >> >> static void >> >> show_architecture (struct ui_file *file, int from_tty, >> >> struct cmd_list_element *c, const char *value) >> >> { >> >> if (target_architecture_user == NULL) >> >> fprintf_filtered (file, _("The target architecture is set " >> >> "automatically (currently %s)\n"), >> >> gdbarch_bfd_arch_info (get_current_arch ())->printable_name); >> >> else >> >> fprintf_filtered (file, _("The target architecture is assumed to be %s\n"), >> >> set_architecture_string); >> >> } >> >> >> >> There is no such equivalent callback for the GDB/MI case. I can't >> >> think of a way to do this that wouldn't be hackish. One thought was to >> >> modify the if() else to specifically look for this case: >> >> >> >> if (ui_out_is_mi_like_p (uiout)) >> >> { >> >> if(c->show_value_func == show_architecture) >> >> { >> >> // reimplement the logic of show_architecture() here, but for MI >> >> // we would need to wipe the existing stb because it might already >> >> have the word "auto" in there >> >> >> >> } >> >> ui_out_field_stream (uiout, "value", stb); >> >> } >> >> else >> >> { >> >> ... >> >> ... >> >> } >> > >> > As you point out I don't think there's a quick fix to your problem, >> > and the comment in 'do_show_command' acknowledges that this area is >> > broken when it comes to MI. >> > >> > As a quick fix how about the patch below. It's not ideal, but it >> > might be enough for you. >> > >> > The basic idea is to wrap the MI return from -gdb-show into a tuple, >> > then add an extra field 'message', which contains the raw output of a >> > CLI 'show' command. >> > >> > [ I think that long term we'd probably switch to a tuple anyway, when >> > we correctly handle things like a variable being 'auto' we'd >> > probably want a reply that looked something like: {value="auto", >> > current="i386"}, so switching to a tuple is probably the way to >> > go. ] >> > >> > For now however, the display of value is not fixed, so the 'auto' >> > value does not get displayed at all, but you do always get the message >> > string, the default reply now looks like this: >> > >> > (gdb) interpreter-exec mi "-gdb-show architecture" >> > ^done,{message="The target architecture is set automatically (currently i386)\n"} >> > >> > You would then have to parse the message string yourself. >> > >> > Hope this helps, >> > Andrew >> > >> > ---- >> > >> > diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c >> > index eb17158..f7597cf 100644 >> > --- a/gdb/cli/cli-setshow.c >> > +++ b/gdb/cli/cli-setshow.c >> > @@ -650,7 +650,23 @@ do_show_command (const char *arg, int from_tty, struct cmd_list_element *c) >> > MI and CLI specific versions. */ >> > >> > if (ui_out_is_mi_like_p (uiout)) >> > - ui_out_field_stream (uiout, "value", stb); >> > + { >> > + struct ui_file *msg_file; >> > + char *value; >> > + >> > + make_cleanup_ui_out_tuple_begin_end (uiout, NULL); >> > + ui_out_field_stream (uiout, "value", stb); >> > + >> > + value = ui_file_xstrdup (stb, NULL); >> > + make_cleanup (xfree, value); >> > + msg_file = mem_fileopen (); >> > + if (c->show_value_func != NULL) >> > + c->show_value_func (msg_file, from_tty, c, value); >> > + else >> > + deprecated_show_value_hack (msg_file, from_tty, c, value); >> > + >> > + ui_out_field_stream (uiout, "message", msg_file); >> > + } >> > else >> > { >> > char *value = ui_file_xstrdup (stb, NULL);