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

[PATCH v3 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

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 for the first time
  gdb/cli: Improve UX when using list with no args
  gdb/cli: add '.' as an argument for 'list' command
  gdb/doc: document '+' argument for 'list' command

 gdb/NEWS                        | 11 ++++
 gdb/cli/cli-cmds.c              | 95 +++++++++++++++++++++++++++------
 gdb/doc/gdb.texinfo             | 16 +++++-
 gdb/source.c                    | 18 +++++++
 gdb/source.h                    |  4 ++
 gdb/testsuite/gdb.base/list.exp | 46 ++++++++++++++--
 gdb/testsuite/gdb.base/list1.c  |  2 +-
 7 files changed, 169 insertions(+), 23 deletions(-)

-- 
2.40.1

[PATCH v3 1/4] gdb/cli: Factor out code to list lines for the first time

[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 | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 638c138e7cb..b0b9c08c2ec 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1200,6 +1200,26 @@ pipe_command_completer (struct cmd_list_element *ignore,
      we don't know how to complete.  */
 }
 
+/* Helper for the list_command function. Resets the location to be printed
+   to the line where the inferior is stopped, then prints the lines.  */
+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 +1241,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.40.1

[PATCH v3 2/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.  If the line that would
be printed is past the end of the file, GDB will now warn that the
previous list command has reached the end of file, and the current line
wil be listed again.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                        |  7 +++++++
 gdb/cli/cli-cmds.c              | 36 +++++++++++++++++++++++++++++++--
 gdb/doc/gdb.texinfo             |  7 +++++--
 gdb/source.c                    | 18 +++++++++++++++++
 gdb/source.h                    |  4 ++++
 gdb/testsuite/gdb.base/list.exp |  8 ++++----
 6 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index fd42864c692..348e73dd15f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -77,6 +77,13 @@
   as the array would be printed by the 'print' command.  This
   functionality is also available for dprintf when dprintf-style is
   'gdb'.
+ 
+* 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, and prints the default
+  location.  Previously, it would error out.  The default location for
+  this purpose is the last solitary line printed, if there was one,
+  else the lines around the main function.
 
 * New commands
 
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index b0b9c08c2ec..5973aebfad3 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1244,8 +1244,40 @@ 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] == '+')
+      /* "l" lists the next few lines, unless we're listing past the end of
+	 the file.  If it would be past the end, re-print the current line.  */
+      else if (arg == nullptr)
+	{
+	  if (can_print_line (cursal.symtab, cursal.line))
+	    print_source_lines (cursal.symtab,
+				source_lines_range (cursal.line), 0);
+	  else
+	    {
+	      warning (_("previous list command has already reached the end "
+			 "of the file. Listing default location again"));
+	      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);
+	    }
+	}
+
+      /* "l +" always lists next few lines.  */
+      else if (arg[0] == '+')
 	print_source_lines (cursal.symtab,
 			    source_lines_range (cursal.line), 0);
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b10c06ae91f..55010f69a1c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9142,9 +9142,12 @@ Print lines centered around the beginning of function
 @item list
 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
+printed; however, if those lines are past the end of the source
+file, or 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..1aa08c6e080 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1484,6 +1484,24 @@ print_source_lines (struct symtab *s, source_lines_range line_range,
 			   line_range.stopline (), flags);
 }
 
+/* See source.h.  */
+
+bool
+can_print_line (struct symtab *s, int line)
+{
+  const std::vector<off_t> *offset_p;
+
+  /* If we can't get the offsets, we definitely can't print the line.  */
+  if (!g_source_cache.get_line_charpos (s, &offset_p))
+    return false;
+  if (offset_p == nullptr)
+    return false;
+
+  /* Otherwise we are able to print LINE if there are at least that many
+     lines in the symtab.  */
+  return line <= offset_p->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..ae6764322d0 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -192,6 +192,10 @@ class source_lines_range
   int m_stopline;
 };
 
+/* 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 18ecd13ac0f..35e099ebaff 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" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \
+	"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" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \
+	"list past end of file"
 }
 
 proc_with_prefix test_list_backwards {} {
-- 
2.40.1

[PATCH v3 3/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
way to ask GDB to print the location where the inferior is stopped.
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 throws an
error.  The test gdb.base/list.exp was updated to test this new argument.

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.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                        |  4 ++++
 gdb/cli/cli-cmds.c              | 26 ++++++++++++++++++++--
 gdb/doc/gdb.texinfo             |  5 +++++
 gdb/testsuite/gdb.base/list.exp | 38 +++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/list1.c  |  2 +-
 5 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 348e73dd15f..d3779c1e953 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -85,6 +85,10 @@
   this purpose is the last solitary line printed, if there was one,
   else the lines around the main function.
 
+* 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 error out.
+
 * New commands
 
 maintenance print record-instruction [ N ]
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5973aebfad3..a966142eaea 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1232,14 +1232,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);
 	}
@@ -1293,6 +1293,27 @@ 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.  */
+	      error (_("Inferior is not running. No current location."));
+	    }
+	  list_around_line (arg, cursal);
+	}
+
       return;
     }
 
@@ -2800,6 +2821,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 55010f69a1c..e4e374d138c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9151,8 +9151,13 @@ it prints the lines around the function @code{main}.
 
 @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.
 @end table
 
+
 @cindex @code{list}, how many lines to display
 By default, @value{GDBN} prints ten source lines with any of these forms of
 the @code{list} command.  You can change this using @code{set listsize}:
diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp
index 35e099ebaff..f853a9b814d 100644
--- a/gdb/testsuite/gdb.base/list.exp
+++ b/gdb/testsuite/gdb.base/list.exp
@@ -400,6 +400,41 @@ 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 correctly identifies that the inferior
+    # isn't running.
+    gdb_test "list ." "Inferior is not running. No current location." \
+	"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"
+}
+
 clean_restart
 gdb_file_cmd ${binfile}
 
@@ -519,4 +554,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.40.1

