Mirror of the gdb mailing list
 help / color / mirror / Atom feed
* How to Read Program Architecture from GDB/MI?
@ 2016-07-25  5:31 Paramjot Oberoi
  2016-07-25 17:27 ` dwk
  0 siblings, 1 reply; 7+ messages in thread
From: Paramjot Oberoi @ 2016-07-25  5:31 UTC (permalink / raw)
  To: gdb

Hi all,

I am attempting to get the CPU architecture of the currently debugged
program via a GDB/MI command. I'm not sure what's I'm doing wrong:

Console interpreter looks good:
(gdb) show architecture
The target architecture is set automatically (currently i386)

MI interpreter succeeds without any data:
(gdb) interpreter-exec mi "-gdb-show architecture"
^done

If I set the arch to auto, then query with MI I get "auto" back...:
(gdb) set arch auto
The target architecture is set automatically (currently i386)
(gdb) interpreter-exec mi "-gdb-show architecture"
^done,value="auto"

Any advice on how I can query the architecture in GDB/MI mode? Thanks
in advance, I appreciate it.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: How to Read Program Architecture from GDB/MI?
  2016-07-25  5:31 How to Read Program Architecture from GDB/MI? Paramjot Oberoi
@ 2016-07-25 17:27 ` dwk
  2016-07-26  6:27   ` Paramjot Oberoi
  0 siblings, 1 reply; 7+ messages in thread
From: dwk @ 2016-07-25 17:27 UTC (permalink / raw)
  To: Paramjot Oberoi; +Cc: gdb

This is a bug in gdb/cli/cli-setshow.c. The current code reads

647   /* FIXME: cagney/2005-02-10: Need to split this in half: code to
648      convert the value into a string (esentially the above); and
649      code to print the value out.  For the latter there should be
650      MI and CLI specific versions.  */
651
652   if (ui_out_is_mi_like_p (uiout))
653     ui_out_field_stream (uiout, "value", stb);
654   else
655     {
656       char *value = ui_file_xstrdup (stb, NULL);
657
658       make_cleanup (xfree, value);
659       if (c->show_value_func != NULL)
660         c->show_value_func (gdb_stdout, from_tty, c, value);
661       else
662         deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
663     }

The if statement is a special case for GDB/MI. It seems to prepare to
generate a value but then never write anything. My guess is that this
code was supposed to generate a nicely parsable version of the string. In
the case of "-gdb-show architecture", there is a c->show_value_func
(show_architecture) to call which prints "unfriendly" output. I tried
playing with different code here but the simplest fix seems to be to
disable the if statement entirely (add 0&& to its test). Then your
"show architecture" command works correctly.

(gdb) interpreter-exec mi "-gdb-show architecture"
~"The target architecture is set automatically (currently i386)\n"
^done
(gdb)

I don't know what cagney was worrying about when this split occurred,
but given that it was in 2005 it may no longer be an issue. Perhaps
try disabling the if statement as I suggested and see if other problems
crop up.

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index eb17158..8d9b691 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -649,7 +649,7 @@ do_show_command (const char *arg, int from_tty,
struct cmd_list_element *c)
      code to print the value out.  For the latter there should be
      MI and CLI specific versions.  */

-  if (ui_out_is_mi_like_p (uiout))
+  if (0 && ui_out_is_mi_like_p (uiout))
     ui_out_field_stream (uiout, "value", stb);
   else
     {


Cheers.

~~ dwk

On Mon, Jul 25, 2016 at 1:30 AM, Paramjot Oberoi <paramjot@gmail.com> wrote:
> Hi all,
>
> I am attempting to get the CPU architecture of the currently debugged
> program via a GDB/MI command. I'm not sure what's I'm doing wrong:
>
> Console interpreter looks good:
> (gdb) show architecture
> The target architecture is set automatically (currently i386)
>
> MI interpreter succeeds without any data:
> (gdb) interpreter-exec mi "-gdb-show architecture"
> ^done
>
> If I set the arch to auto, then query with MI I get "auto" back...:
> (gdb) set arch auto
> The target architecture is set automatically (currently i386)
> (gdb) interpreter-exec mi "-gdb-show architecture"
> ^done,value="auto"
>
> Any advice on how I can query the architecture in GDB/MI mode? Thanks
> in advance, I appreciate it.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: How to Read Program Architecture from GDB/MI?
  2016-07-25 17:27 ` dwk
@ 2016-07-26  6:27   ` Paramjot Oberoi
  2016-07-26  8:09     ` Andrew Burgess
  0 siblings, 1 reply; 7+ messages in thread
From: Paramjot Oberoi @ 2016-07-26  6:27 UTC (permalink / raw)
  To: dwk; +Cc: gdb

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
{
...
...
}

Again this doesn't seem like a good way to do it. Please let me know
what your thoughts are. Thanks in advance.

