提交: 83b9557361c2d79479e17d5967f5180c371fa138 - binutils_os216 (git)

GNU Binutils with patches for OS216

Commit MetaInfo

• 修訂: 83b9557361c2d79479e17d5967f5180c371fa138 (tree)
• 時間: 2017-11-29 05:39:19
• 作者: Keith Seitz <keiths@redh...>
• Commiter: Keith Seitz

Log Message

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.

Change Summary

• modified: gdb/breakpoint.c
• modified: gdb/breakpoint.h
• modified: gdb/infrun.c
• modified: gdb/inline-frame.c
• modified: gdb/inline-frame.h
• modified: gdb/testsuite/gdb.opt/inline-break.c
• modified: gdb/testsuite/gdb.opt/inline-break.exp

差異

--- 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;

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

--- 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.  */

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

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

--- 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;
 }

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