[PATCH v3 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 | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index a966142eaea..b7cbe673985 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2822,6 +2822,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 e4e374d138c..f9c3c90e16a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9149,6 +9149,10 @@ 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, but errors out if the line is
+past the end of the file.
+
 @item list -
 Print lines just before the lines last printed.
 
-- 
2.40.1

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

[Eli Zaretskii]

> From: Bruno Larsen <blarsen@redhat.com>
> Cc: Bruno Larsen <blarsen@redhat.com>,
> 	Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 21 Jun 2023 12:45:45 +0200
> 
> 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 | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index a966142eaea..b7cbe673985 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -2822,6 +2822,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\

This part is OK.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index e4e374d138c..f9c3c90e16a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9149,6 +9149,10 @@ 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, but errors out if the line is
> +past the end of the file.

But here it looks like some copy/pasta error?

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

[Bruno Larsen]

On 21/06/2023 13:13, Eli Zaretskii wrote:
>> From: Bruno Larsen <blarsen@redhat.com>
>> Cc: Bruno Larsen <blarsen@redhat.com>,
>> 	Eli Zaretskii <eliz@gnu.org>
>> Date: Wed, 21 Jun 2023 12:45:45 +0200
>>
>> 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 | 4 ++++
>>   2 files changed, 5 insertions(+)
>>
>> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
>> index a966142eaea..b7cbe673985 100644
>> --- a/gdb/cli/cli-cmds.c
>> +++ b/gdb/cli/cli-cmds.c
>> @@ -2822,6 +2822,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\
> This part is OK.
>
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index e4e374d138c..f9c3c90e16a 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -9149,6 +9149,10 @@ 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, but errors out if the line is
>> +past the end of the file.
> But here it looks like some copy/pasta error?
>
No, this is the main distinguishing feature between "list" and "list +", 
the latter will error out if it is past the end of the file, while the 
former won't.

-- 
Cheers,
Bruno

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

[Eli Zaretskii]

> Date: Wed, 21 Jun 2023 13:20:54 +0200
> Cc: gdb-patches@sourceware.org
> From: Bruno Larsen <blarsen@redhat.com>
> 
> On 21/06/2023 13:13, Eli Zaretskii wrote:
> >>   \"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\
> > This part is OK.
> >
> >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> >> index e4e374d138c..f9c3c90e16a 100644
> >> --- a/gdb/doc/gdb.texinfo
> >> +++ b/gdb/doc/gdb.texinfo
> >> @@ -9149,6 +9149,10 @@ 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, but errors out if the line is
> >> +past the end of the file.
> > But here it looks like some copy/pasta error?
> >
> No, this is the main distinguishing feature between "list" and "list +", 
> the latter will error out if it is past the end of the file, while the 
> former won't.

But then these two descriptions of "list +" describe two
seemingly-different behaviors:

  "list +" lists the ten lines following a previous ten-line listing.

vs

  @item list +
  Same as using with no arguments, but errors out if the line is
  past the end of the file.

Or what am I missing?

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

[Bruno Larsen]

On 21/06/2023 14:44, Eli Zaretskii wrote:
>> Date: Wed, 21 Jun 2023 13:20:54 +0200
>> Cc: gdb-patches@sourceware.org
>> From: Bruno Larsen <blarsen@redhat.com>
>>
>> On 21/06/2023 13:13, Eli Zaretskii wrote:
>>>>    \"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\
>>> This part is OK.
>>>
>>>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>>>> index e4e374d138c..f9c3c90e16a 100644
>>>> --- a/gdb/doc/gdb.texinfo
>>>> +++ b/gdb/doc/gdb.texinfo
>>>> @@ -9149,6 +9149,10 @@ 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, but errors out if the line is
>>>> +past the end of the file.
>>> But here it looks like some copy/pasta error?
>>>
>> No, this is the main distinguishing feature between "list" and "list +",
>> the latter will error out if it is past the end of the file, while the
>> former won't.
> But then these two descriptions of "list +" describe two
> seemingly-different behaviors:
>
>    "list +" lists the ten lines following a previous ten-line listing.
>
> vs
>
>    @item list +
>    Same as using with no arguments, but errors out if the line is
>    past the end of the file.
>
> Or what am I missing?
>
Ah, I see what you mean. I thought that the help text should prioritize 
being short over being complete, and I basically modeled it after the 
no-arguments help text (With no argument, lists ten more lines after or 
around previous listing) whereas the documentation I thought was 
important outlining the difference.

Would you prefer if I made the help text more verbose?

-- 
Cheers,
Bruno

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

[Eli Zaretskii]

> Date: Wed, 21 Jun 2023 14:55:25 +0200
> Cc: gdb-patches@sourceware.org
> From: Bruno Larsen <blarsen@redhat.com>
> 
> >    "list +" lists the ten lines following a previous ten-line listing.
> >
> > vs
> >
> >    @item list +
> >    Same as using with no arguments, but errors out if the line is
> >    past the end of the file.
> >
> > Or what am I missing?
> >
> Ah, I see what you mean. I thought that the help text should prioritize 
> being short over being complete, and I basically modeled it after the 
> no-arguments help text (With no argument, lists ten more lines after or 
> around previous listing) whereas the documentation I thought was 
> important outlining the difference.
> 
> Would you prefer if I made the help text more verbose?

You could say in the help text something like

  "list +" like with no arguments, but errors out if past end of the file

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

[Keith Seitz]

Hi,

Apologies, I'm late to the review here, so if I'm stepping on other's
toes, please ignore me!

On 6/21/23 03:45, 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
> 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.

I've run into this myself over the years. While I've adapted to it, you
(and the original bz reporter) have a very valid point. This is simply
not very user-friendly. So thank you for posting something to address this.

> This commit improves the user experince by changing the behavior of
> "list" slightly when a user passes no arguments.  If the line that would
> be printed is past the end of the file, GDB will now warn that the
> previous list command has reached the end of file, and the current line
> wil be listed again.

[If this is to be your commit message, please note there are several
typos in these two paragraphs ("desireable", "experince").]

An example before(?) / after would help frame the discussion (see below).

> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index b0b9c08c2ec..5973aebfad3 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1244,8 +1244,40 @@ 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] == '+')
> +      /* "l" lists the next few lines, unless we're listing past the end of
> +	 the file.  If it would be past the end, re-print the current line.  */
> +      else if (arg == nullptr)
> +	{
> +	  if (can_print_line (cursal.symtab, cursal.line))
> +	    print_source_lines (cursal.symtab,
> +				source_lines_range (cursal.line), 0);
> +	  else
> +	    {
> +	      warning (_("previous list command has already reached the end "
> +			 "of the file. Listing default location again"));

[Warning: You may want to skip this nit-picking and jump to "HOWEVER" below!]

Nit: Grep'ping through the code, I see that some warnings are implemented
as full sentences and some as incomplete sentences/phrases. This is both.
I'm not a fan. I would prefer full sentences or "previous list command has
already reached the end of the file -- listing default location again".

On that front, I'm not sure I like the whole "Listing default location
again" phrase. Do users typically know what a "default" location is?

I don't see this term specifically defined in either the manual or the
proposed patches? [I see there is a definition in NEWS which seems
insufficient.]

HOWEVER, if I may offer up an example around which to discuss things...

Current behavior, using "list main.c:0" on gdb's sources, and simply hitting
[Enter] until the behavior of interest appears:

(top-gdb)
1481	    gdb_printf (stream, _("\n\
1482	Report bugs to %ps.\n\
1483	"), styled_string (file_name_style.style (), REPORT_BUGS_TO));
1484	  if (stream == gdb_stdout)
1485	    gdb_printf (stream, _("\n\
1486	You can ask GDB-related questions on the GDB users mailing list\n\
1487	(gdb@sourceware.org) or on GDB's IRC channel (#gdb on Libera.Chat).\n"));
1488	}
(top-gdb)

Line number 1489 out of range; ../../src/gdb/main.c has 1488 lines.
(top-gdb)

The proposal here changes this to:

(top-gdb)
1481	    gdb_printf (stream, _("\n\
1482	Report bugs to %ps.\n\
1483	"), styled_string (file_name_style.style (), REPORT_BUGS_TO));
1484	  if (stream == gdb_stdout)
1485	    gdb_printf (stream, _("\n\
1486	You can ask GDB-related questions on the GDB users mailing list\n\
1487	(gdb@sourceware.org) or on GDB's IRC channel (#gdb on Libera.Chat).\n"));
1488	}
(top-gdb)
warning: previous list command has already reached the end of the file. Listing default location again
14	   GNU General Public License for more details.
15	
16	   You should have received a copy of the GNU General Public License
17	   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
18	
19	#include "defs.h"
20	#include "main.h"
21	#include "interps.h"
22	
23	int
(top-gdb)

