Different output from -gdb-show than show

Different output from -gdb-show than show

[Marc Khouzam]

Hi,

while looking for a way know if a target supports reverse execution
I noticed this:

> gdb.7.2 -i mi testing/a.out
(gdb) start
(gdb) -gdb-set exec-direction reverse
^done
(gdb) -gdb-show exec-direction
^done,value="reverse"
(gdb) show exec-direction
&"show exec-direction\n"
~"Forward.\n"
^done

For some reason, -gdb-show is giving a different result than CLI show.
For my "does target support reverse" case, I will be forced to use 'show'.

Bug?

Thanks

Marc
 
 

Re: Different output from -gdb-show than show

[Pedro Alves]

On Tuesday 31 August 2010 16:02:39, Marc Khouzam wrote:
> Hi,
> 
> while looking for a way know if a target supports reverse execution
> I noticed this:
> 
> > gdb.7.2 -i mi testing/a.out
> (gdb) start
> (gdb) -gdb-set exec-direction reverse
> ^done
> (gdb) -gdb-show exec-direction
> ^done,value="reverse"
> (gdb) show exec-direction
> &"show exec-direction\n"
> ~"Forward.\n"
> ^done
> 
> For some reason, -gdb-show is giving a different result than CLI show.
> For my "does target support reverse" case, I will be forced to use 'show'.
> 
> Bug?

Yes.  Here in infrun.c:

> /* User interface for reverse debugging:
>    Set exec-direction / show exec-direction commands
>    (returns error unless target implements to_set_exec_direction method).  */
> 
> enum exec_direction_kind execution_direction = EXEC_FORWARD;
> static const char exec_forward[] = "forward";
> static const char exec_reverse[] = "reverse";
> static const char *exec_direction = exec_forward;
> static const char *exec_direction_names[] = {
>   exec_forward,
>   exec_reverse,
>   NULL
> };
> 
> 
> static void
> set_exec_direction_func (char *args, int from_tty,
> 			 struct cmd_list_element *cmd)
> {
>   if (target_can_execute_reverse)
>     {
>       if (!strcmp (exec_direction, exec_forward))
> 	execution_direction = EXEC_FORWARD;
>       else if (!strcmp (exec_direction, exec_reverse))
> 	execution_direction = EXEC_REVERSE;
>     }
> }
> 

