* [PATCH v1 1/1] list actual code around more than one locations
@ 2017-07-19 4:05 Zhouyi Zhou
2017-08-22 16:00 ` Pedro Alves
0 siblings, 1 reply; 3+ messages in thread
From: Zhouyi Zhou @ 2017-07-19 4:05 UTC (permalink / raw)
To: gdb-patches, palves; +Cc: Zhouyi Zhou
When debugging 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
However, it would be better for gdb to list the actual code around those
two locations, not just print the location.
Under the mentorship of Pedro Alves, I modify the function list_command
so that GDB behaves more instructive:
(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; }
Tested on x86-64 GNU/Linux.
Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
gdb/ChangeLog:
2017-07-19 Zhouyi Zhou <zhouzhouyi@gmail.com>
* cli-cmds.c (list_commands): list actual code around more than one
locations.
---
gdb/cli/cli-cmds.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 8 deletions(-)
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index af750d3..87a89e2 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -980,6 +980,7 @@ 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;
arg1 = arg;
if (*arg1 == ',')
dummy_beg = 1;
@@ -996,15 +997,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. */
@@ -1017,6 +1011,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++;
@@ -1035,11 +1035,17 @@ list_command (char *arg, int from_tty)
filter_sals (&sals_end);
if (sals_end.nelts == 0)
- return;
+ {
+ if (sals.nelts)
+ xfree (sals.sals);
+ return;
+ }
if (sals_end.nelts > 1)
{
ambiguous_line_spec (&sals_end);
xfree (sals_end.sals);
+ if (sals.nelts)
+ xfree (sals.sals);
return;
}
sal_end = sals_end.sals[0];
@@ -1106,13 +1112,37 @@ list_command (char *arg, int from_tty)
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);
+ }
+ }
}
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);
}
/* Subroutine of disassemble_command to simplify it.
--
1.9.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1] list actual code around more than one locations
2017-07-19 4:05 [PATCH v1 1/1] list actual code around more than one locations Zhouyi Zhou
@ 2017-08-22 16:00 ` Pedro Alves
2017-08-22 16:07 ` [pushed] Add test for "List actual code around more than one location" change (Re: [PATCH v1 1/1] list actual code around more than one locations) Pedro Alves
0 siblings, 1 reply; 3+ messages in thread
From: Pedro Alves @ 2017-08-22 16:00 UTC (permalink / raw)
To: Zhouyi Zhou, gdb-patches
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(-)
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.
--
2.5.5
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pushed] Add test for "List actual code around more than one location" change (Re: [PATCH v1 1/1] list actual code around more than one locations)
2017-08-22 16:00 ` Pedro Alves
@ 2017-08-22 16:07 ` Pedro Alves
0 siblings, 0 replies; 3+ messages in thread
From: Pedro Alves @ 2017-08-22 16:07 UTC (permalink / raw)
To: Zhouyi Zhou, gdb-patches
On 08/22/2017 05:00 PM, Pedro Alves wrote:
> Thanks for the patch! I pushed it in with a couple minor
> tweaks. See below.
FYI, I pushed in this testsuite patch to cover the new functionality.
From 5277199aeb328247d5d37ad6f34e4cf200fe42fa Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 22 Aug 2017 14:51:18 +0100
Subject: [PATCH] Add test for "List actual code around more than one location"
change
This adds a test for the "list" command change done in 0d999a6ef0f9
("List actual code around more than one location").
gdb/ChangeLog:
2017-08-22 Pedro Alves <palves@redhat.com>
* gdb.cp/overload.exp (line_range_pattern): New procedure.
(top level): Add "list all overloads" tests.
---
gdb/testsuite/ChangeLog | 5 +++++
gdb/testsuite/gdb.cp/overload.exp | 34 +++++++++++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 2be26a7..d2ccd84 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-08-22 Pedro Alves <palves@redhat.com>
+
+ * gdb.cp/overload.exp (line_range_pattern): New procedure.
+ (top level): Add "list all overloads" tests.
+
2017-08-22 Tom Tromey <tom@tromey.com>
* gdb.gdb/xfullpath.exp: Remove.
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index 77a2505..8cb9311 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -263,10 +263,24 @@ gdb_test "print bar(d)" "= 22"
# List overloaded functions.
+gdb_test_no_output "set listsize 1" ""
+
+# Build source listing pattern based on an inclusive line range.
+
+proc line_range_pattern { range_start range_end } {
+ global line_re
+
+ for {set i $range_start} {$i <= $range_end} {incr i} {
+ append pattern "\r\n$i\[ \t\]\[^\r\n\]*"
+ }
+
+ verbose -log "pattern $pattern"
+ return $pattern
+}
+
# The void case is tricky because some compilers say "(void)"
# and some compilers say "()".
-gdb_test_no_output "set listsize 1" ""
gdb_test_multiple "info func overloadfnarg" "list overloaded function with no args" {
-re ".*overloadfnarg\\(void\\).*$gdb_prompt $" {
# gcc 2
@@ -324,6 +338,24 @@ gdb_test "print N::nsoverload ()" " = 1"
gdb_test "print N::nsoverload (2)" " = 2"
gdb_test "print N::nsoverload (2, 3)" " = 5"
+# Test "list function" when there are multiple "function" overloads.
+
+with_test_prefix "list all overloads" {
+ # Bump up listsize again, to make sure the number of lines to
+ # display before/after each location is computed correctly.
+ gdb_test_no_output "set listsize 10"
+
+ set line_bar_A [gdb_get_line_number "int bar (A)"]
+ set line_bar_B [gdb_get_line_number "int bar (B)"]
+ set lines1 [line_range_pattern [expr $line_bar_A - 5] [expr $line_bar_A + 4]]
+ set lines2 [line_range_pattern [expr $line_bar_B - 5] [expr $line_bar_B + 4]]
+
+ set any "\[^\r\n\]*"
+ set h1_re "file: \"${any}overload.cc\", line number: $line_bar_A"
+ set h2_re "file: \"${any}overload.cc\", line number: $line_bar_B"
+ gdb_test "list bar" "${h1_re}${lines1}\r\n${h2_re}${lines2}"
+}
+
if ![runto 'XXX::marker2'] then {
perror "couldn't run to XXX::marker2"
continue
--
2.5.5
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-22 16:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-19 4:05 [PATCH v1 1/1] list actual code around more than one locations Zhouyi Zhou
2017-08-22 16:00 ` Pedro Alves
2017-08-22 16:07 ` [pushed] Add test for "List actual code around more than one location" change (Re: [PATCH v1 1/1] list actual code around more than one locations) Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).