[v1,1/1] list actual code around more than one locations

Message ID 968c6de4-789d-25f8-7bfc-71a810e26f17@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Aug. 22, 2017, 4 p.m. UTC
  Hi,

Thanks for the patch!  I pushed it in with a couple minor
tweaks.  See below.

On 07/19/2017 05:05 AM, Zhouyi Zhou wrote:

>    else if (no_end)
>      {
>        int first_line = sal.line - get_lines_to_list () / 2;
> +      int i;
>  
>        if (first_line < 1) first_line = 1;
> +      
> +      if (sals.nelts > 1)
> +        {
> +          printf_filtered (_("file: \"%s\", line number: %d\n"),
> +			   symtab_to_filename_for_display (sal.symtab),
> +			   sal.line);
> +        }
>  
>        print_source_lines (sal.symtab,
>  		          first_line,
>  			  first_line + get_lines_to_list (),
>  			  0);
> +      if (sals.nelts > 1)
> +        {
> +          for (i = 1; i < sals.nelts; i++)
> +            {
> +              sal = sals.sals[i];
> +              first_line = sal.line - get_lines_to_list () / 2;
> +              if (first_line < 1) first_line = 1;
> +              printf_filtered (_("file: \"%s\", line number: %d\n"),
> +			       symtab_to_filename_for_display (sal.symtab),
> +			       sal.line);
> +              print_source_lines (sal.symtab,
> +                                  first_line,
> +                                  first_line + get_lines_to_list (),
> +                                  0);
> +            }

We can avoid the nelts == 1 and nelts > 1 duplication by simply
letting the nelts == 1 case be handled by the loop as well.

> +        }
>      }
>    else
>      print_source_lines (sal.symtab, sal.line,
> @@ -1120,6 +1150,8 @@ list_command (char *arg, int from_tty)
>  			 ? sal.line + get_lines_to_list ()
>  			 : sal_end.line + 1),
>  			0);
> +  if (sals.nelts)
> +    xfree (sals.sals);

Also fixed spaces vs tabs throughout, and initialized
sals.sals to avoid all the 'if (sals.nelts)'.  

Note that only really old code uses explicit xfree calls on
exits paths like these, since they're not exception-safe in case
some code between the xmalloc and the xfree throws an exception.
Before GDB's C++ification, that used to be solved with cleanups
(grep for make_cleanup).  However, we're trying to get rid of
cleanups nowadays, in favor of C++ RAII.  That's why I
sent in this series yesterday:

 https://sourceware.org/ml/gdb-patches/2017-08/msg00409.html

However, since this "list" code was already calling xfree
explicitly like that, I though that putting in the patch as is
isn't really making things worse.

Also, you don't seen to have a a copyright assignment for GDB
on file.  You have one for GCC, but that doesn't cover GDB.
The patch is borderline small enough to push in without a GDB
assignment, IMO, particularly if we consider that the boilerplace
xfree calls will be removed soon enough.  In order to be able
accept further patches from you, we'll need that sorted out,
though.  Please feel free to contact me off list if you need
assistance with that.

Thanks again for your contribution!

Pedro Alves

From 0d999a6ef0f98b22430d70951408869864c979e0 Mon Sep 17 00:00:00 2001
From: Zhouyi Zhou <zhouzhouyi@gmail.com>
Date: Tue, 22 Aug 2017 15:32:19 +0100
Subject: [PATCH] List actual code around more than one location

With the following C++ code:
 int bar() { return 0;}
 int bar(int) { return 0; }

GDB behaves as:
 (gdb) list bar
  file: "overload.cc", line number: 1
  file: "overload.cc", line number: 2

It would be better for GDB to list the actual code around those two
locations, not just print the location.  Like:

 (gdb) list bar
 file: "overload.cc", line number: 1
 1       int bar() { return 0;}
 2       int bar(int) { return 0; }
 file: "overload.cc", line number: 2
 1       int bar() { return 0;}
 2       int bar(int) { return 0; }