TBH, I was surprised by this. If a user was looking at the end of a source
file, would they really want to suddenly jump to the top? [They could always
"list 0" to easily accomplish that.]

Also note that as implemented in these patches, hitting [Enter] AGAIN gets "stuck"
in a weird way [this scenario needs tests]:

(top-gdb)
1481	    gdb_printf (stream, _("\n\
1482	Report bugs to %ps.\n\
1483	"), styled_string (file_name_style.style (), REPORT_BUGS_TO));
1484	  if (stream == gdb_stdout)
1485	    gdb_printf (stream, _("\n\
1486	You can ask GDB-related questions on the GDB users mailing list\n\
1487	(gdb@sourceware.org) or on GDB's IRC channel (#gdb on Libera.Chat).\n"));
1488	}
(top-gdb)
warning: previous list command has already reached the end of the file. Listing default location again
14	   GNU General Public License for more details.
15	
16	   You should have received a copy of the GNU General Public License
17	   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
18	
19	#include "defs.h"
20	#include "main.h"
21	#include "interps.h"
22	
23	int
(top-gdb)
24	main (int argc, char **argv)
25	{
26	  struct captured_main_args args;
27	
28	  memset (&args, 0, sizeof args);
29	  args.argc = argc;
30	  args.argv = argv;
31	  args.interpreter_p = INTERP_CONSOLE;
32	  return gdb_main (&args);
33	}
(top-gdb)
warning: previous list command has already reached the end of the file. Listing default location again
14	   GNU General Public License for more details.
15	
16	   You should have received a copy of the GNU General Public License
17	   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
18	
19	#include "defs.h"
20	#include "main.h"
21	#include "interps.h"
22	
23	int
(top-gdb)

The "stuck" behavior/bug here aside, I would like to suggest a simpler solution:
list the last N lines of the file when attempting to list past the last line.

Something like (completely uncoded/artificially created):

(top-gdb)
1481	    gdb_printf (stream, _("\n\
1482	Report bugs to %ps.\n\
1483	"), styled_string (file_name_style.style (), REPORT_BUGS_TO));
1484	  if (stream == gdb_stdout)
1485	    gdb_printf (stream, _("\n\
1486	You can ask GDB-related questions on the GDB users mailing list\n\
1487	(gdb@sourceware.org) or on GDB's IRC channel (#gdb on Libera.Chat).\n"));
1488	}
(top-gdb)
Line number 1489 out of range; ../../src/gdb/main.c has 1488 lines.
1481	    gdb_printf (stream, _("\n\
1482	Report bugs to %ps.\n\
1483	"), styled_string (file_name_style.style (), REPORT_BUGS_TO));
1484	  if (stream == gdb_stdout)
1485	    gdb_printf (stream, _("\n\
1486	You can ask GDB-related questions on the GDB users mailing list\n\
1487	(gdb@sourceware.org) or on GDB's IRC channel (#gdb on Libera.Chat).\n"));
1488	}
[and keep repeating this behavior until a location is explicitly given by the
user]

That keeps the user focused at the end of the file. [AFAIK, we have no
convenient/easy way to jump to the end of a file.]

WDYT?

Keith

PS. OOC do you know if changing this list-past-end-of-file behavior alters the way MI
currently behaves?

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

[Keith Seitz]

