From: Keith Seitz Date: Tue, 22 Aug 2017 16:37:58 +0000 (-0700) Subject: Report stop locations in inlined functions X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=83b9557361c2d79479e17d5967f5180c371fa138;p=thirdparty%2Fbinutils-gdb.git Report stop locations in inlined functions 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; gdb/ChangeLog: * 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: * 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. --- diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index b48c405b084..05605893ac0 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5357,58 +5357,25 @@ need_moribund_for_location_type (struct bp_location *loc) && !target_supports_stopped_by_hw_breakpoint ())); } - -/* 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. */ +/* See breakpoint.h. */ bpstat -bpstat_stop_status (const address_space *aspace, - CORE_ADDR bp_addr, ptid_t ptid, +build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr, const struct target_waitstatus *ws) { - struct breakpoint *b = NULL; - struct bp_location *bl; - struct bp_location *loc; - /* First item of allocated bpstat's. */ + struct breakpoint *b; 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) { if (!breakpoint_enabled (b)) continue; - for (bl = b->loc; bl != NULL; bl = bl->next) + for (bp_location *bl = b->loc; bl != NULL; bl = bl->next) { /* For hardware watchpoints, we look only at the first location. The watchpoint_check function will work on the - entire expression, not the individual locations. For + read watchpoints, the watchpoints_triggered function has checked all locations already. */ if (b->type == bp_hardware_watchpoint && bl != b->loc) @@ -5423,8 +5390,8 @@ bpstat_stop_status (const address_space *aspace, /* Come here if it's a watchpoint, or if the break address matches. */ - bs = new bpstats (bl, &bs_link); /* Alloc a bpstat to - explain stop. */ + bpstat bs = new bpstats (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 @@ -5449,12 +5416,15 @@ bpstat_stop_status (const 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 = new bpstats (loc, &bs_link); + bpstat bs = new bpstats (loc, &bs_link); /* For hits of moribund locations, we should just proceed. */ bs->stop = 0; bs->print = 0; @@ -5463,6 +5433,49 @@ bpstat_stop_status (const address_space *aspace, } } + return bs_head; +} + +/* 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 (const address_space *aspace, + CORE_ADDR bp_addr, ptid_t ptid, + const struct target_waitstatus *ws, + bpstat stop_chain) +{ + struct breakpoint *b = NULL; + /* First item of allocated bpstat's. */ + bpstat bs_head = stop_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) + bs_head = build_bpstat_chain (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 unloaded libraries are correct before we handle "catch load" and @@ -6780,22 +6793,9 @@ describe_other_breakpoints (struct gdbarch *gdbarch, } -/* Return true iff it is meaningful to use the address member of - BPT locations. For some breakpoint types, the locations' address members - are irrelevant and it makes no sense to attempt to compare them to other - addresses (or use them for any other purpose either). - - More specifically, each of the following breakpoint types will - always have a zero valued location address and we don't want to mark - breakpoints of any of these types to be a duplicate of an actual - breakpoint location at address zero: - - bp_watchpoint - bp_catchpoint - -*/ +/* See breakpoint.h. */ -static int +bool breakpoint_address_is_meaningful (struct breakpoint *bpt) { enum bptype type = bpt->type; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 6406a1d32b0..9d1797690ef 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -917,9 +917,35 @@ extern void bpstat_clear (bpstat *); is part of the bpstat is copied as well. */ extern bpstat bpstat_copy (bpstat); +/* Build the bstat chain for the stop information given by ASPACE, + BP_ADDR, and WS. Returns the head of the bpstat chain. */ + +extern bpstat build_bpstat_chain (const address_space *aspace, + CORE_ADDR bp_addr, + const struct target_waitstatus *ws); + extern bpstat bpstat_stop_status (const address_space *aspace, CORE_ADDR pc, ptid_t ptid, - const struct target_waitstatus *ws); + const struct target_waitstatus *ws, + bpstat stop_chain = NULL); + +/* Return true iff it is meaningful to use the address member of + BPT locations. For some breakpoint types, the locations' address members + are irrelevant and it makes no sense to attempt to compare them to other + addresses (or use them for any other purpose either). + + More specifically, each of the following breakpoint types will + always have a zero valued location address and we don't want to mark + breakpoints of any of these types to be a duplicate of an actual + breakpoint location at address zero: + + bp_watchpoint + bp_catchpoint + +*/ + +extern bool breakpoint_address_is_meaningful (struct breakpoint *bpt); + /* This bpstat_what stuff tells wait_for_inferior what to do with a breakpoint (a challenging task). diff --git a/gdb/infrun.c b/gdb/infrun.c index d7df3c7d576..2b1402437bc 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5839,6 +5839,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 @@ -5870,7 +5871,13 @@ handle_signal_stop (struct execution_control_state *ecs) ecs->event_thread->prev_pc, &ecs->ws))) { - skip_inline_frames (ecs->ptid); + struct breakpoint *bpt = NULL; + + stop_chain = build_bpstat_chain (aspace, stop_pc, &ecs->ws); + if (stop_chain != NULL) + bpt = stop_chain->breakpoint_at; + + skip_inline_frames (ecs->ptid, bpt); /* Re-fetch current thread's frame in case that invalidated the frame cache. */ @@ -5919,7 +5926,7 @@ handle_signal_stop (struct execution_control_state *ecs) handles this event. */ ecs->event_thread->control.stop_bpstat = bpstat_stop_status (get_current_regcache ()->aspace (), - stop_pc, ecs->ptid, &ecs->ws); + stop_pc, ecs->ptid, &ecs->ws, stop_chain); /* Following in case break condition called a function. */ diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index b6154cdcc59..f06ecf61a61 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -301,7 +301,7 @@ 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; @@ -327,7 +327,25 @@ skip_inline_frames (ptid_t ptid) if (BLOCK_START (cur_block) == this_pc || block_starting_point_at (this_pc, cur_block)) { - skip_count++; + bool skip_this_frame = true; + + if (bpt != NULL + && user_breakpoint_p (bpt) + && breakpoint_address_is_meaningful (bpt)) + { + for (bp_location *loc = bpt->loc; loc != NULL; + loc = loc->next) + { + if (loc->address == this_pc) + { + skip_this_frame = false; + break; + } + } + } + + 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 e6fe29db1d1..8afaf6c9459 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 2616a0e3d7c..a42bc0bb443 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 20bae071b3d..0a990fbfd55 100644 --- a/gdb/testsuite/gdb.opt/inline-break.exp +++ b/gdb/testsuite/gdb.opt/inline-break.exp @@ -185,4 +185,39 @@ for {set i 1} {$i <= [array size results]} {incr i} { gdb_test "info break $i" $results($i) } +# 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