Re: [RFC] PR 15075 dprintf interferes with "next"

[Hui Zhu <teawater@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3753 bytes --]

Hi,

Because this patch also can handle bug 15434, I updated the test
patches to add test dprintf with non-stop mode.

Thanks,
Hui

2013-05-16  Yao Qi  <yao@codesourcery.com>
	    Hui Zhu  <hui@codesourcery.com>

	PR breakpoints/15075
	PR breakpoints/15434
	* breakpoint.c (bpstat_stop_status): Call b->ops->after_cond.
	(update_dprintf_command_list): Don't append "continue" command
	to the command of dprintf breakpoint.
	(base_breakpoint_after_cond): New.
	(base_breakpoint_ops): Add base_breakpoint_after_cond.
	(dprintf_after_cond): New.
	(initialize_breakpoint_ops): Set dprintf_after_cond.
	* breakpoint.h (breakpoint_ops): Add after_cond.

2013-05-16  Yao Qi  <yao@codesourcery.com>
	    Hui Zhu  <hui@codesourcery.com>

	PR breakpoints/15075
	PR breakpoints/15434
	* gdb.base/dprintf.exp: Don't check "continue" in the output
	of "info breakpoints".
	Add test dprintf with non-stop mode.
	* gdb.base/dprintf.c: Add a sleep for test dprintf with non-stop
	mode.
	* 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.

On Wed, Apr 24, 2013 at 11:18 AM, Hui Zhu <teawater@gmail.com> wrote:
> On Wed, Apr 24, 2013 at 10:44 AM, Yao Qi <yao@codesourcery.com> wrote:
>> On 04/24/2013 07:59 AM, Hui Zhu wrote:
>>>
>>> 2013-04-23  Yao Qi<yao@codesourcery.com>
>>>             Hui Zhu<hui@codesourcery.com>
>>>
>>>         PR gdb/15075
>>
>>         ^^^^^^^^^^^^ "PR breakpoints/15075"
>>
>>
>>>
>>>         * breakpoint.c (bpstat_stop_status): Call b->ops->after_cond.
>>>         (update_dprintf_command_list): Don't append "continue" command
>>>         to the command of dprintf breakpoint.
>>>         (base_breakpoint_after_cond): New.
>>>         (base_breakpoint_ops): Add base_breakpoint_after_cond.
>>>         (dprintf_after_cond): New.
>>>         (initialize_breakpoint_ops): Set dprintf_after_cond.
>>>         * breakpoint.h (breakpoint_ops): Add after_cond.
>>>
>>> 2013-04-23  Hui Zhu<hui@codesourcery.com>
>>>
>>>         PR gdb/15075
>>>
>>>         * gdb.base/dprintf.exp: Remove "continue" from "info breakpoint"
>>>         test.
>>
>> We also need to stop checking "continue" in gdb.mi/mi-breakpoint-changed.exp
>> also, as what patch [1] does.  B.T.W, why don't you add
>> gdb.base/pr15075.[c,exp] from patch [1]?
>>
>> --
>> Yao (齐尧)
>>
>> [1] http://sourceware.org/ml/gdb-patches/2013-02/msg00736.html
>
> Oops, I am sorry that the discussion threads confused me.  Post a new
> version for it.
>
> And I didn't include the "disallows setting commands for dprintf"
> patch because looks it is still in discussion.
>
> Thanks,
> Hui
>
> 2013-04-23  Yao Qi  <yao@codesourcery.com>
>             Hui Zhu  <hui@codesourcery.com>
>
>         PR breakpoints/15075
>
>         * breakpoint.c (bpstat_stop_status): Call b->ops->after_cond.
>         (update_dprintf_command_list): Don't append "continue" command
>         to the command of dprintf breakpoint.
>         (base_breakpoint_after_cond): New.
>         (base_breakpoint_ops): Add base_breakpoint_after_cond.
>         (dprintf_after_cond): New.
>         (initialize_breakpoint_ops): Set dprintf_after_cond.
>         * breakpoint.h (breakpoint_ops): Add after_cond.
>
> 2013-04-23  Yao Qi  <yao@codesourcery.com>
>
>         PR breakpoints/15075
>
>         * 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.

[-- Attachment #2: dprintf-continue.txt --]
[-- Type: text/plain, Size: 3773 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5283,13 +5283,7 @@ bpstat_stop_status (struct address_space
 		    b->enable_state = bp_disabled;
 		  removed_any = 1;
 		}
-	      if (b->silent)
-		bs->print = 0;
-	      bs->commands = b->commands;
-	      incref_counted_command_line (bs->commands);
-	      if (command_line_is_silent (bs->commands
-					  ? bs->commands->commands : NULL))
-		bs->print = 0;
+	      b->ops->after_cond (bs);
 	    }
 
 	}
@@ -8939,25 +8933,16 @@ update_dprintf_command_list (struct brea
 		    _("Invalid dprintf style."));
 
   gdb_assert (printf_line != NULL);
