gdb: Change "list ." command's error when no debuginfo is available

Message ID 20240213143624.140852-1-blarsen@redhat.com
State New
Headers
Series gdb: Change "list ." command's error when no debuginfo is available |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Guinevere Larsen Feb. 13, 2024, 2:36 p.m. UTC
  From: Simon Marchi <simark@simark.ca>

Currently, when a user tries to list the current location, there are 2
different error messages that can happen, either:

    (gdb) list .
    No symbol table is loaded.  Use the "file" command.
or
    (gdb) list .
    No debug information available to print source lines.

The difference here is if gdb can find any symtabs at all or not, which
is not a difference a user should concern themselves with. This commit
changes it so that the error is always:

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

The difference now is if the inferior has started already, which is
controlled by the user and may be useful.

Unfortunately, it isn't as easy to differentiate if the symtab found for
other list parameters is correct, so other invocations, such as "list +"
still retain their original error message.

Co-Authored-By: Simon Marchi <simark@simark.ca>
---
 gdb/cli/cli-cmds.c | 48 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 14 deletions(-)
  

Comments

Guinevere Larsen March 6, 2024, 12:59 p.m. UTC | #1
On 13/02/2024 15:36, Guinevere Larsen wrote:
> From: Simon Marchi <simark@simark.ca>
>
> Currently, when a user tries to list the current location, there are 2
> different error messages that can happen, either:
>
>      (gdb) list .
>      No symbol table is loaded.  Use the "file" command.
> or
>      (gdb) list .
>      No debug information available to print source lines.
>
> The difference here is if gdb can find any symtabs at all or not, which
> is not a difference a user should concern themselves with. This commit
> changes it so that the error is always:
>
>      (gdb) list .
>      No debug information available to print source lines at current PC (0x55555555511d).
> or
>      (gdb) list .
>      No debug information available to print source lines at default location.
>
> The difference now is if the inferior has started already, which is
> controlled by the user and may be useful.
>
> Unfortunately, it isn't as easy to differentiate if the symtab found for
> other list parameters is correct, so other invocations, such as "list +"
> still retain their original error message.
>
> Co-Authored-By: Simon Marchi <simark@simark.ca>

Ping!

I apparently made Simon both the author and co-author oops. One of those 
2 would have my info instead, I don't really mind which.
  
Andrew Burgess March 6, 2024, 1:18 p.m. UTC | #2
Guinevere Larsen <blarsen@redhat.com> writes:

> From: Simon Marchi <simark@simark.ca>
>
> Currently, when a user tries to list the current location, there are 2
> different error messages that can happen, either:
>
>     (gdb) list .
>     No symbol table is loaded.  Use the "file" command.
> or
>     (gdb) list .
>     No debug information available to print source lines.
>
> The difference here is if gdb can find any symtabs at all or not, which
> is not a difference a user should concern themselves with.

Why not?

Maybe "No symbol table is loaded" isn't the clearest, maybe "No debug
information is loaded" might be better, but the first message tells me
that there is no debug information, maybe GDB failed to find it, or
maybe I need to provide it.  The second tells me that GDB did find debug
information that covers this address, but it lacks the required detail
to help, maybe I forgot -g when compiling?

I'm not 100% against this change, I just don't find the motivation super
compelling.

>                                                             This commit
> changes it so that the error is always:
>
>     (gdb) list .
>     No debug information available to print source lines at current PC (0x55555555511d).
> or
>     (gdb) list .
>     No debug information available to print source lines at default location.
>
> The difference now is if the inferior has started already, which is
> controlled by the user and may be useful.
>
> Unfortunately, it isn't as easy to differentiate if the symtab found for
> other list parameters is correct, so other invocations, such as "list +"
> still retain their original error message.
>
> Co-Authored-By: Simon Marchi <simark@simark.ca>
> ---
>  gdb/cli/cli-cmds.c | 48 ++++++++++++++++++++++++++++++++--------------

For sure this patch should have an associated test change.

Thanks,
Andrew

