gdb/cli: fixes to newly added "list ." command
Commit Message
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(-)
Comments
> 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>
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 :)
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
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...
> 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.
@@ -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
@@ -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\
@@ -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
@@ -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