Re: [PATCH 2/2] Report stop locations in inlined functions.

[Pedro Alves <palves@redhat.com>]

On 07/11/2017 03:36 AM, Keith Seitz wrote:
> 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.

I'm still trying to grok these patches fully, but, shouldn't comparing
the breakpoint's bp_location's addresses work the same?  I.e., with this,
gdb.opt/inline-break.exp still passes cleanly here:

From a7b8eb698d2853fc16bca1b933be5c517dd1d446 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 18 Jul 2017 18:09:50 +0100
Subject: [PATCH] no decode

---
 gdb/breakpoint.c   |  2 +-
 gdb/inline-frame.c | 45 +++++++++++----------------------------------
 2 files changed, 12 insertions(+), 35 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 05eb71e..c149e5e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7165,7 +7165,7 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
 
 */
 
-static int
+int
 breakpoint_address_is_meaningful (struct breakpoint *bpt)
 {
   enum bptype type = bpt->type;
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index 006ae0d..9120554 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -297,6 +297,8 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block)
   return 1;
 }
 
+int breakpoint_address_is_meaningful (struct breakpoint *bpt);
+
 /* Skip all inlined functions whose call sites are at the current PC.
    Frames for the hidden functions will not appear in the backtrace until the
    user steps into them.  */
@@ -309,27 +311,6 @@ skip_inline_frames (ptid_t ptid, struct breakpoint *bpt)
   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
@@ -349,23 +330,19 @@ skip_inline_frames (ptid_t ptid, struct breakpoint *bpt)
 	      if (BLOCK_START (cur_block) == this_pc
 		  || block_starting_point_at (this_pc, cur_block))
 		{
-		  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++)
+		  if (bpt != NULL
+		      && breakpoint_address_is_meaningful (bpt))
 		    {
-		      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)
+		      for (bp_location *loc = bpt->loc;
+			   loc != NULL;
+			   loc = loc->next)
+			if (this_pc == loc->address)
+			  {
 			    skip_this_frame = false;
-			}
+			    break;
+			  }
 		    }
 
 		  if (skip_this_frame)
-- 
2.5.5