Change inline frame breakpoint skipping logic (Re: [PATCH] Ensure captured_main has unique address)

Pedro Alves
2018-06-12T17:38:39Z

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(-)

diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index 1ac5835438d..3edd5b2b20b 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -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;
diff --git a/gdb/testsuite/gdb.opt/inline-break.c b/gdb/testsuite/gdb.opt/inline-break.c
index 922102debb6..6ad331fcc63 100644
--- a/gdb/testsuite/gdb.opt/inline-break.c
+++ b/gdb/testsuite/gdb.opt/inline-break.c
@@ -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;
}
diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp
index 008ff1ac33a..f1ab3d23715 100644
--- a/gdb/testsuite/gdb.opt/inline-break.exp
+++ b/gdb/testsuite/gdb.opt/inline-break.exp
@@ -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
--
2.14.3