Use containing function when reporting breakpoint location.
Commit Message
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
Comments
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
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
@@ -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);
}