[PATCHv5 05/11] gdb: don't always print breakpoint location after failed condition check

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Consider the following session:
>
>   (gdb) list some_func
>   1	int
>   2	some_func ()
>   3	{
>   4	  int *p = 0;
>   5	  return *p;
>   6	}
>   7
>   8	void
>   9	foo ()
>   10	{
>   (gdb) break foo if (some_func ())
>   Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
>   (gdb) r
>   Starting program: /tmp/bpcond
>
>   Program received signal SIGSEGV, Segmentation fault.
>   0x0000000000401116 in some_func () at bpcond.c:5
>   5	  return *p;
>   Error in testing condition for breakpoint 1:
>   The program being debugged stopped while in a function called from GDB.
>   Evaluation of the expression containing the function
>   (some_func) will be abandoned.
>   When the function is done executing, GDB will silently stop.
>
>   Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5
>   5	  return *p;
>   (gdb)
>
> What happens here is the breakpoint condition includes a call to an
> inferior function, and the inferior function segfaults.  We can see
> that GDB reports the segfault, and then gives an error message that
> indicates that an inferior function call was interrupted.
>
> After this GDB appears to report that it is stopped at Breakpoint 1,
> inside some_func.
>
> I find this second stop report a little confusing.  While it is true
> that GDB stopped as a result of hitting breakpoint 1, I think the
> message GDB currently prints might give the impression that GDB is
> actually stopped at a location of breakpoint 1, which is not the case.
>
> Also, I find the second stop message draws attention away from
> the "Program received signal SIGSEGV, Segmentation fault" stop
> message, and this second stop might be thought of as replacing in
> someway the earlier message.
>
> In short, I think things would be clearer if the second stop message
> were not reported at all, so the output should, I think, look like
> this:
>
>   (gdb) list some_func
>   1	int
>   2	some_func ()
>   3	{
>   4	  int *p = 0;
>   5	  return *p;
>   6	}
>   7
>   8	void
>   9	foo ()
>   10	{
>   (gdb) break foo if (some_func ())
>   Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
>   (gdb) r
>   Starting program: /tmp/bpcond
>
>   Program received signal SIGSEGV, Segmentation fault.
>   0x0000000000401116 in some_func () at bpcond.c:5
>   5	  return *p;
>   Error in testing condition for breakpoint 1:
>   The program being debugged stopped while in a function called from GDB.
>   Evaluation of the expression containing the function
>   (some_func) will be abandoned.
>   When the function is done executing, GDB will silently stop.
>   (gdb)
>
> The user can still find the number of the breakpoint that triggered
> the initial stop in this line:
>
>   Error in testing condition for breakpoint 1:
>
> But there's now only one stop reason reported, the SIGSEGV, which I
> think is much clearer.
>
> To achieve this change I set the bpstat::print field when:
>
>   (a) a breakpoint condition evaluation failed, and
>
>   (b) the $pc of the thread changed during condition evaluation.
>
> I've updated the existing tests that checked the error message printed
> when a breakpoint condition evaluation failed.

I went ahead and pushed this commit.

If there are any problems, please let me know, I'm happy to address any
issues.

Thanks,
Andrew


> ---
>  gdb/breakpoint.c                           | 12 ++++++++++++
>  gdb/testsuite/gdb.base/infcall-failure.exp | 18 ++++++++----------
>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index dcd42e6b2e2..445b46a706a 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5534,6 +5534,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>  	  else
>  	    within_current_scope = false;
>  	}
> +      CORE_ADDR pc_before_check = get_frame_pc (get_selected_frame (nullptr));
>        if (within_current_scope)
>  	{
>  	  try
> @@ -5545,6 +5546,17 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>  	      exception_fprintf (gdb_stderr, ex,
>  				 "Error in testing condition for breakpoint %d:\n",
>  				 b->number);
> +
> +	      /* If the pc value changed as a result of evaluating the
> +		 condition then we probably stopped within an inferior
> +		 function call due to some unexpected stop, e.g. the thread
> +		 hit another breakpoint, or the thread received an
> +		 unexpected signal.  In this case we don't want to also
> +		 print the information about this breakpoint.  */
> +	      CORE_ADDR pc_after_check
> +		= get_frame_pc (get_selected_frame (nullptr));
> +	      if (pc_before_check != pc_after_check)
> +		bs->print = 0;
>  	    }
>  	}
>        else
> diff --git a/gdb/testsuite/gdb.base/infcall-failure.exp b/gdb/testsuite/gdb.base/infcall-failure.exp
> index 214a64f8de3..5ad179a089d 100644
> --- a/gdb/testsuite/gdb.base/infcall-failure.exp
> +++ b/gdb/testsuite/gdb.base/infcall-failure.exp
> @@ -78,10 +78,7 @@ proc_with_prefix run_cond_hits_breakpoint_test { async_p non_stop_p } {
>  	     "The program being debugged stopped while in a function called from GDB\\." \
>  	     "Evaluation of the expression containing the function" \
>  	     "\\(func_bp\\) will be abandoned\\." \
> -	     "When the function is done executing, GDB will silently stop\\." \
> -	     "" \
> -	     "Breakpoint ${bp_1_num}, \[^\r\n\]+" \
> -	     "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+"]
> +	     "When the function is done executing, GDB will silently stop\\."]
>  }
>  
>  # Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior
> @@ -138,13 +135,12 @@ proc_with_prefix run_cond_hits_segfault_test { async_p non_stop_p } {
>  	     "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \
>  	     "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \
>  	     "Error in testing condition for breakpoint ${bp_1_num}:" \
> -	     "The program being debugged stopped while in a function called from GDB\\." \
> +	     "The program being debugged was signaled while in a function called from GDB\\." \
> +	     "GDB remains in the frame where the signal was received\\." \
> +	     "To change this behavior use \"set unwindonsignal on\"\\." \
>  	     "Evaluation of the expression containing the function" \
>  	     "\\(func_segfault\\) will be abandoned\\." \
> -	     "When the function is done executing, GDB will silently stop\\." \
> -	     "" \
> -	     "Breakpoint ${bp_1_num}, \[^\r\n\]+" \
> -	     "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"]
> +	     "When the function is done executing, GDB will silently stop\\."]
>  }
>  
>  # Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior
> @@ -168,7 +164,9 @@ proc_with_prefix run_call_hits_segfault_test { async_p non_stop_p } {
>  	     "Program received signal SIGSEGV, Segmentation fault\\." \
>  	     "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \
>  	     "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \
> -	     "The program being debugged stopped while in a function called from GDB\\." \
> +	     "The program being debugged was signaled while in a function called from GDB\\." \
> +	     "GDB remains in the frame where the signal was received\\." \
> +	     "To change this behavior use \"set unwindonsignal on\"\\." \
>  	     "Evaluation of the expression containing the function" \
>  	     "\\(func_segfault\\) will be abandoned\\." \
>  	     "When the function is done executing, GDB will silently stop\\."]
> -- 
> 2.25.4