Re: [PATCH] gdb/testsuite: make gdb.base/list-nodebug.exp pass without libc symbols

[Simon Marchi <simark@simark.ca>]

On 1/31/24 09:49, Guinevere Larsen wrote:
> On 30/01/2024 20:51, Simon Marchi wrote:
>> On 1/30/24 11:48, Guinevere Larsen wrote:
>> What I was wondering when looking at it the other day is: why do we
>> need to call get_current_source_symtab_and_line at all in a case
>> where we don't care about the "current" location?
> 
> "current" is a poorly defined term in this context. what the `.`
> argument means with "current" is point of execution of the inferior,
> what get_current_source_symtab_and_line means by "current" is the
> location that is being printed.
> 
> I say this based on the fact that list with no arguments or with '+'
> use this "current" sal to continue printing the file.

Right, but "list ." does not the "current sal".  I said "why do we need
to call get_current_source_symtab_and_line", but in fact it's
"set_default_source_symtab_and_line" that throws the exception, so it's
"set_default_source_symtab_and_line" that we should avoid calling if we
are not going to use it.

>> Early in the function, we have:
>>
>>    symtab_and_line cursal = get_current_source_symtab_and_line ();
>>
>> But in the `else if (arg[0] == '.')` portion of the function, it
>> looks like we override `cursal` with something else.  So, it is
>> possible to call get_current_source_symtab_and_line only in the
>> scopes that actually require it?
> 
> As I mentioned, this gets used by no arg and '+', but I'm not sure if
> it is necessary. We can define cursal only when first printing and for
> '.', using get_last_line_printed for the rest.
> 
> That's besides the point, the important part is that this is the only
> location (other than '.' recently added) that is able to tell if the
> inferior has debuginfo, so we have to handle its error somewhere if we
> want to have a single message for all list errors.

I wrote the patch below to illustrate what I mean.  With it, I at least
get the same message for "list ." on the list-nodebug executable,
regardless of if I have libc debug symbols or not:

    (gdb) list .
    No debug information available to print source lines at current PC (0x55555555511d).

It is still different than if I use "list +", with no debug symbols:

    (gdb) list +
    No symbol table is loaded.  Use the "file" command.

Perhaps that's still not ideal, I don't know, but I think it's already
better than having two different messages for "list .", based on some
unrelated variable (the presence of some debug info for another part of
the program).

Simon

From 292d16f94423758bb08cd14209941a92218878c0 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 31 Jan 2024 14:18:59 -0500
Subject: [PATCH] Test change to list_command

Change-Id: Ia536108a0c69d105cb3c6868c53afb12bb9c4ba9
---
 gdb/cli/cli-cmds.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index df11f956245c..1df632594e8e 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1236,37 +1236,39 @@ list_command (const char *arg, int from_tty)
   /* Pull in the current default source line if necessary.  */
   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 && (arg == nullptr || arg[0] != '.'))
 	{
-	  list_around_line (arg, cursal);
+	  set_default_source_symtab_and_line ();
+	  list_around_line (arg, get_current_source_symtab_and_line ());
 	}

       /* "l" and "l +" lists the next few lines, unless we're listing past
 	 the end of the file.  */
       else if (arg == nullptr || arg[0] == '+')
 	{
+	  set_default_source_symtab_and_line ();
+	  const symtab_and_line cursal = get_current_source_symtab_and_line ();
 	  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"));
-	    }
+	    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.  */
       else if (arg[0] == '-')
 	{
+	  set_default_source_symtab_and_line ();
+	  const symtab_and_line cursal = get_current_source_symtab_and_line ();
+
 	  if (get_first_line_listed () == 1)
 	    error (_("Already at the start of %s."),
 		   symtab_to_filename_for_display (cursal.symtab));
+
 	  source_lines_range range (get_first_line_listed (),
 				    source_lines_range::BACKWARD);
 	  print_source_lines (cursal.symtab, range, 0);
@@ -1275,13 +1277,19 @@ list_command (const char *arg, int from_tty)
       /* "list ." lists the default location again.  */
       else if (arg[0] == '.')
 	{
+	  std::optional<const symtab_and_line> cursal;
 	  if (target_has_stack ())
 	    {
 	      /* 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);
+	      cursal.emplace (find_pc_line (curr_pc, 0));
+
+	      if (cursal->symtab == nullptr)
+		error
+		  (_("No debug information available to print source lines at current PC (%s)."),
+		   paddress (get_frame_arch (frame), curr_pc));
 	    }
 	  else
 	    {
@@ -1289,11 +1297,14 @@ list_command (const char *arg, int from_tty)
 		 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 ();
+	      cursal.emplace (get_current_source_symtab_and_line ());
+
+	      // Not sure if always true, just guessing.
+	      gdb_assert (cursal->symtab != nullptr);
 	    }
-	  if (cursal.symtab == nullptr)
-	    error (_("No debug information available to print source lines."));
-	  list_around_line (arg, cursal);
+
+	  list_around_line (arg, *cursal);
+
 	  /* Set the repeat args so just pressing "enter" after using "list ."
 	     will print the following lines instead of the same lines again. */
 	  if (from_tty)

base-commit: 249e54204b13f9acdd5fbca355fed305e8595f31
-- 
2.43.0