[2/2] Report stop locations in inlined functions.

  On 07/18/2017 10:46 AM, Pedro Alves wrote:

> Maybe we need to move this bit in infrun.c:
> 
>   /* See if there is a breakpoint/watchpoint/catchpoint/etc. that
>      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_chain);
> 
> a bit above, before the skip_inline_frames call, and then
> you don't even need to pass down the bpstat to skip_inline_frames,
> as you can then access it from the thread directly?

I've tried this approach, and it causes a single test regression in gdb.python/py-finish-breakpoint.exp.

It's taken me quite some time to figure out what was happening, but the key is that the regressing test involved a breakpoint condition that was an inferior function call (which failed). In that case, the global inline frame state was left with a stray inline frame and the assertion in skip_inline_frames was triggered.

There are two ways to fix this. The easy way is to call clear_inline_frame_state sometime after the exception occurred, such as where the exception is caught in bpstat_check_breakpoint_conditions(*1). I'm not entirely confident that doing this every time is safe, so I'm leaning toward option #2.

The "more" complex option is to only create the stop_chain, later passing it to bpstat_stop_status(*2). This avoids checking the breakpoint condition before calling skip_inline_frames, avoiding the exception problem altogether.

If you've a preference, I'd like to hear it.

Keith

*1 The "simpler" solution (for illustration only)
  

On 10/20/2017 08:21 PM, Keith Seitz wrote:
> 
> *1 The "simpler" solution (for illustration only)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 32ceea7..4d2ebd8 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -68,6 +68,7 @@
>  #include "format.h"
>  #include "thread-fsm.h"
>  #include "tid-parse.h"
> +#include "inline-frame.h"
>  
>  /* readline include files */
>  #include "readline/readline.h"
> @@ -5359,6 +5360,7 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
>             }
>           CATCH (ex, RETURN_MASK_ALL)
>             {
> +             clear_inline_frame_state (ptid);
>               exception_fprintf (gdb_stderr, ex,
>                                  "Error in testing breakpoint condition:\n");
>             }
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index d00c5f6..60fd166 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -5864,6 +5864,12 @@ handle_signal_stop (struct execution_control_state *ecs)
>    stop_print_frame = 1;
>    stopped_by_random_signal = 0;
>  
> +  /* See if there is a breakpoint/watchpoint/catchpoint/etc. that
> +     handles this event.  */
> +  ecs->event_thread->control.stop_bpstat
> +    = bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
> +			  stop_pc, ecs->ptid, &ecs->ws);
> +
>    /* Hide inlined functions starting here, unless we just performed stepi or
>       nexti.  After stepi and nexti, always show the innermost frame (not any
>       inline function call sites).  */
> @@ -5894,7 +5900,12 @@ handle_signal_stop (struct execution_control_state *ecs)
>  					     ecs->event_thread->prev_pc,
>  					     &ecs->ws)))
>  	{
> -	  skip_inline_frames (ecs->ptid);
> +	  struct breakpoint *bpt = NULL;
> +
> +	  if (ecs->event_thread->control.stop_bpstat != NULL)
> +	    bpt = ecs->event_thread->control.stop_bpstat->breakpoint_at;
> +
> +	  skip_inline_frames (ecs->ptid, bpt);
>  
>  	  /* Re-fetch current thread's frame in case that invalidated
>  	     the frame cache.  */
> @@ -5939,12 +5950,6 @@ handle_signal_stop (struct execution_control_state *ecs)
>  	}
>      }
>  
> -  /* See if there is a breakpoint/watchpoint/catchpoint/etc. that
> -     handles this event.  */
> -  ecs->event_thread->control.stop_bpstat
> -    = bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
> -			  stop_pc, ecs->ptid, &ecs->ws);
> -
>    /* Following in case break condition called a
>       function.  */
>    stop_print_frame = 1;
> 
> 
> 
> *2 The more "complex," safer solution (for illustration only)

Can you send me these as full patches, or send me the
patch they apply on top of too?  I'll need to play with
it a bit to understand it all better.

Thanks,
Pedro Alves