Re: How to Read Program Architecture from GDB/MI?

[Paramjot Oberoi <paramjot at gmail dot com>]

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
<andrew.burgess@embecosm.com> wrote:
> * Paramjot Oberoi <paramjot@gmail.com> [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
>> <andrew.burgess@embecosm.com> wrote:
>> > * Paramjot Oberoi <paramjot@gmail.com> [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);