On 6/21/23 03:45, Bruno Larsen via Gdb-patches wrote:
> Currently, after the user has used the list command once, there is no
> way to ask GDB to print the location where the inferior is stopped.
> 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 throws an
> error.  The test gdb.base/list.exp was updated to test this new argument.

I'm not entirely sure how I feel about throwing an error when using
"list ." with no running inferior. Can you explain why that might be
preferable to, say, just mimicking "list" with no argument or some other
fallback?

[I'm not going to NACK this patch for this, of course. I am just normally
cautious about adding errors when logical, convenient fallbacks could
be used instead.]

However, there is a problem that specifically needs addressing IMO.

Compare (using gdb debugging gdb):

(top-gdb) list 10
5	
6	   This program is free software; you can redistribute it and/or modify
7	   it under the terms of the GNU General Public License as published by
8	   the Free Software Foundation; either version 3 of the License, or
9	   (at your option) any later version.
10	
11	   This program is distributed in the hope that it will be useful,
12	   but WITHOUT ANY WARRANTY; without even the implied warranty of
13	   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
14	   GNU General Public License for more details.
(top-gdb)
15	
16	   You should have received a copy of the GNU General Public License
17	   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
18	
19	#include "defs.h"
20	#include "main.h"
21	#include "interps.h"
22	
23	int
24	main (int argc, char **argv)
(top-gdb)
25	{
26	  struct captured_main_args args;
27	
28	  memset (&args, 0, sizeof args);
29	  args.argc = argc;
30	  args.argv = argv;
31	  args.interpreter_p = INTERP_CONSOLE;
32	  return gdb_main (&args);
33	}
(top-gdb)


With (after "start"):

