diff mbox

Update help of the "frame" command

Message ID 20170106151610.28872-1-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Jan. 6, 2017, 3:16 p.m. UTC 
  The help message of the "frame" command states that nothing is printed
if the command is executed from the command file or user-defined
command.  My testing leads me to think that this is not true (at least
today).

  (gdb) bt
  #0  bar (n=17) at test.c:9
  #1  0x00000000004006e0 in foo (v=17) at test.c:13
  #2  0x00000000004006f0 in main () at test.c:21
  (gdb) frame
  #0  bar (n=17) at test.c:9
  9	    baz(n);
  (gdb) define foo
  Type commands for definition of "foo".
  End with a line saying just "end".
  >frame 1
  >end
  (gdb) foo
  #1  0x00000000004006e0 in foo (v=17) at test.c:13
  13	    bar(v);

This patch simply removes that bit from the help message.  I didn't find
anything corresponding to this in the documentation that needs to be
fixed.

gdb/ChangeLog:

	* stack.c (_initialize_stack): Update "frame" command help message.
---
 gdb/stack.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Luis Machado Jan. 6, 2017, 4:25 p.m. UTC | #1 
On 01/06/2017 09:16 AM, Simon Marchi wrote:
> The help message of the "frame" command states that nothing is printed
> if the command is executed from the command file or user-defined
> command.  My testing leads me to think that this is not true (at least
> today).
>
>   (gdb) bt
>   #0  bar (n=17) at test.c:9
>   #1  0x00000000004006e0 in foo (v=17) at test.c:13
>   #2  0x00000000004006f0 in main () at test.c:21
>   (gdb) frame
>   #0  bar (n=17) at test.c:9
>   9	    baz(n);
>   (gdb) define foo
>   Type commands for definition of "foo".
>   End with a line saying just "end".
>   >frame 1
>   >end
>   (gdb) foo
>   #1  0x00000000004006e0 in foo (v=17) at test.c:13
>   13	    bar(v);
>
> This patch simply removes that bit from the help message.  I didn't find
> anything corresponding to this in the documentation that needs to be
> fixed.
>
> gdb/ChangeLog:
>
> 	* stack.c (_initialize_stack): Update "frame" command help message.
> ---
>  gdb/stack.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 64224d0aad..201a358833 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -2666,9 +2666,7 @@ This is useful in command scripts."));
>  Select and print a stack frame.\nWith no argument, \
>  print the selected stack frame.  (See also \"info frame\").\n\
>  An argument specifies the frame to select.\n\
> -It can be a stack frame number or the address of the frame.\n\
> -With argument, nothing is printed if input is coming from\n\
> -a command file or a user-defined command."));
> +It can be a stack frame number or the address of the frame.\n"));
>
>    add_com_alias ("f", "frame", class_stack, 1);
>
>

Sounds reasonable to me. I ran into other help text block 
inconsistencies. It looks like there is potential for more corrections 
in that area.
Simon Marchi Jan. 6, 2017, 4:28 p.m. UTC | #2 
On 2017-01-06 11:25, Luis Machado wrote:
> Sounds reasonable to me. I ran into other help text block
> inconsistencies. It looks like there is potential for more corrections
> in that area.

If you want to point them out, I can take a look and try to make 
patches.
Luis Machado Jan. 6, 2017, 4:29 p.m. UTC | #3 
On 01/06/2017 10:28 AM, Simon Marchi wrote:
> On 2017-01-06 11:25, Luis Machado wrote:
>> Sounds reasonable to me. I ran into other help text block
>> inconsistencies. It looks like there is potential for more corrections
>> in that area.
>
> If you want to point them out, I can take a look and try to make patches.

I'm about to send an improvement to the "load" command, but basically i 
noticed we are lacking proper usage information in some commands. I 
don't have a list off the top of my head though.
Yao Qi Jan. 9, 2017, 5:27 p.m. UTC | #4 
On 17-01-06 10:16:10, Simon Marchi wrote:
> @@ -2666,9 +2666,7 @@ This is useful in command scripts."));
>  Select and print a stack frame.\nWith no argument, \
>  print the selected stack frame.  (See also \"info frame\").\n\
>  An argument specifies the frame to select.\n\
> -It can be a stack frame number or the address of the frame.\n\
> -With argument, nothing is printed if input is coming from\n\
> -a command file or a user-defined command."));
> +It can be a stack frame number or the address of the frame.\n"));
>  