The above does not complain if target_can_execute_reverse is false, contrary
to what the comment above says.
Leaves exec_direction and execution_direction out of sync in that case.
(your case, because you haven't started the process yet, you're debugging
the executable, which can't do reverse debugging)

> static void
> show_exec_direction_func (struct ui_file *out, int from_tty,
> 			  struct cmd_list_element *cmd, const char *value)
> {
>   switch (execution_direction) {
>   case EXEC_FORWARD:
>     fprintf_filtered (out, _("Forward.\n"));
>     break;
>   case EXEC_REVERSE:
>     fprintf_filtered (out, _("Reverse.\n"));
>     break;
>   case EXEC_ERROR:
>   default:
>     fprintf_filtered (out, 
> 		      _("Forward (target `%s' does not support exec-direction).\n"),
> 		      target_shortname);
>     break;
>   }
> }

"show exec-direction" prints based on execution_direction, while
"-gdb-show exec-direction" prints the raw exec_drection string:

>   add_setshow_enum_cmd ("exec-direction", class_run, exec_direction_names,
> 			&exec_direction, _("Set direction of execution.\n\
                                    ^^^^^^^^^^^^^^^
This is what is used by -gdb-show. --^


-- 
Pedro Alves

Re: Different output from -gdb-show than show

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]

Pedro Alves wrote:
> On Tuesday 31 August 2010 16:02:39, Marc Khouzam wrote:
>> Hi,
>>
>> while looking for a way know if a target supports reverse execution
>> I noticed this:
>>
>>> gdb.7.2 -i mi testing/a.out
>> (gdb) start
>> (gdb) -gdb-set exec-direction reverse
>> ^done
>> (gdb) -gdb-show exec-direction
>> ^done,value="reverse"
>> (gdb) show exec-direction
>> &"show exec-direction\n"
>> ~"Forward.\n"
>> ^done
>>
>> For some reason, -gdb-show is giving a different result than CLI show.
>> For my "does target support reverse" case, I will be forced to use 'show'.
>>
>> Bug?
> 
> Yes.  Here in infrun.c:
> 
>> /* User interface for reverse debugging:
>>    Set exec-direction / show exec-direction commands
>>    (returns error unless target implements to_set_exec_direction method).  */
>>
>> enum exec_direction_kind execution_direction = EXEC_FORWARD;
>> static const char exec_forward[] = "forward";
>> static const char exec_reverse[] = "reverse";
>> static const char *exec_direction = exec_forward;
>> static const char *exec_direction_names[] = {
>>   exec_forward,
>>   exec_reverse,
>>   NULL
>> };
>>
>>
>> static void
>> set_exec_direction_func (char *args, int from_tty,
>> 			 struct cmd_list_element *cmd)
>> {
>>   if (target_can_execute_reverse)
>>     {
>>       if (!strcmp (exec_direction, exec_forward))
>> 	execution_direction = EXEC_FORWARD;
>>       else if (!strcmp (exec_direction, exec_reverse))
>> 	execution_direction = EXEC_REVERSE;
>>     }
>> }
>>
> 
> The above does not complain if target_can_execute_reverse is false, contrary
> to what the comment above says.
> Leaves exec_direction and execution_direction out of sync in that case.
> (your case, because you haven't started the process yet, you're debugging
> the executable, which can't do reverse debugging)
> 
>> static void
>> show_exec_direction_func (struct ui_file *out, int from_tty,
>> 			  struct cmd_list_element *cmd, const char *value)
>> {
>>   switch (execution_direction) {
>>   case EXEC_FORWARD:
>>     fprintf_filtered (out, _("Forward.\n"));
>>     break;
>>   case EXEC_REVERSE:
>>     fprintf_filtered (out, _("Reverse.\n"));
>>     break;
>>   case EXEC_ERROR:
>>   default:
>>     fprintf_filtered (out, 
>> 		      _("Forward (target `%s' does not support exec-direction).\n"),
>> 		      target_shortname);
>>     break;
>>   }
>> }
> 
> "show exec-direction" prints based on execution_direction, while
> "-gdb-show exec-direction" prints the raw exec_drection string:
> 
>>   add_setshow_enum_cmd ("exec-direction", class_run, exec_direction_names,
>> 			&exec_direction, _("Set direction of execution.\n\
>                                     ^^^^^^^^^^^^^^^
> This is what is used by -gdb-show. --^
> 
> 


My bad.

Would this be suitable?



[-- Attachment #2: infrun.txt --]
[-- Type: text/plain, Size: 676 bytes --]

2010-08-31  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>

	* infrun.c (set_exec_direction_func): Error out if target does not
	support reverse execution.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.445
diff -u -p -r1.445 infrun.c
--- infrun.c	1 Jul 2010 15:36:15 -0000	1.445
+++ infrun.c	31 Aug 2010 18:32:36 -0000
@@ -6436,6 +6436,8 @@ set_exec_direction_func (char *args, int
       else if (!strcmp (exec_direction, exec_reverse))
 	execution_direction = EXEC_REVERSE;
     }
+  else
+    error (_("Target does not support this operation."));
 }
 
 static void

RE: Different output from -gdb-show than show

[Marc Khouzam]

 

> -----Original Message-----
> From: Michael Snyder [mailto:msnyder@vmware.com] 
> Sent: Tuesday, August 31, 2010 2:33 PM
> To: Pedro Alves
> Cc: gdb@sourceware.org; Marc Khouzam
> Subject: Re: Different output from -gdb-show than show
> 
> Pedro Alves wrote:
> > On Tuesday 31 August 2010 16:02:39, Marc Khouzam wrote:
> >> Hi,
> >>
> >> while looking for a way know if a target supports reverse execution
> >> I noticed this:
> >>
> >>> gdb.7.2 -i mi testing/a.out
> >> (gdb) start
> >> (gdb) -gdb-set exec-direction reverse
> >> ^done
> >> (gdb) -gdb-show exec-direction
> >> ^done,value="reverse"
> >> (gdb) show exec-direction
> >> &"show exec-direction\n"
> >> ~"Forward.\n"
> >> ^done
> >>
> >> For some reason, -gdb-show is giving a different result 
> than CLI show.
> >> For my "does target support reverse" case, I will be 
> forced to use 'show'.
> >>
> >> Bug?
> > 
> > Yes.  Here in infrun.c:
> > 
> >> /* User interface for reverse debugging:
> >>    Set exec-direction / show exec-direction commands
> >>    (returns error unless target implements 
> to_set_exec_direction method).  */
> >>
> >> enum exec_direction_kind execution_direction = EXEC_FORWARD;
> >> static const char exec_forward[] = "forward";
> >> static const char exec_reverse[] = "reverse";
> >> static const char *exec_direction = exec_forward;
> >> static const char *exec_direction_names[] = {
> >>   exec_forward,
> >>   exec_reverse,
> >>   NULL
> >> };
> >>
> >>
> >> static void
> >> set_exec_direction_func (char *args, int from_tty,
> >> 			 struct cmd_list_element *cmd)
> >> {
> >>   if (target_can_execute_reverse)
> >>     {
> >>       if (!strcmp (exec_direction, exec_forward))
> >> 	execution_direction = EXEC_FORWARD;
> >>       else if (!strcmp (exec_direction, exec_reverse))
> >> 	execution_direction = EXEC_REVERSE;
> >>     }
> >> }
> >>
> > 
> > The above does not complain if target_can_execute_reverse 
> is false, contrary
> > to what the comment above says.
> > Leaves exec_direction and execution_direction out of sync 
> in that case.
> > (your case, because you haven't started the process yet, 
> you're debugging
> > the executable, which can't do reverse debugging)
> > 
> >> static void
> >> show_exec_direction_func (struct ui_file *out, int from_tty,
> >> 			  struct cmd_list_element *cmd, const 
> char *value)
> >> {
> >>   switch (execution_direction) {
> >>   case EXEC_FORWARD:
> >>     fprintf_filtered (out, _("Forward.\n"));
> >>     break;
> >>   case EXEC_REVERSE:
> >>     fprintf_filtered (out, _("Reverse.\n"));
> >>     break;
> >>   case EXEC_ERROR:
> >>   default:
> >>     fprintf_filtered (out, 
> >> 		      _("Forward (target `%s' does not support 
> exec-direction).\n"),
> >> 		      target_shortname);
> >>     break;
> >>   }
> >> }
> > 
> > "show exec-direction" prints based on execution_direction, while
> > "-gdb-show exec-direction" prints the raw exec_drection string:
> > 
> >>   add_setshow_enum_cmd ("exec-direction", class_run, 
> exec_direction_names,
> >> 			&exec_direction, _("Set direction of 
> execution.\n\
> >                                     ^^^^^^^^^^^^^^^
> > This is what is used by -gdb-show. --^
> > 
> > 
> 
> 
> My bad.
> 
> Would this be suitable?
> 
> 2010-08-31  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>
> 
> 	* infrun.c (set_exec_direction_func): Error out if 
> target does not
> 	support reverse execution.
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.445
> diff -u -p -r1.445 infrun.c
> --- infrun.c	1 Jul 2010 15:36:15 -0000	1.445
> +++ infrun.c	31 Aug 2010 18:32:36 -0000
> @@ -6436,6 +6436,8 @@ set_exec_direction_func (char *args, int
>        else if (!strcmp (exec_direction, exec_reverse))
>  	execution_direction = EXEC_REVERSE;
>      }
> +  else

maybe also doing

exec_direction = exec_forward;

before the error? (I stole it from the non_stop case :-))

> +    error (_("Target does not support this operation."));
>  }
>  
>  static void

Re: Different output from -gdb-show than show

[Pedro Alves]

On Tuesday 31 August 2010 19:33:17, Michael Snyder wrote:

> Would this be suitable?

You also need to reset exec_direction back from execution_direction 
in set_exec_direction_func.  The set command callbacks are called _after_
their controlled variable has already been changed.  See
infrun.c:set_non_stop, for example.

-- 
Pedro Alves

Re: Different output from -gdb-show than show

[Michael Snyder]

Pedro Alves wrote:
> On Tuesday 31 August 2010 19:33:17, Michael Snyder wrote:
> 
>> Would this be suitable?
> 
> You also need to reset exec_direction back from execution_direction 
> in set_exec_direction_func.  The set command callbacks are called _after_
> their controlled variable has already been changed.  See
> infrun.c:set_non_stop, for example.
> 

Hmmm, but "exec_direction" doesn't actually seem to be used anywhere.

I did try this patch, and after failing to set "reverse", it
still shows "Forward".

What am I missing?

Re: Different output from -gdb-show than show

[Pedro Alves]

On Tuesday 31 August 2010 20:03:06, Michael Snyder wrote:
> Pedro Alves wrote:
> > On Tuesday 31 August 2010 19:33:17, Michael Snyder wrote:
> > 
> >> Would this be suitable?
> > 
> > You also need to reset exec_direction back from execution_direction 
> > in set_exec_direction_func.  The set command callbacks are called _after_
> > their controlled variable has already been changed.  See
> > infrun.c:set_non_stop, for example.
> > 
> 
> Hmmm, but "exec_direction" doesn't actually seem to be used anywhere.
> 
> I did try this patch, and after failing to set "reverse", it
> still shows "Forward".
> 
> What am I missing?

That "-gdb-show exec-direction" reads from exec_direction.  See
Marc's original bug report upthread.  :-)

-- 
Pedro Alves

Re: Different output from -gdb-show than show

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

Pedro Alves wrote:
> On Tuesday 31 August 2010 20:03:06, Michael Snyder wrote:
>> Pedro Alves wrote:
>>> On Tuesday 31 August 2010 19:33:17, Michael Snyder wrote:
>>>
>>>> Would this be suitable?
>>> You also need to reset exec_direction back from execution_direction 
>>> in set_exec_direction_func.  The set command callbacks are called _after_
>>> their controlled variable has already been changed.  See
>>> infrun.c:set_non_stop, for example.
>>>
>> Hmmm, but "exec_direction" doesn't actually seem to be used anywhere.
>>
>> I did try this patch, and after failing to set "reverse", it
>> still shows "Forward".
>>
>> What am I missing?
> 
> That "-gdb-show exec-direction" reads from exec_direction.  See
> Marc's original bug report upthread.  :-)
> 

I don't see how, since it's a static variable and I've looked at
every local reference to it.

But anyway, here's a new patch:



[-- Attachment #2: infrun.txt --]
[-- Type: text/plain, Size: 731 bytes --]

2010-08-31  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>

	* infrun.c (set_exec_direction_func): Error out if target does not
	support reverse execution.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.445
diff -u -p -r1.445 infrun.c
--- infrun.c	1 Jul 2010 15:36:15 -0000	1.445
+++ infrun.c	31 Aug 2010 19:09:07 -0000
@@ -6436,6 +6436,11 @@ set_exec_direction_func (char *args, int
       else if (!strcmp (exec_direction, exec_reverse))
 	execution_direction = EXEC_REVERSE;
     }
+  else
+    {
+      exec_direction = exec_forward;
+      error (_("Target does not support this operation."));
+    }
 }
 
 static void

Re: Different output from -gdb-show than show

[Pedro Alves]

On Tuesday 31 August 2010 20:10:15, Michael Snyder wrote:
> >> I did try this patch, and after failing to set "reverse", it
> >> still shows "Forward".
> >>
> >> What am I missing?
> > 
> > That "-gdb-show exec-direction" reads from exec_direction.  See
> > Marc's original bug report upthread.  :-)
> > 
> 
> I don't see how, since it's a static variable and I've looked at
> every local reference to it.

A pointer to it is passed to add_setshow_enum_cmd:

  add_setshow_enum_cmd ("exec-direction", class_run, exec_direction_names,
			&exec_direction, _("Set direction of execution.\n\
Options are 'forward' or 'reverse'."),
			_("Show direction of execution (forward/reverse)."),
			_("Tells gdb whether to execute forward or backward."),
			set_exec_direction_func, show_exec_direction_func,
			&setlist, &showlist);

"-gdb-show exec-direction" does not call the set callback, it prints the command's
control enum value directly.

 (gdb) show exec-direction 
 Forward.
 (gdb) interpreter-exec mi "-gdb-show exec-direction"
 ^done,value="forward"
  (gdb) 

Note "Forward." vs "forward".

>   2010-08-31  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>
> 
>         * infrun.c (set_exec_direction_func): Error out if target does not
>         support reverse execution.

Okay.

-- 
Pedro Alves

Re: Different output from -gdb-show than show

[Michael Snyder]

Pedro Alves wrote:

> Note "Forward." vs "forward".
> 
>>   2010-08-31  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>
>>
>>         * infrun.c (set_exec_direction_func): Error out if target does not
>>         support reverse execution.
> 
> Okay.

Thanks, applied to trunk and branch.

Re: Different output from -gdb-show than show

[Tom Tromey]

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Michael> I don't see how, since it's a static variable and I've looked at
Michael> every local reference to it.

The command structure has a pointer to the variable.
do_setshow_command reads this directly, and in the MI case does not call
the show_value_func.

Tom

RE: Different output from -gdb-show than show

[Marc Khouzam]

> -----Original Message-----
> From: gdb-owner@sourceware.org 
> [mailto:gdb-owner@sourceware.org] On Behalf Of Michael Snyder
> Sent: Tuesday, August 31, 2010 3:33 PM
> To: Pedro Alves
> Cc: gdb@sourceware.org; Marc Khouzam
> Subject: Re: Different output from -gdb-show than show
> 
> Pedro Alves wrote:
> 
> > Note "Forward." vs "forward".
> > 
> >>   2010-08-31  Michael Snyder  
> <msnyder@msnyder-server.eng.vmware.com>
> >>
> >>         * infrun.c (set_exec_direction_func): Error out if 
> target does not
> >>         support reverse execution.
> > 
> > Okay.
> 
> Thanks, applied to trunk and branch.

Thanks guys!

Here is what I'm planning to do to see if a target supports reverse
execution:

-gdb-set exec-direction reverse
show exec-direction
-gdb-set exec-direction forward

If the 'show' command shows "Reverse", I'll know that the target
supports reverse execution.  (I have to do it like this because 
I also want it to work for 7.0 and 7.1)

That is the only thing I found that gave me that info without
side-effects.  Makes sense?

Marc

Re: Different output from -gdb-show than show

[Pedro Alves]

On Tuesday 31 August 2010 20:39:51, Marc Khouzam wrote:
> > -----Original Message-----
> > From: gdb-owner@sourceware.org 
> > [mailto:gdb-owner@sourceware.org] On Behalf Of Michael Snyder
> > Sent: Tuesday, August 31, 2010 3:33 PM
> > To: Pedro Alves
> > Cc: gdb@sourceware.org; Marc Khouzam
> > Subject: Re: Different output from -gdb-show than show
> > 
> > Pedro Alves wrote:
> > 
> > > Note "Forward." vs "forward".
> > > 
> > >>   2010-08-31  Michael Snyder  
> > <msnyder@msnyder-server.eng.vmware.com>
> > >>
> > >>         * infrun.c (set_exec_direction_func): Error out if 
> > target does not
> > >>         support reverse execution.
> > > 
> > > Okay.
> > 
> > Thanks, applied to trunk and branch.
> 
> Thanks guys!
> 
> Here is what I'm planning to do to see if a target supports reverse
> execution:
> 
> -gdb-set exec-direction reverse
> show exec-direction
> -gdb-set exec-direction forward
> 
> If the 'show' command shows "Reverse", I'll know that the target
> supports reverse execution.  (I have to do it like this because 
> I also want it to work for 7.0 and 7.1)
> 
> That is the only thing I found that gave me that info without
> side-effects.  Makes sense?

Yes.  You may want to issue an "-gdb-set exec-direction forward"
upfront to clean up any stale state from a previous run/target
as well...  Also, it may wreck havoc in non-stop if the target is
already executing...  I think listing support for reverse
in "-list-target-features" would be nice?

-- 
Pedro Alves

RE: Different output from -gdb-show than show

[Marc Khouzam]

> -----Original Message-----
> From: Pedro Alves [mailto:pedro@codesourcery.com] 
> Sent: Tuesday, August 31, 2010 4:03 PM
> To: Marc Khouzam
> Cc: 'Michael Snyder'; 'gdb@sourceware.org'
> Subject: Re: Different output from -gdb-show than show
> 

> > 
> > Here is what I'm planning to do to see if a target supports reverse
> > execution:
> > 
> > -gdb-set exec-direction reverse
> > show exec-direction
> > -gdb-set exec-direction forward
> > 
> > If the 'show' command shows "Reverse", I'll know that the target
> > supports reverse execution.  (I have to do it like this because 
> > I also want it to work for 7.0 and 7.1)
> > 
> > That is the only thing I found that gave me that info without
> > side-effects.  Makes sense?
> 
> Yes.  You may want to issue an "-gdb-set exec-direction forward"
> upfront to clean up any stale state from a previous run/target
> as well... 

Should be new sessions every time with Eclipse (at least for now).

> Also, it may wreck havoc in non-stop if the target is
> already executing...  

You are right.  Thanks for pointing it out.
Nothing I can do in that case then?

> I think listing support for reverse
> in "-list-target-features" would be nice?

Definitely sounds like there should be some way to get that info.

Re: Different output from -gdb-show than show

[Pedro Alves]

On Tuesday 31 August 2010 21:10:41, Marc Khouzam wrote:
> > Yes.  You may want to issue an "-gdb-set exec-direction forward"
> > upfront to clean up any stale state from a previous run/target
> > as well... 
> 
> Should be new sessions every time with Eclipse (at least for now).
> 
> > Also, it may wreck havoc in non-stop if the target is
> > already executing...  
> 
> You are right.  Thanks for pointing it out.
> Nothing I can do in that case then?

I can't think of anything, sorry.

> > I think listing support for reverse
> > in "-list-target-features" would be nice?
> 
> Definitely sounds like there should be some way to get that info.

Want to post a patch? :-) It should be a two liner addition to
mi-main.c:mi_cmd_list_target_features, plus a similar sized mention
in the docs.

-- 
Pedro Alves

RE: Different output from -gdb-show than show

[Marc Khouzam]

 

> -----Original Message-----
> From: Pedro Alves [mailto:pedro@codesourcery.com] 
> Sent: Tuesday, August 31, 2010 4:28 PM
> To: Marc Khouzam
> Cc: 'Michael Snyder'; 'gdb@sourceware.org'
> Subject: Re: Different output from -gdb-show than show
> 
> On Tuesday 31 August 2010 21:10:41, Marc Khouzam wrote:
> > > Yes.  You may want to issue an "-gdb-set exec-direction forward"
> > > upfront to clean up any stale state from a previous run/target
> > > as well... 
> > 
> > Should be new sessions every time with Eclipse (at least for now).
> > 
> > > Also, it may wreck havoc in non-stop if the target is
> > > already executing...  
> > 
> > You are right.  Thanks for pointing it out.
> > Nothing I can do in that case then?
> 
> I can't think of anything, sorry.
> 
> > > I think listing support for reverse
> > > in "-list-target-features" would be nice?
> > 
> > Definitely sounds like there should be some way to get that info.
> 
> Want to post a patch? :-) It should be a two liner addition to
> mi-main.c:mi_cmd_list_target_features, plus a similar sized mention
> in the docs.

I'll give it a go and post it separately on gdb-patches.

Thanks for the pointer

Marc