That's what this this commit implements.

Tested on x86-64 GNU/Linux.

gdb/ChangeLog:
2017-08-22  Zhouyi Zhou  <zhouzhouyi@gmail.com>

	* cli-cmds.c (list_commands): List actual code around more than
	one location.
---
 gdb/ChangeLog      |  5 +++++
 gdb/cli/cli-cmds.c | 47 +++++++++++++++++++++++++++++++----------------
 2 files changed, 36 insertions(+), 16 deletions(-)
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 74506f8..0908b99 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2017-08-22  Zhouyi Zhou  <zhouzhouyi@gmail.com>
+
+	* cli-cmds.c (list_commands): List actual code around more than
+	one location.
+
 2017-08-21  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-nat.c (fbsd_add_threads): Use array type for `lwps'.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 3fa2499..d4dc539 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -955,6 +955,8 @@  list_command (char *arg, int from_tty)
   if (!have_full_symbols () && !have_partial_symbols ())
     error (_("No symbol table is loaded.  Use the \"file\" command."));
 
+  sals.nelts = 0;
+  sals.sals = NULL;
   arg1 = arg;
   if (*arg1 == ',')
     dummy_beg = 1;
@@ -971,15 +973,8 @@  list_command (char *arg, int from_tty)
 	  /*  C++  */
 	  return;
 	}
-      if (sals.nelts > 1)
-	{
-	  ambiguous_line_spec (&sals);
-	  xfree (sals.sals);
-	  return;
-	}
 
       sal = sals.sals[0];
-      xfree (sals.sals);
     }
 
   /* Record whether the BEG arg is all digits.  */
@@ -992,6 +987,12 @@  list_command (char *arg, int from_tty)
   if (*arg1 == ',')
     {
       no_end = 0;
+      if (sals.nelts > 1)
+	{
+	  ambiguous_line_spec (&sals);
+	  xfree (sals.sals);
+	  return;
+	}
       arg1++;
       while (*arg1 == ' ' || *arg1 == '\t')
 	arg1++;
@@ -1010,11 +1011,15 @@  list_command (char *arg, int from_tty)
 
 	  filter_sals (&sals_end);
 	  if (sals_end.nelts == 0)
-	    return;
+	    {
+	      xfree (sals.sals);
+	      return;
+	    }
 	  if (sals_end.nelts > 1)
 	    {
 	      ambiguous_line_spec (&sals_end);
 	      xfree (sals_end.sals);
+	      xfree (sals.sals);
 	      return;
 	    }
 	  sal_end = sals_end.sals[0];
@@ -1080,14 +1085,23 @@  list_command (char *arg, int from_tty)
     error (_("No default source file yet.  Do \"help list\"."));
   else if (no_end)
     {
-      int first_line = sal.line - get_lines_to_list () / 2;
-
-      if (first_line < 1) first_line = 1;
-
-      print_source_lines (sal.symtab,
-		          first_line,
-			  first_line + get_lines_to_list (),
-			  0);
+      for (int i = 0; i < sals.nelts; i++)
+	{
+	  sal = sals.sals[i];
+	  int first_line = sal.line - get_lines_to_list () / 2;
+	  if (first_line < 1)
+	    first_line = 1;
+	  if (sals.nelts > 1)
+	    {
+	      printf_filtered (_("file: \"%s\", line number: %d\n"),
+			       symtab_to_filename_for_display (sal.symtab),
+			       sal.line);
+	    }
+	  print_source_lines (sal.symtab,
+			      first_line,
+			      first_line + get_lines_to_list (),
+			      0);
+	}
     }
   else
     print_source_lines (sal.symtab, sal.line,
@@ -1095,6 +1109,7 @@  list_command (char *arg, int from_tty)
 			 ? sal.line + get_lines_to_list ()
 			 : sal_end.line + 1),
 			0);
+  xfree (sals.sals);
 }
 
 /* Subroutine of disassemble_command to simplify it.