These two lines were added many years ago,

c906108c (Stan Shebs       1999-04-16 01:35:26 +0000 2632) It can be a stack frame number or the address of the frame.\n\
c906108c (Stan Shebs       1999-04-16 01:35:26 +0000 2633) With argument, nothing is printed if input is coming from\n\

Did you do some archaeology to see how GDB did at that time? and then
we may know these help doc is a leftover of some changes.  If we can
find them, we are confident to remove these doc.  I spent 30 minutes
on c906108c, but didn't find any evidence.
Simon Marchi Jan. 9, 2017, 6:13 p.m. UTC | #5 
On 2017-01-09 12:27, Yao Qi wrote:
> These two lines were added many years ago,
> 
> c906108c (Stan Shebs       1999-04-16 01:35:26 +0000 2632) It can be a
> stack frame number or the address of the frame.\n\
> c906108c (Stan Shebs       1999-04-16 01:35:26 +0000 2633) With
> argument, nothing is printed if input is coming from\n\
> 
> Did you do some archaeology to see how GDB did at that time? and then
> we may know these help doc is a leftover of some changes.  If we can
> find them, we are confident to remove these doc.  I spent 30 minutes
> on c906108c, but didn't find any evidence.

Indeed, it's the commit "Initial creation of sourceware repository".  I 
checked out that commit and looked at the code, but couldn't find 
anything that would suggest that the output of the frame command would 
not be printed when it's executing in a script or user command.

I went earlier using the old tarballs on the website [1], and found that 
in old gdb's, there was code like this:

  965   if (!from_tty)
  966     return;
  967
  968   print_stack_frame (selected_frame, selected_frame_level, 1);

The (!from_tty) check disappeared in gdb 4.3.  I think it's this change:

  873 Thu Oct 24 09:33:44 1991  John Gilmore  (gnu at cygnus.com)
  874
  875         * stack.c (frame_command):  Always print.  Use new
  876         frame_select_command to select a frame without printing.

after that, the frame_command function becomes simply:

  974 static void
  975 frame_command (level_exp, from_tty)
  976      char *level_exp;
  977      int from_tty;
  978 {
  979   select_frame_command (level_exp, from_tty);
  980   print_stack_frame (selected_frame, selected_frame_level, 1);
  981 }

So I think it's safe.

Side-question, is there a git repo somewhere with all these old gdb 
versions, those that predate what's in the current git tree?  It would 
be useful to have a repo with one commit per version.  Here I had to 
download many tarballs and bisect manually, but if they had been in a 
repo it would have been trivial.  If it doesn't exist yet, I think I'll 
do it.

[1] ftp://sourceware.org/pub/gdb/old-releases/
Yao Qi Jan. 10, 2017, 11 a.m. UTC | #6 
On 17-01-09 13:13:42, Simon Marchi wrote:
> Indeed, it's the commit "Initial creation of sourceware repository".
> I checked out that commit and looked at the code, but couldn't find
> anything that would suggest that the output of the frame command
> would not be printed when it's executing in a script or user
> command.
> 
> I went earlier using the old tarballs on the website [1], and found
> that in old gdb's, there was code like this:
> 
>  965   if (!from_tty)
>  966     return;
>  967
>  968   print_stack_frame (selected_frame, selected_frame_level, 1);
> 
> The (!from_tty) check disappeared in gdb 4.3.  I think it's this change:
> 
>  873 Thu Oct 24 09:33:44 1991  John Gilmore  (gnu at cygnus.com)
>  874
>  875         * stack.c (frame_command):  Always print.  Use new
>  876         frame_select_command to select a frame without printing.
> 
> after that, the frame_command function becomes simply:
> 
>  974 static void
>  975 frame_command (level_exp, from_tty)
>  976      char *level_exp;
>  977      int from_tty;
>  978 {
>  979   select_frame_command (level_exp, from_tty);
>  980   print_stack_frame (selected_frame, selected_frame_level, 1);
>  981 }
> 
> So I think it's safe.

Thanks for digging it up.  Patch is OK.

> 
> Side-question, is there a git repo somewhere with all these old gdb
> versions, those that predate what's in the current git tree?  It
> would be useful to have a repo with one commit per version.  Here I
> had to download many tarballs and bisect manually, but if they had
> been in a repo it would have been trivial.  If it doesn't exist yet,
> I think I'll do it.
> 
> [1] ftp://sourceware.org/pub/gdb/old-releases/