-  /* Manufacture a printf/continue sequence.  */
+  /* Manufacture a printf sequence.  */
   {
-    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);
@@ -12743,6 +12728,22 @@ base_breakpoint_explains_signal (struct
   return BPSTAT_SIGNAL_HIDE;
 }
 
+/* The default 'after_cond' method.  */
+
+static void
+base_breakpoint_after_cond (struct bpstats *bs)
+{
+  struct breakpoint *b = bs->breakpoint_at;
+
+  if (b->silent)
+    bs->print = 0;
+  bs->commands = b->commands;
+  incref_counted_command_line (bs->commands);
+  if (command_line_is_silent (bs->commands
+			      ? bs->commands->commands : NULL))
+  bs->print = 0;
+}
+
 struct breakpoint_ops base_breakpoint_ops =
 {
   base_breakpoint_dtor,
@@ -12762,7 +12763,8 @@ struct breakpoint_ops base_breakpoint_op
   base_breakpoint_create_sals_from_address,
   base_breakpoint_create_breakpoints_sal,
   base_breakpoint_decode_linespec,
-  base_breakpoint_explains_signal
+  base_breakpoint_explains_signal,
+  base_breakpoint_after_cond,
 };
 
 /* Default breakpoint_ops methods.  */
@@ -13356,6 +13358,24 @@ dprintf_print_recreate (struct breakpoin
   print_recreate_thread (tp, fp);
 }
 
+/* Implement the "after_cond"  breakpoint_ops method for dprintf.  */
+
+static void
+dprintf_after_cond (struct bpstats *bs)
+{
+  bpstat tmp;
+  struct breakpoint *b = bs->breakpoint_at;
+
+  bs->stop = 0;
+  bs->print = 0;
+  bs->commands = b->commands;
+  tmp = bs->next;
+  bs->next = tmp;
+  incref_counted_command_line (bs->commands);
+  bpstat_do_actions_1 (&bs);
+  bs->next = tmp;
+}
+
 /* The breakpoint_ops structure to be used on static tracepoints with
    markers (`-m').  */
 
@@ -15852,6 +15872,7 @@ initialize_breakpoint_ops (void)
   ops->print_it = bkpt_print_it;
   ops->print_mention = bkpt_print_mention;
   ops->print_recreate = dprintf_print_recreate;
+  ops->after_cond = dprintf_after_cond;
 }
 
 /* Chain containing all defined "enable breakpoint" subcommands.  */
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -614,6 +614,9 @@ struct breakpoint_ops
      'catch signal' interact properly with 'handle'; see
      bpstat_explains_signal.  */
   enum bpstat_signal_value (*explains_signal) (struct breakpoint *);
+
+  /* Do some setup after check condition is true.  */
+  void (*after_cond) (struct bpstats *bs);
 };
 
 /* Helper for breakpoint_ops->print_recreate implementations.  Prints

[-- Attachment #3: dprintf-continue-test.txt --]
[-- Type: text/plain, Size: 4808 bytes --]

--- a/gdb/testsuite/gdb.base/dprintf.c
+++ b/gdb/testsuite/gdb.base/dprintf.c
@@ -39,6 +39,9 @@ main (int argc, char *argv[])
   foo (loc++);
   foo (loc++);
   foo (loc++);
+
+  sleep (3);
+
   return g;
 }
 
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -50,10 +50,8 @@ gdb_test_sequence "info breakpoints" "dp
     "\[\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" \
@@ -136,3 +134,41 @@ if $target_can_dprintf {
 gdb_test "set dprintf-style foobar" "Undefined item: \"foobar\"." \
     "Set dprintf style to an unrecognized type"
 
+
+# Test dprintf with non-stop mode.
+# The testfile uses "run".  The real bug happened only for ![is_remote target].
+if [target_info exists use_gdb_stub] {
+    unsupported "dprintf with non-stop mode"
+    return 0
+}
+gdb_test_no_output "delete 2 5"
+send_gdb "kill\n"
+gdb_expect 120 {
+    -re "Kill the program being debugged. .y or n. $" {
+	send_gdb "y\n"
+	verbose "\t\tKilling previous program being debugged"
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	# OK.
+    }
+}
+gdb_test_no_output "set dprintf-style gdb" "Set dprintf style to gdb"
+gdb_test_no_output "set target-async on"
+gdb_test_no_output "set non-stop on"
+set gdbindex_warning_re "warning: Skipping \[^\r\n\]+ \\.gdb_index section \[^\r\n\]*\r\nDo \"set use-deprecated-index-sections on\" before the file is read\r\nto use the section anyway\\."
+
+gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
+gdb_test "shell echo foo" "foo"
+set test "interrupt"
+gdb_test_multiple $test $test {
+    -re "interrupt\r\n$gdb_prompt " {
+	pass $test
+    }
+}
+set test "process stopped"
+gdb_test_multiple "" $test {
+    -re "\r\n\\\[process \[0-9\]+\\\] #1 stopped\\\.\r\n" {
+	pass $test
+    }
+}
--- /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;
+ }
--- /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"
--- 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