>  1 file changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index df11f956245..31f0b6d9907 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,25 +1277,43 @@ 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
>  	    {
>  	      /* 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 ();
> +	      try
> +		{
> +		  set_default_source_symtab_and_line ();
> +		}
> +	      catch (const gdb_exception &e)
> +		{
> +		  error (_("No debug information available to print source "
> +			   "lines at default location"));
> +		}
> +	      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)
> -- 
> 2.43.0
  
Guinevere Larsen March 6, 2024, 1:38 p.m. UTC | #3
On 06/03/2024 14:18, Andrew Burgess wrote:
> Guinevere Larsen <blarsen@redhat.com> writes:
>
>> From: Simon Marchi <simark@simark.ca>
>>
>> Currently, when a user tries to list the current location, there are 2
>> different error messages that can happen, either:
>>
>>      (gdb) list .
>>      No symbol table is loaded.  Use the "file" command.
>> or
>>      (gdb) list .
>>      No debug information available to print source lines.
>>
>> The difference here is if gdb can find any symtabs at all or not, which
>> is not a difference a user should concern themselves with.
> Why not?
>
> Maybe "No symbol table is loaded" isn't the clearest, maybe "No debug
> information is loaded" might be better, but the first message tells me
> that there is no debug information, maybe GDB failed to find it, or
> maybe I need to provide it.  The second tells me that GDB did find debug
> information that covers this address, but it lacks the required detail
> to help, maybe I forgot -g when compiling?

The situation where I see the second message pops up is when we're, for 
example, in the main function and there are symbols for libc. I may be 
misunderstanding something, but I don't think that debug information 
covers the address the inferior is in.

The situation that we were specifically thinking about was that

>
> I'm not 100% against this change, I just don't find the motivation super
> compelling.
>
>>                                                              This commit
>> changes it so that the error is always:
>>
>>      (gdb) list .
>>      No debug information available to print source lines at current PC (0x55555555511d).
>> or
>>      (gdb) list .
>>      No debug information available to print source lines at default location.
>>
>> The difference now is if the inferior has started already, which is
>> controlled by the user and may be useful.
>>
>> Unfortunately, it isn't as easy to differentiate if the symtab found for
>> other list parameters is correct, so other invocations, such as "list +"
>> still retain their original error message.
>>
>> Co-Authored-By: Simon Marchi <simark@simark.ca>
>> ---
>>   gdb/cli/cli-cmds.c | 48 ++++++++++++++++++++++++++++++++--------------
> For sure this patch should have an associated test change.
That's a good point... Do you know how I'd be able to get a test running 
with no symbols whatsoever, even if the system that is doing the testing 
has libc symbols?
  
Andrew Burgess March 11, 2024, 10:55 a.m. UTC | #4
Guinevere Larsen <blarsen@redhat.com> writes:

> On 06/03/2024 14:18, Andrew Burgess wrote:
>> Guinevere Larsen <blarsen@redhat.com> writes:
>>
>>> From: Simon Marchi <simark@simark.ca>
>>>
>>> Currently, when a user tries to list the current location, there are 2
>>> different error messages that can happen, either:
>>>
>>>      (gdb) list .
>>>      No symbol table is loaded.  Use the "file" command.
>>> or
>>>      (gdb) list .
>>>      No debug information available to print source lines.
>>>
>>> The difference here is if gdb can find any symtabs at all or not, which
>>> is not a difference a user should concern themselves with.
>> Why not?
>>
>> Maybe "No symbol table is loaded" isn't the clearest, maybe "No debug
>> information is loaded" might be better, but the first message tells me
>> that there is no debug information, maybe GDB failed to find it, or
>> maybe I need to provide it.  The second tells me that GDB did find debug
>> information that covers this address, but it lacks the required detail
>> to help, maybe I forgot -g when compiling?
>
> The situation where I see the second message pops up is when we're, for 
> example, in the main function and there are symbols for libc. I may be 
> misunderstanding something, but I don't think that debug information 
> covers the address the inferior is in.
>
> The situation that we were specifically thinking about was that
>
>>
>> I'm not 100% against this change, I just don't find the motivation super
>> compelling.
>>
>>>                                                              This commit
>>> changes it so that the error is always:
>>>
>>>      (gdb) list .
>>>      No debug information available to print source lines at current PC (0x55555555511d).
>>> or
>>>      (gdb) list .
>>>      No debug information available to print source lines at default location.
>>>
>>> The difference now is if the inferior has started already, which is
>>> controlled by the user and may be useful.
>>>
>>> Unfortunately, it isn't as easy to differentiate if the symtab found for
>>> other list parameters is correct, so other invocations, such as "list +"
>>> still retain their original error message.
>>>
>>> Co-Authored-By: Simon Marchi <simark@simark.ca>
>>> ---
>>>   gdb/cli/cli-cmds.c | 48 ++++++++++++++++++++++++++++++++--------------
>> For sure this patch should have an associated test change.
> That's a good point... Do you know how I'd be able to get a test running 
> with no symbols whatsoever, even if the system that is doing the testing 
> has libc symbols?

Could you compile a test binary statically, and then strip all debug
information from it?  Would that do what you need?

Thanks,
Andrew



>
> -- 
> Cheers,
> Guinevere Larsen
> She/Her/Hers
>
>>
>> Thanks,
>> Andrew
>>
>>>   1 file changed, 34 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
>>> index df11f956245..31f0b6d9907 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,25 +1277,43 @@ 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
>>>   	    {
>>>   	      /* 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 ();
>>> +	      try
>>> +		{
>>> +		  set_default_source_symtab_and_line ();
>>> +		}
>>> +	      catch (const gdb_exception &e)
>>> +		{
>>> +		  error (_("No debug information available to print source "
>>> +			   "lines at default location"));
>>> +		}
>>> +	      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)
>>> -- 
>>> 2.43.0
  

Patch

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index df11f956245..31f0b6d9907 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,25 +1277,43 @@  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
 	    {
 	      /* 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 ();
+	      try
+		{
+		  set_default_source_symtab_and_line ();
+		}
+	      catch (const gdb_exception &e)
+		{
+		  error (_("No debug information available to print source "
+			   "lines at default location"));
+		}
+	      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)