[PATCH 1/4] Fix dprintf bugs

[Yao Qi <yao@codesourcery.com>]

Hi,
This patch fixes PR15075.  Besides this bug, this patch also fixes
some other problems:

  - Two dprintf are set on the same address.  Only one printf is
    executed.
  - dprintf and regular breakpoint are set on the same address,
    inferior doesn't stop.

In order to fix these problems, I don't append "continue" command to
dprintf breakpoint, and execute commands of dprintf after
bpstat_check_breakpoint_conditions if condition is true.  The reason I
choose this place (after bpstat_check_breakpoint_conditions instead of
in bpstat_check_breakpoint_conditions) to handle dprintf is that we
have to set BS->stop to zero, but have to update hit count if
condition is true, so we have to do two things together.

With this patch, GDB executes dprintf commands after
bpstat_check_breakpoint_conditions, the different place where
breakpoint commands are executed.  I propose to disable setting
command for dprintf, or disallow user setting commands for dprintf.
If we believe it is important to allow user setting commands for
dprintf (a good use case?), we need a new design like creating struct
dprintf as a 'sub-class' of breakpoint and adding its own field for
printf commands.  The code on sending commands to the remote should be
adjusted as well.

Running gdb.base/pr15075.exp on mainline GDB (unpatched) will get the
following FAIL,

  FAIL: gdb.base/pr15075.exp: p x

and this patch fixes this FAIL.

gdb:

2013-02-28  Yao Qi  <yao@codesourcery.com>

	PR breakpoints/15075
	* breakpoint.c (bpstat_stop_status): Execute commands of
	dprintf if BS->stop is true.  Update hit count and set
	BS->stop to zero.
	(bpstat_what): Set 'this_action' to BPSTAT_WHAT_SINGLE if
	breakpoint is dprintf.
	(update_dprintf_command_list): Don't append "continue" command
	to the commands of dprintf.
	(do_map_commands_command): If breakpoint is dprintf, throw an
	error.

	* NEWS: Mention that GDB disallows setting commands for dprintf.

gdb/testsuite:

2013-02-28  Yao Qi  <yao@codesourcery.com>

	* gdb.base/dprintf.exp: DOn't check "continue" in the output
	of "info breakpoints".
	* gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify):
	Don't check "continue" in script field.
	* gdb.base/pr15075.c: New.
	* gdb.base/pr15075.exp: New.
---
 gdb/NEWS                                       |    2 +
 gdb/breakpoint.c                               |   43 +++++++++++++++--------
 gdb/testsuite/gdb.base/dprintf.exp             |    2 -
 gdb/testsuite/gdb.base/pr15075.c               |   26 ++++++++++++++
 gdb/testsuite/gdb.base/pr15075.exp             |   38 +++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp |    2 +-
 6 files changed, 95 insertions(+), 18 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/pr15075.c
 create mode 100644 gdb/testsuite/gdb.base/pr15075.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 0877aa2..2f0a157 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -70,6 +70,8 @@ Lynx 178 PowerPC		powerpc-*-lynx*178
 * The command 'info tracepoints' can now display 'installed on target'
   or 'not installed on target' for each non-pending location of tracepoint.
 
