Change inline frame breakpoint skipping logic (Re: [PATCH] Ensure captured_main has unique address)
Commit Message
On 06/12/2018 04:06 PM, Tom de Vries wrote:
> Hi,
>
> atm selftest.exp fails for me.
>
> One of the reasons is that after setting a breakpoint in captured_main, we
> stop at:
> ...
> Breakpoint 1, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
> ...
> while selftest_setup expects to stop at captured_main.
>
> The problem is that captured_main_1 has been inlined into captured_main, and
> captured_main has been inlined into gdb_main:
> ...
> $ nm ./build/gdb/gdb | egrep ' [tT] .*captured_main|gdb_main' | c++filt
> 000000000061b950 T gdb_main(captured_main_args*)
> ...
>
> The reason that we seem to be stopping at inline function captured_main_1 has
> probably something to do with commit "Don't elide all inlined frames",
Yes, sounds like it. But the selftest.exp explicitly asks to stop
at "captured_main", not "captured_main_1", so I'm thinking that
it's gdb's behavior that might be wrong:
(top-gdb) b captured_main
Breakpoint 3 at 0x792f99: file src/gdb/main.c, line 492.
(top-gdb) r
Starting program: build/gdb/gdb
Breakpoint 3, captured_main_1 (context=<optimized out>) at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:492
492 lim_at_start = (char *) sbrk (0);
(top-gdb)
With the patch below, we instead get:
(top-gdb) b captured_main
Breakpoint 6 at 0x791339: file src/gdb/main.c, line 492.
(top-gdb) r
Starting program: build/gdb/gdb
Breakpoint 6, captured_main (data=<optimized out>) at src/gdb/main.c:1147
1147 captured_main_1 (context);
(top-gdb)
and:
(top-gdb) b captured_main_1
Breakpoint 7 at 0x791339: file src/gdb/main.c, line 492.
(top-gdb) r
Starting program: build/gdb/gdb
Breakpoint 7, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
492 lim_at_start = (char *) sbrk (0);
(top-gdb)
Note that both captured_main and captured_main_1 resolved to the
same address, 0x791339. The gdb.base/inline-break.exp testcase
currently does not exercise that, but the new test added by the
patch below does. That new test fails without the patch and passes
with the patch. No regressions on x86-64 GNU/Linux. WDYT?
Also, while looking at stopped_by_user_bp_inline_frame I noticed
that comparing the THIS_PC is basically a nop -- if a software or
hardware breakpoint explains the stop, then it must be that it
was installed at the current PC.
From cf6924a2cfca397b80c7c935e048771de77d3105 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 12 Jun 2018 18:22:25 +0100
Subject: [PATCH] Inline breakpoints
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* inline-frame.c (stopped_by_user_bp_inline_frame): Replace PC
parameter with a block parameter. Compare location's block symbol
with the frame's block instead of addresses.
(skip_inline_frames): Pass the current block instead of the
frame's address. Break out as soon as we determine the frame
should not be skipped.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdb.opt/inline-break.c (func_callee, func_caller): New.
(main): Call func_caller.
---
gdb/inline-frame.c | 23 +++++++++++------------
gdb/testsuite/gdb.opt/inline-break.c | 21 +++++++++++++++++++++
gdb/testsuite/gdb.opt/inline-break.exp | 17 +++++++++++++++++
3 files changed, 49 insertions(+), 12 deletions(-)
Comments
On 06/12/2018 07:38 PM, Pedro Alves wrote:
> On 06/12/2018 04:06 PM, Tom de Vries wrote:
>> Hi,
>>
>> atm selftest.exp fails for me.
>>
>> One of the reasons is that after setting a breakpoint in captured_main, we
>> stop at:
>> ...
>> Breakpoint 1, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
>> ...
>> while selftest_setup expects to stop at captured_main.
>>
>> The problem is that captured_main_1 has been inlined into captured_main, and
>> captured_main has been inlined into gdb_main:
>> ...
>> $ nm ./build/gdb/gdb | egrep ' [tT] .*captured_main|gdb_main' | c++filt
>> 000000000061b950 T gdb_main(captured_main_args*)
>> ...
>>
>> The reason that we seem to be stopping at inline function captured_main_1 has
>> probably something to do with commit "Don't elide all inlined frames",
>
> Yes, sounds like it. But the selftest.exp explicitly asks to stop
> at "captured_main", not "captured_main_1", so I'm thinking that
> it's gdb's behavior that might be wrong:
>
> (top-gdb) b captured_main
> Breakpoint 3 at 0x792f99: file src/gdb/main.c, line 492.
> (top-gdb) r
> Starting program: build/gdb/gdb
>
> Breakpoint 3, captured_main_1 (context=<optimized out>) at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:492
> 492 lim_at_start = (char *) sbrk (0);
> (top-gdb)
>
> With the patch below, we instead get:
>
> (top-gdb) b captured_main
> Breakpoint 6 at 0x791339: file src/gdb/main.c, line 492.
> (top-gdb) r
> Starting program: build/gdb/gdb
>
> Breakpoint 6, captured_main (data=<optimized out>) at src/gdb/main.c:1147
> 1147 captured_main_1 (context);
> (top-gdb)
>
> and:
>
> (top-gdb) b captured_main_1
> Breakpoint 7 at 0x791339: file src/gdb/main.c, line 492.
> (top-gdb) r
> Starting program: build/gdb/gdb
> Breakpoint 7, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
> 492 lim_at_start = (char *) sbrk (0);
> (top-gdb)
>
Agreed, that's a better solution.
> Note that both captured_main and captured_main_1 resolved to the
> same address, 0x791339.
Right. I played around a bit with this, and set breakpoints on
captured_main and captured_main_1.
If I set a breakpoint on captured_main_1, we have captured_main unknown:
...
Breakpoint 2, captured_main_1 (context=<optimized out>)
at /home/vries/gdb_versions/devel/src/gdb/main.c:492
492 lim_at_start = (char *) sbrk (0);
(gdb) p captured_main
No symbol "captured_main" in current context.
(gdb) p captured_main_1
$1 = {void (captured_main_args *)} 0x61b959
<gdb_main(captured_main_args*)+25>
...
But If I set a breakpoint on captured_main instead, we have
captured_main_1 unknown:
...
Breakpoint 3, captured_main (data=<optimized out>)
at /home/vries/gdb_versions/devel/src/gdb/main.c:1147
1147 captured_main_1 (context);
(gdb) p captured_main
$2 = {void (void *)} 0x61b959 <gdb_main(captured_main_args*)+25>
(gdb) p captured_main_1
No symbol "captured_main_1" in current context.
...
And if I set a breakpoint on both, captured_main_1 seems to take
precedence (independent of the order used to set the breakpoint):
...
Breakpoint 1, captured_main_1 (context=<optimized out>)
at /home/vries/gdb_versions/devel/src/gdb/main.c:492
492 lim_at_start = (char *) sbrk (0);
(gdb) p captured_main_1
$1 = {void (captured_main_args *)} 0x61b959
<gdb_main(captured_main_args*)+25>
(gdb) p captured_main
No symbol "captured_main" in current context.
...
I don't understand the underlying mechanisms well enough to decide
whether this is a problem or not, but I thought I just mention it.
> The gdb.base/inline-break.exp testcase
> currently does not exercise that, but the new test added by the
> patch below does. That new test fails without the patch and passes
> with the patch. No regressions on x86-64 GNU/Linux. WDYT?
>
AFAICT, the patch looks ok (just one nit below).
> +/* A static inlined function that is called by another static inlined
> + function. */
> +
> +static inline ATTR int
> +func_callee (int x)
> +{
> + return x * 23;
> +}
> +
> +/* A static inlined function that calls another static inlined
> + function. The body of the function is a simple as possible so that
> + both functions are inlined to the same PC address. */
> +
> +static int
inline ATTR ?
> +func_caller (int x)
> +{
> + return func_callee (x);
> +}
> +
Thanks,
- Tom
@@ -286,11 +286,10 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block)
}
/* Loop over the stop chain and determine if execution stopped in an
- inlined frame because of a user breakpoint. THIS_PC is the current
- frame's PC. */
+ inlined frame because of a user breakpoint set at FRAME_BLOCK. */
static bool
-stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain)
+stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain)
{
for (bpstat s = stop_chain; s != NULL; s = s->next)
{
@@ -301,9 +300,9 @@ stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain)
bp_location *loc = s->bp_location_at;
enum bp_loc_type t = loc->loc_type;
- if (loc->address == this_pc
- && (t == bp_loc_software_breakpoint
- || t == bp_loc_hardware_breakpoint))
+ if ((t == bp_loc_software_breakpoint
+ || t == bp_loc_hardware_breakpoint)
+ && frame_block == SYMBOL_BLOCK_VALUE (loc->symbol))
return true;
}
}
@@ -340,12 +339,12 @@ skip_inline_frames (ptid_t ptid, bpstat stop_chain)
{
/* Do not skip the inlined frame if execution
stopped in an inlined frame because of a user
- breakpoint. */
- if (!stopped_by_user_bp_inline_frame (this_pc, stop_chain))
- {
- skip_count++;
- last_sym = BLOCK_FUNCTION (cur_block);
- }
+ breakpoint for this inline function. */
+ if (stopped_by_user_bp_inline_frame (cur_block, stop_chain))
+ break;
+
+ skip_count++;
+ last_sym = BLOCK_FUNCTION (cur_block);
}
else
break;
@@ -176,6 +176,25 @@ not_inline_func3 (int x)
return y + inline_func3 (x);
}
+/* A static inlined function that is called by another static inlined
+ function. */
+
+static inline ATTR int
+func_callee (int x)
+{
+ return x * 23;
+}
+
+/* A static inlined function that calls another static inlined
+ function. The body of the function is a simple as possible so that
+ both functions are inlined to the same PC address. */
+
+static int
+func_caller (int x)
+{
+ return func_callee (x);
+}
+
/* Entry point. */
int
@@ -205,5 +224,7 @@ main (int argc, char *argv[])
x = not_inline_func3 (-21);
+ func_caller (1);
+
return x;
}
@@ -231,4 +231,21 @@ foreach_with_prefix cmd [list "break" "tbreak"] {
}
}
+# func_caller and func_callee are both inline functions, and one calls
+# the other. Test that setting a breakpoint on the caller reports the
+# stop at the caller, and that setting a breakpoint at the callee
+# reports a stop at the callee.
+foreach_with_prefix func {"func_callee" "func_caller"} {
+ clean_restart $binfile
+
+ if {![runto main]} {
+ untested "could not run to main"
+ continue
+ }
+
+ gdb_breakpoint $func
+ gdb_test "continue" "Breakpoint .* $func .*at .*$srcfile.*" \
+ "continue to inline function"
+}
+
unset -nocomplain results