(top-gdb) list .
23	int
24	main (int argc, char **argv)
25	{
26	  struct captured_main_args args;
27	
28	  memset (&args, 0, sizeof args);
29	  args.argc = argc;
30	  args.argv = argv;
31	  args.interpreter_p = INTERP_CONSOLE;
32	  return gdb_main (&args);
(top-gdb)
23	int
24	main (int argc, char **argv)
25	{
26	  struct captured_main_args args;
27	
28	  memset (&args, 0, sizeof args);
29	  args.argc = argc;
30	  args.argv = argv;
31	  args.interpreter_p = INTERP_CONSOLE;
32	  return gdb_main (&args);
(top-gdb)
23	int
24	main (int argc, char **argv)
25	{
26	  struct captured_main_args args;
27	
28	  memset (&args, 0, sizeof args);
29	  args.argc = argc;
30	  args.argv = argv;
31	  args.interpreter_p = INTERP_CONSOLE;
32	  return gdb_main (&args);
(top-gdb)

I don't think this is particularly user-friendly. Is it possible to make
the two use cases behave similarly?

Keith

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

[Bruno Larsen]

On 21/06/2023 18:07, Keith Seitz wrote:
> Hi,
>
> Apologies, I'm late to the review here, so if I'm stepping on other's
> toes, please ignore me!
>
> On 6/21/23 03:45, 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
>> 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.
>
> I've run into this myself over the years. While I've adapted to it, you
> (and the original bz reporter) have a very valid point. This is simply
> not very user-friendly. So thank you for posting something to address 
> this.
>
>> This commit improves the user experince by changing the behavior of
>> "list" slightly when a user passes no arguments.  If the line that would
>> be printed is past the end of the file, GDB will now warn that the
>> previous list command has reached the end of file, and the current line
>> wil be listed again.
>
> [If this is to be your commit message, please note there are several
> typos in these two paragraphs ("desireable", "experince").]
oof, I think my keyboard or USB hub is on its way out o7
>
> An example before(?) / after would help frame the discussion (see below).
Sure!
>
>> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
>> index b0b9c08c2ec..5973aebfad3 100644
>> --- a/gdb/cli/cli-cmds.c
>> +++ b/gdb/cli/cli-cmds.c
>> @@ -1244,8 +1244,40 @@ 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] == '+')
>> +      /* "l" lists the next few lines, unless we're listing past the 
>> end of
>> +     the file.  If it would be past the end, re-print the current 
>> line.  */
>> +      else if (arg == nullptr)
>> +    {
>> +      if (can_print_line (cursal.symtab, cursal.line))
>> +        print_source_lines (cursal.symtab,
>> +                source_lines_range (cursal.line), 0);
>> +      else
>> +        {
>> +          warning (_("previous list command has already reached the 
>> end "
>> +             "of the file. Listing default location again"));
>
> [Warning: You may want to skip this nit-picking and jump to "HOWEVER" 
> below!]
>
> Nit: Grep'ping through the code, I see that some warnings are implemented
> as full sentences and some as incomplete sentences/phrases. This is both.
> I'm not a fan. I would prefer full sentences or "previous list command 
> has
> already reached the end of the file -- listing default location again".
That's fair. I have some more suggestions for the warning message below.
>
> On that front, I'm not sure I like the whole "Listing default location
> again" phrase. Do users typically know what a "default" location is?
>
> I don't see this term specifically defined in either the manual or the
> proposed patches? [I see there is a definition in NEWS which seems
> insufficient.]
>
> HOWEVER, if I may offer up an example around which to discuss things...
>
> Current behavior, using "list main.c:0" on gdb's sources, and simply 
> hitting
> [Enter] until the behavior of interest appears:
>
> (top-gdb)
> 1481        gdb_printf (stream, _("\n\
> 1482    Report bugs to %ps.\n\
> 1483    "), styled_string (file_name_style.style (), REPORT_BUGS_TO));
> 1484      if (stream == gdb_stdout)
> 1485        gdb_printf (stream, _("\n\
> 1486    You can ask GDB-related questions on the GDB users mailing 
> list\n\
> 1487    (gdb@sourceware.org) or on GDB's IRC channel (#gdb on 
> Libera.Chat).\n"));
> 1488    }
> (top-gdb)
>
> Line number 1489 out of range; ../../src/gdb/main.c has 1488 lines.
> (top-gdb)
>
> The proposal here changes this to:
>
> (top-gdb)
> 1481        gdb_printf (stream, _("\n\
> 1482    Report bugs to %ps.\n\
> 1483    "), styled_string (file_name_style.style (), REPORT_BUGS_TO));
> 1484      if (stream == gdb_stdout)
> 1485        gdb_printf (stream, _("\n\
> 1486    You can ask GDB-related questions on the GDB users mailing 
> list\n\
> 1487    (gdb@sourceware.org) or on GDB's IRC channel (#gdb on 
> Libera.Chat).\n"));
> 1488    }
> (top-gdb)
> warning: previous list command has already reached the end of the 
> file. Listing default location again
> 14       GNU General Public License for more details.
> 15
> 16       You should have received a copy of the GNU General Public 
> License
> 17       along with this program.  If not, see 
> <http://www.gnu.org/licenses/>.  */
> 18
> 19    #include "defs.h"
> 20    #include "main.h"
> 21    #include "interps.h"
> 22
> 23    int
> (top-gdb)
>
> TBH, I was surprised by this. If a user was looking at the end of a 
> source
> file, would they really want to suddenly jump to the top? [They could 
> always
> "list 0" to easily accomplish that.]

This is the difficult part of this change proposal: If the last command 
for GDB was "list", we can be reasonably sure that the user would prefer 
the error message, and if the last command was something else 
(especially info locals, backtrace or some other thing that generates a 
lot of output), we can guess that they want to be back at the current 
location. Unfortunately, there is no way (to my knowledge) to get that 
info, so I went with always assuming the user wants to be back at where 
they were.

Also, about the "stuck" behavior, and the confusion about "default 
position" i guess: Since you haven't started the inferior yet, the 
default position is lines around the main function. You're not listing 
gdb/main.c anymore, you're now listing gdb/gdb.c, which only has 33 
lines. The fact that GDB isn't actually listing the function itself, 
ending at the "int" line, is old behavior already

I guess the warning message could be changed to indicate where is being 
listed, saying something like "previous list command has already reached 
the end of file -- inferior not running, listing main again" or 
"previous list (...) -- User examining inferior at line X" (The awkward 
wording of "user examining" is there because we'll list lines around the 
selected frame, so if the user moved up some frames we'll list around 
that instead which is the current behavior for "list" already).

Or I could change it so if the inferior is not running, the command 
errors out, and if it is running, the new behavior is in place. I think 
its better to error out than repeating the last few lines if we can 
assume that the user is just examining the source code (which in this 
case would be when the inferior hasn't been started).

>
> Also note that as implemented in these patches, hitting [Enter] AGAIN 
> gets "stuck"
> in a weird way [this scenario needs tests]:
>
> (top-gdb)
> 1481        gdb_printf (stream, _("\n\
> 1482    Report bugs to %ps.\n\
> 1483    "), styled_string (file_name_style.style (), REPORT_BUGS_TO));
> 1484      if (stream == gdb_stdout)
> 1485        gdb_printf (stream, _("\n\
> 1486    You can ask GDB-related questions on the GDB users mailing 
> list\n\
> 1487    (gdb@sourceware.org) or on GDB's IRC channel (#gdb on 
> Libera.Chat).\n"));
> 1488    }
> (top-gdb)
> warning: previous list command has already reached the end of the 
> file. Listing default location again
> 14       GNU General Public License for more details.
> 15
> 16       You should have received a copy of the GNU General Public 
> License
> 17       along with this program.  If not, see 
> <http://www.gnu.org/licenses/>.  */
> 18
> 19    #include "defs.h"
> 20    #include "main.h"
> 21    #include "interps.h"
> 22
> 23    int
> (top-gdb)
> 24    main (int argc, char **argv)
> 25    {
> 26      struct captured_main_args args;
> 27
> 28      memset (&args, 0, sizeof args);
> 29      args.argc = argc;
> 30      args.argv = argv;
> 31      args.interpreter_p = INTERP_CONSOLE;
> 32      return gdb_main (&args);
> 33    }
> (top-gdb)
> warning: previous list command has already reached the end of the 
> file. Listing default location again
> 14       GNU General Public License for more details.
> 15
> 16       You should have received a copy of the GNU General Public 
> License
> 17       along with this program.  If not, see 
> <http://www.gnu.org/licenses/>.  */
> 18
> 19    #include "defs.h"
> 20    #include "main.h"
> 21    #include "interps.h"
> 22
> 23    int
> (top-gdb)
>
> The "stuck" behavior/bug here aside, I would like to suggest a simpler 
> solution:
> list the last N lines of the file when attempting to list past the 
> last line.
>
> Something like (completely uncoded/artificially created):
>
> (top-gdb)
> 1481        gdb_printf (stream, _("\n\
> 1482    Report bugs to %ps.\n\
> 1483    "), styled_string (file_name_style.style (), REPORT_BUGS_TO));
> 1484      if (stream == gdb_stdout)
> 1485        gdb_printf (stream, _("\n\
> 1486    You can ask GDB-related questions on the GDB users mailing 
> list\n\
> 1487    (gdb@sourceware.org) or on GDB's IRC channel (#gdb on 
> Libera.Chat).\n"));
> 1488    }
> (top-gdb)
> Line number 1489 out of range; ../../src/gdb/main.c has 1488 lines.
> 1481        gdb_printf (stream, _("\n\
> 1482    Report bugs to %ps.\n\
> 1483    "), styled_string (file_name_style.style (), REPORT_BUGS_TO));
> 1484      if (stream == gdb_stdout)
> 1485        gdb_printf (stream, _("\n\
> 1486    You can ask GDB-related questions on the GDB users mailing 
> list\n\
> 1487    (gdb@sourceware.org) or on GDB's IRC channel (#gdb on 
> Libera.Chat).\n"));
> 1488    }
> [and keep repeating this behavior until a location is explicitly given 
> by the
> user]
>
> That keeps the user focused at the end of the file. [AFAIK, we have no
> convenient/easy way to jump to the end of a file.]
>
> WDYT?

I feel like repeating the last lines would not be as useful. The 
use-case I had in mind was someone who is parked close to the end of a 
file and by using list twice or three times ended up there (probably 
listing to the end of a function) and couldn't see it anymore. If the 
user exploring the source file, it is likely more useful to straight up 
say that last list command already reached the end of file.

That said, I think we could also add an argument to list at the end of a 
file, but idk what to call it.

>
> Keith
>
> PS. OOC do you know if changing this list-past-end-of-file behavior 
> alters the way MI
> currently behaves?
>
huh, it doesn't! I thought it used the same base code, but apparently 
not. I'll update the MI version to keep the behaviors synced.

-- 
Cheers,
Bruno

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

[Bruno Larsen]

On 21/06/2023 19:25, Keith Seitz wrote:
> On 6/21/23 03:45, Bruno Larsen via Gdb-patches wrote:
>> Currently, after the user has used the list command once, there is no
>> way to ask GDB to print the location where the inferior is stopped.
>> 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 throws an
>> error.  The test gdb.base/list.exp was updated to test this new 
>> argument.
>
> I'm not entirely sure how I feel about throwing an error when using
> "list ." with no running inferior. Can you explain why that might be
> preferable to, say, just mimicking "list" with no argument or some other
> fallback?
>
> [I'm not going to NACK this patch for this, of course. I am just normally
> cautious about adding errors when logical, convenient fallbacks could
> be used instead.]
My logic was that '.' is supposed to be 'current location', so if the 
inferior is not running, there is no current location. I'm changing it, 
no problem :)
>
> However, there is a problem that specifically needs addressing IMO.
>
> Compare (using gdb debugging gdb):
>
> (top-gdb) list 10
> 5
> 6       This program is free software; you can redistribute it and/or 
> modify
> 7       it under the terms of the GNU General Public License as 
> published by
> 8       the Free Software Foundation; either version 3 of the License, or
> 9       (at your option) any later version.
> 10
> 11       This program is distributed in the hope that it will be useful,
> 12       but WITHOUT ANY WARRANTY; without even the implied warranty of
> 13       MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> 14       GNU General Public License for more details.
> (top-gdb)
> 15
> 16       You should have received a copy of the GNU General Public 
> License
> 17       along with this program.  If not, see 
> <http://www.gnu.org/licenses/>.  */
> 18
> 19    #include "defs.h"
> 20    #include "main.h"
> 21    #include "interps.h"
> 22
> 23    int
> 24    main (int argc, char **argv)
> (top-gdb)
> 25    {
> 26      struct captured_main_args args;
> 27
> 28      memset (&args, 0, sizeof args);
> 29      args.argc = argc;
> 30      args.argv = argv;
> 31      args.interpreter_p = INTERP_CONSOLE;
> 32      return gdb_main (&args);
> 33    }
> (top-gdb)
>
>
> With (after "start"):
>
> (top-gdb) list .
> 23    int
> 24    main (int argc, char **argv)
> 25    {
> 26      struct captured_main_args args;
> 27
> 28      memset (&args, 0, sizeof args);
> 29      args.argc = argc;
> 30      args.argv = argv;
> 31      args.interpreter_p = INTERP_CONSOLE;
> 32      return gdb_main (&args);
> (top-gdb)
> 23    int
> 24    main (int argc, char **argv)
> 25    {
> 26      struct captured_main_args args;
> 27
> 28      memset (&args, 0, sizeof args);
> 29      args.argc = argc;
> 30      args.argv = argv;
> 31      args.interpreter_p = INTERP_CONSOLE;
> 32      return gdb_main (&args);
> (top-gdb)
> 23    int
> 24    main (int argc, char **argv)
> 25    {
> 26      struct captured_main_args args;
> 27
> 28      memset (&args, 0, sizeof args);
> 29      args.argc = argc;
> 30      args.argv = argv;
> 31      args.interpreter_p = INTERP_CONSOLE;
> 32      return gdb_main (&args);
> (top-gdb)
>
> I don't think this is particularly user-friendly. Is it possible to make
> the two use cases behave similarly?

Today I learned about GDB dealing with command arguments! TL;DR yes, 
I'll do that in v4

For future reference (probably for myself) if you advance the arg 
pointer when handling the command, the next command invocation is going 
to use the advanced argument pointer. When handling the number, 
list_command advances the arg, so everything after the first line is 
literally a no arg invocation.

>
> Keith
>

-- 
Cheers,
Bruno

Re: [PATCH v3 1/4] gdb/cli: Factor out code to list lines for the first time

[Andrew Burgess]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> 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 | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 638c138e7cb..b0b9c08c2ec 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1200,6 +1200,26 @@ pipe_command_completer (struct cmd_list_element *ignore,
>       we don't know how to complete.  */
>  }
>  
> +/* Helper for the list_command function. Resets the location to be printed
> +   to the line where the inferior is stopped, then prints the lines.  */

Is this comment correct?  It says "... to the line where the inferior is
stopped", but actually appears to list lines around CURSAL.

Given this isn't a command entry point then you should probably document
what ARG is expected to contain.

Thanks,
Andrew

> +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 +1241,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.40.1

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

[Andrew Burgess]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> 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.  If the line that would
> be printed is past the end of the file, GDB will now warn that the
> previous list command has reached the end of file, and the current line
> wil be listed again.

-1 from me.  I don't like the idea of this auto-wrap around.

I don't feel like this really addresses the original complain anyway.
The complaint seems to be:

  1. User does 'list' and sees the current location,

  2. User runs "other commands" which scrolls the list contents off the
  screen,

  3. User does 'list', but is now unsure what the current location is.

The only way this change helps is if the user keeps "list"-ing until
they wrap around, at which point they can find the current location.

Also, I don't understand what's so cryptic about the error message.  As
far as I can tell, GDB's auto-repeat system only auto-repeats if the
last command is repeatable.  So in order to see the error message at all
the last command typed _must_ have been a 'list' command, so why
wouldn't the error message apply to that command?

I guess maybe the cryptic nature comes from, why is GDB trying to list
line number X at all?  But surely that should suggest to the user that
if they don't want to see like X, they should try 'list NN' instead?

Maybe the right thing to do here is provide more helpful text when
reaching the end of the file?  Or maybe extend the 'help list' text to
give more guidance to users on how to return to the current location?

I did see Keith's suggestion for relisting the last few lines.  I think
I'm neutral on that idea, I wouldn't object to it, but the current just
a warning makes it clear GDB has stopped progressing, which I think
could be useful.

Thanks,
Andrew

>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
>  gdb/NEWS                        |  7 +++++++
>  gdb/cli/cli-cmds.c              | 36 +++++++++++++++++++++++++++++++--
>  gdb/doc/gdb.texinfo             |  7 +++++--
>  gdb/source.c                    | 18 +++++++++++++++++
>  gdb/source.h                    |  4 ++++
>  gdb/testsuite/gdb.base/list.exp |  8 ++++----
>  6 files changed, 72 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index fd42864c692..348e73dd15f 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -77,6 +77,13 @@
>    as the array would be printed by the 'print' command.  This
>    functionality is also available for dprintf when dprintf-style is
>    'gdb'.
> + 
> +* 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, and prints the default
> +  location.  Previously, it would error out.  The default location for
> +  this purpose is the last solitary line printed, if there was one,
> +  else the lines around the main function.
>  
>  * New commands
>  
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index b0b9c08c2ec..5973aebfad3 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1244,8 +1244,40 @@ 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] == '+')
> +      /* "l" lists the next few lines, unless we're listing past the end of
> +	 the file.  If it would be past the end, re-print the current line.  */
> +      else if (arg == nullptr)
> +	{
> +	  if (can_print_line (cursal.symtab, cursal.line))
> +	    print_source_lines (cursal.symtab,
> +				source_lines_range (cursal.line), 0);
> +	  else
> +	    {
> +	      warning (_("previous list command has already reached the end "
> +			 "of the file. Listing default location again"));
> +	      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);
> +	    }
> +	}
> +
> +      /* "l +" always lists next few lines.  */
> +      else if (arg[0] == '+')
>  	print_source_lines (cursal.symtab,
>  			    source_lines_range (cursal.line), 0);
>  
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b10c06ae91f..55010f69a1c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9142,9 +9142,12 @@ Print lines centered around the beginning of function
>  @item list
>  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
> +printed; however, if those lines are past the end of the source
> +file, or 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..1aa08c6e080 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1484,6 +1484,24 @@ print_source_lines (struct symtab *s, source_lines_range line_range,
>  			   line_range.stopline (), flags);
>  }
>  
> +/* See source.h.  */
> +
> +bool
> +can_print_line (struct symtab *s, int line)
> +{
> +  const std::vector<off_t> *offset_p;
> +
> +  /* If we can't get the offsets, we definitely can't print the line.  */
> +  if (!g_source_cache.get_line_charpos (s, &offset_p))
> +    return false;
> +  if (offset_p == nullptr)
> +    return false;
> +
> +  /* Otherwise we are able to print LINE if there are at least that many
> +     lines in the symtab.  */
> +  return line <= offset_p->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..ae6764322d0 100644
> --- a/gdb/source.h
> +++ b/gdb/source.h
> @@ -192,6 +192,10 @@ class source_lines_range
>    int m_stopline;
>  };
>  
> +/* 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 18ecd13ac0f..35e099ebaff 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" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \
> +	"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" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \
> +	"list past end of file"
>  }
>  
>  proc_with_prefix test_list_backwards {} {
> -- 
> 2.40.1

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

[Andrew Burgess]

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> Currently, after the user has used the list command once, there is no
> way to ask GDB to print the location where the inferior is stopped.

  (gdb) list *$pc

should do the job.

> 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 throws an
> error.

If you start GDB with an executable, but don't start the executable, and
then just 'list', GDB does list a location.  Could 'list .' not just
replicate this behaviour instead of throwing an error?

>          The test gdb.base/list.exp was updated to test this new argument.
>
> 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.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
>  gdb/NEWS                        |  4 ++++
>  gdb/cli/cli-cmds.c              | 26 ++++++++++++++++++++--
>  gdb/doc/gdb.texinfo             |  5 +++++
>  gdb/testsuite/gdb.base/list.exp | 38 +++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/list1.c  |  2 +-
>  5 files changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 348e73dd15f..d3779c1e953 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -85,6 +85,10 @@
>    this purpose is the last solitary line printed, if there was one,
>    else the lines around the main function.
>  
> +* 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 error out.
> +
>  * New commands
>  
>  maintenance print record-instruction [ N ]
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 5973aebfad3..a966142eaea 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1232,14 +1232,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);
>  	}
> @@ -1293,6 +1293,27 @@ list_command (const char *arg, int from_tty)
>  	  print_source_lines (cursal.symtab, range, 0);
>  	}
>  
> +      /* "l *" lists the default location again.  */

Is 'l *' here a typo?

Thanks,
Andrew

> +      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.  */
> +	      error (_("Inferior is not running. No current location."));
> +	    }
> +	  list_around_line (arg, cursal);
> +	}
> +
>        return;
>      }
>  
> @@ -2800,6 +2821,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 55010f69a1c..e4e374d138c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9151,8 +9151,13 @@ it prints the lines around the function @code{main}.
>  
>  @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.
>  @end table
>  
> +
>  @cindex @code{list}, how many lines to display
>  By default, @value{GDBN} prints ten source lines with any of these forms of
>  the @code{list} command.  You can change this using @code{set listsize}:
> diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp
> index 35e099ebaff..f853a9b814d 100644
> --- a/gdb/testsuite/gdb.base/list.exp
> +++ b/gdb/testsuite/gdb.base/list.exp
> @@ -400,6 +400,41 @@ 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 correctly identifies that the inferior
> +    # isn't running.
> +    gdb_test "list ." "Inferior is not running. No current location." \
> +	"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"
> +}
> +
>  clean_restart
>  gdb_file_cmd ${binfile}
>  
> @@ -519,4 +554,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.40.1

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