+* GDB now disallows setting commands for dynamic printf breakpoint.
+
 * New configure options
 
 --enable-libmcheck/--disable-libmcheck
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index fb57a57..d4c3690 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1295,6 +1295,10 @@ do_map_commands_command (struct breakpoint *b, void *data)
 {
   struct commands_info *info = data;
 
+  if (b->type == bp_dprintf)
+    error (_("Setting commands for dprintf breakpoint is not "
+	     "allowed"));
+
   if (info->cmd == NULL)
     {
       struct command_line *l;
@@ -5272,6 +5276,25 @@ bpstat_stop_status (struct address_space *aspace,
 	{
 	  bpstat_check_breakpoint_conditions (bs, ptid);
 
+	  if (bs->stop && b->type == bp_dprintf)
+	    {
+	      struct counted_command_line * ccmd = b->commands;
+	      struct command_line *cmd = ccmd ? ccmd->commands : NULL;
+
+	      while (cmd != NULL)
+		{
+		  execute_control_command (cmd);
+		  cmd = cmd->next;
+		}
+
+	      /* Update the hit count of dprintf breakpoint, although
+		 we set BS->stop zero.  */
+	      ++(b->hit_count);
+	      observer_notify_breakpoint_modified (b);
+
+	      bs->stop = 0;
+	    }
+
 	  if (bs->stop)
 	    {
 	      ++(b->hit_count);
@@ -5519,7 +5542,7 @@ bpstat_what (bpstat bs_head)
 	  break;
 
 	case bp_dprintf:
-	  this_action = BPSTAT_WHAT_STOP_SILENT;
+	  this_action = BPSTAT_WHAT_SINGLE;
 	  break;
 
 	default:
@@ -8947,25 +8970,15 @@ update_dprintf_command_list (struct breakpoint *b)
 		    _("Invalid dprintf style."));
 
   gdb_assert (printf_line != NULL);
-  /* Manufacture a printf/continue sequence.  */
+  /* Manufacture a printf command.  */
   {
-    struct command_line *printf_cmd_line, *cont_cmd_line = NULL;
-
-    if (strcmp (dprintf_style, dprintf_style_agent) != 0)
-      {
-	cont_cmd_line = xmalloc (sizeof (struct command_line));
-	cont_cmd_line->control_type = simple_control;
-	cont_cmd_line->body_count = 0;
-	cont_cmd_line->body_list = NULL;
-	cont_cmd_line->next = NULL;
-	cont_cmd_line->line = xstrdup ("continue");
-      }
+    struct command_line *printf_cmd_line
+      = xmalloc (sizeof (struct command_line));
 
-    printf_cmd_line = xmalloc (sizeof (struct command_line));
     printf_cmd_line->control_type = simple_control;
     printf_cmd_line->body_count = 0;
     printf_cmd_line->body_list = NULL;
-    printf_cmd_line->next = cont_cmd_line;
+    printf_cmd_line->next = NULL;
     printf_cmd_line->line = printf_line;
 
     breakpoint_set_commands (b, printf_cmd_line);
diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
index 2b00c24..ffbd3f2 100644
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -48,10 +48,8 @@ gdb_test_sequence "info breakpoints" "dprintf info 1" {
     "\[\r\n\]2       breakpoint"
     "\[\r\n\]3       dprintf"
     "\[\r\n\]        printf \"At foo entry\\\\n\""
-    "\[\r\n\]        continue"
     "\[\r\n\]4       dprintf"
     "\[\r\n\]        printf \"arg=%d, g=%d\\\\n\", arg, g"
-    "\[\r\n\]        continue"
 }
 
 gdb_test "break $bp_location1" \
diff --git a/gdb/testsuite/gdb.base/pr15075.c b/gdb/testsuite/gdb.base/pr15075.c
new file mode 100644
index 0000000..60fe8cf
--- /dev/null
+++ b/gdb/testsuite/gdb.base/pr15075.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main(void)
+ {
+   int x = 5;
+
+   ++x;
+   ++x; /* set dprintf here */
+   return x - 7;
+ }
diff --git a/gdb/testsuite/gdb.base/pr15075.exp b/gdb/testsuite/gdb.base/pr15075.exp
new file mode 100644
index 0000000..86b24cc
--- /dev/null
+++ b/gdb/testsuite/gdb.base/pr15075.exp
@@ -0,0 +1,38 @@
+# Copyright 2013 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+set executable $testfile
+set expfile $testfile.exp
+
+set dp_location [gdb_get_line_number "set dprintf here"]
+
+if [prepare_for_testing $expfile $executable $srcfile {debug}] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return -1
+}
+
+gdb_test "dprintf $dp_location, \"%d\\n\", x" \
+    "Dprintf .*"
+
+gdb_test "next" ".*" "next 1"
+gdb_test "next" ".*" "next 2"
+# Test inferior doesn't exit.
+gdb_test "p x" ".* = 6"
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
index fd32698..24ff55b 100644
--- a/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
+++ b/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp
@@ -96,7 +96,7 @@ proc test_insert_delete_modify { } {
 	$test
     set test "dprintf marker, \"arg\" \""
     mi_gdb_test $test \
-	{.*=breakpoint-created,bkpt=\{number="6",type="dprintf".*,script=\{\"printf \\\\\"arg\\\\\" \\\\\"\",\"continue\"\}.*\}\r\n\^done} \
+	{.*=breakpoint-created,bkpt=\{number="6",type="dprintf".*,script=\{\"printf \\\\\"arg\\\\\" \\\\\"\"\}.*\}\r\n\^done} \
 	$test
 
     # 2. when modifying condition
-- 
1.7.7.6