[PATCH] handle dprintf error more gracefully

[Antoine Tremblay]

ping...

On 02/10/2015 11:27 AM, Antoine Tremblay wrote:
> If a dprintf generates an error, handle it so that we leave
> the program as stopped at location as if a real breakpoint was hit.
> On the mi interface this sends a *stopped event  and avoids
> breaking frontend functionality. On cli it prints as a breakpoint
> would.
>
> Here's an example of what is returned on the mi interface :
>
> &"No symbol \"invalidvar1\" in current context.\n"
> ~"\nBreakpoint "
> ~"5, invalid () at ../../../gdb/testsuite/gdb.mi/mi-dprintf.c:37\n"
> ~"37\t  ++i; /* set dprintf 2 here */\n"
> *stopped,reason="breakpoint-hit",disp="keep",bkptno="5",frame={addr="0x0000000000400665",
> func="invalid",args=[],file="../../../gdb/testsuite/gdb.mi/mi-dprintf.c",
> fullname="/home/eantotr/src/binutils-gdb/gdb/testsuite/gdb.mi/mi-dprintf.c",line="37"},
> thread-id="1",stopped-threads="all",core="2"
>
> Here's an example for the cli :
>
> No symbol "asdf" in current context.
>
> Breakpoint 1, main () at testdprintf.c:8
> 8	    printf("i is %d\n",i);
>
> Note we could also treat the error as a warning and continue
> the program's execution by leaving bs->stop at 0 and continuing.
>
> I sided with the stop on error for now since I though an error
> should be treated as soon as possible, but continuing also
> seems like a possiblity. I'm unsure between the behavior of
> stopping at error or dprintf not stopping...
>
> Feedback is welcome, on the best way to handle this...
>
> gdb/ChangeLog:
> 	PR breakpoints/16551
> 	* breakpoint.c (dprintf_after_condition_true): Handle dprintf error.
> 	(bpstat_what) : Enable dprintf to print on stop.
>
> gdb/testsuite/ChangeLog:
> 	PR breakpoints/16551
> 	* gdb.mi/mi-dprintf.c (invalid): New function to test invalid dprintf.
> 	* gdb.mi/mi-dprintf.exp: Add invalid dprintf test.
> ---
>   gdb/breakpoint.c                    |   21 +++++++++++++++++++--
>   gdb/testsuite/gdb.mi/mi-dprintf.c   |   12 +++++++++++-
>   gdb/testsuite/gdb.mi/mi-dprintf.exp |   15 +++++++++++++++
>   3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 2804453..b51d17c 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5818,7 +5818,12 @@ bpstat_what (bpstat bs_head)
>   
>   	case bp_dprintf:
>   	  if (bs->stop)
> -	    this_action = BPSTAT_WHAT_STOP_SILENT;
> +	    {
> +	      if (bs->print)
> +		this_action = BPSTAT_WHAT_STOP_NOISY;
> +	      else
> +		this_action = BPSTAT_WHAT_STOP_SILENT;
> +	    }
>   	  else
>   	    this_action = BPSTAT_WHAT_SINGLE;
>   	  break;
> @@ -13885,6 +13890,7 @@ dprintf_after_condition_true (struct bpstats *bs)
>     struct cleanup *old_chain;
>     struct bpstats tmp_bs = { NULL };
>     struct bpstats *tmp_bs_p = &tmp_bs;
> +  volatile struct gdb_exception e;
>   
>     /* dprintf's never cause a stop.  This wasn't set in the
>        check_status hook instead because that would make the dprintf's
> @@ -13900,7 +13906,18 @@ dprintf_after_condition_true (struct bpstats *bs)
>     bs->commands = NULL;
>     old_chain = make_cleanup_decref_counted_command_line (&tmp_bs.commands);
>   
> -  bpstat_do_actions_1 (&tmp_bs_p);
> +  TRY_CATCH (e, RETURN_MASK_ERROR)
> +    {
> +      bpstat_do_actions_1 (&tmp_bs_p);
> +    }
> +  if (e.reason < 0)
> +    {
> +      /* Do stop if we have an error executing a dprintf command.  */
> +      bs->stop = 1;
> +      /* Print the breakpoint information.  */
> +      bs->print = 1;
> +      exception_print (gdb_stderr, e);
> +    }
>   
>     /* 'tmp_bs.commands' will usually be NULL by now, but
>        bpstat_do_actions_1 may return early without processing the whole
> diff --git a/gdb/testsuite/gdb.mi/mi-dprintf.c b/gdb/testsuite/gdb.mi/mi-dprintf.c
> index 0b8fc82..ed7d87e 100644
> --- a/gdb/testsuite/gdb.mi/mi-dprintf.c
> +++ b/gdb/testsuite/gdb.mi/mi-dprintf.c
> @@ -29,6 +29,14 @@ foo (int arg)
>     g /= 2.5; /* set breakpoint 1 here */
>   }
>   
> +/* Function to test invalid symbols used in dprinf.  */
> +void
> +invalid (void)
> +{
> +  int i = 0;
> +  ++i; /* set dprintf 2 here */
> +}
> +
>   int
>   main (int argc, char *argv[])
>   {
> @@ -40,7 +48,9 @@ main (int argc, char *argv[])
>   
>     foo (loc++);
>     foo (loc++);
> -  foo (loc++);
> +
> +  invalid ();
> +
>     return g;
>   }
>   
> diff --git a/gdb/testsuite/gdb.mi/mi-dprintf.exp b/gdb/testsuite/gdb.mi/mi-dprintf.exp
> index ea198bd..0cdf2b5 100644
> --- a/gdb/testsuite/gdb.mi/mi-dprintf.exp
> +++ b/gdb/testsuite/gdb.mi/mi-dprintf.exp
> @@ -33,6 +33,7 @@ mi_delete_breakpoints
>   
>   set bp_location1 [gdb_get_line_number "set breakpoint 1 here"]
>   set dp_location1 [gdb_get_line_number "set dprintf 1 here"]
> +set dp_location2 [gdb_get_line_number "set dprintf 2 here"]
>   
>   mi_run_to_main
>   
> @@ -60,12 +61,22 @@ set bps [mi_make_breakpoint -type dprintf -func foo \
>   mi_gdb_test "[incr i]-dprintf-insert $dp_location1 \"arg=%d, g=%d\\n\" arg g" \
>   		   "$i\\^done,$bps" "mi insert dprintf dp_location1"
>   
> +set bps [mi_make_breakpoint -type dprintf -func invalid \
> +		   -file ".*mi-dprintf.c" -fullname ".*mi-dprintf.c" \
> +		   -line $dp_location2]
> +mi_gdb_test "[incr i]-dprintf-insert $dp_location2 \"g=%d\\n\" invalidvar1" \
> +		   "$i\\^done,$bps" "mi insert dprintf dp_location2"
> +
>   set bps {}
>   lappend bps [mi_make_breakpoint -number 3 -type dprintf -func foo \
>   		 -file ".*mi-dprintf.c" -fullname ".*mi-dprintf.c"]
>   lappend bps [mi_make_breakpoint -type dprintf -func foo \
>   		 -file ".*mi-dprintf.c" -fullname ".*mi-dprintf.c" \
>   		 -line $dp_location1]
> +lappend bps [mi_make_breakpoint -type dprintf -func invalid \
> +		 -file ".*mi-dprintf.c" -fullname ".*mi-dprintf.c" \
> +		 -line $dp_location2]
> +
>   mi_gdb_test "[incr i]-break-info" \
>       "$i\\^done,[mi_make_breakpoint_table $bps]" \
>       "mi info dprintf"
> @@ -111,6 +122,10 @@ proc mi_continue_dprintf {args} {
>               }
>   	}
>   	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg 2nd stop"
> +
> +	set msg "mi 3nd dprintf"
> +	mi_send_resuming_command "exec-continue" "$msg continue"
> +	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg 3nd stop"
>       }
>   }
>