* [PATCH] Use containing function when reporting breakpoint location.
@ 2017-03-10 20:59 Keith Seitz
2017-03-20 18:22 ` Jan Kratochvil
0 siblings, 1 reply; 4+ messages in thread
From: Keith Seitz @ 2017-03-10 20:59 UTC (permalink / raw)
To: gdb-patches
print_breakpoint_location currently uses find_pc_sect_function, which always
returns the linkage symbol. This essentially means that it skips over any
inline frames. Thus, when a breakpoint is set in an inlined function,
"info break" may report the calling function as the location of the
breakpoint.
While pedantically true, it is a little misleading to users:
(gdb) b inline_func
Breakpoint 1 at 0x400434: file test.c, line 5.
(gdb) inf br
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000400434 in main at test.c:5
With this patch, print_breakpoint_location now calls a new variation of
find_pc_sect_function, find_pc_sect_containing_function, which does not
skip over inlined functions, and the breakpoint location is now reported
"correctly":
(gdb) inf br
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000400434 in inline_func at test.c:5
gdb/ChangeLog
* blockframe.c (find_pc_sect_containing_function): New function.
* breakpoint.c (print_breakpoint_location): Use
find_pc_sect_containing_function to avoid skipping inline frames.
* symtab.h (find_pc_sect_containing_function): Declare.
gdb/testsuite/ChangeLog
* gdb.opt/inline-break.exp (break_info_1): New procedure.
Use break_info_1 to test "info break" for all breakpoints.
---
gdb/ChangeLog | 7 ++++
gdb/blockframe.c | 12 ++++++
gdb/breakpoint.c | 2 +-
gdb/symtab.h | 7 ++++
gdb/testsuite/ChangeLog | 5 +++
gdb/testsuite/gdb.opt/inline-break.exp | 75 ++++++++++++++++++++++++++++++++++
6 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e4c4432..87dc913 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-MM-DD Keith Seitz <keiths@redhat.com>
+
+ * blockframe.c (find_pc_sect_containing_function): New function.
+ * breakpoint.c (print_breakpoint_location): Use
+ find_pc_sect_containing_function to avoid skipping inline frames.
+ * symtab.h (find_pc_sect_containing_function): Declare.
+
2017-03-10 Keith Seitz <keiths@redhat.com>
PR c++/8218
diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 5ba993c..c602d6b 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -142,6 +142,18 @@ find_pc_sect_function (CORE_ADDR pc, struct obj_section *section)
return block_linkage_function (b);
}
+/* See description in symtab.h. */
+
+struct symbol *
+find_pc_sect_containing_function (CORE_ADDR pc, struct obj_section *section)
+{
+ const struct block *b = block_for_pc_sect (pc, section);
+
+ if (b == NULL)
+ return NULL;
+ return block_containing_function (b);
+}
+
/* Return the function containing pc value PC.
Returns 0 if function is not known.
Backward compatibility, no section */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ab6e9c8..c470f96 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6154,7 +6154,7 @@ print_breakpoint_location (struct breakpoint *b,
else if (loc && loc->symtab)
{
struct symbol *sym
- = find_pc_sect_function (loc->address, loc->section);
+ = find_pc_sect_containing_function (loc->address, loc->section);
if (sym)
{
uiout->text ("in ");
diff --git a/gdb/symtab.h b/gdb/symtab.h
index d8c665c..b16cb18 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1336,6 +1336,13 @@ extern struct symbol *find_pc_function (CORE_ADDR);
extern struct symbol *find_pc_sect_function (CORE_ADDR, struct obj_section *);
+/* Return the function containing pc value PC in section SECTION.
+ Returns NULL if function is not known. Unlike find_pc_sect_function,
+ this function does not skip inline frames. */
+
+extern struct symbol *find_pc_sect_containing_function (CORE_ADDR,
+ struct obj_section *);
+
extern int find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
CORE_ADDR *address,
CORE_ADDR *endaddr,
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0718d76..b32136c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-MM-DD Keith Seitz <keiths@redhat.com>
+
+ * gdb.opt/inline-break.exp (break_info_1): New procedure.
+ Use break_info_1 to test "info break" for all breakpoints.
+
2017-03-10 Keith Seitz <keiths@redhat.com>
PR c++/8128
diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp
index 7be3a34..ff15f1a 100644
--- a/gdb/testsuite/gdb.opt/inline-break.exp
+++ b/gdb/testsuite/gdb.opt/inline-break.exp
@@ -24,6 +24,61 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
return -1
}
+# Return a string that may be used to match the output of "info break NUM".
+#
+# Optional arguments:
+#
+# source - the name of the source file
+# func - the name of the function
+# disp - the breakpoint disposition
+# enabled - breakpoint enable state
+# locs - number of locations
+# line - source line number (ignored without -source)
+
+proc break_info_1 {num args} {
+ global decimal
+
+ # Column delimiter
+ set c {[\t ]+}
+
+ # Row delimiter
+ set end {[\r\n \t]+}
+
+ # Table header
+ set header "[join [list Num Type Disp Enb Address What] ${c}]"
+
+ # Get/configure any optional parameters.
+ parse_args [list {source ""} {func ".*"} {disp "keep"} \
+ {enabled "y"} {locs 1} [list line $decimal]]
+
+ if {$source != ""} {
+ set source "/$source:$line"
+ }
+
+ # Result starts with the standard header.
+ set result "$header${end}"
+
+ # Set up for multi-location breakpoint marker.
+ if {$locs == 1} {
+ set multi ".*"
+ } else {
+ set multi "<MULTIPLE>${end}"
+ }
+ append result "[join [list $num breakpoint $disp $enabled $multi] $c]"
+
+ # Add sub-location info.
+ if {$locs > 1} {
+ for {set i 1} {$i <= $locs} {incr i} {
+ append result "[join [list $num.$i $enabled] $c].*"
+ }
+ }
+
+ # Add function/source file info.
+ append result "in $func at .*$source${end}"
+
+ return $result
+}
+
#
# func1 is a static inlined function that is called once.
# The result should be a single-location breakpoint.
@@ -111,3 +166,23 @@ gdb_test "print func1" \
#
gdb_test "print func2" \
"\\\$.* = {int \\(int\\)} .* <func2>"
+
+# Test that "info break" reports the location of the breakpoints "inside"
+# the inlined functions
+
+set i 0
+set results([incr i]) [break_info_1 1 -source $srcfile -func "func1"]
+set results([incr i]) [break_info_1 2 -locs 2 -source $srcfile -func "func2"]
+set results([incr i]) [break_info_1 3 -source $srcfile -func "func3b"]
+set results([incr i]) [break_info_1 4 -locs 2 -source $srcfile -func "func4b"]
+set results([incr i]) [break_info_1 5 -locs 2 -source $srcfile -func "func5b"]
+set results([incr i]) [break_info_1 6 -locs 3 -source $srcfile -func "func6b"]
+set results([incr i]) [break_info_1 7 -locs 2 -source $srcfile -func "func7b"]
+set results([incr i]) [break_info_1 8 -locs 3 -source $srcfile -func "func8b"]
+
+for {set i 1} {$i <= [llength [array names results]]} {incr i} {
+ send_log "Expecting: $results($i)\n"
+ gdb_test "info break $i" $results($i)
+}
+
+unset -nocomplain results
--
2.1.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Use containing function when reporting breakpoint location.
2017-03-10 20:59 [PATCH] Use containing function when reporting breakpoint location Keith Seitz
@ 2017-03-20 18:22 ` Jan Kratochvil
2017-03-21 18:24 ` Keith Seitz
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kratochvil @ 2017-03-20 18:22 UTC (permalink / raw)
To: Keith Seitz; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]
On Fri, 10 Mar 2017 21:59:13 +0100, Keith Seitz wrote:
> With this patch, print_breakpoint_location now calls a new variation of
> find_pc_sect_function, find_pc_sect_containing_function, which does not
> skip over inlined functions, and the breakpoint location is now reported
> "correctly":
Your patch just exchanges the sets of PASSing and FAILing cases. Goal of this
patch should be to PASS all cases.
------------------------------------------------------------------------------
volatile int i,j;
void f(void) {
i=1;
}
int main(void) {
f();
return 0;
}
------------------------------------------------------------------------------
$ for i in main f;do ./gdb-clean -batch -data-directory data-directory/ ./4 -ex "b $i" -ex 'info break';done
------------------------------------------------------------------------------
Breakpoint 1 at 0x4003b0: file 4.c, line 5.
Num Type Disp Enb Address What
-1 breakpoint keep y 0x00000000004003b0 in main at 4.c:5
+1 breakpoint keep y 0x00000000004003b0 in f at 4.c:5
Breakpoint 1 at 0x4003b0: f. (2 locations)
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
-1.1 y 0x00000000004003b0 in main at 4.c:5
+1.1 y 0x00000000004003b0 in f at 4.c:5
vvv=this line is off-topic for this mail
1.2 y 0x00000000004004c0 in f at 4.c:3
^^^=this line is off-topic for this mail
------------------------------------------------------------------------------
- = FSF GDB HEAD
+ = FSF GDB HEAD + this patch
The goal is to display "main" in the first case and "f" in the second case.
I think this would need to store more information into bp_location to know how
its CORE_ADDR address has been found.
In my attached patch-idea (not compilable) from 2015-06 (RHBZ 1228549#c5)
I chose more a quick-hack to reuse 'addr_string', parse it and select only its
SALs that match by its PC the bp_location being processed/displayed.
Although still the behavior will be confusing afterwards as existing debug
info does not provide enough information for sane -O2 -g debugging.
This should be only fixed by future DWARF+GCC feature by Alexandre Oliva:
https://people.redhat.com/aoliva/papers/sfn/gcc2010.pdf
https://people.redhat.com/aoliva/papers/sfn/slides.pdf
Internal in RH: Message-ID: <orbmto9cir.fsf@lxoliva.fsfla.org>
Jan
[-- Attachment #2: inline-bpt2.patch --]
[-- Type: text/plain, Size: 1588 bytes --]
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index e5d7360..d535dcb 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -301,13 +301,32 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block)
user steps into them. */
void
-skip_inline_frames (ptid_t ptid)
+skip_inline_frames (ptid_t ptid, struct breakpoint *bpt)
{
CORE_ADDR this_pc;
const struct block *frame_block, *cur_block;
struct symbol *last_sym = NULL;
int skip_count = 0;
struct inline_state *state;
+ struct linespec_result canonical;
+
+
+ if (bpt == NULL)
+ canonical.sals = NULL;
+ else
+ {
+ char *addr_string = bpt->addr_string;
+
+ TRY
+ {
+ decode_line_full (&addr_string, DECODE_LINE_FUNFIRSTLINE, NULL, 0,
+ &canonical, multiple_symbols_all, NULL);
+ }
+ CATCH (e, RETURN_MASK_ERROR)
+ {
+ }
+ END_CATCH
+ }
/* This function is called right after reinitializing the frame
cache. We try not to do more unwinding than absolutely
@@ -327,6 +346,20 @@ skip_inline_frames (ptid_t ptid)
if (BLOCK_START (cur_block) == this_pc
|| block_starting_point_at (this_pc, cur_block))
{
+ int lsal_i;
+ struct linespec_sals *lsal;
+
+ for (lsal_i = 0; VEC_iterate (linespec_sals, canonical->sals, lsal_i, lsal); lsal_i++)
+ {
+ struct symtabs_and_lines *sals = lsal->sals;
+
+ for (sals_i = 0; sals_i < sals->nelts; sals_i++)
+ {
+ struct struct symtab_and_line *sal = *sals->sals[sals_i];
+
+ }
+ }
+
skip_count++;
last_sym = BLOCK_FUNCTION (cur_block);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Use containing function when reporting breakpoint location.
2017-03-20 18:22 ` Jan Kratochvil
@ 2017-03-21 18:24 ` Keith Seitz
2017-03-22 0:39 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Keith Seitz @ 2017-03-21 18:24 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 03/20/2017 11:22 AM, Jan Kratochvil wrote:
> The goal is to display "main" in the first case and "f" in the second case.
Bah. Good catch.
> I think this would need to store more information into bp_location to know how
> its CORE_ADDR address has been found.
I considered that, but then decided that was more invasive than I thought it should be. Alas, right now, I don't see another way forward. I'll play with this idea more.
> In my attached patch-idea (not compilable) from 2015-06 (RHBZ 1228549#c5)
> I chose more a quick-hack to reuse 'addr_string', parse it and select only its
> SALs that match by its PC the bp_location being processed/displayed.
I have a patch based on that idea, but I'm a little afraid to introduce that. While skip_inline_frames is known to be an expensive operation, I fear that parsing SaLs in there might kill performance.
Like the bug in $SUBJECT, I have not been able to divine an alternate solution.
As far as this bug is concerned, this patch should be considered rejected/withdrawn.
Thank you, Jan!
Keith
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Use containing function when reporting breakpoint location.
2017-03-21 18:24 ` Keith Seitz
@ 2017-03-22 0:39 ` Pedro Alves
0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2017-03-22 0:39 UTC (permalink / raw)
To: Keith Seitz, Jan Kratochvil; +Cc: gdb-patches
On 03/21/2017 06:24 PM, Keith Seitz wrote:
> As far as this bug is concerned, this patch should be considered rejected/withdrawn.
I think we should step back a bit and understand / discuss what the goal
of "info breakpoints" in this scenario is, and make sure everyone ends up in
the same page.
What would the user ideally see? What information is important for users when
they look at the breakpoint list?
For inlined breakpoints locations, I think it's also important to consider
the case of the code being expanded at multiple sites, resulting in
potentially many locations.
One of the main advantages of showing the multiple breakpoint
locations to the user is to let the user conveniently enable/disable
each location individually. For example, with:
(gdb) b foo::overload1arg
2 breakpoint keep y <MULTIPLE>
2.1 y 0x00000000004009b8 in foo::overload1arg()
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:107
2.2 y 0x00000000004009cd in foo::overload1arg(char)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:110
2.3 y 0x00000000004009e5 in foo::overload1arg(signed char)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:113
2.4 y 0x00000000004009fd in foo::overload1arg(unsigned char)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:116
2.5 y 0x0000000000400a16 in foo::overload1arg(short)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:119
2.6 y 0x0000000000400a32 in foo::overload1arg(unsigned short)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:122
2.7 y 0x0000000000400a4b in foo::overload1arg(int)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:125
2.8 y 0x0000000000400a65 in foo::overload1arg(unsigned int)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:128
2.9 y 0x0000000000400a80 in foo::overload1arg(long)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:131
2.10 y 0x0000000000400a9c in foo::overload1arg(unsigned long)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:134
2.11 y 0x0000000000400ab9 in foo::overload1arg(float)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:137
2.12 y 0x0000000000400ad7 in foo::overload1arg(double)
at src/gdb/testsuite/gdb.cp/ovldbreak.cc:140
Above, it's easy to disable the location for the "(long)" overload, for example.
Or at least, it's doable with the given information, because each method's
prototype is shown.
But if a user sets a breakpoint at an inline function, and then gets back this:
(gdb) inf br
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 breakpoint keep y 0x0000000000400434 in inline_func at test.c:5
1.2 breakpoint keep y 0x0000000000404534 in inline_func at test.c:5
1.3 breakpoint keep y 0x0000000000405404 in inline_func at test.c:5
1.4 breakpoint keep y 0x0000000000402345 in inline_func at test.c:5
1.5 breakpoint keep y 0x0000000000435554 in inline_func at test.c:5
1.6 breakpoint keep y 0x0000000000556566 in inline_func at test.c:5
...
...
1.70 breakpoint keep y 0x0000000000645454 in inline_func at test.c:5
then I don't know what can the user do to quickly distinguish each of
the locations. I would think that ideally a user would want to be able to
quickly figure out from this output where each inline expansion occurred, in order
to be able to conveniently disable a location for a particular inline
expansion site, for being a non-interesting location that triggers
all too often, for example.
So I'd imagine something like this would be more helpful:
(gdb) inf br
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 breakpoint keep y 0x0000000000400434 in inline_func at test.c:5
1.2 breakpoint keep y 0x0000000000404534 in inline_func at test.c:5
inlined in foo at foo.c:1020
1.3 breakpoint keep y 0x0000000000404534 in inline_func at test.c:5
inlined in bar at bar.c:1020
...
I.e., the first location is the out of line copy, and other two are
inline expansions.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-22 0:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 20:59 [PATCH] Use containing function when reporting breakpoint location Keith Seitz
2017-03-20 18:22 ` Jan Kratochvil
2017-03-21 18:24 ` Keith Seitz
2017-03-22 0:39 ` 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).