[Bruno Larsen]

On 22/06/2023 15:46, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> 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.  If the line that would
>> be printed is past the end of the file, GDB will now warn that the
>> previous list command has reached the end of file, and the current line
>> wil be listed again.
> -1 from me.  I don't like the idea of this auto-wrap around.
>
> I don't feel like this really addresses the original complain anyway.
> The complaint seems to be:
>
>    1. User does 'list' and sees the current location,
>
>    2. User runs "other commands" which scrolls the list contents off the
>    screen,
>
>    3. User does 'list', but is now unsure what the current location is.
>
> The only way this change helps is if the user keeps "list"-ing until
> they wrap around, at which point they can find the current location.
>
> Also, I don't understand what's so cryptic about the error message.  As
> far as I can tell, GDB's auto-repeat system only auto-repeats if the
> last command is repeatable.  So in order to see the error message at all
> the last command typed _must_ have been a 'list' command, so why
> wouldn't the error message apply to that command?
>
> I guess maybe the cryptic nature comes from, why is GDB trying to list
> line number X at all?  But surely that should suggest to the user that
> if they don't want to see like X, they should try 'list NN' instead?
>
> Maybe the right thing to do here is provide more helpful text when
> reaching the end of the file?  Or maybe extend the 'help list' text to
> give more guidance to users on how to return to the current location?
>
> I did see Keith's suggestion for relisting the last few lines.  I think
> I'm neutral on that idea, I wouldn't object to it, but the current just
> a warning makes it clear GDB has stopped progressing, which I think
> could be useful.