They were imported into the git repository.  The change you found
above is in b00771232fab861fb31e42dfd5f6643ba1b43cc9
Simon Marchi Jan. 10, 2017, 3:19 p.m. UTC | #7 
On 17-01-10 06:00 AM, Yao Qi wrote:
> On 17-01-09 13:13:42, Simon Marchi wrote:
>> Indeed, it's the commit "Initial creation of sourceware repository".
>> I checked out that commit and looked at the code, but couldn't find
>> anything that would suggest that the output of the frame command
>> would not be printed when it's executing in a script or user
>> command.
>>
>> I went earlier using the old tarballs on the website [1], and found
>> that in old gdb's, there was code like this:
>>
>>  965   if (!from_tty)
>>  966     return;
>>  967
>>  968   print_stack_frame (selected_frame, selected_frame_level, 1);
>>
>> The (!from_tty) check disappeared in gdb 4.3.  I think it's this change:
>>
>>  873 Thu Oct 24 09:33:44 1991  John Gilmore  (gnu at cygnus.com)
>>  874
>>  875         * stack.c (frame_command):  Always print.  Use new
>>  876         frame_select_command to select a frame without printing.
>>
>> after that, the frame_command function becomes simply:
>>
>>  974 static void
>>  975 frame_command (level_exp, from_tty)
>>  976      char *level_exp;
>>  977      int from_tty;
>>  978 {
>>  979   select_frame_command (level_exp, from_tty);
>>  980   print_stack_frame (selected_frame, selected_frame_level, 1);
>>  981 }
>>
>> So I think it's safe.
> 
> Thanks for digging it up.  Patch is OK.
> 
>>
>> Side-question, is there a git repo somewhere with all these old gdb
>> versions, those that predate what's in the current git tree?  It
>> would be useful to have a repo with one commit per version.  Here I
>> had to download many tarballs and bisect manually, but if they had
>> been in a repo it would have been trivial.  If it doesn't exist yet,
>> I think I'll do it.
>>
>> [1] ftp://sourceware.org/pub/gdb/old-releases/
> 
> They were imported into the git repository.  The change you found
> above is in b00771232fab861fb31e42dfd5f6643ba1b43cc9
> 

Ah ok, it seems like there is a disconnect at "Initial creation of sourceware repository",
my blame tool doesn't allow me to go past that.  But indeed, if I manually checkout a
previous commit I see the older versions.

I added a reference to that commit in my commit log and pushed the patch, thanks.
Pedro Alves Jan. 10, 2017, 4 p.m. UTC | #8 
On 01/10/2017 11:00 AM, Yao Qi wrote:
> On 17-01-09 13:13:42, Simon Marchi wrote:

>> Side-question, is there a git repo somewhere with all these old gdb
>> versions, those that predate what's in the current git tree?  It
>> would be useful to have a repo with one commit per version.  Here I
>> had to download many tarballs and bisect manually, but if they had
>> been in a repo it would have been trivial.  If it doesn't exist yet,
>> I think I'll do it.
>>
>> [1] ftp://sourceware.org/pub/gdb/old-releases/
> 
> They were imported into the git repository.  The change you found
> above is in b00771232fab861fb31e42dfd5f6643ba1b43cc9
> 

Yes, they were imported from here:

 https://github.com/palves/gdb-old-releases

The git history around the initial CVS import is quite
annoying, since there were a series of commits that added
the whole of gdb, then removed it, then re-added it, etc.
I think there are ways in git nowadays to tell (local) git to
see through a range of commits, so effectively
"virtually squash" those deletions as if they never existed,
but I never investigated exactly how to do it.  If someone pulls
that off, I think it'd be great.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/stack.c b/gdb/stack.c
index 64224d0aad..201a358833 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2666,9 +2666,7 @@  This is useful in command scripts."));
 Select and print a stack frame.\nWith no argument, \
 print the selected stack frame.  (See also \"info frame\").\n\
 An argument specifies the frame to select.\n\
-It can be a stack frame number or the address of the frame.\n\
-With argument, nothing is printed if input is coming from\n\
-a command file or a user-defined command."));
+It can be a stack frame number or the address of the frame.\n"));
 
   add_com_alias ("f", "frame", class_stack, 1);