[PATCH] handle dprintf error more gracefully

Antoine Tremblay antoine.tremblay@ericsson.com
Tue Feb 10 16:27:00 GMT 2015


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"
     }
 }
 
-- 
1.7.9.5



More information about the Gdb-patches mailing list