I'm reworking the patch already, but I wanted to document my thoughts so 
you have context for the next version.

You hit the nail on the head that it isn't the command erroring out that 
is the problem, but rather that the user will think "why is GDB trying 
to list a line that doesn't exist when I requested the current one?". 
And while it is easy to figure out that "list NN" is the answer, 
figuring out what N should be is not obvious, it either requires that 
the user use some other command to find the current line (such as frame 
or bt 1), or that they know about convenience registers/using registers 
in GDB and use "list *$pc" as you suggested in patch 3 of this series. I 
dont think we should tie functionality of one command to knowledge of 
unrelated things.

So a reasonable way to meet you in the middle, I think, is to have the 
error for the no-arg invocation of list continue erroring out, but 
reference using "list ." to go back to the current location in the 
warning message. Does that sound reasonable?

-- 
Cheers,
Bruno

> Thanks,
> Andrew
>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30497
>> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>> ---
>>   gdb/NEWS                        |  7 +++++++
>>   gdb/cli/cli-cmds.c              | 36 +++++++++++++++++++++++++++++++--
>>   gdb/doc/gdb.texinfo             |  7 +++++--
>>   gdb/source.c                    | 18 +++++++++++++++++
>>   gdb/source.h                    |  4 ++++
>>   gdb/testsuite/gdb.base/list.exp |  8 ++++----
>>   6 files changed, 72 insertions(+), 8 deletions(-)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index fd42864c692..348e73dd15f 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -77,6 +77,13 @@
>>     as the array would be printed by the 'print' command.  This
>>     functionality is also available for dprintf when dprintf-style is
>>     'gdb'.
>> +
>> +* 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, and prints the default
>> +  location.  Previously, it would error out.  The default location for
>> +  this purpose is the last solitary line printed, if there was one,
>> +  else the lines around the main function.
>>   
>>   * New commands
>>   
>> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
>> index b0b9c08c2ec..5973aebfad3 100644
>> --- a/gdb/cli/cli-cmds.c
>> +++ b/gdb/cli/cli-cmds.c
>> @@ -1244,8 +1244,40 @@ 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] == '+')
>> +      /* "l" lists the next few lines, unless we're listing past the end of
>> +	 the file.  If it would be past the end, re-print the current line.  */
>> +      else if (arg == nullptr)
>> +	{
>> +	  if (can_print_line (cursal.symtab, cursal.line))
>> +	    print_source_lines (cursal.symtab,
>> +				source_lines_range (cursal.line), 0);
>> +	  else
>> +	    {
>> +	      warning (_("previous list command has already reached the end "
>> +			 "of the file. Listing default location again"));
>> +	      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);
>> +	    }
>> +	}
>> +
>> +      /* "l +" always lists next few lines.  */
>> +      else if (arg[0] == '+')
>>   	print_source_lines (cursal.symtab,
>>   			    source_lines_range (cursal.line), 0);
>>   
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index b10c06ae91f..55010f69a1c 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -9142,9 +9142,12 @@ Print lines centered around the beginning of function
>>   @item list
>>   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
>> +printed; however, if those lines are past the end of the source
>> +file, or 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..1aa08c6e080 100644
>> --- a/gdb/source.c
>> +++ b/gdb/source.c
>> @@ -1484,6 +1484,24 @@ print_source_lines (struct symtab *s, source_lines_range line_range,
>>   			   line_range.stopline (), flags);
>>   }
>>   
>> +/* See source.h.  */
>> +
>> +bool
>> +can_print_line (struct symtab *s, int line)
>> +{
>> +  const std::vector<off_t> *offset_p;
>> +
>> +  /* If we can't get the offsets, we definitely can't print the line.  */
>> +  if (!g_source_cache.get_line_charpos (s, &offset_p))
>> +    return false;
>> +  if (offset_p == nullptr)
>> +    return false;
>> +
>> +  /* Otherwise we are able to print LINE if there are at least that many
>> +     lines in the symtab.  */
>> +  return line <= offset_p->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..ae6764322d0 100644
>> --- a/gdb/source.h
>> +++ b/gdb/source.h
>> @@ -192,6 +192,10 @@ class source_lines_range
>>     int m_stopline;
>>   };
>>   
>> +/* 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 18ecd13ac0f..35e099ebaff 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" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \
>> +	"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" "warning: previous list command has already reached the end of the file. Listing default location again.*1\[ \t\]+#include \"list0.h\".*" \
>> +	"list past end of file"
>>   }
>>   
>>   proc_with_prefix test_list_backwards {} {
>> -- 
>> 2.40.1