[PATCH v4 0/4] Small changes to "list" command

[PATCH v4 0/4] Small changes to "list" command

[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

[PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line

[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

Re: [PATCH v4 1/4] gdb/cli: Factor out code to list lines around a given line

[Tom Tromey]

>>>>> "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

[PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[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

Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[Eli Zaretskii]

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

Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[Tom Tromey]

>>>>> "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

Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[Pedro Alves]

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

Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[Bruno Larsen]

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

Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[Andrew Burgess]

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

Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command

[Pedro Alves]

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.

[PATCH] gdb/cli: fixes to newly added "list ." command

[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

Re: [PATCH] gdb/cli: fixes to newly added "list ." command

[Eli Zaretskii]

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

Re: [PATCH] gdb/cli: fixes to newly added "list ." command

[Bruno Larsen]

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

Re: [PATCH] gdb/cli: fixes to newly added "list ." command

[Eli Zaretskii]

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

Re: [PATCH] gdb/cli: fixes to newly added "list ." command

[Pedro Alves]

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

Re: [PATCH] gdb/cli: fixes to newly added "list ." command

[Bruno Larsen]

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
>

[PATCH v2] gdb/cli: fixes to newly added "list ." command

[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

Re: [PATCH v2] gdb/cli: fixes to newly added "list ." command

[Eli Zaretskii]

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

[PING][PATCH v2] gdb/cli: fixes to newly added "list ." command

[Bruno Larsen]

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

[PINGv2][PATCH v2] gdb/cli: fixes to newly added "list ." command

[Guinevere Larsen]

On 04/08/2023 10:37, Bruno Larsen wrote:
> Ping!
>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

Re: [PATCH v2] gdb/cli: fixes to newly added "list ." command

[Andrew Burgess]

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

[PATCH v3] gdb/cli: fixes to newly added "list ." command

[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

[PING][PATCH v3] gdb/cli: fixes to newly added "list ." command

[Guinevere Larsen]

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

Re: [PATCH v3] gdb/cli: fixes to newly added "list ." command

[Andrew Burgess]

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

[PATCH v4] gdb/cli: fixes to newly added "list ." command

[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.

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

Re: [PATCH v4] gdb/cli: fixes to newly added "list ." command

[Eli Zaretskii]

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

Re: [PATCH v4] gdb/cli: fixes to newly added "list ." command

[Guinevere Larsen]

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

[PATCH v4 3/4] gdb/cli: Improve UX when using list with no args

[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

Re: [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args

[Eli Zaretskii]

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

Re: [PATCH v4 3/4] gdb/cli: Improve UX when using list with no args

[Keith Seitz]

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

[PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command

[Bruno Larsen]

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

Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command

[Keith Seitz]

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

Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command

[Matt Rice]

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
>

Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command

[Bruno Larsen]

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

Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command

[Tom Tromey]

>>>>> "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

Re: [PATCH v4 4/4] gdb/doc: document '+' argument for 'list' command

[Matt Rice]

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.

Re: [PATCH v4 0/4] Small changes to "list" command

[Tom Tromey]

>>>>> "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