This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH 2/2] Report stop locations in inlined functions.
- From: Keith Seitz <keiths at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Mon, 10 Jul 2017 19:36:41 -0700
- Subject: [PATCH 2/2] Report stop locations in inlined functions.
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=keiths at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E6C7D80F63
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E6C7D80F63
- References: <1499740601-15957-1-git-send-email-keiths@redhat.com>
This is a patch for a very related inline function problem. Using the
test case from breakpoints/17534,
3 static inline void NVIC_EnableIRQ(int IRQn)
4 {
5 volatile int y;
6 y = IRQn;
7 }
8
9 __attribute__( ( always_inline ) ) static inline void __WFI(void)
10 {
11 __asm volatile ("nop");
12 }
13
14 int main(void) {
15
16 x= 42;
17
18 if (x)
19 NVIC_EnableIRQ(16);
20 else
21 NVIC_EnableIRQ(18);
(gdb) b NVIC_EnableIRQ
Breakpoint 1 at 0x4003e4: NVIC_EnableIRQ. (2 locations)
(gdb) r
Starting program: 17534
Breakpoint 1, main () at 17534.c:19
19 NVIC_EnableIRQ(16);
This happens because skip_inline_frames automatically skips every inlined
frame. Based on a suggestion by Jan, this patch introduces a new function,
breakpoint_for_stop, which attempts to ascertain which breakpoint, if any,
caused a particular stop in the inferior. That breakpoint is then passed
to skip_inline_frames so that it can decide if a particular inlined frame
should be skipped.
I've had to separate the bpstat chain building from bpstat_stop_status --
py-finish-breakpoint.exp did not like me calling bpstat_stop_status multiple
times. So I've added the ability to allocate the chain separately and
optionally pass it to bpstat_stop_status, which remains otherwise unchanged.
With this patch, GDB now correctly reports that the inferior has stopped
inside the inlined function:
(gdb) r
Starting program: 17534
Breakpoint 1, NVIC_EnableIRQ (IRQn=16) at 17534.c:6
6 y = IRQn;
I don't quite like this, though. This solution involves calling
decode_line_full, and that is really expensive, so I would be grateful if
maintaienrs could offer advice on how to better tackle this.
gdb/ChangeLog:
2017-MM-DD Keith Seitz <keiths@redhat.com>
* breakpoint.c (bpstat_explains_signal): Add output parameter for
breakpoint and save the breakpoint if one is found to explain
the signal.
All callers updated.
(build_bpstat_chain): New function, moved from bpstat_stop_status.
(breakpoint_for_stop): New function.
(bpstat_stop_status): Add new optional parameter for the bpstat chain.
If this new parameter is NULL, call build_bpstat_chain.
All callers updated.
* breakpoint.h (breakpoint_for_stop): Declare.
(bpstat_explains_signal): Update declaration.
* infrun.c (handle_signal_stop): Before calling skip_inline_frames,
use breakpoint_for_stop to find the breakpoint that caused us
to stop.
Save the bpstat chain for later invocation of bpstat_stop_status.
* inline-frame.c: Include linespec.h.
(skip_inline_frames): Add struct breakpoint parameter.
Re-parse the location of the breakpoint causing the stop, if any,
and only skip frames that did not cause the stop.
* inline-frame.h (skip_inline_frames): Update declaration.
gdb/testsuite/ChangeLog:
2017-MM-DD Keith Seitz <keiths@redhat.com>
* gdb.opt/inline-break.c (inline_func1, not_inline_func1)
(inline_func2, not_inline_func2, inline_func3, not_inline_func3):
New functions.
(main): Call not_inline_func3.
* gdb.opt/inline-break.exp: Start inferior and set breakpoints at
inline_func1, inline_func2, and inline_func3. Test that when each
breakpoint is hit, GDB properly reports both the stop location
and the backtrace.
---
gdb/breakpoint.c | 128 +++++++++++++++++++++------------
gdb/breakpoint.h | 19 ++++-
gdb/infrun.c | 22 +++---
gdb/inline-frame.c | 46 +++++++++++-
gdb/inline-frame.h | 2 +-
gdb/testsuite/gdb.opt/inline-break.c | 50 +++++++++++++
gdb/testsuite/gdb.opt/inline-break.exp | 35 +++++++++
7 files changed, 244 insertions(+), 58 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c283ec0..0a087ec 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4547,7 +4547,7 @@ bpstat_find_breakpoint (bpstat bsp, struct breakpoint *breakpoint)
/* See breakpoint.h. */
int
-bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
+bpstat_explains_signal (bpstat bsp, enum gdb_signal sig, struct breakpoint **bp)
{
for (; bsp != NULL; bsp = bsp->next)
{
@@ -4556,13 +4556,21 @@ bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
/* A moribund location can never explain a signal other than
GDB_SIGNAL_TRAP. */
if (sig == GDB_SIGNAL_TRAP)
- return 1;
+ {
+ if (bp != NULL)
+ *bp = NULL;
+ return 1;
+ }
}
else
{
if (bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at,
sig))
- return 1;
+ {
+ if (bp != NULL)
+ *bp = bsp->breakpoint_at;;
+ return 1;
+ }
}
}
@@ -5597,47 +5605,16 @@ need_moribund_for_location_type (struct bp_location *loc)
&& !target_supports_stopped_by_hw_breakpoint ()));
}
+/* Build the bstat chain for the stop information given by ASPACE,
+ BP_ADDR, and WS. BS_LINK should point to storage which may be subsequently
+ passed to bpstat_stop_status to avoid rebuilding the stop chain. */
-/* Get a bpstat associated with having just stopped at address
- BP_ADDR in thread PTID.
-
- Determine whether we stopped at a breakpoint, etc, or whether we
- don't understand this stop. Result is a chain of bpstat's such
- that:
-
- if we don't understand the stop, the result is a null pointer.
-
- if we understand why we stopped, the result is not null.
-
- Each element of the chain refers to a particular breakpoint or
- watchpoint at which we have stopped. (We may have stopped for
- several reasons concurrently.)
-
- Each element of the chain has valid next, breakpoint_at,
- commands, FIXME??? fields. */
-
-bpstat
-bpstat_stop_status (struct address_space *aspace,
- CORE_ADDR bp_addr, ptid_t ptid,
- const struct target_waitstatus *ws)
+static void
+build_bpstat_chain (bpstat *bs_link, struct address_space *aspace,
+ CORE_ADDR bp_addr, const struct target_waitstatus *ws)
{
- struct breakpoint *b = NULL;
+ struct breakpoint *b;
struct bp_location *bl;
- struct bp_location *loc;
- /* First item of allocated bpstat's. */
- bpstat bs_head = NULL, *bs_link = &bs_head;
- /* Pointer to the last thing in the chain currently. */
- bpstat bs;
- int ix;
- int need_remove_insert;
- int removed_any;
-
- /* First, build the bpstat chain with locations that explain a
- target stop, while being careful to not set the target running,
- as that may invalidate locations (in particular watchpoint
- locations are recreated). Resuming will happen here with
- breakpoint conditions or watchpoint expressions that include
- inferior function calls. */
ALL_BREAKPOINTS (b)
{
@@ -5663,8 +5640,8 @@ bpstat_stop_status (struct address_space *aspace,
/* Come here if it's a watchpoint, or if the break address
matches. */
- bs = bpstat_alloc (bl, &bs_link); /* Alloc a bpstat to
- explain stop. */
+ bpstat bs = bpstat_alloc (bl, &bs_link); /* Alloc a bpstat to
+ explain stop. */
/* Assume we stop. Should we find a watchpoint that is not
actually triggered, or if the condition of the breakpoint
@@ -5689,12 +5666,15 @@ bpstat_stop_status (struct address_space *aspace,
if (!target_supports_stopped_by_sw_breakpoint ()
|| !target_supports_stopped_by_hw_breakpoint ())
{
- for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+ struct bp_location *loc;
+
+ for (int ix = 0;
+ VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
{
if (breakpoint_location_address_match (loc, aspace, bp_addr)
&& need_moribund_for_location_type (loc))
{
- bs = bpstat_alloc (loc, &bs_link);
+ bpstat bs = bpstat_alloc (loc, &bs_link);
/* For hits of moribund locations, we should just proceed. */
bs->stop = 0;
bs->print = 0;
@@ -5702,6 +5682,64 @@ bpstat_stop_status (struct address_space *aspace,
}
}
}
+}
+
+/* See breakpoint.h. */
+
+struct breakpoint *
+breakpoint_for_stop (struct address_space *aspace, CORE_ADDR stop_pc,
+ enum gdb_signal stop_signal,
+ const struct target_waitstatus *ws,
+ bpstat *storage)
+{
+ build_bpstat_chain (storage, aspace, stop_pc, ws);
+
+ struct breakpoint *bpt = NULL;
+ if (bpstat_explains_signal (*storage, stop_signal, &bpt))
+ return bpt;
+
+ return NULL;
+}
+
+/* Get a bpstat associated with having just stopped at address
+ BP_ADDR in thread PTID.
+
+ Determine whether we stopped at a breakpoint, etc, or whether we
+ don't understand this stop. Result is a chain of bpstat's such
+ that:
+
+ if we don't understand the stop, the result is a null pointer.
+
+ if we understand why we stopped, the result is not null.
+
+ Each element of the chain refers to a particular breakpoint or
+ watchpoint at which we have stopped. (We may have stopped for
+ several reasons concurrently.)
+
+ Each element of the chain has valid next, breakpoint_at,
+ commands, FIXME??? fields. */
+
+bpstat
+bpstat_stop_status (struct address_space *aspace,
+ CORE_ADDR bp_addr, ptid_t ptid,
+ const struct target_waitstatus *ws,
+ bpstat opt_chain)
+{
+ struct breakpoint *b = NULL;
+ /* First item of allocated bpstat's. */
+ bpstat bs_head = opt_chain;
+ bpstat bs;
+ int need_remove_insert;
+ int removed_any;
+
+ /* First, build the bpstat chain with locations that explain a
+ target stop, while being careful to not set the target running,
+ as that may invalidate locations (in particular watchpoint
+ locations are recreated). Resuming will happen here with
+ breakpoint conditions or watchpoint expressions that include
+ inferior function calls. */
+ if (bs_head == NULL)
+ build_bpstat_chain (&bs_head, aspace, bp_addr, ws);
/* A bit of special processing for shlib breakpoints. We need to
process solib loading here, so that the lists of loaded and
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 2f10c3b..1d7ed48 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -918,7 +918,21 @@ extern bpstat bpstat_copy (bpstat);
extern bpstat bpstat_stop_status (struct address_space *aspace,
CORE_ADDR pc, ptid_t ptid,
- const struct target_waitstatus *ws);
+ const struct target_waitstatus *ws,
+ bpstat opt_chain);
+
+/* If the stop described by ASPACE, STOP_PC, STOP_SIGNAL, and WS was
+ the result of a breakpoint, return that breakpoint; otherwise return
+ NULL.
+
+ STORAGE will hold the bpstat chain and may be passed to bpstat_stop_status
+ to avoid rebuilding the entire stop chain multiple times. */
+
+extern struct breakpoint *
+ breakpoint_for_stop (struct address_space *aspace, CORE_ADDR stop_pc,
+ enum gdb_signal stop_signal,
+ const struct target_waitstatus *ws,
+ bpstat *storage);
/* This bpstat_what stuff tells wait_for_inferior what to do with a
breakpoint (a challenging task).
@@ -1028,7 +1042,8 @@ bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *);
/* Nonzero if a signal that we got in target_wait() was due to
circumstances explained by the bpstat; the signal is therefore not
random. */
-extern int bpstat_explains_signal (bpstat, enum gdb_signal);
+extern int bpstat_explains_signal (bpstat, enum gdb_signal,
+ struct breakpoint **);
/* Nonzero is this bpstat causes a stop. */
extern int bpstat_causes_stop (bpstat);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 5e4cd51..7efafa6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4276,7 +4276,7 @@ handle_syscall_event (struct execution_control_state *ecs)
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (regcache),
- stop_pc, ecs->ptid, &ecs->ws);
+ stop_pc, ecs->ptid, &ecs->ws, NULL);
if (handle_stop_requested (ecs))
return 0;
@@ -4979,7 +4979,7 @@ handle_inferior_event_1 (struct execution_control_state *ecs)
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (regcache),
- stop_pc, ecs->ptid, &ecs->ws);
+ stop_pc, ecs->ptid, &ecs->ws, NULL);
if (handle_stop_requested (ecs))
return;
@@ -5233,7 +5233,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
- stop_pc, ecs->ptid, &ecs->ws);
+ stop_pc, ecs->ptid, &ecs->ws, NULL);
if (handle_stop_requested (ecs))
return;
@@ -5345,7 +5345,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
- stop_pc, ecs->ptid, &ecs->ws);
+ stop_pc, ecs->ptid, &ecs->ws, NULL);
/* Note that this may be referenced from inside
bpstat_stop_status above, through inferior_has_execd. */
@@ -5884,6 +5884,7 @@ handle_signal_stop (struct execution_control_state *ecs)
ecs->event_thread->control.stop_step = 0;
stop_print_frame = 1;
stopped_by_random_signal = 0;
+ bpstat stop_chain = NULL;
/* Hide inlined functions starting here, unless we just performed stepi or
nexti. After stepi and nexti, always show the innermost frame (not any
@@ -5915,7 +5916,12 @@ handle_signal_stop (struct execution_control_state *ecs)
ecs->event_thread->prev_pc,
&ecs->ws)))
{
- skip_inline_frames (ecs->ptid);
+ struct breakpoint *bpt
+ = breakpoint_for_stop (aspace, stop_pc,
+ ecs->event_thread->suspend.stop_signal,
+ &ecs->ws, &stop_chain);
+
+ skip_inline_frames (ecs->ptid, bpt);
/* Re-fetch current thread's frame in case that invalidated
the frame cache. */
@@ -5964,7 +5970,7 @@ handle_signal_stop (struct execution_control_state *ecs)
handles this event. */
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
- stop_pc, ecs->ptid, &ecs->ws);
+ stop_pc, ecs->ptid, &ecs->ws, stop_chain);
/* Following in case break condition called a
function. */
@@ -5981,7 +5987,7 @@ handle_signal_stop (struct execution_control_state *ecs)
if (debug_infrun
&& ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
&& !bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
- GDB_SIGNAL_TRAP)
+ GDB_SIGNAL_TRAP, NULL)
&& stopped_by_watchpoint)
fprintf_unfiltered (gdb_stdlog,
"infrun: no user watchpoint explains "
@@ -6010,7 +6016,7 @@ handle_signal_stop (struct execution_control_state *ecs)
/* See if the breakpoints module can explain the signal. */
random_signal
= !bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
- ecs->event_thread->suspend.stop_signal);
+ ecs->event_thread->suspend.stop_signal, NULL);
/* Maybe this was a trap for a software breakpoint that has since
been removed. */
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index b6154cd..006ae0d 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -27,6 +27,7 @@
#include "symtab.h"
#include "vec.h"
#include "frame.h"
+#include "linespec.h"
/* We need to save a few variables for every thread stopped at the
virtual call site of an inlined function. If there was always a
@@ -301,13 +302,34 @@ 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;
+
+ canonical.sals = NULL;
+ if (bpt != NULL)
+ {
+ const struct event_location *location = bpt->location.get ();
+
+ if (location != NULL && event_location_type (location) != PROBE_LOCATION)
+ {
+ TRY
+ {
+ decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, bpt->pspace,
+ 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,7 +349,27 @@ skip_inline_frames (ptid_t ptid)
if (BLOCK_START (cur_block) == this_pc
|| block_starting_point_at (this_pc, cur_block))
{
- skip_count++;
+ int lsal_i;
+ struct linespec_sals *lsal;
+ bool skip_this_frame = true;
+
+ for (lsal_i = 0;
+ VEC_iterate (linespec_sals, canonical.sals,
+ lsal_i, lsal); lsal_i++)
+ {
+ struct symtabs_and_lines &sals = lsal->sals;
+
+ for (int sals_i = 0; sals_i < sals.nelts; sals_i++)
+ {
+ struct symtab_and_line &sal = sals.sals[sals_i];
+
+ if (sal.pc == this_pc)
+ skip_this_frame = false;
+ }
+ }
+
+ if (skip_this_frame)
+ skip_count++;
last_sym = BLOCK_FUNCTION (cur_block);
}
else
diff --git a/gdb/inline-frame.h b/gdb/inline-frame.h
index e6fe29d..8afaf6c 100644
--- a/gdb/inline-frame.h
+++ b/gdb/inline-frame.h
@@ -31,7 +31,7 @@ extern const struct frame_unwind inline_frame_unwind;
Frames for the hidden functions will not appear in the backtrace until the
user steps into them. */
-void skip_inline_frames (ptid_t ptid);
+void skip_inline_frames (ptid_t ptid, struct breakpoint *bpt);
/* Forget about any hidden inlined functions in PTID, which is new or
about to be resumed. If PTID is minus_one_ptid, forget about all
diff --git a/gdb/testsuite/gdb.opt/inline-break.c b/gdb/testsuite/gdb.opt/inline-break.c
index 2616a0e..a42bc0b 100644
--- a/gdb/testsuite/gdb.opt/inline-break.c
+++ b/gdb/testsuite/gdb.opt/inline-break.c
@@ -128,6 +128,54 @@ func8a (int x)
return func8b (x * 31);
}
+static inline ATTR int
+inline_func1 (int x)
+{
+ int y = 1; /* inline_func1 */
+
+ return y + x;
+}
+
+static int
+not_inline_func1 (int x)
+{
+ int y = 2; /* not_inline_func1 */
+
+ return y + inline_func1 (x);
+}
+
+inline ATTR int
+inline_func2 (int x)
+{
+ int y = 3; /* inline_func2 */
+
+ return y + not_inline_func1 (x);
+}
+
+int
+not_inline_func2 (int x)
+{
+ int y = 4; /* not_inline_func2 */
+
+ return y + inline_func2 (x);
+}
+
+static inline ATTR int
+inline_func3 (int x)
+{
+ int y = 5; /* inline_func3 */
+
+ return y + not_inline_func2 (x);
+}
+
+static int
+not_inline_func3 (int x)
+{
+ int y = 6; /* not_inline_func3 */
+
+ return y + inline_func3 (x);
+}
+
/* Entry point. */
int
@@ -155,5 +203,7 @@ main (int argc, char *argv[])
x = func8a (x) + func8b (x);
+ x = not_inline_func3 (-21);
+
return x;
}
diff --git a/gdb/testsuite/gdb.opt/inline-break.exp b/gdb/testsuite/gdb.opt/inline-break.exp
index 91766d4..7965c77 100644
--- a/gdb/testsuite/gdb.opt/inline-break.exp
+++ b/gdb/testsuite/gdb.opt/inline-break.exp
@@ -211,4 +211,39 @@ for {set i 1} {$i <= [llength [array names results]]} {incr i} {
gdb_test "interpreter-exec mi -break-info 1" \
".*,call-site-func=\"main\",call-site-file=\".*\",call-site-fullname=\".*\",call-site-line=\".*\".*"
+# Start us running.
+if {![runto main]} {
+ untested "could not run to main"
+ return -1
+}
+
+# Insert breakpoints for all inline_func? and not_inline_func? and check
+# that we actually stop where we think we should.
+
+for {set i 1} {$i < 4} {incr i} {
+ foreach inline {"not_inline" "inline"} {
+ gdb_breakpoint "${inline}_func$i" message
+ }
+}
+
+set ws {[\r\n\t ]+}
+set backtrace [list "(in|at)? main"]
+for {set i 3} {$i > 0} {incr i -1} {
+
+ foreach inline {"not_inline" "inline"} {
+
+ # Check that we stop at the correct location and print out
+ # the (possibly) inlined frames.
+ set num [gdb_get_line_number "/* ${inline}_func$i */"]
+ set pattern ".*/$srcfile:$num${ws}.*$num${ws}int y = $decimal;"
+ append pattern "${ws}/\\\* ${inline}_func$i \\\*/"
+ send_log "Expecting $pattern\n"
+ gdb_continue_to_breakpoint "${inline}_func$i" $pattern
+
+ # Also check for the correct backtrace.
+ set backtrace [linsert $backtrace 0 "(in|at)?${ws}${inline}_func$i"]
+ gdb_test_sequence "bt" "bt stopped in ${inline}_func$i" $backtrace
+ }
+}
+
unset -nocomplain results
--
2.1.0