* [PATCH v4 0/4] Small changes to "list" command @ 2023-07-13 10:24 Bruno Larsen 2023-07-13 10:24 ` [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line Bruno Larsen ` (4 more replies) 0 siblings, 5 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-13 10:24 UTC (permalink / raw) To: gdb-patches; +Cc: Bruno Larsen I decided to tackle PR cli/30497, and while doing so, Andrew mentioned that it would also be nice if we could explicitly ask GDB to print the current location, so I also decided to add that into a series. The first patch is just some groundwork preparation to make the rest smooth. On the second pass, I realized that 'list +' isn't properly documented, so I added it to the docs as well. After last round of reviews, I changed my approach to fixing cli/30497 to only have a more obvious error message to the end user instead of jumping back to the current location. Changes from v3: * Reordered patches - now '.' comes first so the UX change can reference it * completely rewrote approach to patch 3 * made "list ." no longer throw an error when inferior isn't started Changes from v2: * Addressed Eli's comments * Added Eli reviews. Changes from v1: * added new arguments to the docs * Added patch 4, completely new. * Fixed wording, per Eli's suggestions. Bruno Larsen (4): gdb/cli: Factor out code to list lines around a given line gdb/cli: add '.' as an argument for 'list' command gdb/cli: Improve UX when using list with no args gdb/doc: document '+' argument for 'list' command gdb/NEWS | 9 ++++ gdb/cli/cli-cmds.c | 85 +++++++++++++++++++++++++-------- gdb/doc/gdb.texinfo | 12 ++++- gdb/source.c | 16 +++++++ gdb/source.h | 7 +++ gdb/testsuite/gdb.base/list.exp | 47 ++++++++++++++++-- gdb/testsuite/gdb.base/list1.c | 2 +- 7 files changed, 153 insertions(+), 25 deletions(-) -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line 2023-07-13 10:24 [PATCH v4 0/4] Small changes to "list" command Bruno Larsen @ 2023-07-13 10:24 ` Bruno Larsen 2023-07-13 16:53 ` Tom Tromey 2023-07-13 10:24 ` [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Bruno Larsen ` (3 subsequent siblings) 4 siblings, 1 reply; 37+ messages in thread From: Bruno Larsen @ 2023-07-13 10:24 UTC (permalink / raw) To: gdb-patches; +Cc: Bruno Larsen A future patch will add more situations that calculates "lines around a certain point" to be printed using print_source_lines, and the logic could be re-used. As a preparation for those commits, this one factors out that part of the logic of the list command into its own function. No functional changes are expected --- gdb/cli/cli-cmds.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 638c138e7cb..00977bc2ee3 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1200,6 +1200,28 @@ pipe_command_completer (struct cmd_list_element *ignore, we don't know how to complete. */ } +/* Helper for the list_command function. Prints the lines around (and + including) line stored in CURSAL. ARG contains the arguments used in + the command invocation, and is used to determine a special case when + printing backwards. */ +static void +list_around_line (const char *arg, symtab_and_line cursal) +{ + int first; + + first = std::max (cursal.line - get_lines_to_list () / 2, 1); + + /* A small special case --- if listing backwards, and we + should list only one line, list the preceding line, + instead of the exact line we've just shown after e.g., + stopping for a breakpoint. */ + if (arg != NULL && arg[0] == '-' + && get_lines_to_list () == 1 && first > 1) + first -= 1; + + print_source_lines (cursal.symtab, source_lines_range (first), 0); +} + static void list_command (const char *arg, int from_tty) { @@ -1221,19 +1243,7 @@ list_command (const char *arg, int from_tty) source line, center the listing around that line. */ if (get_first_line_listed () == 0) { - int first; - - first = std::max (cursal.line - get_lines_to_list () / 2, 1); - - /* A small special case --- if listing backwards, and we - should list only one line, list the preceding line, - instead of the exact line we've just shown after e.g., - stopping for a breakpoint. */ - if (arg != NULL && arg[0] == '-' - && get_lines_to_list () == 1 && first > 1) - first -= 1; - - print_source_lines (cursal.symtab, source_lines_range (first), 0); + list_around_line (arg, cursal); } /* "l" or "l +" lists next ten lines. */ -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line 2023-07-13 10:24 ` [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line Bruno Larsen @ 2023-07-13 16:53 ` Tom Tromey 0 siblings, 0 replies; 37+ messages in thread From: Tom Tromey @ 2023-07-13 16:53 UTC (permalink / raw) To: Bruno Larsen via Gdb-patches; +Cc: Bruno Larsen >>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: Bruno> A future patch will add more situations that calculates "lines around a Bruno> certain point" to be printed using print_source_lines, and the logic Bruno> could be re-used. As a preparation for those commits, this one factors Bruno> out that part of the logic of the list command into its own function. Bruno> No functional changes are expected Thank you. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-13 10:24 [PATCH v4 0/4] Small changes to "list" command Bruno Larsen 2023-07-13 10:24 ` [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line Bruno Larsen @ 2023-07-13 10:24 ` Bruno Larsen 2023-07-13 11:05 ` Eli Zaretskii ` (2 more replies) 2023-07-13 10:24 ` [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args Bruno Larsen ` (2 subsequent siblings) 4 siblings, 3 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-13 10:24 UTC (permalink / raw) To: gdb-patches; +Cc: Bruno Larsen Currently, after the user has used the list command once, there is no self-contained way to ask GDB to print the location where the inferior is stopped. The current best options require either using a separate command to scope out where the inferior is stopped, or using "list *$pc" requiring knowledge of GDB standard registers. This commit adds a way to do that using '.' as a new argument for the 'list' command. If the inferior isn't running, the command prints around the main function. Because this necessitated having the inferior running and the test was (seemingly unnecessarily) using printf in a non-essential way and it would make the resulting log harder to read for no benefit, it was replaced by a differen t statement. --- gdb/NEWS | 4 ++++ gdb/cli/cli-cmds.c | 31 ++++++++++++++++++++++++-- gdb/doc/gdb.texinfo | 5 +++++ gdb/testsuite/gdb.base/list.exp | 39 +++++++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/list1.c | 2 +- 5 files changed, 78 insertions(+), 3 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index b924834d3d7..eef440a5242 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -84,6 +84,10 @@ is 64k. To print longer strings you should increase 'max-value-size'. +* The 'list' command now accepts '.' as an argument, which tells GDB to + print the location where the inferior is stopped. If the inferior hasn't + started yet, the command will print around the main function. + * New commands maintenance print record-instruction [ N ] diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 00977bc2ee3..1c459afdc97 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1234,14 +1234,14 @@ list_command (const char *arg, int from_tty) const char *p; /* Pull in the current default source line if necessary. */ - if (arg == NULL || ((arg[0] == '+' || arg[0] == '-') && arg[1] == '\0')) + if (arg == NULL || ((arg[0] == '+' || arg[0] == '-' || arg[0] == '.') && arg[1] == '\0')) { set_default_source_symtab_and_line (); symtab_and_line cursal = get_current_source_symtab_and_line (); /* If this is the first "list" since we've set the current source line, center the listing around that line. */ - if (get_first_line_listed () == 0) + if (get_first_line_listed () == 0 && (arg == nullptr || arg[0] != '.')) { list_around_line (arg, cursal); } @@ -1263,6 +1263,32 @@ list_command (const char *arg, int from_tty) print_source_lines (cursal.symtab, range, 0); } + /* "l ." lists the default location again. */ + else if (arg[0] == '.') + { + try + { + /* Find the current line by getting the PC of the currently + selected frame, and finding the line associated to it. */ + frame_info_ptr frame = get_selected_frame (nullptr); + CORE_ADDR curr_pc = get_frame_pc (frame); + cursal = find_pc_line (curr_pc, 0); + } + catch (const gdb_exception &e) + { + /* If there was an exception above, it means the inferior + is not running, so reset the current source location to + the default. */ + clear_current_source_symtab_and_line (); + set_default_source_symtab_and_line (); + cursal = get_current_source_symtab_and_line (); + } + list_around_line (arg, cursal); + /* Advance argument so just pressing "enter" after using "list ." + will print the following lines instead of the same lines again. */ + arg++; + } + return; } @@ -2770,6 +2796,7 @@ and send its output to SHELL_COMMAND.")); = add_com ("list", class_files, list_command, _("\ List specified function or line.\n\ With no argument, lists ten more lines after or around previous listing.\n\ +\"list .\" lists ten lines arond where the inferior is stopped.\n\ \"list -\" lists the ten lines before a previous ten-line listing.\n\ One argument specifies a line, and ten lines are listed around that line.\n\ Two arguments with comma between specify starting and ending lines to list.\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index b10c06ae91f..7619efe3de9 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9148,6 +9148,11 @@ Stack}), this prints lines centered around that line. @item list - Print lines just before the lines last printed. + +@item list . +Print the lines surrounding the location that is where the inferior +is stopped. If the inferior is not running, print around the main +function instead. @end table @cindex @code{list}, how many lines to display diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index 18ecd13ac0f..ed178a1dd95 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/gdb/testsuite/gdb.base/list.exp @@ -400,6 +400,42 @@ proc test_list_invalid_args {} { "second use of \"list +INVALID\"" } +proc test_list_current_location {} { + global binfile + # If the first "list" command that GDB runs is "list ." GDB may be + # unable to recognize that the inferior isn't running, so we should + # reload the inferior to test that condition. + clean_restart + gdb_file_cmd ${binfile} + + # Ensure that we are printing 10 lines + if {![set_listsize 10]} { + return + } + + # First guarantee that GDB prints around the main function correctly + gdb_test "list ." \ + "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ + "list . with inferior not running" + + if {![runto_main]} { + warning "couldn't start inferior" + return + } + + # Walk forward some lines + gdb_test "until 15" ".*15.*foo.*" + + # Test that the correct location is printed and that + # using just "list" will print the following lines. + gdb_test "list ." ".*" "list current line after starting" + gdb_test "list" ".*" "confirm we are printing the following lines" + + # Test that list . will reset to current location + gdb_test "list ." ".*" "list around current line again" + gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" +} + clean_restart gdb_file_cmd ${binfile} @@ -519,4 +555,7 @@ test_list "list -" 10 2 "7-8" "5-6" # the current line. test_list "list -" 10 1 "7" "6" +# Test printing the location where the inferior is stopped. +test_list_current_location + remote_exec build "rm -f list0.h" diff --git a/gdb/testsuite/gdb.base/list1.c b/gdb/testsuite/gdb.base/list1.c index d694495c3fb..9297f958f46 100644 --- a/gdb/testsuite/gdb.base/list1.c +++ b/gdb/testsuite/gdb.base/list1.c @@ -7,7 +7,7 @@ void bar (int x) - - */ { - printf ("%d\n", x); + x++; long_line (); } -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-13 10:24 ` [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Bruno Larsen @ 2023-07-13 11:05 ` Eli Zaretskii 2023-07-13 16:53 ` Tom Tromey 2023-07-14 17:50 ` Pedro Alves 2 siblings, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-07-13 11:05 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches > Cc: Bruno Larsen <blarsen@redhat.com> > Date: Thu, 13 Jul 2023 12:24:09 +0200 > From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> > > Currently, after the user has used the list command once, there is no > self-contained way to ask GDB to print the location where the inferior is > stopped. The current best options require either using a separate > command to scope out where the inferior is stopped, or using "list *$pc" > requiring knowledge of GDB standard registers. This commit adds a way > to do that using '.' as a new argument for the 'list' command. If the > inferior isn't running, the command prints around the main function. > > Because this necessitated having the inferior running and the test was > (seemingly unnecessarily) using printf in a non-essential way and it > would make the resulting log harder to read for no benefit, it was > replaced by a differen t statement. > --- > gdb/NEWS | 4 ++++ > gdb/cli/cli-cmds.c | 31 ++++++++++++++++++++++++-- > gdb/doc/gdb.texinfo | 5 +++++ > gdb/testsuite/gdb.base/list.exp | 39 +++++++++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/list1.c | 2 +- > 5 files changed, 78 insertions(+), 3 deletions(-) The documentation parts are okay, thanks. Reviewed-By: Eli Zaretskii <eliz@gnu.org> ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-13 10:24 ` [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Bruno Larsen 2023-07-13 11:05 ` Eli Zaretskii @ 2023-07-13 16:53 ` Tom Tromey 2023-07-14 17:50 ` Pedro Alves 2 siblings, 0 replies; 37+ messages in thread From: Tom Tromey @ 2023-07-13 16:53 UTC (permalink / raw) To: Bruno Larsen via Gdb-patches; +Cc: Bruno Larsen >>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: Bruno> Because this necessitated having the inferior running and the test was Bruno> (seemingly unnecessarily) using printf in a non-essential way and it Bruno> would make the resulting log harder to read for no benefit, it was Bruno> replaced by a differen t statement. Extra space in there. Normally I wouldn't point it out but I had one other small nit. Bruno> + try Bruno> + { Bruno> + /* Find the current line by getting the PC of the currently Bruno> + selected frame, and finding the line associated to it. */ Bruno> + frame_info_ptr frame = get_selected_frame (nullptr); Bruno> + CORE_ADDR curr_pc = get_frame_pc (frame); Bruno> + cursal = find_pc_line (curr_pc, 0); Bruno> + } Bruno> + catch (const gdb_exception &e) Bruno> + { Bruno> + /* If there was an exception above, it means the inferior Bruno> + is not running, so reset the current source location to Bruno> + the default. */ Bruno> + clear_current_source_symtab_and_line (); Bruno> + set_default_source_symtab_and_line (); Bruno> + cursal = get_current_source_symtab_and_line (); The indentation in the 'catch' part looks off. This is ok with these nits fixed, no need to re-send. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-13 10:24 ` [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Bruno Larsen 2023-07-13 11:05 ` Eli Zaretskii 2023-07-13 16:53 ` Tom Tromey @ 2023-07-14 17:50 ` Pedro Alves 2023-07-17 8:21 ` Bruno Larsen 2023-07-18 11:21 ` [PATCH] gdb/cli: fixes to newly added "list ." command Bruno Larsen 2 siblings, 2 replies; 37+ messages in thread From: Pedro Alves @ 2023-07-14 17:50 UTC (permalink / raw) To: Bruno Larsen, gdb-patches Hi, Sorry for coming late to the party here, but while trying to catch up on the list I spotted a few things that I think should be fixed. See below. BTW, I think the feature is cool! On 2023-07-13 11:24, Bruno Larsen via Gdb-patches wrote: > Currently, after the user has used the list command once, there is no > self-contained way to ask GDB to print the location where the inferior is > stopped. The current best options require either using a separate > command to scope out where the inferior is stopped, or using "list *$pc" > requiring knowledge of GDB standard registers. This commit adds a way > to do that using '.' as a new argument for the 'list' command. If the > inferior isn't running, the command prints around the main function. > > Because this necessitated having the inferior running and the test was > (seemingly unnecessarily) using printf in a non-essential way and it > would make the resulting log harder to read for no benefit, it was > replaced by a differen t statement. > --- > gdb/NEWS | 4 ++++ > gdb/cli/cli-cmds.c | 31 ++++++++++++++++++++++++-- > gdb/doc/gdb.texinfo | 5 +++++ > gdb/testsuite/gdb.base/list.exp | 39 +++++++++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/list1.c | 2 +- > 5 files changed, 78 insertions(+), 3 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index b924834d3d7..eef440a5242 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -84,6 +84,10 @@ > is 64k. To print longer strings you should increase > 'max-value-size'. > > +* The 'list' command now accepts '.' as an argument, which tells GDB to > + print the location where the inferior is stopped. If the inferior hasn't > + started yet, the command will print around the main function. It would be more accurate to say that it's where the current thread is stopped. Say you run to breakpoint hit in thread 1, and then switch to thread 2, and then do "list .". That will show you the current frame of thread 2, while "where the inferior is stopped" could very well be interpreted as "thread 1". > + > * New commands > > maintenance print record-instruction [ N ] > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 00977bc2ee3..1c459afdc97 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1234,14 +1234,14 @@ list_command (const char *arg, int from_tty) > const char *p; > > /* Pull in the current default source line if necessary. */ > - if (arg == NULL || ((arg[0] == '+' || arg[0] == '-') && arg[1] == '\0')) > + if (arg == NULL || ((arg[0] == '+' || arg[0] == '-' || arg[0] == '.') && arg[1] == '\0')) > { > set_default_source_symtab_and_line (); > symtab_and_line cursal = get_current_source_symtab_and_line (); > > /* If this is the first "list" since we've set the current > source line, center the listing around that line. */ > - if (get_first_line_listed () == 0) > + if (get_first_line_listed () == 0 && (arg == nullptr || arg[0] != '.')) > { > list_around_line (arg, cursal); > } > @@ -1263,6 +1263,32 @@ list_command (const char *arg, int from_tty) > print_source_lines (cursal.symtab, range, 0); > } > > + /* "l ." lists the default location again. */ Spelling out "list ." would be better for grepping, IMHO. > + else if (arg[0] == '.') > + { > + try > + { > + /* Find the current line by getting the PC of the currently > + selected frame, and finding the line associated to it. */ > + frame_info_ptr frame = get_selected_frame (nullptr); So this is actually using the selected frame, not the current frame. So even the "where the thread is stopped" description would be incorrect. If you really wanted to use frame #0 (where the thread stopped), then this should use get_current_frame. > + CORE_ADDR curr_pc = get_frame_pc (frame); > + cursal = find_pc_line (curr_pc, 0); > + } > + catch (const gdb_exception &e) > + { > + /* If there was an exception above, it means the inferior > + is not running, so reset the current source location to > + the default. */ Aww. Please don't use a try/catch for this... You can just check target_has_execution. Also, if an exception would be good for this (which it isn't), please don't catch gdb_exception -- it catches QUIT/Ctrl-C as well, which is most often wrong. You would want gdb_exception_error normally. > + clear_current_source_symtab_and_line (); > + set_default_source_symtab_and_line (); > + cursal = get_current_source_symtab_and_line (); > + } > + list_around_line (arg, cursal); > + /* Advance argument so just pressing "enter" after using "list ." > + will print the following lines instead of the same lines again. */ > + arg++; > + } > + > return; > } > > @@ -2770,6 +2796,7 @@ and send its output to SHELL_COMMAND.")); > = add_com ("list", class_files, list_command, _("\ > List specified function or line.\n\ > With no argument, lists ten more lines after or around previous listing.\n\ > +\"list .\" lists ten lines arond where the inferior is stopped.\n\ arond -> around > \"list -\" lists the ten lines before a previous ten-line listing.\n\ > One argument specifies a line, and ten lines are listed around that line.\n\ > Two arguments with comma between specify starting and ending lines to list.\n\ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index b10c06ae91f..7619efe3de9 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9148,6 +9148,11 @@ Stack}), this prints lines centered around that line. > > @item list - > Print lines just before the lines last printed. > + > +@item list . > +Print the lines surrounding the location that is where the inferior > +is stopped. If the inferior is not running, print around the main > +function instead. This should be clarified as well. > @end table > > @cindex @code{list}, how many lines to display > diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp > index 18ecd13ac0f..ed178a1dd95 100644 > --- a/gdb/testsuite/gdb.base/list.exp > +++ b/gdb/testsuite/gdb.base/list.exp > @@ -400,6 +400,42 @@ proc test_list_invalid_args {} { > "second use of \"list +INVALID\"" > } > > +proc test_list_current_location {} { > + global binfile > + # If the first "list" command that GDB runs is "list ." GDB may be > + # unable to recognize that the inferior isn't running, so we should > + # reload the inferior to test that condition. I don't understand this comment. Why would it be unable to do so? > + clean_restart > + gdb_file_cmd ${binfile} > + > + # Ensure that we are printing 10 lines > + if {![set_listsize 10]} { > + return > + } > + > + # First guarantee that GDB prints around the main function correctly > + gdb_test "list ." \ > + "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ > + "list . with inferior not running" > + > + if {![runto_main]} { > + warning "couldn't start inferior" > + return > + } > + > + # Walk forward some lines Missing period. > + gdb_test "until 15" ".*15.*foo.*" > + > + # Test that the correct location is printed and that > + # using just "list" will print the following lines. > + gdb_test "list ." ".*" "list current line after starting" > + gdb_test "list" ".*" "confirm we are printing the following lines" > + > + # Test that list . will reset to current location > + gdb_test "list ." ".*" "list around current line again" > + gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" I don't understand -- these 4 tests match ".*", so how how they ensuring what they claim they test? Looks like WIP? Pedro Alves > +} > + > clean_restart > gdb_file_cmd ${binfile} > > @@ -519,4 +555,7 @@ test_list "list -" 10 2 "7-8" "5-6" > # the current line. > test_list "list -" 10 1 "7" "6" > > +# Test printing the location where the inferior is stopped. > +test_list_current_location > + > remote_exec build "rm -f list0.h" > diff --git a/gdb/testsuite/gdb.base/list1.c b/gdb/testsuite/gdb.base/list1.c > index d694495c3fb..9297f958f46 100644 > --- a/gdb/testsuite/gdb.base/list1.c > +++ b/gdb/testsuite/gdb.base/list1.c > @@ -7,7 +7,7 @@ void bar (int x) > - > - */ > { > - printf ("%d\n", x); > + x++; > > long_line (); > } ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-14 17:50 ` Pedro Alves @ 2023-07-17 8:21 ` Bruno Larsen 2023-07-17 8:44 ` Andrew Burgess 2023-07-17 14:14 ` Pedro Alves 2023-07-18 11:21 ` [PATCH] gdb/cli: fixes to newly added "list ." command Bruno Larsen 1 sibling, 2 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-17 8:21 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 14/07/2023 19:50, Pedro Alves wrote: > Hi, > > Sorry for coming late to the party here, but while trying to catch up on the > list I spotted a few things that I think should be fixed. See below. I forgot to send the email, but I've already pushed this patch... I'll fix up the things you pointed in a moment and send a new patch, I just have a few questions. > > BTW, I think the feature is cool! > > On 2023-07-13 11:24, Bruno Larsen via Gdb-patches wrote: >> Currently, after the user has used the list command once, there is no >> self-contained way to ask GDB to print the location where the inferior is >> stopped. The current best options require either using a separate >> command to scope out where the inferior is stopped, or using "list *$pc" >> requiring knowledge of GDB standard registers. This commit adds a way >> to do that using '.' as a new argument for the 'list' command. If the >> inferior isn't running, the command prints around the main function. >> >> Because this necessitated having the inferior running and the test was >> (seemingly unnecessarily) using printf in a non-essential way and it >> would make the resulting log harder to read for no benefit, it was >> replaced by a differen t statement. >> --- >> gdb/NEWS | 4 ++++ >> gdb/cli/cli-cmds.c | 31 ++++++++++++++++++++++++-- >> gdb/doc/gdb.texinfo | 5 +++++ >> gdb/testsuite/gdb.base/list.exp | 39 +++++++++++++++++++++++++++++++++ >> gdb/testsuite/gdb.base/list1.c | 2 +- >> 5 files changed, 78 insertions(+), 3 deletions(-) >> >> diff --git a/gdb/NEWS b/gdb/NEWS >> index b924834d3d7..eef440a5242 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -84,6 +84,10 @@ >> is 64k. To print longer strings you should increase >> 'max-value-size'. >> >> +* The 'list' command now accepts '.' as an argument, which tells GDB to >> + print the location where the inferior is stopped. If the inferior hasn't >> + started yet, the command will print around the main function. > It would be more accurate to say that it's where the current thread is stopped. > > Say you run to breakpoint hit in thread 1, and then switch to thread 2, and then do > "list .". That will show you the current frame of thread 2, while "where the > inferior is stopped" could very well be interpreted as "thread 1". The wording does need changing, I agree. I think using the wording that already exists in the documentation is the best way to go: "prints around the last solitary line printed as part of displaying a stack frame". The wording I originally used (and the improvement you suggested) could make it sound like I would always print the lowermost frame, when that is not the point I wanted. What I wanted was to re-do what the first usage of "list" does without needing to know the lines. My big question: Can I change the NEWS entry, or is that some taboo that's best avoided? > >> + >> * New commands >> >> maintenance print record-instruction [ N ] >> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c >> index 00977bc2ee3..1c459afdc97 100644 >> --- a/gdb/cli/cli-cmds.c >> +++ b/gdb/cli/cli-cmds.c >> @@ -1234,14 +1234,14 @@ list_command (const char *arg, int from_tty) >> const char *p; >> >> /* Pull in the current default source line if necessary. */ >> - if (arg == NULL || ((arg[0] == '+' || arg[0] == '-') && arg[1] == '\0')) >> + if (arg == NULL || ((arg[0] == '+' || arg[0] == '-' || arg[0] == '.') && arg[1] == '\0')) >> { >> set_default_source_symtab_and_line (); >> symtab_and_line cursal = get_current_source_symtab_and_line (); >> >> /* If this is the first "list" since we've set the current >> source line, center the listing around that line. */ >> - if (get_first_line_listed () == 0) >> + if (get_first_line_listed () == 0 && (arg == nullptr || arg[0] != '.')) >> { >> list_around_line (arg, cursal); >> } >> @@ -1263,6 +1263,32 @@ list_command (const char *arg, int from_tty) >> print_source_lines (cursal.symtab, range, 0); >> } >> >> + /* "l ." lists the default location again. */ > Spelling out "list ." would be better for grepping, IMHO. I followed the convention of the code around it. All used only 'l' instead of "list", but I can change that easily enough. > >> + else if (arg[0] == '.') >> + { >> + try >> + { >> + /* Find the current line by getting the PC of the currently >> + selected frame, and finding the line associated to it. */ >> + frame_info_ptr frame = get_selected_frame (nullptr); > So this is actually using the selected frame, not the current frame. So even > the "where the thread is stopped" description would be incorrect. If you really > wanted to use frame #0 (where the thread stopped), then this should use get_current_frame. Yeah... that's the confusion I mentioned earlier... I want the selected frame, since its what "list" does when you first call it after entering a frame. > > >> + CORE_ADDR curr_pc = get_frame_pc (frame); >> + cursal = find_pc_line (curr_pc, 0); >> + } >> + catch (const gdb_exception &e) >> + { >> + /* If there was an exception above, it means the inferior >> + is not running, so reset the current source location to >> + the default. */ > Aww. Please don't use a try/catch for this... You can just check target_has_execution. Oh, I thought target_has_execution would query if the target can execute the inferior, rather than looking if the inferior is being executed. Will change > > Also, if an exception would be good for this (which it isn't), please don't catch > gdb_exception -- it catches QUIT/Ctrl-C as well, which is most often wrong. You would > want gdb_exception_error normally. Also news to me! I probably won't use it, but its good to know for when I need it :) >> + clear_current_source_symtab_and_line (); >> + set_default_source_symtab_and_line (); >> + cursal = get_current_source_symtab_and_line (); >> + } >> + list_around_line (arg, cursal); >> + /* Advance argument so just pressing "enter" after using "list ." >> + will print the following lines instead of the same lines again. */ >> + arg++; >> + } >> + >> return; >> } >> >> @@ -2770,6 +2796,7 @@ and send its output to SHELL_COMMAND.")); >> = add_com ("list", class_files, list_command, _("\ >> List specified function or line.\n\ >> With no argument, lists ten more lines after or around previous listing.\n\ >> +\"list .\" lists ten lines arond where the inferior is stopped.\n\ > arond -> around > >> \"list -\" lists the ten lines before a previous ten-line listing.\n\ >> One argument specifies a line, and ten lines are listed around that line.\n\ >> Two arguments with comma between specify starting and ending lines to list.\n\ >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >> index b10c06ae91f..7619efe3de9 100644 >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -9148,6 +9148,11 @@ Stack}), this prints lines centered around that line. >> >> @item list - >> Print lines just before the lines last printed. >> + >> +@item list . >> +Print the lines surrounding the location that is where the inferior >> +is stopped. If the inferior is not running, print around the main >> +function instead. > This should be clarified as well. > >> @end table >> >> @cindex @code{list}, how many lines to display >> diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp >> index 18ecd13ac0f..ed178a1dd95 100644 >> --- a/gdb/testsuite/gdb.base/list.exp >> +++ b/gdb/testsuite/gdb.base/list.exp >> @@ -400,6 +400,42 @@ proc test_list_invalid_args {} { >> "second use of \"list +INVALID\"" >> } >> >> +proc test_list_current_location {} { >> + global binfile >> + # If the first "list" command that GDB runs is "list ." GDB may be >> + # unable to recognize that the inferior isn't running, so we should >> + # reload the inferior to test that condition. > I don't understand this comment. Why would it be unable to do so? doh, this is a comment form previous iterations. Back when I thought "list ." should error, we were entering the "never before listed" if clause, instead of finding "list ." which was a problem. Now that the behavior is the same, there isn't a reason to restart the inferior. > >> + clean_restart >> + gdb_file_cmd ${binfile} >> + >> + # Ensure that we are printing 10 lines >> + if {![set_listsize 10]} { >> + return >> + } >> + >> + # First guarantee that GDB prints around the main function correctly >> + gdb_test "list ." \ >> + "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ >> + "list . with inferior not running" >> + >> + if {![runto_main]} { >> + warning "couldn't start inferior" >> + return >> + } >> + >> + # Walk forward some lines > Missing period. > >> + gdb_test "until 15" ".*15.*foo.*" >> + > >> + # Test that the correct location is printed and that >> + # using just "list" will print the following lines. >> + gdb_test "list ." ".*" "list current line after starting" >> + gdb_test "list" ".*" "confirm we are printing the following lines" >> + >> + # Test that list . will reset to current location >> + gdb_test "list ." ".*" "list around current line again" >> + gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" > I don't understand -- these 4 tests match ".*", so how how they ensuring what > they claim they test? Looks like WIP? you are correct, it is WIP... taking time off while developing this patch was clearly a bad choice hahaha -- Cheers, Bruno > > Pedro Alves > >> +} >> + >> clean_restart >> gdb_file_cmd ${binfile} >> >> @@ -519,4 +555,7 @@ test_list "list -" 10 2 "7-8" "5-6" >> # the current line. >> test_list "list -" 10 1 "7" "6" >> >> +# Test printing the location where the inferior is stopped. >> +test_list_current_location >> + >> remote_exec build "rm -f list0.h" >> diff --git a/gdb/testsuite/gdb.base/list1.c b/gdb/testsuite/gdb.base/list1.c >> index d694495c3fb..9297f958f46 100644 >> --- a/gdb/testsuite/gdb.base/list1.c >> +++ b/gdb/testsuite/gdb.base/list1.c >> @@ -7,7 +7,7 @@ void bar (int x) >> - >> - */ >> { >> - printf ("%d\n", x); >> + x++; >> >> long_line (); >> } ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-17 8:21 ` Bruno Larsen @ 2023-07-17 8:44 ` Andrew Burgess 2023-07-17 14:14 ` Pedro Alves 1 sibling, 0 replies; 37+ messages in thread From: Andrew Burgess @ 2023-07-17 8:44 UTC (permalink / raw) To: Bruno Larsen, Pedro Alves, gdb-patches Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: > On 14/07/2023 19:50, Pedro Alves wrote: >> Hi, >> >> Sorry for coming late to the party here, but while trying to catch up on the >> list I spotted a few things that I think should be fixed. See below. > I forgot to send the email, but I've already pushed this patch... I'll > fix up the things you pointed in a moment and send a new patch, I just > have a few questions. >> >> BTW, I think the feature is cool! >> >> On 2023-07-13 11:24, Bruno Larsen via Gdb-patches wrote: >>> Currently, after the user has used the list command once, there is no >>> self-contained way to ask GDB to print the location where the inferior is >>> stopped. The current best options require either using a separate >>> command to scope out where the inferior is stopped, or using "list *$pc" >>> requiring knowledge of GDB standard registers. This commit adds a way >>> to do that using '.' as a new argument for the 'list' command. If the >>> inferior isn't running, the command prints around the main function. >>> >>> Because this necessitated having the inferior running and the test was >>> (seemingly unnecessarily) using printf in a non-essential way and it >>> would make the resulting log harder to read for no benefit, it was >>> replaced by a differen t statement. >>> --- >>> gdb/NEWS | 4 ++++ >>> gdb/cli/cli-cmds.c | 31 ++++++++++++++++++++++++-- >>> gdb/doc/gdb.texinfo | 5 +++++ >>> gdb/testsuite/gdb.base/list.exp | 39 +++++++++++++++++++++++++++++++++ >>> gdb/testsuite/gdb.base/list1.c | 2 +- >>> 5 files changed, 78 insertions(+), 3 deletions(-) >>> >>> diff --git a/gdb/NEWS b/gdb/NEWS >>> index b924834d3d7..eef440a5242 100644 >>> --- a/gdb/NEWS >>> +++ b/gdb/NEWS >>> @@ -84,6 +84,10 @@ >>> is 64k. To print longer strings you should increase >>> 'max-value-size'. >>> >>> +* The 'list' command now accepts '.' as an argument, which tells GDB to >>> + print the location where the inferior is stopped. If the inferior hasn't >>> + started yet, the command will print around the main function. >> It would be more accurate to say that it's where the current thread is stopped. >> >> Say you run to breakpoint hit in thread 1, and then switch to thread 2, and then do >> "list .". That will show you the current frame of thread 2, while "where the >> inferior is stopped" could very well be interpreted as "thread 1". > > The wording does need changing, I agree. I think using the wording that > already exists in the documentation is the best way to go: "prints > around the last solitary line printed as part of displaying a stack > frame". The wording I originally used (and the improvement you > suggested) could make it sound like I would always print the lowermost > frame, when that is not the point I wanted. What I wanted was to re-do > what the first usage of "list" does without needing to know the lines. > > My big question: Can I change the NEWS entry, or is that some taboo > that's best avoided? It's fine to change the section of the NEWS file for the next release, that is, the part currently under the 'Changes since GDB 13' heading. We shouldn't change anything under a 'Change in GDB xxx' heading, as that describes content that has already been released. Thanks, Andrew ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command 2023-07-17 8:21 ` Bruno Larsen 2023-07-17 8:44 ` Andrew Burgess @ 2023-07-17 14:14 ` Pedro Alves 1 sibling, 0 replies; 37+ messages in thread From: Pedro Alves @ 2023-07-17 14:14 UTC (permalink / raw) To: Bruno Larsen, gdb-patches On 2023-07-17 09:21, Bruno Larsen wrote: > On 14/07/2023 19:50, Pedro Alves wrote: >> Hi, >> >> Sorry for coming late to the party here, but while trying to catch up on the >> list I spotted a few things that I think should be fixed. See below. > I forgot to send the email, but I've already pushed this patch... No worries, I had seen that. That's what I meant by being late to the party. :-) > I'll fix up the things you pointed in a moment and send a new patch, I just have a few questions. Thanks. >> >> BTW, I think the feature is cool! >> >> On 2023-07-13 11:24, Bruno Larsen via Gdb-patches wrote: >>> diff --git a/gdb/NEWS b/gdb/NEWS >>> index b924834d3d7..eef440a5242 100644 >>> --- a/gdb/NEWS >>> +++ b/gdb/NEWS >>> @@ -84,6 +84,10 @@ >>> is 64k. To print longer strings you should increase >>> 'max-value-size'. >>> +* The 'list' command now accepts '.' as an argument, which tells GDB to >>> + print the location where the inferior is stopped. If the inferior hasn't >>> + started yet, the command will print around the main function. >> It would be more accurate to say that it's where the current thread is stopped. >> >> Say you run to breakpoint hit in thread 1, and then switch to thread 2, and then do >> "list .". That will show you the current frame of thread 2, while "where the >> inferior is stopped" could very well be interpreted as "thread 1". > > The wording does need changing, I agree. I think using the wording that already exists in the documentation is the best way to go: "prints around the last solitary line printed as part of displaying a stack frame". The wording I originally used (and the improvement you suggested) could make it sound like I would always print the lowermost frame, when that is not the point I wanted. What I wanted was to re-do what the first usage of "list" does without needing to know the lines. Makes sense. So this is really just a shortcut for: (gdb) frame (gdb) list >>> + else if (arg[0] == '.') >>> + { >>> + try >>> + { >>> + /* Find the current line by getting the PC of the currently >>> + selected frame, and finding the line associated to it. */ >>> + frame_info_ptr frame = get_selected_frame (nullptr); >> So this is actually using the selected frame, not the current frame. So even >> the "where the thread is stopped" description would be incorrect. If you really >> wanted to use frame #0 (where the thread stopped), then this should use get_current_frame. > Yeah... that's the confusion I mentioned earlier... I want the selected frame, since its what "list" does when you first call it after entering a frame. >> >> >>> + CORE_ADDR curr_pc = get_frame_pc (frame); >>> + cursal = find_pc_line (curr_pc, 0); >>> + } >>> + catch (const gdb_exception &e) >>> + { >>> + /* If there was an exception above, it means the inferior >>> + is not running, so reset the current source location to >>> + the default. */ >> Aww. Please don't use a try/catch for this... You can just check target_has_execution. > Oh, I thought target_has_execution would query if the target can execute the inferior, rather than looking if the inferior is being executed. Will change Actually, target_has_stack would be even more appropriate. ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH] gdb/cli: fixes to newly added "list ." command 2023-07-14 17:50 ` Pedro Alves 2023-07-17 8:21 ` Bruno Larsen @ 2023-07-18 11:21 ` Bruno Larsen 2023-07-18 12:54 ` Eli Zaretskii ` (2 more replies) 1 sibling, 3 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-18 11:21 UTC (permalink / raw) To: gdb-patches; +Cc: Bruno Larsen After the series that added this command was pushed, Pedro mentioned that the news description could easily be misinterpreted, as well as some code and test improvements that should be made. While fixing the test, I realized that code repetition wasn't happening as it should, so I took care of that too. --- gdb/NEWS | 5 +++-- gdb/cli/cli-cmds.c | 18 +++++++++--------- gdb/doc/gdb.texinfo | 6 +++--- gdb/testsuite/gdb.base/list.exp | 19 +++++++++---------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index ac5dc424d3f..dd2fc0103bc 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -87,8 +87,9 @@ * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. * The 'list' command now accepts '.' as an argument, which tells GDB to - print the location where the inferior is stopped. If the inferior hasn't - started yet, the command will print around the main function. + print the location around the last solitary line printed as part of + displaying a stack frame. If the inferior hasn't started yet, the + command will print around the main function. * Using the 'list' command with no arguments in a situation where the command would attempt to list past the end of the file now warns the diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 0fa24fd3df9..13d1ce71a9f 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) print_source_lines (cursal.symtab, range, 0); } - /* "l ." lists the default location again. */ + /* "list ." lists the default location again. */ else if (arg[0] == '.') { - try + if (target_has_stack ()) { /* Find the current line by getting the PC of the currently selected frame, and finding the line associated to it. */ @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) CORE_ADDR curr_pc = get_frame_pc (frame); cursal = find_pc_line (curr_pc, 0); } - catch (const gdb_exception &e) + else { - /* If there was an exception above, it means the inferior - is not running, so reset the current source location to - the default. */ + /* The inferior is not running, so reset the current source + location to the default. */ clear_current_source_symtab_and_line (); set_default_source_symtab_and_line (); cursal = get_current_source_symtab_and_line (); } list_around_line (arg, cursal); - /* Advance argument so just pressing "enter" after using "list ." + /* Set the repeat args so just pressing "enter" after using "list ." will print the following lines instead of the same lines again. */ - arg++; + if (from_tty) + set_repeat_arguments (""); } return; @@ -2805,7 +2805,7 @@ and send its output to SHELL_COMMAND.")); = add_com ("list", class_files, list_command, _("\ List specified function or line.\n\ With no argument, lists ten more lines after or around previous listing.\n\ -\"list .\" lists ten lines arond where the inferior is stopped.\n\ +\"list .\" lists ten lines around where the inferior is stopped.\n\ \"list +\" lists the ten lines following a previous ten-line listing.\n\ \"list -\" lists the ten lines before a previous ten-line listing.\n\ One argument specifies a line, and ten lines are listed around that line.\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index cd86da50f46..c9377846bdc 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9155,9 +9155,9 @@ Same as using with no arguments. Print lines just before the lines last printed. @item list . -Print the lines surrounding the location that is where the inferior -is stopped. If the inferior is not running, print around the main -function instead. +Print the lines surrounding the last solitary line printed as part +of displaying a fram. If the inferior is not running, print around +the main function instead. @end table @cindex @code{list}, how many lines to display diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index 582355996b0..520424c37c6 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/gdb/testsuite/gdb.base/list.exp @@ -402,18 +402,16 @@ proc test_list_invalid_args {} { proc test_list_current_location {} { global binfile - # If the first "list" command that GDB runs is "list ." GDB may be - # unable to recognize that the inferior isn't running, so we should - # reload the inferior to test that condition. + # Restart the inferior to test "list ." before the inferior is started. clean_restart gdb_file_cmd ${binfile} - # Ensure that we are printing 10 lines + # Ensure that we are printing 10 lines. if {![set_listsize 10]} { return } - # First guarantee that GDB prints around the main function correctly + # First guarantee that GDB prints around the main function correctly. gdb_test "list ." \ "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ "list . with inferior not running" @@ -423,17 +421,18 @@ proc test_list_current_location {} { return } - # Walk forward some lines + # Walk forward some lines. gdb_test "until 15" ".*15.*foo.*" # Test that the correct location is printed and that # using just "list" will print the following lines. - gdb_test "list ." ".*" "list current line after starting" - gdb_test "list" ".*" "confirm we are printing the following lines" + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list current line after starting" + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "confirm we are printing the following lines" # Test that list . will reset to current location - gdb_test "list ." ".*" "list around current line again" - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" + # and that an empty line lists the following lines. + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list around current line again" + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "testing repeated invocations with GDB's auto-repeat" } clean_restart -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] gdb/cli: fixes to newly added "list ." command 2023-07-18 11:21 ` [PATCH] gdb/cli: fixes to newly added "list ." command Bruno Larsen @ 2023-07-18 12:54 ` Eli Zaretskii 2023-07-18 13:40 ` Bruno Larsen 2023-07-18 13:43 ` Pedro Alves 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen 2 siblings, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2023-07-18 12:54 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches > Cc: Bruno Larsen <blarsen@redhat.com> > Date: Tue, 18 Jul 2023 13:21:41 +0200 > From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> > > diff --git a/gdb/NEWS b/gdb/NEWS > index ac5dc424d3f..dd2fc0103bc 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -87,8 +87,9 @@ > * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the last solitary line printed as part of > + displaying a stack frame. If the inferior hasn't started yet, the > + command will print around the main function. "Last solitary line" is not necessarily clear (why "solitary"?). How about * The 'list' command now accepts '.' as an argument, which tells GDB to print source code around the default location. The default location is where the inferior stopped; if the inferior didn't start yet, the command will print around the beginning of the 'main' function. > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9155,9 +9155,9 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the lines surrounding the last solitary line printed as part > +of displaying a fram. If the inferior is not running, print around > +the main function instead. > @end table Same here. Thanks. Reviewed-By: Eli Zaretskii <eliz@gnu.org> ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] gdb/cli: fixes to newly added "list ." command 2023-07-18 12:54 ` Eli Zaretskii @ 2023-07-18 13:40 ` Bruno Larsen 2023-07-18 16:17 ` Eli Zaretskii 0 siblings, 1 reply; 37+ messages in thread From: Bruno Larsen @ 2023-07-18 13:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches On 18/07/2023 14:54, Eli Zaretskii wrote: >> Cc: Bruno Larsen <blarsen@redhat.com> >> Date: Tue, 18 Jul 2023 13:21:41 +0200 >> From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> >> >> diff --git a/gdb/NEWS b/gdb/NEWS >> index ac5dc424d3f..dd2fc0103bc 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -87,8 +87,9 @@ >> * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. >> >> * The 'list' command now accepts '.' as an argument, which tells GDB to >> - print the location where the inferior is stopped. If the inferior hasn't >> - started yet, the command will print around the main function. >> + print the location around the last solitary line printed as part of >> + displaying a stack frame. If the inferior hasn't started yet, the >> + command will print around the main function. > "Last solitary line" is not necessarily clear (why "solitary"?). I picked "solitary line" from the description of list with no argments: however, if the last line printed was a solitary line printed as part of displaying a stack frame (@pxref{Stack, ,Examining the Stack}), this prints lines centered around that line. when examined in context, I think it makes sense, but I can revisit this if you want. > How > about > > * The 'list' command now accepts '.' as an argument, which tells GDB > to print source code around the default location. The default > location is where the inferior stopped; The problem with "where the inferior is stopped", as mentioned by pedro in an earlier email, that there are a few different interpretations, and only one is correct. 1. list around the PC that triggered a breakpoint. So if Thread 1 hit a breakpoint, but a user switched to thread 2, they could expect that "list ." would list around thread 1, but that's not what would happen 2. list the lowermost frame of a given thread. This was Pedro's initial interpretation of the command, so if the inferior stopped and the user went up a few frames, technically the place where it is stopped is still the same PC, but we're not listing around those lines 3. List around the frame that is being examined. This is what I meant with the command, so if you go up some frames in a different thread, "list ." will list the same thing as the first call to "list". Its effectively an alias for "(gdb) frame; (gdb) list", as pedro put it. I'm not exactly happy with the final wording I went with, though, so I'm happy for other suggestions :) -- Cheers, Bruno > if the inferior didn't > start yet, the command will print around the beginning of the > 'main' function. > >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -9155,9 +9155,9 @@ Same as using with no arguments. >> Print lines just before the lines last printed. >> >> @item list . >> -Print the lines surrounding the location that is where the inferior >> -is stopped. If the inferior is not running, print around the main >> -function instead. >> +Print the lines surrounding the last solitary line printed as part >> +of displaying a fram. If the inferior is not running, print around >> +the main function instead. >> @end table > Same here. > > Thanks. > > Reviewed-By: Eli Zaretskii <eliz@gnu.org> > ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] gdb/cli: fixes to newly added "list ." command 2023-07-18 13:40 ` Bruno Larsen @ 2023-07-18 16:17 ` Eli Zaretskii 0 siblings, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-07-18 16:17 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches > Date: Tue, 18 Jul 2023 15:40:24 +0200 > Cc: gdb-patches@sourceware.org > From: Bruno Larsen <blarsen@redhat.com> > > > * The 'list' command now accepts '.' as an argument, which tells GDB > > to print source code around the default location. The default > > location is where the inferior stopped; > > The problem with "where the inferior is stopped", as mentioned by pedro > in an earlier email, that there are a few different interpretations, and > only one is correct. > > 1. list around the PC that triggered a breakpoint. So if Thread 1 hit a > breakpoint, but a user switched to thread 2, they could expect that > "list ." would list around thread 1, but that's not what would happen > 2. list the lowermost frame of a given thread. This was Pedro's initial > interpretation of the command, so if the inferior stopped and the user > went up a few frames, technically the place where it is stopped is still > the same PC, but we're not listing around those lines > 3. List around the frame that is being examined. This is what I meant > with the command, so if you go up some frames in a different thread, > "list ." will list the same thing as the first call to "list". Its > effectively an alias for "(gdb) frame; (gdb) list", as pedro put it. We are splitting hair, I think. To make this more accurate, I can suggest The default location is where the inferior stopped in the current stack frame. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] gdb/cli: fixes to newly added "list ." command 2023-07-18 11:21 ` [PATCH] gdb/cli: fixes to newly added "list ." command Bruno Larsen 2023-07-18 12:54 ` Eli Zaretskii @ 2023-07-18 13:43 ` Pedro Alves 2023-07-18 14:55 ` Bruno Larsen 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen 2 siblings, 1 reply; 37+ messages in thread From: Pedro Alves @ 2023-07-18 13:43 UTC (permalink / raw) To: Bruno Larsen, gdb-patches On 2023-07-18 12:21, Bruno Larsen via Gdb-patches wrote: > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as Per the discussion, it's more than misinterpreted, it's factually incorrect. :-) > some code and test improvements that should be made. > > return; > @@ -2805,7 +2805,7 @@ and send its output to SHELL_COMMAND.")); > = add_com ("list", class_files, list_command, _("\ > List specified function or line.\n\ > With no argument, lists ten more lines after or around previous listing.\n\ > -\"list .\" lists ten lines arond where the inferior is stopped.\n\ > +\"list .\" lists ten lines around where the inferior is stopped.\n\ "where the inferior is stopped" should be fixed here too, as it's not what GDB does. > \"list +\" lists the ten lines following a previous ten-line listing.\n\ > \"list -\" lists the ten lines before a previous ten-line listing.\n\ > One argument specifies a line, and ten lines are listed around that line.\n\ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index cd86da50f46..c9377846bdc 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9155,9 +9155,9 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the lines surrounding the last solitary line printed as part > +of displaying a fram. If the inferior is not running, print around "fram" -> "frame". > +the main function instead. > @end table ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] gdb/cli: fixes to newly added "list ." command 2023-07-18 13:43 ` Pedro Alves @ 2023-07-18 14:55 ` Bruno Larsen 0 siblings, 0 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-18 14:55 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 18/07/2023 15:43, Pedro Alves wrote: > On 2023-07-18 12:21, Bruno Larsen via Gdb-patches wrote: >> After the series that added this command was pushed, Pedro mentioned >> that the news description could easily be misinterpreted, as well as > Per the discussion, it's more than misinterpreted, it's factually incorrect. :-) > >> some code and test improvements that should be made. >> >> return; >> @@ -2805,7 +2805,7 @@ and send its output to SHELL_COMMAND.")); >> = add_com ("list", class_files, list_command, _("\ >> List specified function or line.\n\ >> With no argument, lists ten more lines after or around previous listing.\n\ >> -\"list .\" lists ten lines arond where the inferior is stopped.\n\ >> +\"list .\" lists ten lines around where the inferior is stopped.\n\ > "where the inferior is stopped" should be fixed here too, as it's not what GDB does. I'm having a hard time thinking of a concise description for it. Best I could come up was +\"list .\" lists ten lines around current location in the selected frame.\n\ but that feels very jargon-y... -- Cheers, Bruno > >> \"list +\" lists the ten lines following a previous ten-line listing.\n\ >> \"list -\" lists the ten lines before a previous ten-line listing.\n\ >> One argument specifies a line, and ten lines are listed around that line.\n\ >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >> index cd86da50f46..c9377846bdc 100644 >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -9155,9 +9155,9 @@ Same as using with no arguments. >> Print lines just before the lines last printed. >> >> @item list . >> -Print the lines surrounding the location that is where the inferior >> -is stopped. If the inferior is not running, print around the main >> -function instead. >> +Print the lines surrounding the last solitary line printed as part >> +of displaying a fram. If the inferior is not running, print around > "fram" -> "frame". > >> +the main function instead. >> @end table > ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2] gdb/cli: fixes to newly added "list ." command 2023-07-18 11:21 ` [PATCH] gdb/cli: fixes to newly added "list ." command Bruno Larsen 2023-07-18 12:54 ` Eli Zaretskii 2023-07-18 13:43 ` Pedro Alves @ 2023-07-21 10:26 ` Bruno Larsen 2023-07-21 11:05 ` Eli Zaretskii ` (3 more replies) 2 siblings, 4 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-21 10:26 UTC (permalink / raw) To: gdb-patches; +Cc: pedro, Bruno Larsen After the series that added this command was pushed, Pedro mentioned that the news description could easily be misinterpreted, as well as some code and test improvements that should be made. While fixing the test, I realized that code repetition wasn't happening as it should, so I took care of that too. --- Changes for v2: * reworded documentation and help text --- gdb/NEWS | 6 ++++-- gdb/cli/cli-cmds.c | 21 ++++++++++++--------- gdb/doc/gdb.texinfo | 6 +++--- gdb/testsuite/gdb.base/list.exp | 19 +++++++++---------- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index ac5dc424d3f..93010178364 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -87,8 +87,10 @@ * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. * The 'list' command now accepts '.' as an argument, which tells GDB to - print the location where the inferior is stopped. If the inferior hasn't - started yet, the command will print around the main function. + print the location around the default location. The default location + is where the inferior is stopped in the current stack frame; if the + inferior didn't start yet, the command will print around the beginning + of the 'main' function. * Using the 'list' command with no arguments in a situation where the command would attempt to list past the end of the file now warns the diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 0fa24fd3df9..9b047513e02 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) print_source_lines (cursal.symtab, range, 0); } - /* "l ." lists the default location again. */ + /* "list ." lists the default location again. */ else if (arg[0] == '.') { - try + if (target_has_stack ()) { /* Find the current line by getting the PC of the currently selected frame, and finding the line associated to it. */ @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) CORE_ADDR curr_pc = get_frame_pc (frame); cursal = find_pc_line (curr_pc, 0); } - catch (const gdb_exception &e) + else { - /* If there was an exception above, it means the inferior - is not running, so reset the current source location to - the default. */ + /* The inferior is not running, so reset the current source + location to the default. */ clear_current_source_symtab_and_line (); set_default_source_symtab_and_line (); cursal = get_current_source_symtab_and_line (); } list_around_line (arg, cursal); - /* Advance argument so just pressing "enter" after using "list ." + /* Set the repeat args so just pressing "enter" after using "list ." will print the following lines instead of the same lines again. */ - arg++; + if (from_tty) + set_repeat_arguments (""); } return; @@ -2805,9 +2805,12 @@ and send its output to SHELL_COMMAND.")); = add_com ("list", class_files, list_command, _("\ List specified function or line.\n\ With no argument, lists ten more lines after or around previous listing.\n\ -\"list .\" lists ten lines arond where the inferior is stopped.\n\ \"list +\" lists the ten lines following a previous ten-line listing.\n\ \"list -\" lists the ten lines before a previous ten-line listing.\n\ +\"list .\" lists ten lines around the default location.\n\ +The default location is around the location where the inferior is stopped\n\ +in the current frame, or around the main function if the inferior didn't start.\n\ +\n\ One argument specifies a line, and ten lines are listed around that line.\n\ Two arguments with comma between specify starting and ending lines to list.\n\ Lines can be specified in these ways:\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index cd86da50f46..de44317ec6f 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9155,9 +9155,9 @@ Same as using with no arguments. Print lines just before the lines last printed. @item list . -Print the lines surrounding the location that is where the inferior -is stopped. If the inferior is not running, print around the main -function instead. +Print the default location. The default location is where the inferior +is stopped in the current frame or around the beginning of the 'main' +function, if the inferior didn't start yet. @end table @cindex @code{list}, how many lines to display diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index 582355996b0..520424c37c6 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/gdb/testsuite/gdb.base/list.exp @@ -402,18 +402,16 @@ proc test_list_invalid_args {} { proc test_list_current_location {} { global binfile - # If the first "list" command that GDB runs is "list ." GDB may be - # unable to recognize that the inferior isn't running, so we should - # reload the inferior to test that condition. + # Restart the inferior to test "list ." before the inferior is started. clean_restart gdb_file_cmd ${binfile} - # Ensure that we are printing 10 lines + # Ensure that we are printing 10 lines. if {![set_listsize 10]} { return } - # First guarantee that GDB prints around the main function correctly + # First guarantee that GDB prints around the main function correctly. gdb_test "list ." \ "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ "list . with inferior not running" @@ -423,17 +421,18 @@ proc test_list_current_location {} { return } - # Walk forward some lines + # Walk forward some lines. gdb_test "until 15" ".*15.*foo.*" # Test that the correct location is printed and that # using just "list" will print the following lines. - gdb_test "list ." ".*" "list current line after starting" - gdb_test "list" ".*" "confirm we are printing the following lines" + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list current line after starting" + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "confirm we are printing the following lines" # Test that list . will reset to current location - gdb_test "list ." ".*" "list around current line again" - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" + # and that an empty line lists the following lines. + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list around current line again" + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "testing repeated invocations with GDB's auto-repeat" } clean_restart -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2] gdb/cli: fixes to newly added "list ." command 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen @ 2023-07-21 11:05 ` Eli Zaretskii 2023-08-04 8:37 ` [PING][PATCH " Bruno Larsen ` (2 subsequent siblings) 3 siblings, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-07-21 11:05 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches, pedro > Cc: pedro@palves.net, > Bruno Larsen <blarsen@redhat.com> > Date: Fri, 21 Jul 2023 12:26:56 +0200 > From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> > > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as > some code and test improvements that should be made. > > While fixing the test, I realized that code repetition wasn't > happening as it should, so I took care of that too. > --- > Changes for v2: > * reworded documentation and help text Thanks. > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -87,8 +87,10 @@ > * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the default location. The default location ^^ Two spaces there, please. > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9155,9 +9155,9 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the default location. The default location is where the inferior ^^ Likewise. Reviewed-By: Eli Zaretskii <eliz@gnu.org> ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PING][PATCH v2] gdb/cli: fixes to newly added "list ." command 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen 2023-07-21 11:05 ` Eli Zaretskii @ 2023-08-04 8:37 ` Bruno Larsen 2023-08-23 10:03 ` [PINGv2][PATCH " Guinevere Larsen 2023-08-23 15:00 ` [PATCH " Andrew Burgess 2023-08-28 15:50 ` [PATCH v3] " Guinevere Larsen 3 siblings, 1 reply; 37+ messages in thread From: Bruno Larsen @ 2023-08-04 8:37 UTC (permalink / raw) To: gdb-patches, Bruno Larsen; +Cc: pedro Ping! -- Cheers, Bruno On 21/07/2023 12:26, Bruno Larsen wrote: > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as > some code and test improvements that should be made. > > While fixing the test, I realized that code repetition wasn't > happening as it should, so I took care of that too. > --- > Changes for v2: > * reworded documentation and help text > > --- > gdb/NEWS | 6 ++++-- > gdb/cli/cli-cmds.c | 21 ++++++++++++--------- > gdb/doc/gdb.texinfo | 6 +++--- > gdb/testsuite/gdb.base/list.exp | 19 +++++++++---------- > 4 files changed, 28 insertions(+), 24 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index ac5dc424d3f..93010178364 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -87,8 +87,10 @@ > * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the default location. The default location > + is where the inferior is stopped in the current stack frame; if the > + inferior didn't start yet, the command will print around the beginning > + of the 'main' function. > > * Using the 'list' command with no arguments in a situation where the > command would attempt to list past the end of the file now warns the > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 0fa24fd3df9..9b047513e02 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) > print_source_lines (cursal.symtab, range, 0); > } > > - /* "l ." lists the default location again. */ > + /* "list ." lists the default location again. */ > else if (arg[0] == '.') > { > - try > + if (target_has_stack ()) > { > /* Find the current line by getting the PC of the currently > selected frame, and finding the line associated to it. */ > @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) > CORE_ADDR curr_pc = get_frame_pc (frame); > cursal = find_pc_line (curr_pc, 0); > } > - catch (const gdb_exception &e) > + else > { > - /* If there was an exception above, it means the inferior > - is not running, so reset the current source location to > - the default. */ > + /* The inferior is not running, so reset the current source > + location to the default. */ > clear_current_source_symtab_and_line (); > set_default_source_symtab_and_line (); > cursal = get_current_source_symtab_and_line (); > } > list_around_line (arg, cursal); > - /* Advance argument so just pressing "enter" after using "list ." > + /* Set the repeat args so just pressing "enter" after using "list ." > will print the following lines instead of the same lines again. */ > - arg++; > + if (from_tty) > + set_repeat_arguments (""); > } > > return; > @@ -2805,9 +2805,12 @@ and send its output to SHELL_COMMAND.")); > = add_com ("list", class_files, list_command, _("\ > List specified function or line.\n\ > With no argument, lists ten more lines after or around previous listing.\n\ > -\"list .\" lists ten lines arond where the inferior is stopped.\n\ > \"list +\" lists the ten lines following a previous ten-line listing.\n\ > \"list -\" lists the ten lines before a previous ten-line listing.\n\ > +\"list .\" lists ten lines around the default location.\n\ > +The default location is around the location where the inferior is stopped\n\ > +in the current frame, or around the main function if the inferior didn't start.\n\ > +\n\ > One argument specifies a line, and ten lines are listed around that line.\n\ > Two arguments with comma between specify starting and ending lines to list.\n\ > Lines can be specified in these ways:\n\ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index cd86da50f46..de44317ec6f 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9155,9 +9155,9 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the default location. The default location is where the inferior > +is stopped in the current frame or around the beginning of the 'main' > +function, if the inferior didn't start yet. > @end table > > @cindex @code{list}, how many lines to display > diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp > index 582355996b0..520424c37c6 100644 > --- a/gdb/testsuite/gdb.base/list.exp > +++ b/gdb/testsuite/gdb.base/list.exp > @@ -402,18 +402,16 @@ proc test_list_invalid_args {} { > > proc test_list_current_location {} { > global binfile > - # If the first "list" command that GDB runs is "list ." GDB may be > - # unable to recognize that the inferior isn't running, so we should > - # reload the inferior to test that condition. > + # Restart the inferior to test "list ." before the inferior is started. > clean_restart > gdb_file_cmd ${binfile} > > - # Ensure that we are printing 10 lines > + # Ensure that we are printing 10 lines. > if {![set_listsize 10]} { > return > } > > - # First guarantee that GDB prints around the main function correctly > + # First guarantee that GDB prints around the main function correctly. > gdb_test "list ." \ > "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ > "list . with inferior not running" > @@ -423,17 +421,18 @@ proc test_list_current_location {} { > return > } > > - # Walk forward some lines > + # Walk forward some lines. > gdb_test "until 15" ".*15.*foo.*" > > # Test that the correct location is printed and that > # using just "list" will print the following lines. > - gdb_test "list ." ".*" "list current line after starting" > - gdb_test "list" ".*" "confirm we are printing the following lines" > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list current line after starting" > + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "confirm we are printing the following lines" > > # Test that list . will reset to current location > - gdb_test "list ." ".*" "list around current line again" > - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" > + # and that an empty line lists the following lines. > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list around current line again" > + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "testing repeated invocations with GDB's auto-repeat" > } > > clean_restart ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PINGv2][PATCH v2] gdb/cli: fixes to newly added "list ." command 2023-08-04 8:37 ` [PING][PATCH " Bruno Larsen @ 2023-08-23 10:03 ` Guinevere Larsen 0 siblings, 0 replies; 37+ messages in thread From: Guinevere Larsen @ 2023-08-23 10:03 UTC (permalink / raw) To: gdb-patches, Bruno Larsen; +Cc: pedro On 04/08/2023 10:37, Bruno Larsen wrote: > Ping! > -- Cheers, Guinevere Larsen She/Her/Hers ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2] gdb/cli: fixes to newly added "list ." command 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen 2023-07-21 11:05 ` Eli Zaretskii 2023-08-04 8:37 ` [PING][PATCH " Bruno Larsen @ 2023-08-23 15:00 ` Andrew Burgess 2023-08-28 15:50 ` [PATCH v3] " Guinevere Larsen 3 siblings, 0 replies; 37+ messages in thread From: Andrew Burgess @ 2023-08-23 15:00 UTC (permalink / raw) To: Bruno Larsen via Gdb-patches, gdb-patches; +Cc: pedro, Bruno Larsen Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as > some code and test improvements that should be made. > > While fixing the test, I realized that code repetition wasn't > happening as it should, so I took care of that too. > --- > Changes for v2: > * reworded documentation and help text > > --- > gdb/NEWS | 6 ++++-- > gdb/cli/cli-cmds.c | 21 ++++++++++++--------- > gdb/doc/gdb.texinfo | 6 +++--- > gdb/testsuite/gdb.base/list.exp | 19 +++++++++---------- > 4 files changed, 28 insertions(+), 24 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index ac5dc424d3f..93010178364 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -87,8 +87,10 @@ > * The Ada 2022 Enum_Rep and Enum_Val attributes are now supported. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the default location. The default location > + is where the inferior is stopped in the current stack frame; if the While reading this patch I struggled with the definition(s) of 'default location'. Here you talk about 'where the inferior is stopped', but I'm not sure about that -- what about multi-threaded inferiors? So maybe we should say 'current thread is stopped'? But then you also pull in the idea of the 'current stack frame'. So I wonder if we can actually drop reference to inferior/thread completely. How about: The default location is the point of execution within the currently selected frame. The phrase 'point of execution' is something I pulled from the manual -- it's used in several places. > + inferior didn't start yet, the command will print around the beginning > + of the 'main' function. s/inferior didn't start/inferior hasn't started/ -- I think this reads better. > > * Using the 'list' command with no arguments in a situation where the > command would attempt to list past the end of the file now warns the > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 0fa24fd3df9..9b047513e02 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) > print_source_lines (cursal.symtab, range, 0); > } > > - /* "l ." lists the default location again. */ > + /* "list ." lists the default location again. */ > else if (arg[0] == '.') > { > - try > + if (target_has_stack ()) > { > /* Find the current line by getting the PC of the currently > selected frame, and finding the line associated to it. */ > @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) > CORE_ADDR curr_pc = get_frame_pc (frame); > cursal = find_pc_line (curr_pc, 0); > } > - catch (const gdb_exception &e) > + else > { > - /* If there was an exception above, it means the inferior > - is not running, so reset the current source location to > - the default. */ > + /* The inferior is not running, so reset the current source > + location to the default. */ > clear_current_source_symtab_and_line (); > set_default_source_symtab_and_line (); > cursal = get_current_source_symtab_and_line (); > } > list_around_line (arg, cursal); > - /* Advance argument so just pressing "enter" after using "list ." > + /* Set the repeat args so just pressing "enter" after using "list ." > will print the following lines instead of the same lines again. */ > - arg++; > + if (from_tty) > + set_repeat_arguments (""); > } > > return; > @@ -2805,9 +2805,12 @@ and send its output to SHELL_COMMAND.")); > = add_com ("list", class_files, list_command, _("\ > List specified function or line.\n\ > With no argument, lists ten more lines after or around previous listing.\n\ > -\"list .\" lists ten lines arond where the inferior is stopped.\n\ > \"list +\" lists the ten lines following a previous ten-line listing.\n\ > \"list -\" lists the ten lines before a previous ten-line listing.\n\ > +\"list .\" lists ten lines around the default location.\n\ > +The default location is around the location where the inferior is stopped\n\ > +in the current frame, or around the main function if the inferior didn't start.\n\ > +\n\ > One argument specifies a line, and ten lines are listed around that line.\n\ > Two arguments with comma between specify starting and ending lines to list.\n\ > Lines can be specified in these ways:\n\ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index cd86da50f46..de44317ec6f 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9155,9 +9155,9 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the default location. The default location is where the inferior Maybe: Print lines surrounding the default location. The default location is the point of execution within the currently selected frame. > +is stopped in the current frame or around the beginning of the 'main' > +function, if the inferior didn't start yet. > @end table > > @cindex @code{list}, how many lines to display > diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp > index 582355996b0..520424c37c6 100644 > --- a/gdb/testsuite/gdb.base/list.exp > +++ b/gdb/testsuite/gdb.base/list.exp > @@ -402,18 +402,16 @@ proc test_list_invalid_args {} { > > proc test_list_current_location {} { > global binfile > - # If the first "list" command that GDB runs is "list ." GDB may be > - # unable to recognize that the inferior isn't running, so we should > - # reload the inferior to test that condition. > + # Restart the inferior to test "list ." before the inferior is started. I think you mean 'Restart GDB to test ....', otherwise this sentence makes no sense! > clean_restart > gdb_file_cmd ${binfile} > > - # Ensure that we are printing 10 lines > + # Ensure that we are printing 10 lines. > if {![set_listsize 10]} { > return > } > > - # First guarantee that GDB prints around the main function correctly > + # First guarantee that GDB prints around the main function correctly. > gdb_test "list ." \ > "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ > "list . with inferior not running" > @@ -423,17 +421,18 @@ proc test_list_current_location {} { > return > } > > - # Walk forward some lines > + # Walk forward some lines. > gdb_test "until 15" ".*15.*foo.*" > > # Test that the correct location is printed and that > # using just "list" will print the following lines. > - gdb_test "list ." ".*" "list current line after starting" > - gdb_test "list" ".*" "confirm we are printing the following lines" > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list current line after starting" > + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "confirm we are printing the following lines" Here, and below, you should wrap these long lines. Within the regexp you should consider using a '^' anchor, e.g.: gdb_test "list ." "^10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ "list current line after starting" this ensures there is no output before the lines starting '10 ....'. Obviously that suggestion applies in the other 3 locations too. > > # Test that list . will reset to current location > - gdb_test "list ." ".*" "list around current line again" > - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" > + # and that an empty line lists the following lines. > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" "list around current line again" > + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" "testing repeated invocations with GDB's auto-repeat" > } > > clean_restart > -- > 2.41.0 Thanks, Andrew ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] gdb/cli: fixes to newly added "list ." command 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen ` (2 preceding siblings ...) 2023-08-23 15:00 ` [PATCH " Andrew Burgess @ 2023-08-28 15:50 ` Guinevere Larsen 2023-09-14 13:00 ` [PING][PATCH " Guinevere Larsen ` (2 more replies) 3 siblings, 3 replies; 37+ messages in thread From: Guinevere Larsen @ 2023-08-28 15:50 UTC (permalink / raw) To: gdb-patches; +Cc: Guinevere Larsen After the series that added this command was pushed, Pedro mentioned that the news description could easily be misinterpreted, as well as some code and test improvements that should be made. While fixing the test, I realized that code repetition wasn't happening as it should, so I took care of that too. --- Changes for v3: * Changed documentation wording again, from "defualt location" to "point of execution", seeing as it is already in the manual. Since I changed docs, I removed Eli's review tag. Changes for v2: * reworded documentation and help text --- gdb/NEWS | 5 +++-- gdb/cli/cli-cmds.c | 18 +++++++++--------- gdb/doc/gdb.texinfo | 5 ++--- gdb/testsuite/gdb.base/list.exp | 24 +++++++++++++----------- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index c4b1f7a7e3b..f34bda60a88 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -90,8 +90,9 @@ expression parser. * The 'list' command now accepts '.' as an argument, which tells GDB to - print the location where the inferior is stopped. If the inferior hasn't - started yet, the command will print around the main function. + print the location around the point of execution within the current frame. + If the inferior hasn't started yet, the command wil print around the + beginning of the 'main' function. * Using the 'list' command with no arguments in a situation where the command would attempt to list past the end of the file now warns the diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 0fa24fd3df9..6bf10c98c54 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) print_source_lines (cursal.symtab, range, 0); } - /* "l ." lists the default location again. */ + /* "list ." lists the default location again. */ else if (arg[0] == '.') { - try + if (target_has_stack ()) { /* Find the current line by getting the PC of the currently selected frame, and finding the line associated to it. */ @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) CORE_ADDR curr_pc = get_frame_pc (frame); cursal = find_pc_line (curr_pc, 0); } - catch (const gdb_exception &e) + else { - /* If there was an exception above, it means the inferior - is not running, so reset the current source location to - the default. */ + /* The inferior is not running, so reset the current source + location to the point of execution. */ clear_current_source_symtab_and_line (); set_default_source_symtab_and_line (); cursal = get_current_source_symtab_and_line (); } list_around_line (arg, cursal); - /* Advance argument so just pressing "enter" after using "list ." + /* Set the repeat args so just pressing "enter" after using "list ." will print the following lines instead of the same lines again. */ - arg++; + if (from_tty) + set_repeat_arguments (""); } return; @@ -2805,9 +2805,9 @@ and send its output to SHELL_COMMAND.")); = add_com ("list", class_files, list_command, _("\ List specified function or line.\n\ With no argument, lists ten more lines after or around previous listing.\n\ -\"list .\" lists ten lines arond where the inferior is stopped.\n\ \"list +\" lists the ten lines following a previous ten-line listing.\n\ \"list -\" lists the ten lines before a previous ten-line listing.\n\ +\"list .\" lists ten lines around the point of execution in the current frame.\n\ One argument specifies a line, and ten lines are listed around that line.\n\ Two arguments with comma between specify starting and ending lines to list.\n\ Lines can be specified in these ways:\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 8be9725d1a2..784da709ea2 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9212,9 +9212,8 @@ Same as using with no arguments. Print lines just before the lines last printed. @item list . -Print the lines surrounding the location that is where the inferior -is stopped. If the inferior is not running, print around the main -function instead. +Print the point of execution within the currently selected frame. +If the inferior is not running, print around the main function instead. @end table @cindex @code{list}, how many lines to display diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index 582355996b0..306fe41c3a3 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/gdb/testsuite/gdb.base/list.exp @@ -402,18 +402,15 @@ proc test_list_invalid_args {} { proc test_list_current_location {} { global binfile - # If the first "list" command that GDB runs is "list ." GDB may be - # unable to recognize that the inferior isn't running, so we should - # reload the inferior to test that condition. - clean_restart + # Reload the inferior to test "list ." before the inferior is started. gdb_file_cmd ${binfile} - # Ensure that we are printing 10 lines + # Ensure that we are printing 10 lines. if {![set_listsize 10]} { return } - # First guarantee that GDB prints around the main function correctly + # First guarantee that GDB prints around the main function correctly. gdb_test "list ." \ "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ "list . with inferior not running" @@ -423,17 +420,22 @@ proc test_list_current_location {} { return } - # Walk forward some lines + # Walk forward some lines. gdb_test "until 15" ".*15.*foo.*" # Test that the correct location is printed and that # using just "list" will print the following lines. - gdb_test "list ." ".*" "list current line after starting" - gdb_test "list" ".*" "confirm we are printing the following lines" + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ + "list current line after starting" + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ + "confirm we are printing the following lines" # Test that list . will reset to current location - gdb_test "list ." ".*" "list around current line again" - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" + # and that an empty line lists the following lines. + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ + "list around current line again" + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ + "testing repeated invocations with GDB's auto-repeat" } clean_restart -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PING][PATCH v3] gdb/cli: fixes to newly added "list ." command 2023-08-28 15:50 ` [PATCH v3] " Guinevere Larsen @ 2023-09-14 13:00 ` Guinevere Larsen 2023-09-18 13:16 ` [PATCH " Andrew Burgess 2023-09-19 9:06 ` [PATCH v4] " Guinevere Larsen 2 siblings, 0 replies; 37+ messages in thread From: Guinevere Larsen @ 2023-09-14 13:00 UTC (permalink / raw) To: Guinevere Larsen, Bruno Larsen via Gdb-patches Ping! -- Cheers, Guinevere Larsen She/Her/Hers On 28/08/2023 17:50, Guinevere Larsen wrote: > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as > some code and test improvements that should be made. > > While fixing the test, I realized that code repetition wasn't > happening as it should, so I took care of that too. > --- > Changes for v3: > * Changed documentation wording again, from "defualt location" to > "point of execution", seeing as it is already in the manual. > > Since I changed docs, I removed Eli's review tag. > > Changes for v2: > * reworded documentation and help text > > --- > gdb/NEWS | 5 +++-- > gdb/cli/cli-cmds.c | 18 +++++++++--------- > gdb/doc/gdb.texinfo | 5 ++--- > gdb/testsuite/gdb.base/list.exp | 24 +++++++++++++----------- > 4 files changed, 27 insertions(+), 25 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index c4b1f7a7e3b..f34bda60a88 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -90,8 +90,9 @@ > expression parser. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the point of execution within the current frame. > + If the inferior hasn't started yet, the command wil print around the > + beginning of the 'main' function. > > * Using the 'list' command with no arguments in a situation where the > command would attempt to list past the end of the file now warns the > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 0fa24fd3df9..6bf10c98c54 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) > print_source_lines (cursal.symtab, range, 0); > } > > - /* "l ." lists the default location again. */ > + /* "list ." lists the default location again. */ > else if (arg[0] == '.') > { > - try > + if (target_has_stack ()) > { > /* Find the current line by getting the PC of the currently > selected frame, and finding the line associated to it. */ > @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) > CORE_ADDR curr_pc = get_frame_pc (frame); > cursal = find_pc_line (curr_pc, 0); > } > - catch (const gdb_exception &e) > + else > { > - /* If there was an exception above, it means the inferior > - is not running, so reset the current source location to > - the default. */ > + /* The inferior is not running, so reset the current source > + location to the point of execution. */ > clear_current_source_symtab_and_line (); > set_default_source_symtab_and_line (); > cursal = get_current_source_symtab_and_line (); > } > list_around_line (arg, cursal); > - /* Advance argument so just pressing "enter" after using "list ." > + /* Set the repeat args so just pressing "enter" after using "list ." > will print the following lines instead of the same lines again. */ > - arg++; > + if (from_tty) > + set_repeat_arguments (""); > } > > return; > @@ -2805,9 +2805,9 @@ and send its output to SHELL_COMMAND.")); > = add_com ("list", class_files, list_command, _("\ > List specified function or line.\n\ > With no argument, lists ten more lines after or around previous listing.\n\ > -\"list .\" lists ten lines arond where the inferior is stopped.\n\ > \"list +\" lists the ten lines following a previous ten-line listing.\n\ > \"list -\" lists the ten lines before a previous ten-line listing.\n\ > +\"list .\" lists ten lines around the point of execution in the current frame.\n\ > One argument specifies a line, and ten lines are listed around that line.\n\ > Two arguments with comma between specify starting and ending lines to list.\n\ > Lines can be specified in these ways:\n\ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 8be9725d1a2..784da709ea2 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9212,9 +9212,8 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the point of execution within the currently selected frame. > +If the inferior is not running, print around the main function instead. > @end table > > @cindex @code{list}, how many lines to display > diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp > index 582355996b0..306fe41c3a3 100644 > --- a/gdb/testsuite/gdb.base/list.exp > +++ b/gdb/testsuite/gdb.base/list.exp > @@ -402,18 +402,15 @@ proc test_list_invalid_args {} { > > proc test_list_current_location {} { > global binfile > - # If the first "list" command that GDB runs is "list ." GDB may be > - # unable to recognize that the inferior isn't running, so we should > - # reload the inferior to test that condition. > - clean_restart > + # Reload the inferior to test "list ." before the inferior is started. > gdb_file_cmd ${binfile} > > - # Ensure that we are printing 10 lines > + # Ensure that we are printing 10 lines. > if {![set_listsize 10]} { > return > } > > - # First guarantee that GDB prints around the main function correctly > + # First guarantee that GDB prints around the main function correctly. > gdb_test "list ." \ > "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ > "list . with inferior not running" > @@ -423,17 +420,22 @@ proc test_list_current_location {} { > return > } > > - # Walk forward some lines > + # Walk forward some lines. > gdb_test "until 15" ".*15.*foo.*" > > # Test that the correct location is printed and that > # using just "list" will print the following lines. > - gdb_test "list ." ".*" "list current line after starting" > - gdb_test "list" ".*" "confirm we are printing the following lines" > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ > + "list current line after starting" > + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ > + "confirm we are printing the following lines" > > # Test that list . will reset to current location > - gdb_test "list ." ".*" "list around current line again" > - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" > + # and that an empty line lists the following lines. > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ > + "list around current line again" > + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ > + "testing repeated invocations with GDB's auto-repeat" > } > > clean_restart ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v3] gdb/cli: fixes to newly added "list ." command 2023-08-28 15:50 ` [PATCH v3] " Guinevere Larsen 2023-09-14 13:00 ` [PING][PATCH " Guinevere Larsen @ 2023-09-18 13:16 ` Andrew Burgess 2023-09-19 9:06 ` [PATCH v4] " Guinevere Larsen 2 siblings, 0 replies; 37+ messages in thread From: Andrew Burgess @ 2023-09-18 13:16 UTC (permalink / raw) To: Guinevere Larsen, gdb-patches Guinevere Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as > some code and test improvements that should be made. > > While fixing the test, I realized that code repetition wasn't > happening as it should, so I took care of that too. > --- > Changes for v3: > * Changed documentation wording again, from "defualt location" to > "point of execution", seeing as it is already in the manual. > > Since I changed docs, I removed Eli's review tag. > > Changes for v2: > * reworded documentation and help text > > --- > gdb/NEWS | 5 +++-- > gdb/cli/cli-cmds.c | 18 +++++++++--------- > gdb/doc/gdb.texinfo | 5 ++--- > gdb/testsuite/gdb.base/list.exp | 24 +++++++++++++----------- > 4 files changed, 27 insertions(+), 25 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index c4b1f7a7e3b..f34bda60a88 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -90,8 +90,9 @@ > expression parser. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the point of execution within the current frame. > + If the inferior hasn't started yet, the command wil print around the > + beginning of the 'main' function. > > * Using the 'list' command with no arguments in a situation where the > command would attempt to list past the end of the file now warns the > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 0fa24fd3df9..6bf10c98c54 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) > print_source_lines (cursal.symtab, range, 0); > } > > - /* "l ." lists the default location again. */ > + /* "list ." lists the default location again. */ > else if (arg[0] == '.') > { > - try > + if (target_has_stack ()) > { > /* Find the current line by getting the PC of the currently > selected frame, and finding the line associated to it. */ > @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) > CORE_ADDR curr_pc = get_frame_pc (frame); > cursal = find_pc_line (curr_pc, 0); > } > - catch (const gdb_exception &e) > + else > { > - /* If there was an exception above, it means the inferior > - is not running, so reset the current source location to > - the default. */ > + /* The inferior is not running, so reset the current source > + location to the point of execution. */ I'm not sure this comment makes sense -- if the inferior is NOT running then set the location to the point of execution! How about: /* The inferior is not running, so reset the current source location the default (usually the main function). */ If this, or something like this is acceptable to you then: > clear_current_source_symtab_and_line (); > set_default_source_symtab_and_line (); > cursal = get_current_source_symtab_and_line (); > } > list_around_line (arg, cursal); > - /* Advance argument so just pressing "enter" after using "list ." > + /* Set the repeat args so just pressing "enter" after using "list ." > will print the following lines instead of the same lines again. */ > - arg++; > + if (from_tty) > + set_repeat_arguments (""); > } > > return; > @@ -2805,9 +2805,9 @@ and send its output to SHELL_COMMAND.")); > = add_com ("list", class_files, list_command, _("\ > List specified function or line.\n\ > With no argument, lists ten more lines after or around previous listing.\n\ > -\"list .\" lists ten lines arond where the inferior is stopped.\n\ > \"list +\" lists the ten lines following a previous ten-line listing.\n\ > \"list -\" lists the ten lines before a previous ten-line listing.\n\ > +\"list .\" lists ten lines around the point of execution in the current frame.\n\ > One argument specifies a line, and ten lines are listed around that line.\n\ > Two arguments with comma between specify starting and ending lines to list.\n\ > Lines can be specified in these ways:\n\ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 8be9725d1a2..784da709ea2 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9212,9 +9212,8 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the point of execution within the currently selected frame. > +If the inferior is not running, print around the main function instead. I think saying 'print the point of execution' might be confusing. How about retaining the mention of 'lines surrounding', something like: Print the lines surrounding the point of execution within the currently selected frame. If the inferior is not running print lines around the start of the main function instead. If you're happy with the comment change, then, at least for the code portion: Approved-By: Andrew Burgess <aburgess@redhat.com> If you Cc Eli with your final doc version then you'll usually get a quick reply. Thanks, Andrew > @end table > > @cindex @code{list}, how many lines to display > diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp > index 582355996b0..306fe41c3a3 100644 > --- a/gdb/testsuite/gdb.base/list.exp > +++ b/gdb/testsuite/gdb.base/list.exp > @@ -402,18 +402,15 @@ proc test_list_invalid_args {} { > > proc test_list_current_location {} { > global binfile > - # If the first "list" command that GDB runs is "list ." GDB may be > - # unable to recognize that the inferior isn't running, so we should > - # reload the inferior to test that condition. > - clean_restart > + # Reload the inferior to test "list ." before the inferior is started. > gdb_file_cmd ${binfile} > > - # Ensure that we are printing 10 lines > + # Ensure that we are printing 10 lines. > if {![set_listsize 10]} { > return > } > > - # First guarantee that GDB prints around the main function correctly > + # First guarantee that GDB prints around the main function correctly. > gdb_test "list ." \ > "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ > "list . with inferior not running" > @@ -423,17 +420,22 @@ proc test_list_current_location {} { > return > } > > - # Walk forward some lines > + # Walk forward some lines. > gdb_test "until 15" ".*15.*foo.*" > > # Test that the correct location is printed and that > # using just "list" will print the following lines. > - gdb_test "list ." ".*" "list current line after starting" > - gdb_test "list" ".*" "confirm we are printing the following lines" > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ > + "list current line after starting" > + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ > + "confirm we are printing the following lines" > > # Test that list . will reset to current location > - gdb_test "list ." ".*" "list around current line again" > - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" > + # and that an empty line lists the following lines. > + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ > + "list around current line again" > + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ > + "testing repeated invocations with GDB's auto-repeat" > } > > clean_restart > -- > 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4] gdb/cli: fixes to newly added "list ." command 2023-08-28 15:50 ` [PATCH v3] " Guinevere Larsen 2023-09-14 13:00 ` [PING][PATCH " Guinevere Larsen 2023-09-18 13:16 ` [PATCH " Andrew Burgess @ 2023-09-19 9:06 ` Guinevere Larsen 2023-09-19 11:27 ` Eli Zaretskii 2 siblings, 1 reply; 37+ messages in thread From: Guinevere Larsen @ 2023-09-19 9:06 UTC (permalink / raw) To: gdb-patches; +Cc: eliz, Guinevere Larsen, Andrew Burgess After the series that added this command was pushed, Pedro mentioned that the news description could easily be misinterpreted, as well as some code and test improvements that should be made. While fixing the test, I realized that code repetition wasn't happening as it should, so I took care of that too. Approved-By: Andrew Burgess <aburgess@redhat.com> --- Changes for v4: * Adopted Andrew's suggestions for wording, and added his approval tag. I'm just waiting for Eli to review the changes again. Changes for v3: * Changed documentation wording again, from "defualt location" to "point of execution", seeing as it is already in the manual. Since I changed docs, I removed Eli's review tag. Changes for v2: * reworded documentation and help text --- gdb/NEWS | 5 +++-- gdb/cli/cli-cmds.c | 18 +++++++++--------- gdb/doc/gdb.texinfo | 6 +++--- gdb/testsuite/gdb.base/list.exp | 24 +++++++++++++----------- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 98ff00d5efc..2837c62b163 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -90,8 +90,9 @@ expression parser. * The 'list' command now accepts '.' as an argument, which tells GDB to - print the location where the inferior is stopped. If the inferior hasn't - started yet, the command will print around the main function. + print the location around the point of execution within the current frame. + If the inferior hasn't started yet, the command wil print around the + beginning of the 'main' function. * Using the 'list' command with no arguments in a situation where the command would attempt to list past the end of the file now warns the diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 0fa24fd3df9..b801e210d6e 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1272,10 +1272,10 @@ list_command (const char *arg, int from_tty) print_source_lines (cursal.symtab, range, 0); } - /* "l ." lists the default location again. */ + /* "list ." lists the default location again. */ else if (arg[0] == '.') { - try + if (target_has_stack ()) { /* Find the current line by getting the PC of the currently selected frame, and finding the line associated to it. */ @@ -1283,19 +1283,19 @@ list_command (const char *arg, int from_tty) CORE_ADDR curr_pc = get_frame_pc (frame); cursal = find_pc_line (curr_pc, 0); } - catch (const gdb_exception &e) + else { - /* If there was an exception above, it means the inferior - is not running, so reset the current source location to - the default. */ + /* The inferior is not running, so reset the current source + location to the default (usually the main function). */ clear_current_source_symtab_and_line (); set_default_source_symtab_and_line (); cursal = get_current_source_symtab_and_line (); } list_around_line (arg, cursal); - /* Advance argument so just pressing "enter" after using "list ." + /* Set the repeat args so just pressing "enter" after using "list ." will print the following lines instead of the same lines again. */ - arg++; + if (from_tty) + set_repeat_arguments (""); } return; @@ -2805,9 +2805,9 @@ and send its output to SHELL_COMMAND.")); = add_com ("list", class_files, list_command, _("\ List specified function or line.\n\ With no argument, lists ten more lines after or around previous listing.\n\ -\"list .\" lists ten lines arond where the inferior is stopped.\n\ \"list +\" lists the ten lines following a previous ten-line listing.\n\ \"list -\" lists the ten lines before a previous ten-line listing.\n\ +\"list .\" lists ten lines around the point of execution in the current frame.\n\ One argument specifies a line, and ten lines are listed around that line.\n\ Two arguments with comma between specify starting and ending lines to list.\n\ Lines can be specified in these ways:\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index aa3c6778887..2d812b221bc 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9212,9 +9212,9 @@ Same as using with no arguments. Print lines just before the lines last printed. @item list . -Print the lines surrounding the location that is where the inferior -is stopped. If the inferior is not running, print around the main -function instead. +Print the lines surrounding the point of execution within the +currently selected frame. If the inferior is not running print lines +around the start of the main function instead. @end table @cindex @code{list}, how many lines to display diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index 582355996b0..306fe41c3a3 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/gdb/testsuite/gdb.base/list.exp @@ -402,18 +402,15 @@ proc test_list_invalid_args {} { proc test_list_current_location {} { global binfile - # If the first "list" command that GDB runs is "list ." GDB may be - # unable to recognize that the inferior isn't running, so we should - # reload the inferior to test that condition. - clean_restart + # Reload the inferior to test "list ." before the inferior is started. gdb_file_cmd ${binfile} - # Ensure that we are printing 10 lines + # Ensure that we are printing 10 lines. if {![set_listsize 10]} { return } - # First guarantee that GDB prints around the main function correctly + # First guarantee that GDB prints around the main function correctly. gdb_test "list ." \ "1.*\r\n2\[ \t\]+\r\n3\[ \t\]+int main \[)(\]+.*5\[ \t\]+int x;.*" \ "list . with inferior not running" @@ -423,17 +420,22 @@ proc test_list_current_location {} { return } - # Walk forward some lines + # Walk forward some lines. gdb_test "until 15" ".*15.*foo.*" # Test that the correct location is printed and that # using just "list" will print the following lines. - gdb_test "list ." ".*" "list current line after starting" - gdb_test "list" ".*" "confirm we are printing the following lines" + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ + "list current line after starting" + gdb_test "list" "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ + "confirm we are printing the following lines" # Test that list . will reset to current location - gdb_test "list ." ".*" "list around current line again" - gdb_test " " ".*" "testing repeated invocations with GDB's auto-repeat" + # and that an empty line lists the following lines. + gdb_test "list ." "10\[ \t\]+foo \(.*\);.*19\[ \t\]+foo \(.*\);" \ + "list around current line again" + gdb_test " " "20\[ \t\]+foo \(.*\);.*29\[ \t\]+foo \(.*\);" \ + "testing repeated invocations with GDB's auto-repeat" } clean_restart -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] gdb/cli: fixes to newly added "list ." command 2023-09-19 9:06 ` [PATCH v4] " Guinevere Larsen @ 2023-09-19 11:27 ` Eli Zaretskii 2023-09-19 12:07 ` Guinevere Larsen 0 siblings, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2023-09-19 11:27 UTC (permalink / raw) To: Guinevere Larsen; +Cc: gdb-patches, blarsen, aburgess > From: Guinevere Larsen <blarsen@redhat.com> > Cc: eliz@gnu.org, > Guinevere Larsen <blarsen@redhat.com>, > Andrew Burgess <aburgess@redhat.com> > Date: Tue, 19 Sep 2023 11:06:28 +0200 > > After the series that added this command was pushed, Pedro mentioned > that the news description could easily be misinterpreted, as well as > some code and test improvements that should be made. > > While fixing the test, I realized that code repetition wasn't > happening as it should, so I took care of that too. Thanks. > diff --git a/gdb/NEWS b/gdb/NEWS > index 98ff00d5efc..2837c62b163 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -90,8 +90,9 @@ > expression parser. > > * The 'list' command now accepts '.' as an argument, which tells GDB to > - print the location where the inferior is stopped. If the inferior hasn't > - started yet, the command will print around the main function. > + print the location around the point of execution within the current frame. > + If the inferior hasn't started yet, the command wil print around the > + beginning of the 'main' function. ^^^ Typo. > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9212,9 +9212,9 @@ Same as using with no arguments. > Print lines just before the lines last printed. > > @item list . > -Print the lines surrounding the location that is where the inferior > -is stopped. If the inferior is not running, print around the main > -function instead. > +Print the lines surrounding the point of execution within the > +currently selected frame. If the inferior is not running print lines > +around the start of the main function instead. ^ A comma is missing there. Reviewed-By: Eli Zaretskii <eliz@gnu.org> ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4] gdb/cli: fixes to newly added "list ." command 2023-09-19 11:27 ` Eli Zaretskii @ 2023-09-19 12:07 ` Guinevere Larsen 0 siblings, 0 replies; 37+ messages in thread From: Guinevere Larsen @ 2023-09-19 12:07 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches, aburgess On 19/09/2023 13:27, Eli Zaretskii wrote: >> From: Guinevere Larsen <blarsen@redhat.com> >> Cc: eliz@gnu.org, >> Guinevere Larsen <blarsen@redhat.com>, >> Andrew Burgess <aburgess@redhat.com> >> Date: Tue, 19 Sep 2023 11:06:28 +0200 >> >> After the series that added this command was pushed, Pedro mentioned >> that the news description could easily be misinterpreted, as well as >> some code and test improvements that should be made. >> >> While fixing the test, I realized that code repetition wasn't >> happening as it should, so I took care of that too. > Thanks. >> diff --git a/gdb/NEWS b/gdb/NEWS >> index 98ff00d5efc..2837c62b163 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -90,8 +90,9 @@ >> expression parser. >> >> * The 'list' command now accepts '.' as an argument, which tells GDB to >> - print the location where the inferior is stopped. If the inferior hasn't >> - started yet, the command will print around the main function. >> + print the location around the point of execution within the current frame. >> + If the inferior hasn't started yet, the command wil print around the >> + beginning of the 'main' function. ^^^ > Typo. > >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -9212,9 +9212,9 @@ Same as using with no arguments. >> Print lines just before the lines last printed. >> >> @item list . >> -Print the lines surrounding the location that is where the inferior >> -is stopped. If the inferior is not running, print around the main >> -function instead. >> +Print the lines surrounding the point of execution within the >> +currently selected frame. If the inferior is not running print lines >> +around the start of the main function instead. ^ > A comma is missing there. > > Reviewed-By: Eli Zaretskii <eliz@gnu.org> > Thanks, pushed! -- Cheers, Guinevere Larsen She/Her/Hers ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args 2023-07-13 10:24 [PATCH v4 0/4] Small changes to "list" command Bruno Larsen 2023-07-13 10:24 ` [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line Bruno Larsen 2023-07-13 10:24 ` [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Bruno Larsen @ 2023-07-13 10:24 ` Bruno Larsen 2023-07-13 11:06 ` Eli Zaretskii 2023-07-13 17:41 ` Keith Seitz 2023-07-13 10:24 ` [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command Bruno Larsen 2023-07-13 17:31 ` [PATCH v4 0/4] Small changes to "list" command Tom Tromey 4 siblings, 2 replies; 37+ messages in thread From: Bruno Larsen @ 2023-07-13 10:24 UTC (permalink / raw) To: gdb-patches; +Cc: Bruno Larsen When using "list" with no arguments, GDB will first print the lines around where the inferior is stopped, then print the next N lines until reaching the end of file, at which point it wanrs the user "Line X out of range, file Y only has X-1 lines.". This is usually desireable, but if the user can no longer see the original line, they may have forgotten the current line or that a list command was used at all, making GDB's error message look cryptic. It was reported in bugzilla as PR cli/30497. This commit improves the user experince by changing the behavior of "list" slightly when a user passes no arguments. It now prints that the end of the file has been reached and recommends that the user use the command "list ." instead. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497 --- gdb/NEWS | 5 +++++ gdb/cli/cli-cmds.c | 17 +++++++++++++---- gdb/doc/gdb.texinfo | 4 +++- gdb/source.c | 16 ++++++++++++++++ gdb/source.h | 7 +++++++ gdb/testsuite/gdb.base/list.exp | 8 ++++---- 6 files changed, 48 insertions(+), 9 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index eef440a5242..df26606c9a8 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -88,6 +88,11 @@ print the location where the inferior is stopped. If the inferior hasn't started yet, the command will print around the main function. +* Using the 'list' command with no arguments in a situation where the + command would attempt to list past the end of the file now warns the + user that the end of file has been reached, refers the user to the + newly added '.' argument + * New commands maintenance print record-instruction [ N ] diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 1c459afdc97..5f5933e7963 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1246,10 +1246,19 @@ list_command (const char *arg, int from_tty) list_around_line (arg, cursal); } - /* "l" or "l +" lists next ten lines. */ - else if (arg == NULL || arg[0] == '+') - print_source_lines (cursal.symtab, - source_lines_range (cursal.line), 0); + /* "l" and "l +" lists the next few lines, unless we're listing past + the end of the file. */ + else if (arg == nullptr || arg[0] == '+') + { + if (last_symtab_line (cursal.symtab) >= cursal.line) + print_source_lines (cursal.symtab, + source_lines_range (cursal.line), 0); + else + { + error (_("End of the file was already reached, use \"list .\" to" + " list the current location again")); + } + } /* "l -" lists previous ten lines, the ones before the ten just listed. */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 7619efe3de9..20c9b24400d 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9144,7 +9144,9 @@ Print more lines. If the last lines printed were printed with a @code{list} command, this prints lines following the last lines printed; however, if the last line printed was a solitary line printed as part of displaying a stack frame (@pxref{Stack, ,Examining the -Stack}), this prints lines centered around that line. +Stack}), this prints lines centered around that line. If no +@code{list} command has been used and no solitary line was printed, +it prints the lines around the function @code{main}. @item list - Print lines just before the lines last printed. diff --git a/gdb/source.c b/gdb/source.c index 9997cccb31b..08adc6671b7 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -1484,6 +1484,22 @@ print_source_lines (struct symtab *s, source_lines_range line_range, line_range.stopline (), flags); } +/* See source.h. */ + +int +last_symtab_line (struct symtab *s) +{ + const std::vector<off_t> *offsets; + + /* Try to get the offsets for the start of each line. */ + if (!g_source_cache.get_line_charpos (s, &offsets)) + return false; + if (offsets == nullptr) + return false; + + return offsets->size (); +} + \f /* Print info on range of pc's in a specified line. */ diff --git a/gdb/source.h b/gdb/source.h index 8fbc365680d..be80e003890 100644 --- a/gdb/source.h +++ b/gdb/source.h @@ -192,6 +192,13 @@ class source_lines_range int m_stopline; }; +/* Get the number of the last line in the given symtab. */ +extern int last_symtab_line (struct symtab *s); + +/* Check if the line LINE can be found in the symtab S, so that it can be + printed. */ +extern bool can_print_line (struct symtab *s, int line); + /* Variation of previous print_source_lines that takes a range instead of a start and end line number. */ extern void print_source_lines (struct symtab *s, source_lines_range r, diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index ed178a1dd95..582355996b0 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/gdb/testsuite/gdb.base/list.exp @@ -175,8 +175,8 @@ proc_with_prefix test_list_forward {} { "list 25-34" gdb_test "list" "35\[ \t\]+foo \\(.*\\);.*${last_line_re}" \ "list 35-42" - gdb_test "list" "Line number 44 out of range; \[^\r\n\]+ has 43 lines\." \ - "end of file error after \"list\" command" + gdb_test "list" "End of the file was already reached, use \"list .\" to list the current location again" \ + "list past end of file" } # Test that repeating the list linenum command doesn't print the same @@ -194,8 +194,8 @@ proc_with_prefix test_repeat_list_command {} { "list 25-34" gdb_test " " "35\[ \t\]+foo \\(.*\\);.*${last_line_re}" \ "list 35-42" - gdb_test "list" "Line number 44 out of range; \[^\r\n\]+ has 43 lines\." \ - "end of file error after using 'return' to repeat the list command" + gdb_test "list" "End of the file was already reached, use \"list .\" to list the current location again" \ + "list past end of file" } proc_with_prefix test_list_backwards {} { -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args 2023-07-13 10:24 ` [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args Bruno Larsen @ 2023-07-13 11:06 ` Eli Zaretskii 2023-07-13 17:41 ` Keith Seitz 1 sibling, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-07-13 11:06 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches > Cc: Bruno Larsen <blarsen@redhat.com> > Date: Thu, 13 Jul 2023 12:24:10 +0200 > From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> > > When using "list" with no arguments, GDB will first print the lines > around where the inferior is stopped, then print the next N lines until > reaching the end of file, at which point it wanrs the user "Line X out > of range, file Y only has X-1 lines.". This is usually desireable, but > if the user can no longer see the original line, they may have forgotten > the current line or that a list command was used at all, making GDB's > error message look cryptic. It was reported in bugzilla as PR cli/30497. > > This commit improves the user experince by changing the behavior of > "list" slightly when a user passes no arguments. It now prints that the > end of the file has been reached and recommends that the user use the > command "list ." instead. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497 > --- > gdb/NEWS | 5 +++++ > gdb/cli/cli-cmds.c | 17 +++++++++++++---- > gdb/doc/gdb.texinfo | 4 +++- > gdb/source.c | 16 ++++++++++++++++ > gdb/source.h | 7 +++++++ > gdb/testsuite/gdb.base/list.exp | 8 ++++---- > 6 files changed, 48 insertions(+), 9 deletions(-) Thanks, the documentation parts are okay. Reviewed-By: Eli Zaretskii <eliz@gnu.org> ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args 2023-07-13 10:24 ` [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args Bruno Larsen 2023-07-13 11:06 ` Eli Zaretskii @ 2023-07-13 17:41 ` Keith Seitz 1 sibling, 0 replies; 37+ messages in thread From: Keith Seitz @ 2023-07-13 17:41 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches Just wanted to point out some trivial typos: On 7/13/23 03:24, Bruno Larsen via Gdb-patches wrote: > When using "list" with no arguments, GDB will first print the lines > around where the inferior is stopped, then print the next N lines until > reaching the end of file, at which point it wanrs the user "Line X out ^^^^^ "warns" > of range, file Y only has X-1 lines.". This is usually desireable, but "desirable" ^^^^^^^^^^ > if the user can no longer see the original line, they may have forgotten > the current line or that a list command was used at all, making GDB's > error message look cryptic. It was reported in bugzilla as PR cli/30497. > > This commit improves the user experince by changing the behavior of ^^^^^^^^^ "experience" > "list" slightly when a user passes no arguments. It now prints that the > end of the file has been reached and recommends that the user use the > command "list ." instead. Apologies for the nitpicking after the patch has been approved. Keith ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command 2023-07-13 10:24 [PATCH v4 0/4] Small changes to "list" command Bruno Larsen ` (2 preceding siblings ...) 2023-07-13 10:24 ` [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args Bruno Larsen @ 2023-07-13 10:24 ` Bruno Larsen 2023-07-13 17:35 ` Keith Seitz 2023-07-13 17:31 ` [PATCH v4 0/4] Small changes to "list" command Tom Tromey 4 siblings, 1 reply; 37+ messages in thread From: Bruno Larsen @ 2023-07-13 10:24 UTC (permalink / raw) To: gdb-patches; +Cc: Bruno Larsen, Eli Zaretskii The command 'list' has accepted the argument '+' for many years already, but this option wasn't documented either in the texinfo docs or in the help text for the command. This commit documents it. Reviewed-By: Eli Zaretskii <eliz@gnu.org> --- gdb/cli/cli-cmds.c | 1 + gdb/doc/gdb.texinfo | 3 +++ 2 files changed, 4 insertions(+) diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 5f5933e7963..418b04212eb 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -2806,6 +2806,7 @@ and send its output to SHELL_COMMAND.")); List specified function or line.\n\ With no argument, lists ten more lines after or around previous listing.\n\ \"list .\" lists ten lines arond where the inferior is stopped.\n\ +\"list +\" lists the ten lines following a previous ten-line listing.\n\ \"list -\" lists the ten lines before a previous ten-line listing.\n\ One argument specifies a line, and ten lines are listed around that line.\n\ Two arguments with comma between specify starting and ending lines to list.\n\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 20c9b24400d..cd86da50f46 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -9148,6 +9148,9 @@ Stack}), this prints lines centered around that line. If no @code{list} command has been used and no solitary line was printed, it prints the lines around the function @code{main}. +@item list + +Same as using with no arguments. + @item list - Print lines just before the lines last printed. -- 2.41.0 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command 2023-07-13 10:24 ` [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command Bruno Larsen @ 2023-07-13 17:35 ` Keith Seitz 2023-07-13 21:30 ` Matt Rice 0 siblings, 1 reply; 37+ messages in thread From: Keith Seitz @ 2023-07-13 17:35 UTC (permalink / raw) To: Bruno Larsen; +Cc: gdb-patches On 7/13/23 03:24, Bruno Larsen via Gdb-patches wrote: > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 20c9b24400d..cd86da50f46 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -9148,6 +9148,9 @@ Stack}), this prints lines centered around that line. If no > @code{list} command has been used and no solitary line was printed, > it prints the lines around the function @code{main}. > > +@item list + > +Same as using with no arguments. > + > @item list - > Print lines just before the lines last printed. > Grepping gdb.texinfo for "list +" definitely has a result: $ grep 'list +' gdb.texinfo @item list + list +[NSText initialize] This is from the same "Printing Source Line" section of the Manual that your patch touches. This is the blurb which gives a "complete description of the possible arguments for list". What your patch does is actually add "list +" to the "most commonly used" blurb at the top of the section. I'm not sure I really find that compelling, given that it is essentially synonymous with just repeating/hitting enter, documented just above it. To be clear, I am not entirely against it. I'll leave it to the documentation experts. [And I see we have a near mid-air collision with an Approved-by. So ignore me!] Keith ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command 2023-07-13 17:35 ` Keith Seitz @ 2023-07-13 21:30 ` Matt Rice 2023-07-14 8:53 ` Bruno Larsen 0 siblings, 1 reply; 37+ messages in thread From: Matt Rice @ 2023-07-13 21:30 UTC (permalink / raw) To: Keith Seitz; +Cc: Bruno Larsen, gdb-patches On Thu, Jul 13, 2023 at 5:36 PM Keith Seitz via Gdb-patches <gdb-patches@sourceware.org> wrote: > > On 7/13/23 03:24, Bruno Larsen via Gdb-patches wrote: > > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 20c9b24400d..cd86da50f46 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -9148,6 +9148,9 @@ Stack}), this prints lines centered around that line. If no > > @code{list} command has been used and no solitary line was printed, > > it prints the lines around the function @code{main}. > > > > +@item list + > > +Same as using with no arguments. > > + > > @item list - > > Print lines just before the lines last printed. > > > > Grepping gdb.texinfo for "list +" definitely has a result: > > $ grep 'list +' gdb.texinfo > @item list + > list +[NSText initialize] > > This is from the same "Printing Source Line" section of the Manual > that your patch touches. This is the blurb which gives a "complete > description of the possible arguments for list". I guess it is worth noting that this usage of 'list +[NSText initialize]', is completely different than the first match '@item list +' Keith refers to, it comes from the objective-c section and in that case the '+' signifies that the initialize method is a class method. That one comes from the section "Method Names in Commands", an awkward bit of ambiguity. But it took me a bit to figure out what you were actually referring to, not seeing the NSText match in that section. > What your patch does is actually add "list +" to the "most commonly used" > blurb at the top of the section. > > I'm not sure I really find that compelling, given that it is essentially > synonymous with just repeating/hitting enter, documented just above it. > To be clear, I am not entirely against it. I'll leave it to the documentation > experts. > > [And I see we have a near mid-air collision with an Approved-by. So ignore me!] > > Keith > ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command 2023-07-13 21:30 ` Matt Rice @ 2023-07-14 8:53 ` Bruno Larsen 2023-07-14 16:30 ` Tom Tromey 0 siblings, 1 reply; 37+ messages in thread From: Bruno Larsen @ 2023-07-14 8:53 UTC (permalink / raw) To: Matt Rice, Keith Seitz; +Cc: gdb-patches On 13/07/2023 23:30, Matt Rice wrote: > On Thu, Jul 13, 2023 at 5:36 PM Keith Seitz via Gdb-patches > <gdb-patches@sourceware.org> wrote: >> On 7/13/23 03:24, Bruno Larsen via Gdb-patches wrote: >> >>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >>> index 20c9b24400d..cd86da50f46 100644 >>> --- a/gdb/doc/gdb.texinfo >>> +++ b/gdb/doc/gdb.texinfo >>> @@ -9148,6 +9148,9 @@ Stack}), this prints lines centered around that line. If no >>> @code{list} command has been used and no solitary line was printed, >>> it prints the lines around the function @code{main}. >>> >>> +@item list + >>> +Same as using with no arguments. >>> + >>> @item list - >>> Print lines just before the lines last printed. >>> >> Grepping gdb.texinfo for "list +" definitely has a result: >> >> $ grep 'list +' gdb.texinfo >> @item list + >> list +[NSText initialize] >> >> This is from the same "Printing Source Line" section of the Manual >> that your patch touches. This is the blurb which gives a "complete >> description of the possible arguments for list". > I guess it is worth noting that this usage of 'list +[NSText > initialize]', is completely different than the first match '@item list > +' Keith refers to, > it comes from the objective-c section and in that case the '+' > signifies that the initialize method is a class method. > That one comes from the section "Method Names in Commands", an awkward > bit of ambiguity. But it took me a bit to figure out what you > were actually referring to, not seeing the NSText match in that section. Yeah, it does sound like some unfortunate bit of ambiguity. I'm sure that "list +" is different to "list +[NSText initialize]" because the code has a special case for "arg[0] == '+' && arg[1] == '\0'", so it has to be a different case. Would be nice if we could change this to a less ambiguous option, as future work... > >> What your patch does is actually add "list +" to the "most commonly used" >> blurb at the top of the section. >> >> I'm not sure I really find that compelling, given that it is essentially >> synonymous with just repeating/hitting enter, documented just above it. >> To be clear, I am not entirely against it. I'll leave it to the documentation >> experts. My reasoning to document it is that all options should be in the documentation. If this option is redundant, I think it is a code issue, not a documentation issue. -- Cheers, Bruno >> >> [And I see we have a near mid-air collision with an Approved-by. So ignore me!] >> >> Keith >> ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command 2023-07-14 8:53 ` Bruno Larsen @ 2023-07-14 16:30 ` Tom Tromey 2023-07-14 21:30 ` Matt Rice 0 siblings, 1 reply; 37+ messages in thread From: Tom Tromey @ 2023-07-14 16:30 UTC (permalink / raw) To: Bruno Larsen via Gdb-patches; +Cc: Matt Rice, Keith Seitz, Bruno Larsen >>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: Bruno> Yeah, it does sound like some unfortunate bit of ambiguity. I'm sure Bruno> that "list +" is different to "list +[NSText initialize]" because the Bruno> code has a special case for "arg[0] == '+' && arg[1] == '\0'", so it Bruno> has to be a different case. Bruno> Would be nice if we could change this to a less ambiguous option, as Bruno> future work... I wonder if the ObjC code even works. Most of the test cases can't even be compiled, and no one has maintained it in many years -- I think since before I worked on gdb. Tom ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command 2023-07-14 16:30 ` Tom Tromey @ 2023-07-14 21:30 ` Matt Rice 0 siblings, 0 replies; 37+ messages in thread From: Matt Rice @ 2023-07-14 21:30 UTC (permalink / raw) To: Tom Tromey; +Cc: Bruno Larsen via Gdb-patches, Keith Seitz, Bruno Larsen On Fri, Jul 14, 2023 at 4:30 PM Tom Tromey <tom@tromey.com> wrote: > > >>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: > > Bruno> Yeah, it does sound like some unfortunate bit of ambiguity. I'm sure > Bruno> that "list +" is different to "list +[NSText initialize]" because the > Bruno> code has a special case for "arg[0] == '+' && arg[1] == '\0'", so it > Bruno> has to be a different case. > > Bruno> Would be nice if we could change this to a less ambiguous option, as > Bruno> future work... > > I wonder if the ObjC code even works. Most of the test cases can't even > be compiled, and no one has maintained it in many years -- I think since > before I worked on gdb. > Yeah most of the test cases rely upon stuff which the gcc objc runtime removed and released before I noticed and could object to their removal (stuff like removing the Object class following suit from apples removal of the same things). At the time I had been working on fixing up and expanding the tests... A good way forward might be to re-add them to gcc in a separate new library That might make it possible to get it working with some past and future gcc versions. Without that gdb would have to build it's own runtime for the testsuite, or rely upon a 3rd party root object in order to test things like allocating objects, so we can invoke instance methods... I neither had enough interest, nor thought it was a good idea for either gdb to maintain such a thing or alternately rely upon a external library just for testing... Because without objects to actually instantiate gdb is pretty limited in what it can test, and the code for allocating objects from the runtime is quite runtime dependent. As such I wouldn't be surprised if it were completely broken either. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 0/4] Small changes to "list" command 2023-07-13 10:24 [PATCH v4 0/4] Small changes to "list" command Bruno Larsen ` (3 preceding siblings ...) 2023-07-13 10:24 ` [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command Bruno Larsen @ 2023-07-13 17:31 ` Tom Tromey 4 siblings, 0 replies; 37+ messages in thread From: Tom Tromey @ 2023-07-13 17:31 UTC (permalink / raw) To: Bruno Larsen via Gdb-patches; +Cc: Bruno Larsen >>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: Bruno> I decided to tackle PR cli/30497, and while doing so, Andrew mentioned Bruno> that it would also be nice if we could explicitly ask GDB to print the Bruno> current location, so I also decided to add that into a series. The first Bruno> patch is just some groundwork preparation to make the rest smooth. On Bruno> the second pass, I realized that 'list +' isn't properly documented, so Bruno> I added it to the docs as well. Bruno> After last round of reviews, I changed my approach to fixing cli/30497 Bruno> to only have a more obvious error message to the end user instead of Bruno> jumping back to the current location. This all looks good to me, modulo a nit I sent earlier. Thank you. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2023-09-19 12:07 UTC | newest] Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-13 10:24 [PATCH v4 0/4] Small changes to "list" command Bruno Larsen 2023-07-13 10:24 ` [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line Bruno Larsen 2023-07-13 16:53 ` Tom Tromey 2023-07-13 10:24 ` [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Bruno Larsen 2023-07-13 11:05 ` Eli Zaretskii 2023-07-13 16:53 ` Tom Tromey 2023-07-14 17:50 ` Pedro Alves 2023-07-17 8:21 ` Bruno Larsen 2023-07-17 8:44 ` Andrew Burgess 2023-07-17 14:14 ` Pedro Alves 2023-07-18 11:21 ` [PATCH] gdb/cli: fixes to newly added "list ." command Bruno Larsen 2023-07-18 12:54 ` Eli Zaretskii 2023-07-18 13:40 ` Bruno Larsen 2023-07-18 16:17 ` Eli Zaretskii 2023-07-18 13:43 ` Pedro Alves 2023-07-18 14:55 ` Bruno Larsen 2023-07-21 10:26 ` [PATCH v2] " Bruno Larsen 2023-07-21 11:05 ` Eli Zaretskii 2023-08-04 8:37 ` [PING][PATCH " Bruno Larsen 2023-08-23 10:03 ` [PINGv2][PATCH " Guinevere Larsen 2023-08-23 15:00 ` [PATCH " Andrew Burgess 2023-08-28 15:50 ` [PATCH v3] " Guinevere Larsen 2023-09-14 13:00 ` [PING][PATCH " Guinevere Larsen 2023-09-18 13:16 ` [PATCH " Andrew Burgess 2023-09-19 9:06 ` [PATCH v4] " Guinevere Larsen 2023-09-19 11:27 ` Eli Zaretskii 2023-09-19 12:07 ` Guinevere Larsen 2023-07-13 10:24 ` [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args Bruno Larsen 2023-07-13 11:06 ` Eli Zaretskii 2023-07-13 17:41 ` Keith Seitz 2023-07-13 10:24 ` [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command Bruno Larsen 2023-07-13 17:35 ` Keith Seitz 2023-07-13 21:30 ` Matt Rice 2023-07-14 8:53 ` Bruno Larsen 2023-07-14 16:30 ` Tom Tromey 2023-07-14 21:30 ` Matt Rice 2023-07-13 17:31 ` [PATCH v4 0/4] Small changes to "list" command Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).