On Mon, Jul 25, 2016 at 1:27 PM, dwk <dwks42@gmail.com> wrote:
> This is a bug in gdb/cli/cli-setshow.c. The current code reads
>
> 647   /* FIXME: cagney/2005-02-10: Need to split this in half: code to
> 648      convert the value into a string (esentially the above); and
> 649      code to print the value out.  For the latter there should be
> 650      MI and CLI specific versions.  */
> 651
> 652   if (ui_out_is_mi_like_p (uiout))
> 653     ui_out_field_stream (uiout, "value", stb);
> 654   else
> 655     {
> 656       char *value = ui_file_xstrdup (stb, NULL);
> 657
> 658       make_cleanup (xfree, value);
> 659       if (c->show_value_func != NULL)
> 660         c->show_value_func (gdb_stdout, from_tty, c, value);
> 661       else
> 662         deprecated_show_value_hack (gdb_stdout, from_tty, c, value);
> 663     }
>
> The if statement is a special case for GDB/MI. It seems to prepare to
> generate a value but then never write anything. My guess is that this
> code was supposed to generate a nicely parsable version of the string. In
> the case of "-gdb-show architecture", there is a c->show_value_func
> (show_architecture) to call which prints "unfriendly" output. I tried
> playing with different code here but the simplest fix seems to be to
> disable the if statement entirely (add 0&& to its test). Then your
> "show architecture" command works correctly.
>
> (gdb) interpreter-exec mi "-gdb-show architecture"
> ~"The target architecture is set automatically (currently i386)\n"
> ^done
> (gdb)
>
> I don't know what cagney was worrying about when this split occurred,
> but given that it was in 2005 it may no longer be an issue. Perhaps
> try disabling the if statement as I suggested and see if other problems
> crop up.
>
> diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
> index eb17158..8d9b691 100644
> --- a/gdb/cli/cli-setshow.c
> +++ b/gdb/cli/cli-setshow.c
> @@ -649,7 +649,7 @@ do_show_command (const char *arg, int from_tty,
> struct cmd_list_element *c)
>       code to print the value out.  For the latter there should be
>       MI and CLI specific versions.  */
>
> -  if (ui_out_is_mi_like_p (uiout))
> +  if (0 && ui_out_is_mi_like_p (uiout))
>      ui_out_field_stream (uiout, "value", stb);
>    else
>      {
>
>
> Cheers.
>
> ~~ dwk
>
> On Mon, Jul 25, 2016 at 1:30 AM, Paramjot Oberoi <paramjot@gmail.com> wrote:
>> Hi all,
>>
>> I am attempting to get the CPU architecture of the currently debugged
>> program via a GDB/MI command. I'm not sure what's I'm doing wrong:
>>
>> Console interpreter looks good:
>> (gdb) show architecture
>> The target architecture is set automatically (currently i386)
>>
>> MI interpreter succeeds without any data:
>> (gdb) interpreter-exec mi "-gdb-show architecture"
>> ^done
>>
>> If I set the arch to auto, then query with MI I get "auto" back...:
>> (gdb) set arch auto
>> The target architecture is set automatically (currently i386)
>> (gdb) interpreter-exec mi "-gdb-show architecture"
>> ^done,value="auto"
>>
>> Any advice on how I can query the architecture in GDB/MI mode? Thanks
>> in advance, I appreciate it.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: How to Read Program Architecture from GDB/MI?
  2016-07-26  6:27   ` Paramjot Oberoi
@ 2016-07-26  8:09     ` Andrew Burgess
  2016-07-26 18:36       ` Paramjot Oberoi
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2016-07-26  8:09 UTC (permalink / raw)
  To: Paramjot Oberoi; +Cc: dwk, gdb

* 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);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: How to Read Program Architecture from GDB/MI?
  2016-07-26  8:09     ` Andrew Burgess
@ 2016-07-26 18:36       ` Paramjot Oberoi
  2016-07-26 23:22         ` Andrew Burgess
  0 siblings, 1 reply; 7+ messages in thread
From: Paramjot Oberoi @ 2016-07-26 18:36 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: dwk, gdb

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.

-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);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: How to Read Program Architecture from GDB/MI?
  2016-07-26 18:36       ` Paramjot Oberoi
@ 2016-07-26 23:22         ` Andrew Burgess
  2016-07-28  5:32           ` Paramjot Oberoi
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2016-07-26 23:22 UTC (permalink / raw)
  To: Paramjot Oberoi; +Cc: dwk, gdb

* 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);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: How to Read Program Architecture from GDB/MI?
  2016-07-26 23:22         ` Andrew Burgess
@ 2016-07-28  5:32           ` Paramjot Oberoi
  0 siblings, 0 replies; 7+ messages in thread
From: Paramjot Oberoi @ 2016-07-28  5:32 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb

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);


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-07-28  5:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-25  5:31 How to Read Program Architecture from GDB/MI? Paramjot Oberoi
2016-07-25 17:27 ` dwk
2016-07-26  6:27   ` Paramjot Oberoi
2016-07-26  8:09     ` Andrew Burgess
2016-07-26 18:36       ` Paramjot Oberoi
2016-07-26 23:22         ` Andrew Burgess
2016-07-28  5:32           ` Paramjot Oberoi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox