[PATCH] add -s option to make -break-insert support dprintf

[PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi,

This is the patches to add -s flags to make -break-info support dprintf.
mi-dprintf.txt is for the code.
mi-dprintf-doc.txt is for the doc.
mi-dprintf-test.txt is for the test.

After this patch, you can use -s option to add dprintf.  To see mi-dprintf-doc.txt to get info about how to use this function.

Please help me review it.

Thanks,
Hui

This is for NEWS:
   ** The -s of MI command -break-insert can set a dynamic printf.

2013-03-28  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (mi_cmd_break_insert): Add "-s".

2013-03-28  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Add "-s".

2013-03-28  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.c, gdb.mi/mi-dprintf.h: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -305,7 +305,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1211,6 +1211,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -99,15 +99,17 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
+  int dprintf = 0;
   struct cleanup *back_to;
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
       HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
       IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT, DISABLE_OPT,
-      TRACEPOINT_OPT,
+      TRACEPOINT_OPT, DPRINTF_OPT,
     };
   static const struct mi_opt opts[] =
   {
@@ -119,6 +121,7 @@ mi_cmd_break_insert (char *command, char
     {"f", PENDING_OPT, 0},
     {"d", DISABLE_OPT, 0},
     {"a", TRACEPOINT_OPT, 0},
+    {"s", DPRINTF_OPT, 1},
     { 0, 0, 0 }
   };
 
@@ -159,9 +162,15 @@ mi_cmd_break_insert (char *command, char
 	case TRACEPOINT_OPT:
 	  tracepoint = 1;
 	  break;
+	case DPRINTF_OPT:
+	  extra_string = oarg;
+	  dprintf = 1;
+	  break;
 	}
     }
 
+  if (hardware && dprintf)
+    error (_("-break-insert: -h and -s cannot be use together"));
   if (oind >= argc)
     error (_("-break-insert: Missing <location>"));
   if (oind < argc - 1)
@@ -180,11 +189,14 @@ mi_cmd_break_insert (char *command, char
      regular non-jump based tracepoints.  */
   type_wanted = (tracepoint
 		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+		 : (hardware ? bp_hardware_breakpoint
+		    : (dprintf ? bp_dprintf : bp_breakpoint)));
+  ops = tracepoint ? &tracepoint_breakpoint_ops
+		   : (dprintf ? &dprintf_breakpoint_ops
+			      : &bkpt_breakpoint_ops);
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 1599 bytes --]

--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28784,7 +28784,9 @@ N.A.
 @smallexample
  -break-insert [ -t ] [ -h ] [ -f ] [ -d ] [ -a ]
     [ -c @var{condition} ] [ -i @var{ignore-count} ]
-    [ -p @var{thread-id} ] [ @var{location} ]
+    [ -p @var{thread-id} ]
+    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
+    [ @var{location} ]
 @end smallexample
 
 @noindent
@@ -28824,6 +28826,29 @@ Make the breakpoint conditional on @var{
 Initialize the @var{ignore-count}.
 @item -p @var{thread-id}
 Restrict the breakpoint to the specified @var{thread-id}.
+@item -s "@var{location},@var{template},@var{expression}[,@var{expression}@dots{}]"
+Set a @ref{Dynamic Printf}.  The @var{location}, @var{template}
+and @var{expression} should be within double quotations and be escaped
+by being preceded with a backslash.
+
+@smallexample
+(gdb)
+4-break-insert -s "\"At foo entry\\n\"" foo
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-break-insert -s "\"arg=%d, g=%d\\n\", arg, g" 26
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @end table
 
 @subsubheading Result

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7845 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,58 @@
+/* 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/>.  */
+
+#include <stdio.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+#include <stdlib.h>
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,123 @@
+#   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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile .c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-dprintf.exp
+     return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-break-insert -s" \
+    "1\\^error,msg=\"-break-insert: Option -s requires an argument\"" "mi insert without location"
+
+mi_gdb_test "2-break-insert -s foo" \
+    "2\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-break-insert -s 29" \
+    "3\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-break-insert -s \"\\\"At foo entry\\\\n\\\"\" foo" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"25\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"25\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+mi_run_cmd
+
+mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf, gdb"
+
+mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf, gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf, call"
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf, call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf, fprintf"
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf, fprintf"
+}
+
+set target_can_dprintf 1
+set msg "Set dprintf style to agent"
+mi_gdb_test "1set dprintf-style agent" "\[^\n\]*\r\ndone"
+gdb_expect {
+    -re "\\^done" {
+	pass "$msg - can do"
+    }
+    -re ".*" {
+	set target_can_dprintf 0
+	pass "$msg - cannot do"
+    }
+    timeout {
+       fail "resume all, waiting for program exit (timeout)"
+    }
+}
+
+if $target_can_dprintf {
+
+    mi_run_cmd
+
+    mi_gdb_test "continue" "Breakpoint \[0-9\]+, foo .*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" "Breakpoint \[0-9\]+, foo .*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" \
+	"6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"25\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+	"mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Eli Zaretskii]

> Date: Thu, 28 Mar 2013 22:21:30 +0800
> From: Hui Zhu <hui_zhu@mentor.com>
> CC: Marc Khouzam <marc.khouzam@ericsson.com>, Eli Zaretskii <eliz@gnu.org>
> 
> This is the patches to add -s flags to make -break-info support dprintf.
> mi-dprintf.txt is for the code.
> mi-dprintf-doc.txt is for the doc.
> mi-dprintf-test.txt is for the test.
> 
> After this patch, you can use -s option to add dprintf.  To see mi-dprintf-doc.txt to get info about how to use this function.

Thanks.

> This is for NEWS:
>    ** The -s of MI command -break-insert can set a dynamic printf.

 "The new option -s of the MI command -break-insert sets a dynamic
  printf breakpoint."

> 2013-03-28  Hui Zhu  <hui@codesourcery.com>
> 
> 	* gdb.texinfo (GDB/MI Breakpoint Commands): Add "-s".
                                                    ^^^^^^^^
"Describe the -s option."

> --- a/doc/gdb.texinfo
> +++ b/doc/gdb.texinfo
> @@ -28784,7 +28784,9 @@ N.A.
>  @smallexample
>   -break-insert [ -t ] [ -h ] [ -f ] [ -d ] [ -a ]
>      [ -c @var{condition} ] [ -i @var{ignore-count} ]
> -    [ -p @var{thread-id} ] [ @var{location} ]
> +    [ -p @var{thread-id} ]
> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
> +    [ @var{location} ]
>  @end smallexample
>  
>  @noindent
> @@ -28824,6 +28826,29 @@ Make the breakpoint conditional on @var{
>  Initialize the @var{ignore-count}.
>  @item -p @var{thread-id}
>  Restrict the breakpoint to the specified @var{thread-id}.
> +@item -s "@var{location},@var{template},@var{expression}[,@var{expression}@dots{}]"

The format of the -s argument in @item is different from the format in
the @smallexample above.  Which one is correct?

> +Set a @ref{Dynamic Printf}.

Please don't use @ref as if it were an HTML "<a href" attribute; it is
not.  This produces ugly Info output.  Please use this style instead:

  Set a dynamic printf breakpoint (@pxref{Dynamic Printf}).

or this:

  Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi Eli,

Thanks for your review.

On Thu, Mar 28, 2013 at 10:38 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 28 Mar 2013 22:21:30 +0800
>> From: Hui Zhu <hui_zhu@mentor.com>
>> CC: Marc Khouzam <marc.khouzam@ericsson.com>, Eli Zaretskii <eliz@gnu.org>
>>
>> This is the patches to add -s flags to make -break-info support dprintf.
>> mi-dprintf.txt is for the code.
>> mi-dprintf-doc.txt is for the doc.
>> mi-dprintf-test.txt is for the test.
>>
>> After this patch, you can use -s option to add dprintf.  To see mi-dprintf-doc.txt to get info about how to use this function.
>
> Thanks.
>
>> This is for NEWS:
>>    ** The -s of MI command -break-insert can set a dynamic printf.
>
>  "The new option -s of the MI command -break-insert sets a dynamic
>   printf breakpoint."

Thanks.  I will use it.

>
>> 2013-03-28  Hui Zhu  <hui@codesourcery.com>
>>
>>       * gdb.texinfo (GDB/MI Breakpoint Commands): Add "-s".
>                                                     ^^^^^^^^
> "Describe the -s option."
>
>> --- a/doc/gdb.texinfo
>> +++ b/doc/gdb.texinfo
>> @@ -28784,7 +28784,9 @@ N.A.
>>  @smallexample
>>   -break-insert [ -t ] [ -h ] [ -f ] [ -d ] [ -a ]
>>      [ -c @var{condition} ] [ -i @var{ignore-count} ]
>> -    [ -p @var{thread-id} ] [ @var{location} ]
>> +    [ -p @var{thread-id} ]
>> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
>> +    [ @var{location} ]
>>  @end smallexample
>>
>>  @noindent
>> @@ -28824,6 +28826,29 @@ Make the breakpoint conditional on @var{
>>  Initialize the @var{ignore-count}.
>>  @item -p @var{thread-id}
>>  Restrict the breakpoint to the specified @var{thread-id}.
>> +@item -s "@var{location},@var{template},@var{expression}[,@var{expression}@dots{}]"
>
> The format of the -s argument in @item is different from the format in
> the @smallexample above.  Which one is correct?

Oops, the @item is wrong.  It is fixed now.

>
>> +Set a @ref{Dynamic Printf}.
>
> Please don't use @ref as if it were an HTML "<a href" attribute; it is
> not.  This produces ugly Info output.  Please use this style instead:
>
>   Set a dynamic printf breakpoint (@pxref{Dynamic Printf}).
>
> or this:
>
>   Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
>

Fixed.

Post a new version according to your comments.

Thanks,
Hui

  ** The new option -s of the MI command -break-insert sets a dynamic
     printf breakpoint.

2013-03-29  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (mi_cmd_break_insert): Describe the "-s" option.

2013-03-29  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the "-s" option.

2013-03-29  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.c, gdb.mi/mi-dprintf.h: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -305,7 +305,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1211,6 +1211,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -99,15 +99,17 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
+  int dprintf = 0;
   struct cleanup *back_to;
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
       HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
       IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT, DISABLE_OPT,
-      TRACEPOINT_OPT,
+      TRACEPOINT_OPT, DPRINTF_OPT,
     };
   static const struct mi_opt opts[] =
   {
@@ -119,6 +121,7 @@ mi_cmd_break_insert (char *command, char
     {"f", PENDING_OPT, 0},
     {"d", DISABLE_OPT, 0},
     {"a", TRACEPOINT_OPT, 0},
+    {"s", DPRINTF_OPT, 1},
     { 0, 0, 0 }
   };
 
@@ -159,9 +162,15 @@ mi_cmd_break_insert (char *command, char
 	case TRACEPOINT_OPT:
 	  tracepoint = 1;
 	  break;
+	case DPRINTF_OPT:
+	  extra_string = oarg;
+	  dprintf = 1;
+	  break;
 	}
     }
 
+  if (hardware && dprintf)
+    error (_("-break-insert: -h and -s cannot be use together"));
   if (oind >= argc)
     error (_("-break-insert: Missing <location>"));
   if (oind < argc - 1)
@@ -180,11 +189,14 @@ mi_cmd_break_insert (char *command, char
      regular non-jump based tracepoints.  */
   type_wanted = (tracepoint
 		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+		 : (hardware ? bp_hardware_breakpoint
+		    : (dprintf ? bp_dprintf : bp_breakpoint)));
+  ops = tracepoint ? &tracepoint_breakpoint_ops
+		   : (dprintf ? &dprintf_breakpoint_ops
+			      : &bkpt_breakpoint_ops);
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 1623 bytes --]

--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28784,7 +28784,9 @@ N.A.
 @smallexample
  -break-insert [ -t ] [ -h ] [ -f ] [ -d ] [ -a ]
     [ -c @var{condition} ] [ -i @var{ignore-count} ]
-    [ -p @var{thread-id} ] [ @var{location} ]
+    [ -p @var{thread-id} ]
+    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
+    [ @var{location} ]
 @end smallexample
 
 @noindent
@@ -28824,6 +28826,29 @@ Make the breakpoint conditional on @var{
 Initialize the @var{ignore-count}.
 @item -p @var{thread-id}
 Restrict the breakpoint to the specified @var{thread-id}.
+@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
+Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
+The @var{location}, @var{template} and @var{expression} should be
+within double quotations and be escaped by being preceded with a backslash.
+
+@smallexample
+(gdb)
+4-break-insert -s "\"At foo entry\\n\"" foo
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-break-insert -s "\"arg=%d, g=%d\\n\", arg, g" 26
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @end table
 
 @subsubheading Result

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7845 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,58 @@
+/* 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/>.  */
+
+#include <stdio.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+#include <stdlib.h>
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,123 @@
+#   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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile .c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-dprintf.exp
+     return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-break-insert -s" \
+    "1\\^error,msg=\"-break-insert: Option -s requires an argument\"" "mi insert without location"
+
+mi_gdb_test "2-break-insert -s foo" \
+    "2\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-break-insert -s 29" \
+    "3\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-break-insert -s \"\\\"At foo entry\\\\n\\\"\" foo" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"25\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"25\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+mi_run_cmd
+
+mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf, gdb"
+
+mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf, gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf, call"
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf, call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf, fprintf"
+
+    mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf, fprintf"
+}
+
+set target_can_dprintf 1
+set msg "Set dprintf style to agent"
+mi_gdb_test "1set dprintf-style agent" "\[^\n\]*\r\ndone"
+gdb_expect {
+    -re "\\^done" {
+	pass "$msg - can do"
+    }
+    -re ".*" {
+	set target_can_dprintf 0
+	pass "$msg - cannot do"
+    }
+    timeout {
+       fail "resume all, waiting for program exit (timeout)"
+    }
+}
+
+if $target_can_dprintf {
+
+    mi_run_cmd
+
+    mi_gdb_test "continue" "Breakpoint \[0-9\]+, foo .*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" "Breakpoint \[0-9\]+, foo .*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" \
+	"6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"25\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+	"mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Fri, 29 Mar 2013 16:01:18 +0800
> Cc: Hui Zhu <hui_zhu@mentor.com>, gdb-patches@sourceware.org, 
> 	marc.khouzam@ericsson.com
> 
> Post a new version according to your comments.

This version is OK, thanks.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Pedro Alves]

Hi Hui,

Thanks for the patch.

New MI features need a NEWS entry.

On 03/29/2013 08:01 AM, Hui Zhu wrote:

> +  if (hardware && dprintf)
> +    error (_("-break-insert: -h and -s cannot be use together"));

"cannot be used"

> @@ -180,11 +189,14 @@ mi_cmd_break_insert (char *command, char
>       regular non-jump based tracepoints.  */
>    type_wanted = (tracepoint
>  		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
> -		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
> -  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
> +		 : (hardware ? bp_hardware_breakpoint
> +		    : (dprintf ? bp_dprintf : bp_breakpoint)));
> +  ops = tracepoint ? &tracepoint_breakpoint_ops
> +		   : (dprintf ? &dprintf_breakpoint_ops
> +			      : &bkpt_breakpoint_ops);

This is getting unnecessarily hard for humans to grok.  Write
instead as (untested):

  if (tracepoint)
    {
      /* move existing comment on fast tracepoints here */
      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
      ops = &tracepoint_breakpoint_ops;
    }
  else if (dprintf)
    {
      type_wanted = bp_dprintf;
      ops = &dprintf_breakpoint_ops;
    }
  else
    {
      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
      ops = &bkpt_breakpoint_ops;
    }


> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
> +The @var{location}, @var{template} and @var{expression} should be
> +within double quotations and be escaped by being preceded with a backslash.

Please remove the "location" mention here.  It's stale.

I think you either say "double quotation marks" or "double quotes",
never "double quotation".

>
> 2013-03-29  Hui Zhu  <hui@codesourcery.com>
>
> 	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
> 	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
> 	* mi/mi-cmd-break.c (mi_cmd_break_insert): Describe the "-s" option.

Not really describing...  "Handle"?

> 2013-03-29  Hui Zhu  <hui@codesourcery.com>
>
> 	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
> 	* gdb.mi/mi-dprintf.c, gdb.mi/mi-dprintf.h: New.

Missing reference to mi-dprintf.exp.


> +  foo (loc++);
> +  foo (loc++);
> +  foo (loc++);
> +  return g;
> +}
> +
> +#include <stdlib.h>

Headers at the top, please.

> +/* Make sure function 'malloc' is linked into program.  On some bare-metal
> +   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
> +   is needed, otherwise we'll see such error message
> +


> +standard_testfile .c

.c is the default.

> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +     untested mi-dprintf.exp

http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls




> +set target_can_dprintf 1

This should start out as 0.  More below.

> +set msg "Set dprintf style to agent"
> +mi_gdb_test "1set dprintf-style agent" "\[^\n\]*\r\ndone"
> +gdb_expect {
> +    -re "\\^done" {
> +	pass "$msg - can do"

and be set to 1 here.

Should expect ${mi_gdb_prompt} too.

> +    }
> +    -re ".*" {

Should expect ${mi_gdb_prompt} too.
But what this actually expecting?  Is it:

 "warning: Target cannot run dprintf commands, falling back to GDB printf"

?  Please adjust the -re accordingly.

> +	set target_can_dprintf 0
> +	pass "$msg - cannot do"

> +    }
> +    timeout {
> +       fail "resume all, waiting for program exit (timeout)"

Certainly "resume all" is a pasto here.

Related to the comment to "set target_can_dprintf 1" above,
e.g., this failure path didn't set target_can_dprintf to 0.

> +    }
> +}
> +
> +if $target_can_dprintf {

Why do I get:

 PASS: gdb.mi/mi-dprintf.exp: Set dprintf style to agent - cannot do

with gdbserver?

(The test tries the same thing with a few different options.  I suspect
it could be simplified with loops and with_test_prefix.)

-- 
Pedro Alves

RE: [PATCH] add -s option to make -break-insert support dprintf

[Marc Khouzam]

> ________________________________________
> From: gdb-patches-owner@sourceware.org [gdb-patches-owner@sourceware.org] on behalf of Pedro Alves [palves@redhat.com]
> Sent: April 9, 2013 1:50 PM
> To: Hui Zhu
> Cc: Eli Zaretskii; Hui Zhu; gdb-patches@sourceware.org; Marc Khouzam
> Subject: Re: [PATCH] add -s option to make -break-insert support dprintf
> 
> Hi Hui,
> 
> Thanks for the patch.
> 
> New MI features need a NEWS entry.
> 
> On 03/29/2013 08:01 AM, Hui Zhu wrote:
> 
> > +  if (hardware && dprintf)
> > +    error (_("-break-insert: -h and -s cannot be use together"));
> 
> "cannot be used"
> 
> > @@ -180,11 +189,14 @@ mi_cmd_break_insert (char *command, char
> >       regular non-jump based tracepoints.  */
> >    type_wanted = (tracepoint
> >                ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
> > -              : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
> > -  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
> > +              : (hardware ? bp_hardware_breakpoint
> > +                 : (dprintf ? bp_dprintf : bp_breakpoint)));
> > +  ops = tracepoint ? &tracepoint_breakpoint_ops
> > +                : (dprintf ? &dprintf_breakpoint_ops
> > +                           : &bkpt_breakpoint_ops);
> 
> This is getting unnecessarily hard for humans to grok.  Write
> instead as (untested):
> 
>   if (tracepoint)
>     {
>       /* move existing comment on fast tracepoints here */
>       type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
>       ops = &tracepoint_breakpoint_ops;
>     }
>   else if (dprintf)
>     {
>       type_wanted = bp_dprintf;
>       ops = &dprintf_breakpoint_ops;
>     }
>   else
>     {
>       type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
>       ops = &bkpt_breakpoint_ops;
>     }


In the orginal patch, having both 'hardware' and 'dprintf' true would 
create a hardware breakpoint (not dprintf), but would still set 'ops'
to &dprintf_breakpoint_ops.  This didn't look right to me.

A side-effect of Pedro's change is that the hardware dprintf case
will be handled properly.  I think that is a good thing.  However,
I wanted to mention it, as I don't know if there are other changes
needed to handle a hardware dprintf (or if it really should be allowed).
I am allowed to create a hardware breakpoint with a printf condition,
so I guess a hardware dprintf would make sense, but I'm not sure.

Marc

Re: [PATCH] add -s option to make -break-insert support dprintf

[Pedro Alves]

Hi Marc,

On 04/10/2013 11:31 AM, Marc Khouzam wrote:

> In the orginal patch, having both 'hardware' and 'dprintf' true would 
> create a hardware breakpoint (not dprintf), but would still set 'ops'
> to &dprintf_breakpoint_ops.  This didn't look right to me.

In fairness, it actually wouldn't, because of:

>>> +  if (hardware && dprintf)
>>> +    error (_("-break-insert: -h and -s cannot be use together"));

I guess my change at least makes it more obvious in that other
spot too.

> A side-effect of Pedro's change is that the hardware dprintf case
> will be handled properly.  I think that is a good thing.  However,
> I wanted to mention it, as I don't know if there are other changes
> needed to handle a hardware dprintf (or if it really should be allowed).
> I am allowed to create a hardware breakpoint with a printf condition,
> so I guess a hardware dprintf would make sense, but I'm not sure.

Yeah, I don't see why it couldn't work.  Any kind of
watchpoint/breakpoint, really.  To me, this exposes that the whole
dprintf feature hasn't been exposed to the user at the right
abstraction level.  It should have been something lower level that
allowed constructing dprintfs, or hardware-dprintfs, or
watchpoints-dprintfs, etc.  The python Stop hook is close, except
for the agent side part.  Maybe equipping breakpoints with a separate
list of "commands that don't interfere with the current execution
command at time of hit" would have been closer to home.

-- 
Pedro Alves

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi Pedro,

Thanks for your review.

On Wed, Apr 10, 2013 at 1:50 AM, Pedro Alves <palves@redhat.com> wrote:
> Hi Hui,
>
> Thanks for the patch.
>
> New MI features need a NEWS entry.

This is for NEWS:
  ** The -s of MI command -break-insert can set a dynamic printf.

>
> On 03/29/2013 08:01 AM, Hui Zhu wrote:
>
>> +  if (hardware && dprintf)
>> +    error (_("-break-insert: -h and -s cannot be use together"));
>
> "cannot be used"

Fixed.

>
>> @@ -180,11 +189,14 @@ mi_cmd_break_insert (char *command, char
>>       regular non-jump based tracepoints.  */
>>    type_wanted = (tracepoint
>>                ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
>> -              : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
>> -  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
>> +              : (hardware ? bp_hardware_breakpoint
>> +                 : (dprintf ? bp_dprintf : bp_breakpoint)));
>> +  ops = tracepoint ? &tracepoint_breakpoint_ops
>> +                : (dprintf ? &dprintf_breakpoint_ops
>> +                           : &bkpt_breakpoint_ops);
>
> This is getting unnecessarily hard for humans to grok.  Write
> instead as (untested):
>
>   if (tracepoint)
>     {
>       /* move existing comment on fast tracepoints here */
>       type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
>       ops = &tracepoint_breakpoint_ops;
>     }
>   else if (dprintf)
>     {
>       type_wanted = bp_dprintf;
>       ops = &dprintf_breakpoint_ops;
>     }
>   else
>     {
>       type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
>       ops = &bkpt_breakpoint_ops;
>     }
>

Fixed.

>
>> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
>> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
>> +The @var{location}, @var{template} and @var{expression} should be
>> +within double quotations and be escaped by being preceded with a backslash.
>
> Please remove the "location" mention here.  It's stale.
>
> I think you either say "double quotation marks" or "double quotes",
> never "double quotation".

Fixed.

>
>>
>> 2013-03-29  Hui Zhu  <hui@codesourcery.com>
>>
>>       * breakpoint.c (dprintf_breakpoint_ops): Remove its static.
>>       * breakpoint.h (dprintf_breakpoint_ops): Add extern.
>>       * mi/mi-cmd-break.c (mi_cmd_break_insert): Describe the "-s" option.
>
> Not really describing...  "Handle"?

Fixed.

>
>> 2013-03-29  Hui Zhu  <hui@codesourcery.com>
>>
>>       * gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
>>       * gdb.mi/mi-dprintf.c, gdb.mi/mi-dprintf.h: New.
>
> Missing reference to mi-dprintf.exp.

Fixed.

>
>
>> +  foo (loc++);
>> +  foo (loc++);
>> +  foo (loc++);
>> +  return g;
>> +}
>> +
>> +#include <stdlib.h>
>
> Headers at the top, please.

Fixed.

>
>> +/* Make sure function 'malloc' is linked into program.  On some bare-metal
>> +   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
>> +   is needed, otherwise we'll see such error message
>> +
>
>
>> +standard_testfile .c
>
> .c is the default.

Fixed.

>
>> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
>> +     untested mi-dprintf.exp
>
> http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls
>

Looks test doesn't need it now.  So I removed it.

>
>
>
>> +set target_can_dprintf 1
>
> This should start out as 0.  More below.
>
>> +set msg "Set dprintf style to agent"
>> +mi_gdb_test "1set dprintf-style agent" "\[^\n\]*\r\ndone"
>> +gdb_expect {
>> +    -re "\\^done" {
>> +     pass "$msg - can do"
>
> and be set to 1 here.
>
> Should expect ${mi_gdb_prompt} too.
>
>> +    }
>> +    -re ".*" {
>
> Should expect ${mi_gdb_prompt} too.
> But what this actually expecting?  Is it:
>
>  "warning: Target cannot run dprintf commands, falling back to GDB printf"
>
> ?  Please adjust the -re accordingly.
>
>> +     set target_can_dprintf 0
>> +     pass "$msg - cannot do"
>
>> +    }
>> +    timeout {
>> +       fail "resume all, waiting for program exit (timeout)"
>
> Certainly "resume all" is a pasto here.

pasto?

>
> Related to the comment to "set target_can_dprintf 1" above,
> e.g., this failure path didn't set target_can_dprintf to 0.
>
>> +    }
>> +}
>> +
>> +if $target_can_dprintf {
>
> Why do I get:
>
>  PASS: gdb.mi/mi-dprintf.exp: Set dprintf style to agent - cannot do
>
> with gdbserver?

Set dprintf style to agent need test with gdbserver.
I update this pass to unsupported.

And also update this part to make it test OK with gdbserver.

>
> (The test tries the same thing with a few different options.  I suspect
> it could be simplified with loops and with_test_prefix.)

I moved them to a proc and add with_test_prefix.

I post a new version according to your comments.  Please help me review it.

Best,
Hui

>
> --
> Pedro Alves


2013-04-11  Hui Zhu  <hui@codesourcery.com>

* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
* breakpoint.h (dprintf_breakpoint_ops): Add extern.
* mi/mi-cmd-break.c (mi_cmd_break_insert): Handle the "-s" option.

2013-04-11  Hui Zhu  <hui@codesourcery.com>

* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the "-s" option.

2013-04-11  Hui Zhu  <hui@codesourcery.com>

* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -305,7 +305,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -99,15 +99,17 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
+  int dprintf = 0;
   struct cleanup *back_to;
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
       HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
       IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT, DISABLE_OPT,
-      TRACEPOINT_OPT,
+      TRACEPOINT_OPT, DPRINTF_OPT,
     };
   static const struct mi_opt opts[] =
   {
@@ -119,6 +121,7 @@ mi_cmd_break_insert (char *command, char
     {"f", PENDING_OPT, 0},
     {"d", DISABLE_OPT, 0},
     {"a", TRACEPOINT_OPT, 0},
+    {"s", DPRINTF_OPT, 1},
     { 0, 0, 0 }
   };
 
@@ -159,9 +162,15 @@ mi_cmd_break_insert (char *command, char
 	case TRACEPOINT_OPT:
 	  tracepoint = 1;
 	  break;
+	case DPRINTF_OPT:
+	  extra_string = oarg;
+	  dprintf = 1;
+	  break;
 	}
     }
 
+  if (hardware && dprintf)
+    error (_("-break-insert: -h and -s cannot be used together"));
   if (oind >= argc)
     error (_("-break-insert: Missing <location>"));
   if (oind < argc - 1)
@@ -171,20 +180,31 @@ mi_cmd_break_insert (char *command, char
   /* Now we have what we need, let's insert the breakpoint!  */
   back_to = setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 1603 bytes --]

--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28702,7 +28702,9 @@ N.A.
 @smallexample
  -break-insert [ -t ] [ -h ] [ -f ] [ -d ] [ -a ]
     [ -c @var{condition} ] [ -i @var{ignore-count} ]
-    [ -p @var{thread-id} ] [ @var{location} ]
+    [ -p @var{thread-id} ]
+    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
+    [ @var{location} ]
 @end smallexample
 
 @noindent
@@ -28742,6 +28744,29 @@ Make the breakpoint conditional on @var{
 Initialize the @var{ignore-count}.
 @item -p @var{thread-id}
 Restrict the breakpoint to the specified @var{thread-id}.
+@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
+Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
+The @var{template} and @var{expression} should be within double
+quotes and be escaped by being preceded with a backslash.
+
+@smallexample
+(gdb)
+4-break-insert -s "\"At foo entry\\n\"" foo
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-break-insert -s "\"arg=%d, g=%d\\n\", arg, g" 26
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @end table
 
 @subsubheading Result

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7179 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,120 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-dprintf.exp
+     return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-break-insert -s" \
+    "1\\^error,msg=\"-break-insert: Option -s requires an argument\"" "mi insert without location"
+
+mi_gdb_test "2-break-insert -s foo" \
+    "2\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-break-insert -s 29" \
+    "3\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-break-insert -s \"\\\"At foo entry\\\\n\\\"\" foo" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	mi_run_cmd
+	mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf"
+	mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf"
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+# Sleep 1 second to make sure set dprintf-style agent get right outout.
+sleep 1
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re "done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+       fail "resume all, waiting for program exit (timeout)"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Pedro Alves]

On 04/11/2013 03:40 AM, Hui Zhu wrote:

>> New MI features need a NEWS entry.
> 
> This is for NEWS:
>   ** The -s of MI command -break-insert can set a dynamic printf.
> 

Please send that as a real patch, so e.g., we can see what section of
NEWS you're proposing adding it to.

> 
>>
>>> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
>>> +     untested mi-dprintf.exp
>>
>> http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls
>>
> 
> Looks test doesn't need it now.  So I removed it.

I still see it in the patch.  But, you can simplify and
replace that gdb_compile call with a build_executable call, I think?
Then you don't need the untested call.

>>
>>> +     set target_can_dprintf 0
>>> +     pass "$msg - cannot do"
>>
>>> +    }
>>> +    timeout {
>>> +       fail "resume all, waiting for program exit (timeout)"
>>
>> Certainly "resume all" is a pasto here.
> 
> pasto?

Typo -> pasto.  Pasto is like a typo, but refers to
blindly copy/pasting text from elsewhere.

"resume all, waiting for program exit" makes no sense here.
It should be $msg, no?

>> Why do I get:
>>
>>  PASS: gdb.mi/mi-dprintf.exp: Set dprintf style to agent - cannot do
>>
>> with gdbserver?
> 
> Set dprintf style to agent need test with gdbserver.
> I update this pass to unsupported.
> 
> And also update this part to make it test OK with gdbserver.

I still get:

$ make check gdbserver RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"
...
FAIL: gdb.mi/mi-dprintf.exp: set dprintf style to agent

                === gdb Summary ===

# of expected passes            13
# of unexpected failures        1

gdb.log:

~"arg=1235, g=2222\n"
*running,thread-id="1"
=breakpoint-modified,bkpt={number="5",type="breakpoint",disp="keep",enabled="y",addr="0x00000000004006c2",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29",thread-groups=["i1"],times="2",original-location="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c:29"}
*stopped,reason="breakpoint-hit",disp="keep",bkptno="5",frame={addr="0x00000000004006c2",func="foo",args=[{name="arg",value="1235"}],file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29"},thread-id="1",stopped-threads="all",core="1"
(gdb)
PASS: gdb.mi/mi-dprintf.exp: gdb: mi 2nd dprintf
&"continue\n"
~"Continuing.\n"
^running
*running,thread-id="1"
(gdb)
=breakpoint-modified,bkpt={number="3",type="dprintf",disp="keep",enabled="y",addr="0x00000000004006a3",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="27",thread-groups=["i1"],times="3",script={"printf \\"At foo entry\\\\n\\"","continue"},original-location="foo"}
*stopped
~"At foo entry\n"
*running,thread-id="1"
=breakpoint-modified,bkpt={number="4",type="dprintf",disp="keep",enabled="y",addr="0x00000000004006b4",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="28",thread-groups=["i1"],times="3",script={"printf \\"arg=%d, g=%d\\\\n\\", arg, g","continue"},original-location="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c:28"}
*stopped
~"arg=1236, g=3013\n"
*running,thread-id="1"
=breakpoint-modified,bkpt={number="5",type="breakpoint",disp="keep",enabled="y",addr="0x00000000004006c2",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29",thread-groups=["i1"],times="3",original-location="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c:29"}
*stopped,reason="breakpoint-hit",disp="keep",bkptno="5",frame={addr="0x00000000004006c2",func="foo",args=[{name="arg",value="1236"}],file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29"},thread-id="1",stopped-threads="all",core="0"
(gdb)
FAIL: gdb.mi/mi-dprintf.exp: set dprintf style to agent


> +# Sleep 1 second to make sure set dprintf-style agent get right outout.

s/outout/output/ ?

-- 
Pedro Alves

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi Pedro,

Thanks for your review.

On Thu, Apr 11, 2013 at 5:14 PM, Pedro Alves <palves@redhat.com> wrote:
> On 04/11/2013 03:40 AM, Hui Zhu wrote:
>
>>> New MI features need a NEWS entry.
>>
>> This is for NEWS:
>>   ** The -s of MI command -break-insert can set a dynamic printf.
>>
>
> Please send that as a real patch, so e.g., we can see what section of
> NEWS you're proposing adding it to.

OK.  I add this change to doc patch.

>
>>
>>>
>>>> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
>>>> +     untested mi-dprintf.exp
>>>
>>> http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls
>>>
>>
>> Looks test doesn't need it now.  So I removed it.
>
> I still see it in the patch.  But, you can simplify and
> replace that gdb_compile call with a build_executable call, I think?
> Then you don't need the untested call.

Fixed.

>
>>>
>>>> +     set target_can_dprintf 0
>>>> +     pass "$msg - cannot do"
>>>
>>>> +    }
>>>> +    timeout {
>>>> +       fail "resume all, waiting for program exit (timeout)"
>>>
>>> Certainly "resume all" is a pasto here.
>>
>> pasto?
>
> Typo -> pasto.  Pasto is like a typo, but refers to
> blindly copy/pasting text from elsewhere.
>
> "resume all, waiting for program exit" makes no sense here.
> It should be $msg, no?

Fixed.

>
>>> Why do I get:
>>>
>>>  PASS: gdb.mi/mi-dprintf.exp: Set dprintf style to agent - cannot do
>>>
>>> with gdbserver?
>>
>> Set dprintf style to agent need test with gdbserver.
>> I update this pass to unsupported.
>>
>> And also update this part to make it test OK with gdbserver.
>
> I still get:
>
> $ make check gdbserver RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"

This part is really odd.
In my part, without "sleep 1" will random get fail with "Set dprintf
style to agent ".
The reason of fail is test try to check the output before it call
send_gdb "set dprintf-style agent\n".
This is why I add a "sleep 1" for it.

But looks it still not OK in your part, so I change it to:
mi_gdb_test "pwd" ".*"

If it is still not OK in your part, I suggest remove this part of test
because it is not very important for this test.   The "set
dprintf-style agent" is tested in "dprintf.exp".

> ...
> FAIL: gdb.mi/mi-dprintf.exp: set dprintf style to agent
>
>                 === gdb Summary ===
>
> # of expected passes            13
> # of unexpected failures        1
>
> gdb.log:
>
> ~"arg=1235, g=2222\n"
> *running,thread-id="1"
> =breakpoint-modified,bkpt={number="5",type="breakpoint",disp="keep",enabled="y",addr="0x00000000004006c2",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29",thread-groups=["i1"],times="2",original-location="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c:29"}
> *stopped,reason="breakpoint-hit",disp="keep",bkptno="5",frame={addr="0x00000000004006c2",func="foo",args=[{name="arg",value="1235"}],file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29"},thread-id="1",stopped-threads="all",core="1"
> (gdb)
> PASS: gdb.mi/mi-dprintf.exp: gdb: mi 2nd dprintf
> &"continue\n"
> ~"Continuing.\n"
> ^running
> *running,thread-id="1"
> (gdb)
> =breakpoint-modified,bkpt={number="3",type="dprintf",disp="keep",enabled="y",addr="0x00000000004006a3",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="27",thread-groups=["i1"],times="3",script={"printf \\"At foo entry\\\\n\\"","continue"},original-location="foo"}
> *stopped
> ~"At foo entry\n"
> *running,thread-id="1"
> =breakpoint-modified,bkpt={number="4",type="dprintf",disp="keep",enabled="y",addr="0x00000000004006b4",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="28",thread-groups=["i1"],times="3",script={"printf \\"arg=%d, g=%d\\\\n\\", arg, g","continue"},original-location="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c:28"}
> *stopped
> ~"arg=1236, g=3013\n"
> *running,thread-id="1"
> =breakpoint-modified,bkpt={number="5",type="breakpoint",disp="keep",enabled="y",addr="0x00000000004006c2",func="foo",file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29",thread-groups=["i1"],times="3",original-location="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c:29"}
> *stopped,reason="breakpoint-hit",disp="keep",bkptno="5",frame={addr="0x00000000004006c2",func="foo",args=[{name="arg",value="1236"}],file="../../../src/gdb/testsuite/gdb.mi/mi-dprintf.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.mi/mi-dprintf.c",line="29"},thread-id="1",stopped-threads="all",core="0"
> (gdb)
> FAIL: gdb.mi/mi-dprintf.exp: set dprintf style to agent
>
>
>> +# Sleep 1 second to make sure set dprintf-style agent get right outout.
>
> s/outout/output/ ?

Fixed.

>
> --
> Pedro Alves
>

Thanks,
Hui

2013-04-12  Hui Zhu  <hui@codesourcery.com>

* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
* breakpoint.h (dprintf_breakpoint_ops): Add extern.
* mi/mi-cmd-break.c (mi_cmd_break_insert): Handle the "-s" option.

2013-04-12  Hui Zhu  <hui@codesourcery.com>

* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the "-s" option.

2013-04-12  Hui Zhu  <hui@codesourcery.com>

* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -305,7 +305,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -99,15 +99,17 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
+  int dprintf = 0;
   struct cleanup *back_to;
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
       HARDWARE_OPT, TEMP_OPT, CONDITION_OPT,
       IGNORE_COUNT_OPT, THREAD_OPT, PENDING_OPT, DISABLE_OPT,
-      TRACEPOINT_OPT,
+      TRACEPOINT_OPT, DPRINTF_OPT,
     };
   static const struct mi_opt opts[] =
   {
@@ -119,6 +121,7 @@ mi_cmd_break_insert (char *command, char
     {"f", PENDING_OPT, 0},
     {"d", DISABLE_OPT, 0},
     {"a", TRACEPOINT_OPT, 0},
+    {"s", DPRINTF_OPT, 1},
     { 0, 0, 0 }
   };
 
@@ -159,9 +162,15 @@ mi_cmd_break_insert (char *command, char
 	case TRACEPOINT_OPT:
 	  tracepoint = 1;
 	  break;
+	case DPRINTF_OPT:
+	  extra_string = oarg;
+	  dprintf = 1;
+	  break;
 	}
     }
 
+  if (hardware && dprintf)
+    error (_("-break-insert: -h and -s cannot be used together"));
   if (oind >= argc)
     error (_("-break-insert: Missing <location>"));
   if (oind < argc - 1)
@@ -171,20 +180,31 @@ mi_cmd_break_insert (char *command, char
   /* Now we have what we need, let's insert the breakpoint!  */
   back_to = setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 1951 bytes --]

--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,9 @@ show remote trace-status-packet
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
+  ** The new option -s of the MI command -break-insert sets a dynamic
+     printf breakpoint.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28745,7 +28745,9 @@ N.A.
 @smallexample
  -break-insert [ -t ] [ -h ] [ -f ] [ -d ] [ -a ]
     [ -c @var{condition} ] [ -i @var{ignore-count} ]
-    [ -p @var{thread-id} ] [ @var{location} ]
+    [ -p @var{thread-id} ]
+    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
+    [ @var{location} ]
 @end smallexample
 
 @noindent
@@ -28785,6 +28787,29 @@ Make the breakpoint conditional on @var{
 Initialize the @var{ignore-count}.
 @item -p @var{thread-id}
 Restrict the breakpoint to the specified @var{thread-id}.
+@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
+Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
+The @var{template} and @var{expression} should be within double
+quotes and be escaped by being preceded with a backslash.
+
+@smallexample
+(gdb)
+4-break-insert -s "\"At foo entry\\n\"" foo
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-break-insert -s "\"arg=%d, g=%d\\n\", arg, g" 26
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @end table
 
 @subsubheading Result

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7126 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,121 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-break-insert -s" \
+    "1\\^error,msg=\"-break-insert: Option -s requires an argument\"" "mi insert without location"
+
+mi_gdb_test "2-break-insert -s foo" \
+    "2\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-break-insert -s 29" \
+    "3\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-break-insert -s \"\\\"At foo entry\\\\n\\\"\" foo" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	mi_run_cmd
+	mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf"
+	mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf"
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+# To make sure set dprintf-style agent get right output.
+mi_gdb_test "pwd" ".*"
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Pedro Alves]

On 04/12/2013 11:38 AM, Hui Zhu wrote:
>> >
>> > I still get:
>> >
>> > $ make check gdbserver RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"
> This part is really odd.
> In my part, without "sleep 1" will random get fail with "Set dprintf
> style to agent ".
> The reason of fail is test try to check the output before it call
> send_gdb "set dprintf-style agent\n".
> This is why I add a "sleep 1" for it.
> 
> But looks it still not OK in your part, so I change it to:
> mi_gdb_test "pwd" ".*"
> 
> If it is still not OK in your part, I suggest remove this part of test
> because it is not very important for this test.   The "set
> dprintf-style agent" is tested in "dprintf.exp".
> 

No, it's not okay.  And it's not okay to just sweep it under the rug.

I believe the problem is that the test is sending CLI resumption commands:

+	mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf"
+	mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf"

"continue" sent from MI causes two prompts to appear:

(gdb)
continue
&"continue\n"
~"Continuing.\n"
^running
*running,thread-id="all"
(gdb)
=breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x000000000045a3ff",func="main",file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32",thread-groups=["i1"],times="1",original-location="/home/pedro/gdb/mygit/src/gdb/gdb.c:32"}
~"\nBreakpoint "
~"6, main (argc=1, argv=0x7fffffffdc58) at ../../src/gdb/gdb.c:32\n"
~"32\t  args.use_windows = 0;\n"
*stopped,reason="breakpoint-hit",disp="keep",bkptno="6",frame={addr="0x000000000045a3ff",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdc58"}],file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32"},thread-id="1",stopped-threads="all",core="0"
(gdb)

and that is confusing the test (the regex for the prompt is probably stopping
at the first prompt sometimes).

The test should be adjusted to do MI -exec-continue instead, with
mi_execute_to "exec-continue", mi_send_resuming_command "exec-continue"
or something like that.

BTW, you don't need to use gdbserver to trigger the issue.  Native
works too.  E.g., leave this running, and eventually, it should FAIL
and stop:

$ (set -e; while true; do make check RUNTESTFLAGS="mi-dprintf.exp"; done)

gdb/contrib/expect-read1.sh probably makes this reproducible all
the time, though I haven't tried.

-- 
Pedro Alves

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

On Fri, Apr 12, 2013 at 7:34 PM, Pedro Alves <palves@redhat.com> wrote:
> On 04/12/2013 11:38 AM, Hui Zhu wrote:
>>> >
>>> > I still get:
>>> >
>>> > $ make check gdbserver RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"
>> This part is really odd.
>> In my part, without "sleep 1" will random get fail with "Set dprintf
>> style to agent ".
>> The reason of fail is test try to check the output before it call
>> send_gdb "set dprintf-style agent\n".
>> This is why I add a "sleep 1" for it.
>>
>> But looks it still not OK in your part, so I change it to:
>> mi_gdb_test "pwd" ".*"
>>
>> If it is still not OK in your part, I suggest remove this part of test
>> because it is not very important for this test.   The "set
>> dprintf-style agent" is tested in "dprintf.exp".
>>
>
> No, it's not okay.  And it's not okay to just sweep it under the rug.
>
> I believe the problem is that the test is sending CLI resumption commands:
>
> +       mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf"
> +       mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf"
>
> "continue" sent from MI causes two prompts to appear:
>
> (gdb)
> continue
> &"continue\n"
> ~"Continuing.\n"
> ^running
> *running,thread-id="all"
> (gdb)
> =breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x000000000045a3ff",func="main",file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32",thread-groups=["i1"],times="1",original-location="/home/pedro/gdb/mygit/src/gdb/gdb.c:32"}
> ~"\nBreakpoint "
> ~"6, main (argc=1, argv=0x7fffffffdc58) at ../../src/gdb/gdb.c:32\n"
> ~"32\t  args.use_windows = 0;\n"
> *stopped,reason="breakpoint-hit",disp="keep",bkptno="6",frame={addr="0x000000000045a3ff",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdc58"}],file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32"},thread-id="1",stopped-threads="all",core="0"
> (gdb)
>
> and that is confusing the test (the regex for the prompt is probably stopping
> at the first prompt sometimes).
>
> The test should be adjusted to do MI -exec-continue instead, with
> mi_execute_to "exec-continue", mi_send_resuming_command "exec-continue"
> or something like that.
>
> BTW, you don't need to use gdbserver to trigger the issue.  Native
> works too.  E.g., leave this running, and eventually, it should FAIL
> and stop:
>
> $ (set -e; while true; do make check RUNTESTFLAGS="mi-dprintf.exp"; done)
>
> gdb/contrib/expect-read1.sh probably makes this reproducible all
> the time, though I haven't tried.


Post a new version change the continue to mi_run_cmd, gdb_expect and
mi_send_resuming_command.
It works OK with loop test.

Please help me review it.

Thanks,
Hui




>
> --
> Pedro Alves
>

2013-04-15  Hui Zhu  <hui@codesourcery.com>

* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

[-- Attachment #2: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7565 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,148 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-break-insert -s" \
+    "1\\^error,msg=\"-break-insert: Option -s requires an argument\"" "mi insert without location"
+
+mi_gdb_test "2-break-insert -s foo" \
+    "2\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-break-insert -s 29" \
+    "3\\^error,msg=\"-break-insert: Missing <location>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-break-insert -s \"\\\"At foo entry\\\\n\\\"\" foo" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	global mi_gdb_prompt
+
+	mi_run_cmd
+	set msg "mi 1st dprintf"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1234, g=1234.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"
+
+	set msg "mi 2nd dprintf"
+	mi_send_resuming_command "exec-continue" "$msg continue"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1235, g=2222.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+# To make sure set dprintf-style agent get right output.
+mi_gdb_test "pwd" ".*"
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Pedro Alves]

On 04/15/2013 03:57 PM, Hui Zhu wrote:
> On Fri, Apr 12, 2013 at 7:34 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 04/12/2013 11:38 AM, Hui Zhu wrote:
>>>>>
>>>>> I still get:
>>>>>
>>>>> $ make check gdbserver RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"
>>> This part is really odd.
>>> In my part, without "sleep 1" will random get fail with "Set dprintf
>>> style to agent ".
>>> The reason of fail is test try to check the output before it call
>>> send_gdb "set dprintf-style agent\n".
>>> This is why I add a "sleep 1" for it.
>>>
>>> But looks it still not OK in your part, so I change it to:
>>> mi_gdb_test "pwd" ".*"
>>>
>>> If it is still not OK in your part, I suggest remove this part of test
>>> because it is not very important for this test.   The "set
>>> dprintf-style agent" is tested in "dprintf.exp".
>>>
>>
>> No, it's not okay.  And it's not okay to just sweep it under the rug.
>>
>> I believe the problem is that the test is sending CLI resumption commands:
>>
>> +       mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf"
>> +       mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf"
>>
>> "continue" sent from MI causes two prompts to appear:
>>
>> (gdb)
>> continue
>> &"continue\n"
>> ~"Continuing.\n"
>> ^running
>> *running,thread-id="all"
>> (gdb)
>> =breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x000000000045a3ff",func="main",file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32",thread-groups=["i1"],times="1",original-location="/home/pedro/gdb/mygit/src/gdb/gdb.c:32"}
>> ~"\nBreakpoint "
>> ~"6, main (argc=1, argv=0x7fffffffdc58) at ../../src/gdb/gdb.c:32\n"
>> ~"32\t  args.use_windows = 0;\n"
>> *stopped,reason="breakpoint-hit",disp="keep",bkptno="6",frame={addr="0x000000000045a3ff",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdc58"}],file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32"},thread-id="1",stopped-threads="all",core="0"
>> (gdb)
>>
>> and that is confusing the test (the regex for the prompt is probably stopping
>> at the first prompt sometimes).
>>
>> The test should be adjusted to do MI -exec-continue instead, with
>> mi_execute_to "exec-continue", mi_send_resuming_command "exec-continue"
>> or something like that.
>>
>> BTW, you don't need to use gdbserver to trigger the issue.  Native
>> works too.  E.g., leave this running, and eventually, it should FAIL
>> and stop:
>>
>> $ (set -e; while true; do make check RUNTESTFLAGS="mi-dprintf.exp"; done)
>>
>> gdb/contrib/expect-read1.sh probably makes this reproducible all
>> the time, though I haven't tried.
> 
> 
> Post a new version change the continue to mi_run_cmd, gdb_expect and
> mi_send_resuming_command.
> It works OK with loop test.

$ (set -e; while true; do make check RUNTESTFLAGS="mi-dprintf.exp"; done)
...
Running ../../../src/gdb/testsuite/gdb.mi/mi-dprintf.exp ...
FAIL: gdb.mi/mi-dprintf.exp: gdb: mi 1st dprintf stop (timeout)
FAIL: gdb.mi/mi-dprintf.exp: mi 1st dprintf, agent
FAIL: gdb.mi/mi-dprintf.exp: mi 2nd dprintf, agent
FAIL: gdb.mi/mi-dprintf.exp: mi info dprintf second time

Did you try gdb/contrib/expect-read1.sh ?

-- 
Pedro Alves

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

On Tue, Apr 16, 2013 at 12:23 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/15/2013 03:57 PM, Hui Zhu wrote:
>> On Fri, Apr 12, 2013 at 7:34 PM, Pedro Alves <palves@redhat.com> wrote:
>>> On 04/12/2013 11:38 AM, Hui Zhu wrote:
>>>>>>
>>>>>> I still get:
>>>>>>
>>>>>> $ make check gdbserver RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"
>>>> This part is really odd.
>>>> In my part, without "sleep 1" will random get fail with "Set dprintf
>>>> style to agent ".
>>>> The reason of fail is test try to check the output before it call
>>>> send_gdb "set dprintf-style agent\n".
>>>> This is why I add a "sleep 1" for it.
>>>>
>>>> But looks it still not OK in your part, so I change it to:
>>>> mi_gdb_test "pwd" ".*"
>>>>
>>>> If it is still not OK in your part, I suggest remove this part of test
>>>> because it is not very important for this test.   The "set
>>>> dprintf-style agent" is tested in "dprintf.exp".
>>>>
>>>
>>> No, it's not okay.  And it's not okay to just sweep it under the rug.
>>>
>>> I believe the problem is that the test is sending CLI resumption commands:
>>>
>>> +       mi_gdb_test "continue" ".*At foo entry.*arg=1234, g=1234.*" "mi 1st dprintf"
>>> +       mi_gdb_test "continue" ".*At foo entry.*arg=1235, g=2222.*" "mi 2nd dprintf"
>>>
>>> "continue" sent from MI causes two prompts to appear:
>>>
>>> (gdb)
>>> continue
>>> &"continue\n"
>>> ~"Continuing.\n"
>>> ^running
>>> *running,thread-id="all"
>>> (gdb)
>>> =breakpoint-modified,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x000000000045a3ff",func="main",file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32",thread-groups=["i1"],times="1",original-location="/home/pedro/gdb/mygit/src/gdb/gdb.c:32"}
>>> ~"\nBreakpoint "
>>> ~"6, main (argc=1, argv=0x7fffffffdc58) at ../../src/gdb/gdb.c:32\n"
>>> ~"32\t  args.use_windows = 0;\n"
>>> *stopped,reason="breakpoint-hit",disp="keep",bkptno="6",frame={addr="0x000000000045a3ff",func="main",args=[{name="argc",value="1"},{name="argv",value="0x7fffffffdc58"}],file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="32"},thread-id="1",stopped-threads="all",core="0"
>>> (gdb)
>>>
>>> and that is confusing the test (the regex for the prompt is probably stopping
>>> at the first prompt sometimes).
>>>
>>> The test should be adjusted to do MI -exec-continue instead, with
>>> mi_execute_to "exec-continue", mi_send_resuming_command "exec-continue"
>>> or something like that.
>>>
>>> BTW, you don't need to use gdbserver to trigger the issue.  Native
>>> works too.  E.g., leave this running, and eventually, it should FAIL
>>> and stop:
>>>
>>> $ (set -e; while true; do make check RUNTESTFLAGS="mi-dprintf.exp"; done)
>>>
>>> gdb/contrib/expect-read1.sh probably makes this reproducible all
>>> the time, though I haven't tried.
>>
>>
>> Post a new version change the continue to mi_run_cmd, gdb_expect and
>> mi_send_resuming_command.
>> It works OK with loop test.
>
> $ (set -e; while true; do make check RUNTESTFLAGS="mi-dprintf.exp"; done)
> ...
> Running ../../../src/gdb/testsuite/gdb.mi/mi-dprintf.exp ...
> FAIL: gdb.mi/mi-dprintf.exp: gdb: mi 1st dprintf stop (timeout)
> FAIL: gdb.mi/mi-dprintf.exp: mi 1st dprintf, agent
> FAIL: gdb.mi/mi-dprintf.exp: mi 2nd dprintf, agent
> FAIL: gdb.mi/mi-dprintf.exp: mi info dprintf second time
>
> Did you try gdb/contrib/expect-read1.sh ?


What I did is:
EXPECT=../contrib/expect-read1.sh make check RUNTESTFLAGS="mi-dprintf.exp"
EXPECT=../contrib/expect-read1.sh make check
RUNTESTFLAGS="--target_board=native-gdbserver mi-dprintf.exp"

And it is OK in my part.  I did something wrong?

Thanks,
Hui



>
> --
> Pedro Alves
>

Re: [PATCH] add -s option to make -break-insert support dprintf

[Yao Qi]

On 04/15/2013 10:57 PM, Hui Zhu wrote:
> +	mi_run_cmd
> +	set msg "mi 1st dprintf"
> +	gdb_expect {
> +            -re ".*At foo entry.*arg=1234, g=1234.*" {

Hui,
Probably we need to remove the trailing ".*" above.  Then, the rest of 
the output will be matched ...


> +		pass $msg
> +	    }
> +	    -re ".*$mi_gdb_prompt$" {
> +		fail $msg
> +	    }
> +            timeout {
> +		fail $msg
> +            }
> +	}
> +	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"

... here.

> +
> +	set msg "mi 2nd dprintf"
> +	mi_send_resuming_command "exec-continue" "$msg continue"
> +	gdb_expect {
> +            -re ".*At foo entry.*arg=1235, g=2222.*" {

We need append "$mi_gdb_prompt$" to the end of the pattern above to eat 
all the output, otherwise, these output will make troubles to the rest 
of the tests.

> +		pass $msg
> +	    }
> +	    -re ".*$mi_gdb_prompt$" {
> +		fail $msg
> +	    }
> +            timeout {
> +		fail $msg
> +            }
> +	}

I can get some fails on my machine, similar to fails that Pedro pointed 
out.  With these change above, I don't see fails any more.

b.t.w,

+# To make sure set dprintf-style agent get right output.
+mi_gdb_test "pwd" ".*"

This line is odd to me, and looks it only papers over the problem 
somewhere else.

-- 
Yao (齐尧)

Re: [PATCH] add -s option to make -break-insert support dprintf

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Fri, 12 Apr 2013 18:38:46 +0800
> Cc: Eli Zaretskii <eliz@gnu.org>, Hui Zhu <hui_zhu@mentor.com>, 
> 	gdb-patches ml <gdb-patches@sourceware.org>, Marc Khouzam <marc.khouzam@ericsson.com>
> 
> >>> New MI features need a NEWS entry.
> >>
> >> This is for NEWS:
> >>   ** The -s of MI command -break-insert can set a dynamic printf.
> >>
> >
> > Please send that as a real patch, so e.g., we can see what section of
> > NEWS you're proposing adding it to.
> 
> OK.  I add this change to doc patch.

It is OK, thanks.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
Hui> +    [ @var{location} ]

Hui> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
Hui> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
Hui> +The @var{template} and @var{expression} should be within double
Hui> +quotes and be escaped by being preceded with a backslash.

MI already defines a quoting approach and allows multiple arguments.
In fact one of its selling points is that it doesn't have to be as
free-form as the CLI -- it can be more predictable for programs to use.

So, I think the above approach is not that great.
It adds a second layer of parsing to MI, I guess just to work around
internal deficiencies in gdb.

It seems like you could use positional arguments instead:

-break-insert -s FORMAT LOCATION ARG ARG ARG ...


I don't really understand the part about how the expressions should be
in double quotes.  The test suite doesn't do that:

    +mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \

I suggest just dropping that text.  I think it probably means that the
argument should be properly quoted for MI, but that is redundant.

Tom

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

On Sat, Apr 13, 2013 at 12:32 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
> Hui> +    [ @var{location} ]
>
> Hui> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
> Hui> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
> Hui> +The @var{template} and @var{expression} should be within double
> Hui> +quotes and be escaped by being preceded with a backslash.
>
> MI already defines a quoting approach and allows multiple arguments.
> In fact one of its selling points is that it doesn't have to be as
> free-form as the CLI -- it can be more predictable for programs to use.
>
> So, I think the above approach is not that great.
> It adds a second layer of parsing to MI, I guess just to work around
> internal deficiencies in gdb.
>
> It seems like you could use positional arguments instead:
>
> -break-insert -s FORMAT LOCATION ARG ARG ARG ...
>
>
> I don't really understand the part about how the expressions should be
> in double quotes.  The test suite doesn't do that:
>
>     +mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
>
> I suggest just dropping that text.  I think it probably means that the
> argument should be properly quoted for MI, but that is redundant.
>
> Tom

This design is because the MI inferior will auto convert the format
string of argument of mi command.  But dprintf need format sting keep
the original of the format string.  So I use the current format.

Thanks,
Hui

Re: [PATCH] add -s option to make -break-insert support dprintf

[Pedro Alves]

On 04/15/2013 02:33 PM, Hui Zhu wrote:
> On Sat, Apr 13, 2013 at 12:32 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>>
>> Hui> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
>> Hui> +    [ @var{location} ]
>>
>> Hui> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
>> Hui> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
>> Hui> +The @var{template} and @var{expression} should be within double
>> Hui> +quotes and be escaped by being preceded with a backslash.
>>
>> MI already defines a quoting approach and allows multiple arguments.
>> In fact one of its selling points is that it doesn't have to be as
>> free-form as the CLI -- it can be more predictable for programs to use.
>>
>> So, I think the above approach is not that great.
>> It adds a second layer of parsing to MI, I guess just to work around
>> internal deficiencies in gdb.
>>
>> It seems like you could use positional arguments instead:
>>
>> -break-insert -s FORMAT LOCATION ARG ARG ARG ...
>>
>>
>> I don't really understand the part about how the expressions should be
>> in double quotes.  The test suite doesn't do that:
>>
>>     +mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
>>
>> I suggest just dropping that text.  I think it probably means that the
>> argument should be properly quoted for MI, but that is redundant.

I stopped for this as well before while reviewing the patches.  I ended
up not say anything as after trying things out, I realized that unlike
with other arguments, which only require quoting if the argument itself
contains spaces or quotes, etc., in this case, the quotes really are
necessary (as in, GDB complains if you don't add the quotes).  I didn't
think about the split arguments idea though.  That's a good question.

>>
>> Tom
> 
> This design is because the MI inferior will auto convert the format
> string of argument of mi command.  But dprintf need format sting keep
> the original of the format string.  So I use the current format.

I have a bit of trouble understanding exactly what you meant.  :-(  I see
that with "target-printf" (what dprintf uses as command when the dprintf
runs on the target/agent side), the whole format string plus the expressions are
passed down to the target as a single chunk, unanalyzed and unmodified.
E.g., even 'dprintf foo, "foo" "bar"' or 'agent-printf "foo" "bar"' is
accepted, and passed down to the target as is (whatever that might mean).
Is that what you were referring to?  What did you mean by "MI inferior"
and "auto convert" ?

-- 
Pedro Alves

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

On Tue, Apr 16, 2013 at 12:05 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/15/2013 02:33 PM, Hui Zhu wrote:
>> On Sat, Apr 13, 2013 at 12:32 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>>>
>>> Hui> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
>>> Hui> +    [ @var{location} ]
>>>
>>> Hui> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
>>> Hui> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
>>> Hui> +The @var{template} and @var{expression} should be within double
>>> Hui> +quotes and be escaped by being preceded with a backslash.
>>>
>>> MI already defines a quoting approach and allows multiple arguments.
>>> In fact one of its selling points is that it doesn't have to be as
>>> free-form as the CLI -- it can be more predictable for programs to use.
>>>
>>> So, I think the above approach is not that great.
>>> It adds a second layer of parsing to MI, I guess just to work around
>>> internal deficiencies in gdb.
>>>
>>> It seems like you could use positional arguments instead:
>>>
>>> -break-insert -s FORMAT LOCATION ARG ARG ARG ...
>>>
>>>
>>> I don't really understand the part about how the expressions should be
>>> in double quotes.  The test suite doesn't do that:
>>>
>>>     +mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
>>>
>>> I suggest just dropping that text.  I think it probably means that the
>>> argument should be properly quoted for MI, but that is redundant.
>
> I stopped for this as well before while reviewing the patches.  I ended
> up not say anything as after trying things out, I realized that unlike
> with other arguments, which only require quoting if the argument itself
> contains spaces or quotes, etc., in this case, the quotes really are
> necessary (as in, GDB complains if you don't add the quotes).  I didn't
> think about the split arguments idea though.  That's a good question.
>
>>>
>>> Tom
>>
>> This design is because the MI inferior will auto convert the format
>> string of argument of mi command.  But dprintf need format sting keep
>> the original of the format string.  So I use the current format.
>
> I have a bit of trouble understanding exactly what you meant.  :-(  I see
> that with "target-printf" (what dprintf uses as command when the dprintf
> runs on the target/agent side), the whole format string plus the expressions are
> passed down to the target as a single chunk, unanalyzed and unmodified.
> E.g., even 'dprintf foo, "foo" "bar"' or 'agent-printf "foo" "bar"' is
> accepted, and passed down to the target as is (whatever that might mean).
> Is that what you were referring to?  What did you mean by "MI inferior"
> and "auto convert" ?

The MI handle the argument is different with CLI.  For example:
Input -break-insert -s "1\n" to mi.  The mi_cmd_break_insert will get:
Breakpoint 1, mi_cmd_break_insert (command=0x233fb50 "break-insert",
argv=0x233fd30, argc=2)
    at ../../gdb/gdb/mi/mi-cmd-break.c:93
93  char *address = NULL;
(gdb) p argv[0]
$1 = 0x233fd50 "-s"
(gdb) p argv[1]
$2 = 0x233fd70 "1\n"
(gdb) p argv[1][0]
$3 = 49 '1'
(gdb) p argv[1][1]
$4 = 10 '\n'

But the dprint want to get is:
(gdb) p argv[1][1]
$7 = 49 '1'
(gdb) p argv[1][2]
$8 = 92 '\\'
(gdb) p argv[1][3]
$9 = 110 'n'

Thanks,
Hui


>
> --
> Pedro Alves
>

Re: [PATCH] add -s option to make -break-insert support dprintf

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> The MI handle the argument is different with CLI.  For example:
Hui> Input -break-insert -s "1\n" to mi.  The mi_cmd_break_insert will get:
[...]

This seems to mean that extra quoting is needed for the format string.
Like you can't send just:

-break-insert -s "\"1\n\""

It seems like you have to send:

-break-insert -s "\"1\\n\""


But an alternative is to keep the MI interface simple and "MI-like" --
meaning using the quoting and argument parsing convention already
defined by MI, one of its big benefits over the CLI -- and let gdb
handle the requirements of the protocol and/or its own internals.  That
is, send:

-break-insert -s "1\n"

... and gdb can rewrite that format string to whatever is needed.

Tom

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

On Sat, Apr 20, 2013 at 3:56 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> The MI handle the argument is different with CLI.  For example:
> Hui> Input -break-insert -s "1\n" to mi.  The mi_cmd_break_insert will get:
> [...]
>
> This seems to mean that extra quoting is needed for the format string.
> Like you can't send just:
>
> -break-insert -s "\"1\n\""
>
> It seems like you have to send:
>
> -break-insert -s "\"1\\n\""
>
>
> But an alternative is to keep the MI interface simple and "MI-like" --
> meaning using the quoting and argument parsing convention already
> defined by MI, one of its big benefits over the CLI -- and let gdb
> handle the requirements of the protocol and/or its own internals.  That
> is, send:
>
> -break-insert -s "1\n"
>
> ... and gdb can rewrite that format string to whatever is needed.
>
> Tom


Does GDB have some function can convert string to "" inside string?

Could you give me some comments about the new format for -s.

My thought is:
-s format argv1 argv2, other options

"-break-insert -s FORMAT LOCATION ARG ARG ARG ..." is too different
with current options of  -break-insert and very hard to support.

Thanks,
Hui

Re: [PATCH] add -s option to make -break-insert support dprintf

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> Does GDB have some function can convert string to "" inside string?

I wouldn't rule it out but I couldn't think of one offhand.

Hui> Could you give me some comments about the new format for -s.
Hui> My thought is:
Hui> -s format argv1 argv2, other options
Hui> "-break-insert -s FORMAT LOCATION ARG ARG ARG ..." is too different
Hui> with current options of  -break-insert and very hard to support.

What is hard about it?

In mi_cmd_break_insert, if '-s' was given, use argv[oind+1] through
argv[argc-1] as the arguments.  Maybe the hard part is getting this
through create_breakpoint?  That is just further proof that it is a bad
API.

TBH I don't see why this needs to be in -break-insert.  A new command
seems cleaner to me.  But I don't want to press too hard either.

Tom

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

On Thu, Apr 25, 2013 at 4:35 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> Does GDB have some function can convert string to "" inside string?
>
> I wouldn't rule it out but I couldn't think of one offhand.
>
> Hui> Could you give me some comments about the new format for -s.
> Hui> My thought is:
> Hui> -s format argv1 argv2, other options
> Hui> "-break-insert -s FORMAT LOCATION ARG ARG ARG ..." is too different
> Hui> with current options of  -break-insert and very hard to support.
>
> What is hard about it?
>
> In mi_cmd_break_insert, if '-s' was given, use argv[oind+1] through
> argv[argc-1] as the arguments.  Maybe the hard part is getting this
> through create_breakpoint?  That is just further proof that it is a bad
> API.
>
> TBH I don't see why this needs to be in -break-insert.  A new command
> seems cleaner to me.  But I don't want to press too hard either.
>
> Tom

Hi Tom,

According to your comments.  I did some update with these patches to
added special command -dprintf-insert to insert dprintf.  Its format
is close to simple dprintf command:
-dprintf-insert LOCATION FORMAT ARG ARG ...

Thanks,
Hui

2013-05-03  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (ctype.h): New include.
	(mi_argv_to_format, mi_cmd_break_insert_1): New.
	(mi_cmd_break_insert): Call mi_cmd_break_insert_1.
	(mi_cmd_dprintf_insert): New.
	* mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
	* mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.

2013-05-03  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
	"-dprintf-insert" command.

2013-05-03  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -301,7 +301,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -30,6 +30,7 @@
 #include "observer.h"
 #include "mi-main.h"
 #include "mi-cmd-break.h"
+#include <ctype.h>
 
 enum
   {
@@ -84,11 +85,87 @@ setup_breakpoint_reporting (void)
 }
 
 
-/* Implements the -break-insert command.
-   See the MI manual for the list of possible options.  */
+static char *
+mi_argv_to_format (int format_num, char **argv, int argc)
+{
+  char *format;
+  int format_size, i, format_current_size = 0;
 
-void
-mi_cmd_break_insert (char *command, char **argv, int argc)
+  if (format_num >= argc)
+    error (_("-dprintf-insert: Missing <format>"));
+
+  /* If all the string need convert to \ddd mode, so * 2.
+     + 2 for two ".
+     + 1 for \0.  */
+  format_size = strlen (argv[format_num]) * 4 + 3;
+  format = xmalloc (format_size);
+
+  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
+  *(format + format_current_size) = '\"';
+  ++format_current_size;
+  for (i = 0; i < strlen (argv[format_num]); i++)
+    {
+      switch (argv[format_num][i])
+	{
+	case '\\':
+	  strcpy (format + format_current_size, "\\\\");
+	  break;
+	case '\a':
+	  strcpy (format + format_current_size, "\\a");
+	  break;
+	case '\b':
+	  strcpy (format + format_current_size, "\\b");
+	  break;
+	case '\f':
+	  strcpy (format + format_current_size, "\\f");
+	  break;
+	case '\n':
+	  strcpy (format + format_current_size, "\\n");
+	  break;
+	case '\r':
+	  strcpy (format + format_current_size, "\\r");
+	  break;
+	case '\t':
+	  strcpy (format + format_current_size, "\\t");
+	  break;
+	case '\v':
+	  strcpy (format + format_current_size, "\\v");
+	  break;
+	default:
+	  if (isprint(argv[format_num][i]))
+	    {
+	      *(format + format_current_size) = argv[format_num][i];
+	      *(format + format_current_size + 1) = '\0';
+	    }
+	  else
+	    sprintf (format + format_current_size, "\\%o",
+		     argv[format_num][i]);
+	  break;
+	}
+      format_current_size += strlen (format + format_current_size);
+    }
+  *(format + format_current_size) = '\"';
+  ++format_current_size;
+  *(format + format_current_size) = '\0';
+
+  /* Apply other argv to FORMAT.  */
+  for (i = format_num + 1; i < argc; i++)
+    {
+      if (2 + strlen (argv[i]) > format_size - format_current_size)
+        {
+	  format_size = format_current_size + strlen (argv[i]) + 2;
+          format = xrealloc (format, format_size);
+	}
+      *(format + format_current_size) = ',';
+      strcpy (format + format_current_size + 1, argv[i]);
+      format_current_size += strlen (format + format_current_size);
+    }
+
+  return format;
+}
+
+static void
+mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
 {
   char *address = NULL;
   int hardware = 0;
@@ -102,6 +179,7 @@ mi_cmd_break_insert (char *command, char
   struct cleanup *back_to;
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
@@ -163,35 +241,75 @@ mi_cmd_break_insert (char *command, char
     }
 
   if (oind >= argc)
-    error (_("-break-insert: Missing <location>"));
-  if (oind < argc - 1)
-    error (_("-break-insert: Garbage following <location>"));
+    error (_("-%s-insert: Missing <location>"),
+	   dprintf ? "dprintf" : "break");
   address = argv[oind];
+  if (dprintf)
+    {
+      if (hardware)
+	error (_("-dprintf-insert: does not support -h"));
+
+      extra_string = mi_argv_to_format (oind + 1, argv, argc);
+      make_cleanup (xfree, extra_string);
+    }
+  else
+    {
+      if (oind < argc - 1)
+	error (_("-break-insert: Garbage following <location>"));
+    }
 
   /* Now we have what we need, let's insert the breakpoint!  */
   back_to = setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
   do_cleanups (back_to);
+}
+
+/* Implements the -break-insert command.
+   See the MI manual for the list of possible options.  */
 
+void
+mi_cmd_break_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (0, command, argv, argc);
+}
+
+/* Implements the -dprintf-insert command.
+   See the MI manual for the list of possible options.  */
+
+void
+mi_cmd_dprintf_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (1, command, argv, argc);
 }
 
 enum wp_type
--- a/mi/mi-cmds.c
+++ b/mi/mi-cmds.c
@@ -61,6 +61,8 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
   DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
 		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("dprintf-insert", mi_cmd_dprintf_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
   DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
 		   &mi_suppress_notification.breakpoint),
--- a/mi/mi-cmds.h
+++ b/mi/mi-cmds.h
@@ -39,6 +39,7 @@ typedef void (mi_cmd_argv_ftype) (char *
 extern mi_cmd_argv_ftype mi_cmd_ada_task_info;
 extern mi_cmd_argv_ftype mi_cmd_add_inferior;
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
+extern mi_cmd_argv_ftype mi_cmd_dprintf_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
 extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 2763 bytes --]

--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,8 @@ show remote trace-status-packet
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
+  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28946,6 +28946,84 @@ times="0"@}]@}
 @c (gdb)
 @end smallexample
 
+@subheading The @code{-dprintf-insert} Command
+@findex -dprintf-insert
+
+@subsubheading Synopsis
+
+@smallexample
+ -dprintf-insert [ -t ] [ -f ] [ -d ]
+    [ -c @var{condition} ] [ -i @var{ignore-count} ]
+    [ -p @var{thread-id} ] [ @var{location} ] [ @var{format} ]
+    [ @var{argument} ]
+@end smallexample
+
+@noindent
+If specified, @var{location}, can be one of:
+
+@itemize @bullet
+@item function
+@c @item +offset
+@c @item -offset
+@c @item linenum
+@item filename:linenum
+@item filename:function
+@item *address
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary breakpoint.
+@item -f
+If @var{location} cannot be parsed (for example if it
+refers to unknown files or functions), create a pending
+breakpoint. Without this flag, @value{GDBN} will report
+an error, and won't create a breakpoint, if @var{location}
+cannot be parsed.
+@item -d
+Create a disabled breakpoint.
+@item -c @var{condition}
+Make the breakpoint conditional on @var{condition}.
+@item -i @var{ignore-count}
+Initialize the @var{ignore-count}.
+@item -p @var{thread-id}
+Restrict the breakpoint to the specified @var{thread-id}.
+@end table
+
+@subsubheading Result
+
+@xref{GDB/MI Breakpoint Information}, for details on the format of the
+resulting breakpoint.
+
+Note: this format is open to change.
+@c An out-of-band breakpoint instead of part of the result?
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} commands are @samp{dprintf}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+4-dprintf-insert foo "At foo entry\n"
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-dprintf-insert 26 "arg=%d, g=%d\n" arg g
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @subheading The @code{-break-list} Command
 @findex -break-list
 

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7444 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,145 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-dprintf-insert" \
+    "1\\^error,msg=\"-dprintf-insert: Missing <location>\"" "mi insert without location"
+
+mi_gdb_test "2-dprintf-insert foo" \
+    "2\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-dprintf-insert 29" \
+    "3\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-dprintf-insert foo \"At foo entry\\n\"" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-dprintf-insert $dp_location1 \"arg=%d, g=%d\\n\" arg g" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	global mi_gdb_prompt
+
+	mi_run_cmd
+	set msg "mi 1st dprintf"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1234, g=1234.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"
+
+	set msg "mi 2nd dprintf"
+	mi_send_resuming_command "exec-continue" "$msg continue"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1235, g=2222.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> According to your comments.  I did some update with these patches to
Hui> added special command -dprintf-insert to insert dprintf.  Its format
Hui> is close to simple dprintf command:
Hui> -dprintf-insert LOCATION FORMAT ARG ARG ...

I like this approach much more.
Thanks for doing it.

Hui> +static char *
Hui> +mi_argv_to_format (int format_num, char **argv, int argc)

This needs an introductory comment explaining the arguments and result.

Hui> +  /* If all the string need convert to \ddd mode, so * 2.
Hui> +     + 2 for two ".
Hui> +     + 1 for \0.  */
Hui> +  format_size = strlen (argv[format_num]) * 4 + 3;

The comment says "* 2" but the code says "* 4".
Personally I'd just use an obstack rather than mess around with explicit
reallocs and the like.

Hui> +	    sprintf (format + format_current_size, "\\%o",
Hui> +		     argv[format_num][i]);

It seems that this could do the wrong thing for a char that
sign-extends.

Hui> +  /* Apply other argv to FORMAT.  */
Hui> +  for (i = format_num + 1; i < argc; i++)

It seems to me that it would be better to just pass in argv+argc to
mi_argv_to_format, and omit the format_num argument entirely.

Hui> +static void
Hui> +mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)

Intro comment.

Hui> +      extra_string = mi_argv_to_format (oind + 1, argv, argc);
Hui> +      make_cleanup (xfree, extra_string);

This makes a dangling cleanup.

Hui> +  if (tracepoint)
Hui> +    {
Hui> +      /* Note that to request a fast tracepoint, the client uses the
Hui> +	 "hardware" flag, although there's nothing of hardware related to
Hui> +	 fast tracepoints -- one can implement slow tracepoints with
Hui> +	 hardware breakpoints, but fast tracepoints are always software.
Hui> +	 "fast" is a misnomer, actually, "jump" would be more appropriate.
Hui> +	 A simulator or an emulator could conceivably implement fast
Hui> +	 regular non-jump based tracepoints.  */
Hui> +      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
Hui> +      ops = &tracepoint_breakpoint_ops;
Hui> +    }
Hui> +  else if (dprintf)
Hui> +    {

It seems that 'tracepoint' and 'dprintf' are exclusive, so this should
be checked above where "hardware" is checked.

Tom

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi Tom,

Thanks for your review.

On Wed, May 8, 2013 at 4:49 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> According to your comments.  I did some update with these patches to
> Hui> added special command -dprintf-insert to insert dprintf.  Its format
> Hui> is close to simple dprintf command:
> Hui> -dprintf-insert LOCATION FORMAT ARG ARG ...
>
> I like this approach much more.
> Thanks for doing it.
>
> Hui> +static char *
> Hui> +mi_argv_to_format (int format_num, char **argv, int argc)
>
> This needs an introductory comment explaining the arguments and result.

Add:
/* Convert arguments in ARGV to the string in "format",argv,argv...
   and return it.  */

>
> Hui> +  /* If all the string need convert to \ddd mode, so * 2.
> Hui> +     + 2 for two ".
> Hui> +     + 1 for \0.  */
> Hui> +  format_size = strlen (argv[format_num]) * 4 + 3;
>
> The comment says "* 2" but the code says "* 4".
> Personally I'd just use an obstack rather than mess around with explicit
> reallocs and the like.

Obstack is better than what I did in before.   Thanks for that.
Updated patch for it.

>
> Hui> +      sprintf (format + format_current_size, "\\%o",
> Hui> +               argv[format_num][i]);
>
> It seems that this could do the wrong thing for a char that
> sign-extends.

I changed this part to:
sprintf (tmp, "\\%o", (unsigned char)argv[format_num][i]);

>
> Hui> +  /* Apply other argv to FORMAT.  */
> Hui> +  for (i = format_num + 1; i < argc; i++)
>
> It seems to me that it would be better to just pass in argv+argc to
> mi_argv_to_format, and omit the format_num argument entirely.

Fixed.

>
> Hui> +static void
> Hui> +mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
>
> Intro comment.

/* Insert breakpoint.
   If dprintf is true, it will insert dprintf.
   If not, it will insert other type breakpoint.  */

>
> Hui> +      extra_string = mi_argv_to_format (oind + 1, argv, argc);
> Hui> +      make_cleanup (xfree, extra_string);
>
> This makes a dangling cleanup.

Fixed.

>
> Hui> +  if (tracepoint)
> Hui> +    {
> Hui> +      /* Note that to request a fast tracepoint, the client uses the
> Hui> +   "hardware" flag, although there's nothing of hardware related to
> Hui> +   fast tracepoints -- one can implement slow tracepoints with
> Hui> +   hardware breakpoints, but fast tracepoints are always software.
> Hui> +   "fast" is a misnomer, actually, "jump" would be more appropriate.
> Hui> +   A simulator or an emulator could conceivably implement fast
> Hui> +   regular non-jump based tracepoints.  */
> Hui> +      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
> Hui> +      ops = &tracepoint_breakpoint_ops;
> Hui> +    }
> Hui> +  else if (dprintf)
> Hui> +    {
>
> It seems that 'tracepoint' and 'dprintf' are exclusive, so this should
> be checked above where "hardware" is checked.

Fixed.

I post  the new patches that updated according to your comments.

Thanks,
Hui

2013-05-10  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (ctype.h): New include.
	(gdb_obstack.h): New include.
	(mi_argv_to_format, mi_cmd_break_insert_1): New.
	(mi_cmd_break_insert): Call mi_cmd_break_insert_1.
	(mi_cmd_dprintf_insert): New.
	* mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
	* mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.

2013-05-10  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
	"-dprintf-insert" command.

2013-05-10  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -301,7 +301,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -30,6 +30,8 @@
 #include "observer.h"
 #include "mi-main.h"
 #include "mi-cmd-break.h"
+#include "gdb_obstack.h"
+#include <ctype.h>
 
 enum
   {
@@ -84,11 +86,82 @@ setup_breakpoint_reporting (void)
 }
 
 
-/* Implements the -break-insert command.
-   See the MI manual for the list of possible options.  */
+/* Convert arguments in ARGV to the string in "format",argv,argv...
+   and return it.  */
 
-void
-mi_cmd_break_insert (char *command, char **argv, int argc)
+static char *
+mi_argv_to_format (char **argv, int argc)
+{
+  int i;
+  struct obstack obstack;
+  char *ret;
+
+  obstack_init (&obstack);
+
+  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
+  obstack_1grow (&obstack, '\"');
+  for (i = 0; i < strlen (argv[0]); i++)
+    {
+      switch (argv[0][i])
+	{
+	case '\\':
+	  obstack_grow (&obstack, "\\\\", 2);
+	  break;
+	case '\a':
+	  obstack_grow (&obstack, "\\a", 2);
+	  break;
+	case '\b':
+	  obstack_grow (&obstack, "\\b", 2);
+	  break;
+	case '\f':
+	  obstack_grow (&obstack, "\\f", 2);
+	  break;
+	case '\n':
+	  obstack_grow (&obstack, "\\n", 2);
+	  break;
+	case '\r':
+	  obstack_grow (&obstack, "\\r", 2);
+	  break;
+	case '\t':
+	  obstack_grow (&obstack, "\\t", 2);
+	  break;
+	case '\v':
+	  obstack_grow (&obstack, "\\v", 2);
+	  break;
+	default:
+	  if (isprint(argv[0][i]))
+	    obstack_grow (&obstack, argv[0] + i, 1);
+	  else
+	    {
+	      char tmp[5];
+	      sprintf (tmp, "\\%o", (unsigned char)argv[0][i]);
+	      obstack_grow (&obstack, tmp, strlen (tmp));
+	    }
+	  break;
+	}
+    }
+  obstack_1grow (&obstack, '\"');
+
+  /* Apply other argv to FORMAT.  */
+  for (i = 1; i < argc; i++)
+    {
+      obstack_1grow (&obstack, ',');
+      obstack_grow (&obstack, argv[i], strlen (argv[i]));
+    }
+  obstack_1grow (&obstack, '\0');
+
+  ret = xstrdup (obstack_finish (&obstack));
+  obstack_free (&obstack, NULL);
+
+  return ret;
+}
+
+/* Insert breakpoint.
+   If dprintf is true, it will insert dprintf.
+   If not, it will insert other type breakpoint.  */
+
+static void
+mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
 {
   char *address = NULL;
   int hardware = 0;
@@ -102,6 +175,8 @@ mi_cmd_break_insert (char *command, char
   struct cleanup *back_to;
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
+  struct cleanup *extra_string_cleanup;
 
   enum opt
     {
@@ -163,35 +238,81 @@ mi_cmd_break_insert (char *command, char
     }
 
   if (oind >= argc)
-    error (_("-break-insert: Missing <location>"));
-  if (oind < argc - 1)
-    error (_("-break-insert: Garbage following <location>"));
+    error (_("-%s-insert: Missing <location>"),
+	   dprintf ? "dprintf" : "break");
   address = argv[oind];
+  if (dprintf)
+    {
+      int format_num = oind + 1;
+
+      if (hardware || tracepoint)
+	error (_("-dprintf-insert: does not support -h or -a"));
+      if (format_num >= argc)
+	error (_("-dprintf-insert: Missing <format>"));
+
+      extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
+      extra_string_cleanup = make_cleanup (xfree, extra_string);
+    }
+  else
+    {
+      if (oind < argc - 1)
+	error (_("-break-insert: Garbage following <location>"));
+    }
 
   /* Now we have what we need, let's insert the breakpoint!  */
   back_to = setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
   do_cleanups (back_to);
+  if (dprintf)
+    do_cleanups (extra_string_cleanup);
+}
+
+/* Implements the -break-insert command.
+   See the MI manual for the list of possible options.  */
 
+void
+mi_cmd_break_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (0, command, argv, argc);
+}
+
+/* Implements the -dprintf-insert command.
+   See the MI manual for the list of possible options.  */
+
+void
+mi_cmd_dprintf_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (1, command, argv, argc);
 }
 
 enum wp_type
--- a/mi/mi-cmds.c
+++ b/mi/mi-cmds.c
@@ -61,6 +61,8 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
   DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
 		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("dprintf-insert", mi_cmd_dprintf_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
   DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
 		   &mi_suppress_notification.breakpoint),
--- a/mi/mi-cmds.h
+++ b/mi/mi-cmds.h
@@ -39,6 +39,7 @@ typedef void (mi_cmd_argv_ftype) (char *
 extern mi_cmd_argv_ftype mi_cmd_ada_task_info;
 extern mi_cmd_argv_ftype mi_cmd_add_inferior;
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
+extern mi_cmd_argv_ftype mi_cmd_dprintf_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
 extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 2748 bytes --]

--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,8 @@ show debug nios2
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
+  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28971,6 +28971,84 @@ times="0"@}]@}
 @c (gdb)
 @end smallexample
 
+@subheading The @code{-dprintf-insert} Command
+@findex -dprintf-insert
+
+@subsubheading Synopsis
+
+@smallexample
+ -dprintf-insert [ -t ] [ -f ] [ -d ]
+    [ -c @var{condition} ] [ -i @var{ignore-count} ]
+    [ -p @var{thread-id} ] [ @var{location} ] [ @var{format} ]
+    [ @var{argument} ]
+@end smallexample
+
+@noindent
+If specified, @var{location}, can be one of:
+
+@itemize @bullet
+@item function
+@c @item +offset
+@c @item -offset
+@c @item linenum
+@item filename:linenum
+@item filename:function
+@item *address
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary breakpoint.
+@item -f
+If @var{location} cannot be parsed (for example if it
+refers to unknown files or functions), create a pending
+breakpoint. Without this flag, @value{GDBN} will report
+an error, and won't create a breakpoint, if @var{location}
+cannot be parsed.
+@item -d
+Create a disabled breakpoint.
+@item -c @var{condition}
+Make the breakpoint conditional on @var{condition}.
+@item -i @var{ignore-count}
+Initialize the @var{ignore-count}.
+@item -p @var{thread-id}
+Restrict the breakpoint to the specified @var{thread-id}.
+@end table
+
+@subsubheading Result
+
+@xref{GDB/MI Breakpoint Information}, for details on the format of the
+resulting breakpoint.
+
+Note: this format is open to change.
+@c An out-of-band breakpoint instead of part of the result?
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} commands are @samp{dprintf}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+4-dprintf-insert foo "At foo entry\n"
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-dprintf-insert 26 "arg=%d, g=%d\n" arg g
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @subheading The @code{-break-list} Command
 @findex -break-list
 

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7444 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,145 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-dprintf-insert" \
+    "1\\^error,msg=\"-dprintf-insert: Missing <location>\"" "mi insert without location"
+
+mi_gdb_test "2-dprintf-insert foo" \
+    "2\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-dprintf-insert 29" \
+    "3\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-dprintf-insert foo \"At foo entry\\n\"" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-dprintf-insert $dp_location1 \"arg=%d, g=%d\\n\" arg g" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	global mi_gdb_prompt
+
+	mi_run_cmd
+	set msg "mi 1st dprintf"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1234, g=1234.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"
+
+	set msg "mi 2nd dprintf"
+	mi_send_resuming_command "exec-continue" "$msg continue"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1235, g=2222.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Tom Tromey]

Hui> +	  if (isprint(argv[0][i]))

Missing space before "(".

Hui> +	      char tmp[5];
Hui> +	      sprintf (tmp, "\\%o", (unsigned char)argv[0][i]);

Missing newline between these lines.
Missing space after ")".

Hui> +  if (dprintf)
Hui> +    {
Hui> +      int format_num = oind + 1;
Hui> +
Hui> +      if (hardware || tracepoint)
Hui> +	error (_("-dprintf-insert: does not support -h or -a"));
Hui> +      if (format_num >= argc)
Hui> +	error (_("-dprintf-insert: Missing <format>"));
Hui> +
Hui> +      extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
Hui> +      extra_string_cleanup = make_cleanup (xfree, extra_string);

It is better to just install an outer null cleanup and invoke that at
the end.

Tom

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi Tom,

Thanks for your review.

On Fri, May 10, 2013 at 11:21 PM, Tom Tromey <tromey@redhat.com> wrote:
> Hui> +    if (isprint(argv[0][i]))
>
> Missing space before "(".

Fixed.

>
> Hui> +        char tmp[5];
> Hui> +        sprintf (tmp, "\\%o", (unsigned char)argv[0][i]);
>
> Missing newline between these lines.
> Missing space after ")".

Fixed.

>
> Hui> +  if (dprintf)
> Hui> +    {
> Hui> +      int format_num = oind + 1;
> Hui> +
> Hui> +      if (hardware || tracepoint)
> Hui> +  error (_("-dprintf-insert: does not support -h or -a"));
> Hui> +      if (format_num >= argc)
> Hui> +  error (_("-dprintf-insert: Missing <format>"));
> Hui> +
> Hui> +      extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
> Hui> +      extra_string_cleanup = make_cleanup (xfree, extra_string);
>
> It is better to just install an outer null cleanup and invoke that at
> the end.

Fixed.

The attachments is the new version of patches.

Thanks,
Hui

2013-05-11  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (ctype.h): New include.
	(gdb_obstack.h): New include.
	(mi_argv_to_format, mi_cmd_break_insert_1): New.
	(mi_cmd_break_insert): Call mi_cmd_break_insert_1.
	(mi_cmd_dprintf_insert): New.
	* mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
	* mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.

2013-05-11  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
	"-dprintf-insert" command.

2013-05-11  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -301,7 +301,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -30,6 +30,8 @@
 #include "observer.h"
 #include "mi-main.h"
 #include "mi-cmd-break.h"
+#include "gdb_obstack.h"
+#include <ctype.h>
 
 enum
   {
@@ -84,11 +86,83 @@ setup_breakpoint_reporting (void)
 }
 
 
-/* Implements the -break-insert command.
-   See the MI manual for the list of possible options.  */
+/* Convert arguments in ARGV to the string in "format",argv,argv...
+   and return it.  */
 
-void
-mi_cmd_break_insert (char *command, char **argv, int argc)
+static char *
+mi_argv_to_format (char **argv, int argc)
+{
+  int i;
+  struct obstack obstack;
+  char *ret;
+
+  obstack_init (&obstack);
+
+  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
+  obstack_1grow (&obstack, '\"');
+  for (i = 0; i < strlen (argv[0]); i++)
+    {
+      switch (argv[0][i])
+	{
+	case '\\':
+	  obstack_grow (&obstack, "\\\\", 2);
+	  break;
+	case '\a':
+	  obstack_grow (&obstack, "\\a", 2);
+	  break;
+	case '\b':
+	  obstack_grow (&obstack, "\\b", 2);
+	  break;
+	case '\f':
+	  obstack_grow (&obstack, "\\f", 2);
+	  break;
+	case '\n':
+	  obstack_grow (&obstack, "\\n", 2);
+	  break;
+	case '\r':
+	  obstack_grow (&obstack, "\\r", 2);
+	  break;
+	case '\t':
+	  obstack_grow (&obstack, "\\t", 2);
+	  break;
+	case '\v':
+	  obstack_grow (&obstack, "\\v", 2);
+	  break;
+	default:
+	  if (isprint (argv[0][i]))
+	    obstack_grow (&obstack, argv[0] + i, 1);
+	  else
+	    {
+	      char tmp[5];
+
+	      sprintf (tmp, "\\%o", (unsigned char) argv[0][i]);
+	      obstack_grow (&obstack, tmp, strlen (tmp));
+	    }
+	  break;
+	}
+    }
+  obstack_1grow (&obstack, '\"');
+
+  /* Apply other argv to FORMAT.  */
+  for (i = 1; i < argc; i++)
+    {
+      obstack_1grow (&obstack, ',');
+      obstack_grow (&obstack, argv[i], strlen (argv[i]));
+    }
+  obstack_1grow (&obstack, '\0');
+
+  ret = xstrdup (obstack_finish (&obstack));
+  obstack_free (&obstack, NULL);
+
+  return ret;
+}
+
+/* Insert breakpoint.
+   If dprintf is true, it will insert dprintf.
+   If not, it will insert other type breakpoint.  */
+
+static void
+mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
 {
   char *address = NULL;
   int hardware = 0;
@@ -99,9 +173,10 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
-  struct cleanup *back_to;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
@@ -163,35 +238,79 @@ mi_cmd_break_insert (char *command, char
     }
 
   if (oind >= argc)
-    error (_("-break-insert: Missing <location>"));
-  if (oind < argc - 1)
-    error (_("-break-insert: Garbage following <location>"));
+    error (_("-%s-insert: Missing <location>"),
+	   dprintf ? "dprintf" : "break");
   address = argv[oind];
+  if (dprintf)
+    {
+      int format_num = oind + 1;
+
+      if (hardware || tracepoint)
+	error (_("-dprintf-insert: does not support -h or -a"));
+      if (format_num >= argc)
+	error (_("-dprintf-insert: Missing <format>"));
+
+      extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
+      make_cleanup (xfree, extra_string);
+    }
+  else
+    {
+      if (oind < argc - 1)
+	error (_("-break-insert: Garbage following <location>"));
+    }
 
   /* Now we have what we need, let's insert the breakpoint!  */
-  back_to = setup_breakpoint_reporting ();
+  setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
   do_cleanups (back_to);
+}
+
+/* Implements the -break-insert command.
+   See the MI manual for the list of possible options.  */
+
+void
+mi_cmd_break_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (0, command, argv, argc);
+}
+
+/* Implements the -dprintf-insert command.
+   See the MI manual for the list of possible options.  */
 
+void
+mi_cmd_dprintf_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (1, command, argv, argc);
 }
 
 enum wp_type
--- a/mi/mi-cmds.c
+++ b/mi/mi-cmds.c
@@ -61,6 +61,8 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
   DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
 		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("dprintf-insert", mi_cmd_dprintf_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
   DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
 		   &mi_suppress_notification.breakpoint),
--- a/mi/mi-cmds.h
+++ b/mi/mi-cmds.h
@@ -39,6 +39,7 @@ typedef void (mi_cmd_argv_ftype) (char *
 extern mi_cmd_argv_ftype mi_cmd_ada_task_info;
 extern mi_cmd_argv_ftype mi_cmd_add_inferior;
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
+extern mi_cmd_argv_ftype mi_cmd_dprintf_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
 extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 2748 bytes --]

--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,8 @@ show debug nios2
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
+  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28971,6 +28971,84 @@ times="0"@}]@}
 @c (gdb)
 @end smallexample
 
+@subheading The @code{-dprintf-insert} Command
+@findex -dprintf-insert
+
+@subsubheading Synopsis
+
+@smallexample
+ -dprintf-insert [ -t ] [ -f ] [ -d ]
+    [ -c @var{condition} ] [ -i @var{ignore-count} ]
+    [ -p @var{thread-id} ] [ @var{location} ] [ @var{format} ]
+    [ @var{argument} ]
+@end smallexample
+
+@noindent
+If specified, @var{location}, can be one of:
+
+@itemize @bullet
+@item function
+@c @item +offset
+@c @item -offset
+@c @item linenum
+@item filename:linenum
+@item filename:function
+@item *address
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary breakpoint.
+@item -f
+If @var{location} cannot be parsed (for example if it
+refers to unknown files or functions), create a pending
+breakpoint. Without this flag, @value{GDBN} will report
+an error, and won't create a breakpoint, if @var{location}
+cannot be parsed.
+@item -d
+Create a disabled breakpoint.
+@item -c @var{condition}
+Make the breakpoint conditional on @var{condition}.
+@item -i @var{ignore-count}
+Initialize the @var{ignore-count}.
+@item -p @var{thread-id}
+Restrict the breakpoint to the specified @var{thread-id}.
+@end table
+
+@subsubheading Result
+
+@xref{GDB/MI Breakpoint Information}, for details on the format of the
+resulting breakpoint.
+
+Note: this format is open to change.
+@c An out-of-band breakpoint instead of part of the result?
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} commands are @samp{dprintf}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+4-dprintf-insert foo "At foo entry\n"
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-dprintf-insert 26 "arg=%d, g=%d\n" arg g
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @subheading The @code{-break-list} Command
 @findex -break-list
 

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7444 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,145 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-dprintf-insert" \
+    "1\\^error,msg=\"-dprintf-insert: Missing <location>\"" "mi insert without location"
+
+mi_gdb_test "2-dprintf-insert foo" \
+    "2\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-dprintf-insert 29" \
+    "3\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-dprintf-insert foo \"At foo entry\\n\"" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-dprintf-insert $dp_location1 \"arg=%d, g=%d\\n\" arg g" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	global mi_gdb_prompt
+
+	mi_run_cmd
+	set msg "mi 1st dprintf"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1234, g=1234.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"
+
+	set msg "mi 2nd dprintf"
+	mi_send_resuming_command "exec-continue" "$msg continue"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1235, g=2222.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Sat, 11 May 2013 10:37:56 +0800
> Cc: Pedro Alves <palves@redhat.com>, Eli Zaretskii <eliz@gnu.org>, Hui Zhu <hui_zhu@mentor.com>, 
> 	gdb-patches ml <gdb-patches@sourceware.org>, Marc Khouzam <marc.khouzam@ericsson.com>
> 
> --- a/NEWS
> +++ b/NEWS
> @@ -60,6 +60,8 @@ show debug nios2
>    ** The -trace-save MI command can optionally save trace buffer in Common
>       Trace Format now.
>  
> +  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
> +
>  *** Changes in GDB 7.6

This part is OK.

> +If specified, @var{location}, can be one of:
> +
> +@itemize @bullet
> +@item function
> +@c @item +offset
> +@c @item -offset
> +@c @item linenum
> +@item filename:linenum
> +@item filename:function
> +@item *address
> +@end itemize

Each of "function", "filename", "linenum", and "address" above
should be in @var{}, because they do not stand for themselves.

> +If @var{location} cannot be parsed (for example if it
                                                  ^
A comma is missing.

> +refers to unknown files or functions), create a pending
> +breakpoint. Without this flag, @value{GDBN} will report
             ^^
Two spaces between sentences.

> +@item -i @var{ignore-count}
> +Initialize the @var{ignore-count}.

"Set the ignore count of the breakpoint (@pxref{Conditions, ignore
count}) to @var{ignore-count}."

> +Note: this format is open to change.

Why do we need this note in the manual?

> +The corresponding @value{GDBN} commands are @samp{dprintf}.
                                  ^^^^^^^^^^^^
"command is"

Thanks.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

Hi Eli,

Thanks for your review.

On Sat, May 11, 2013 at 3:28 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Hui Zhu <teawater@gmail.com>
>> Date: Sat, 11 May 2013 10:37:56 +0800
>> Cc: Pedro Alves <palves@redhat.com>, Eli Zaretskii <eliz@gnu.org>, Hui Zhu <hui_zhu@mentor.com>,
>>       gdb-patches ml <gdb-patches@sourceware.org>, Marc Khouzam <marc.khouzam@ericsson.com>
>>
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -60,6 +60,8 @@ show debug nios2
>>    ** The -trace-save MI command can optionally save trace buffer in Common
>>       Trace Format now.
>>
>> +  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
>> +
>>  *** Changes in GDB 7.6
>
> This part is OK.
>
>> +If specified, @var{location}, can be one of:
>> +
>> +@itemize @bullet
>> +@item function
>> +@c @item +offset
>> +@c @item -offset
>> +@c @item linenum
>> +@item filename:linenum
>> +@item filename:function
>> +@item *address
>> +@end itemize
>
> Each of "function", "filename", "linenum", and "address" above
> should be in @var{}, because they do not stand for themselves.

Fixed.

>
>> +If @var{location} cannot be parsed (for example if it
>                                                   ^
> A comma is missing.

The comma is after the right parenthesis: refers to unknown files or functions),
Do I need change this part to following format?
If @var{location} cannot be parsed, (for example if it
refers to unknown files or functions) create a pending


>
>> +refers to unknown files or functions), create a pending
>> +breakpoint. Without this flag, @value{GDBN} will report
>              ^^
> Two spaces between sentences.

Fixed.

>
>> +@item -i @var{ignore-count}
>> +Initialize the @var{ignore-count}.
>
> "Set the ignore count of the breakpoint (@pxref{Conditions, ignore
> count}) to @var{ignore-count}."
>
>> +Note: this format is open to change.
>
> Why do we need this note in the manual?

Removed.

>
>> +The corresponding @value{GDBN} commands are @samp{dprintf}.
>                                   ^^^^^^^^^^^^
> "command is"

Fixed.

Post a new version according to your comments.

Thanks,
Hui

2013-05-13  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (ctype.h): New include.
	(gdb_obstack.h): New include.
	(mi_argv_to_format, mi_cmd_break_insert_1): New.
	(mi_cmd_break_insert): Call mi_cmd_break_insert_1.
	(mi_cmd_dprintf_insert): New.
	* mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
	* mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.

2013-05-13  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
	"-dprintf-insert" command.

2013-05-13  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -301,7 +301,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -30,6 +30,8 @@
 #include "observer.h"
 #include "mi-main.h"
 #include "mi-cmd-break.h"
+#include "gdb_obstack.h"
+#include <ctype.h>
 
 enum
   {
@@ -84,11 +86,83 @@ setup_breakpoint_reporting (void)
 }
 
 
-/* Implements the -break-insert command.
-   See the MI manual for the list of possible options.  */
+/* Convert arguments in ARGV to the string in "format",argv,argv...
+   and return it.  */
 
-void
-mi_cmd_break_insert (char *command, char **argv, int argc)
+static char *
+mi_argv_to_format (char **argv, int argc)
+{
+  int i;
+  struct obstack obstack;
+  char *ret;
+
+  obstack_init (&obstack);
+
+  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
+  obstack_1grow (&obstack, '\"');
+  for (i = 0; i < strlen (argv[0]); i++)
+    {
+      switch (argv[0][i])
+	{
+	case '\\':
+	  obstack_grow (&obstack, "\\\\", 2);
+	  break;
+	case '\a':
+	  obstack_grow (&obstack, "\\a", 2);
+	  break;
+	case '\b':
+	  obstack_grow (&obstack, "\\b", 2);
+	  break;
+	case '\f':
+	  obstack_grow (&obstack, "\\f", 2);
+	  break;
+	case '\n':
+	  obstack_grow (&obstack, "\\n", 2);
+	  break;
+	case '\r':
+	  obstack_grow (&obstack, "\\r", 2);
+	  break;
+	case '\t':
+	  obstack_grow (&obstack, "\\t", 2);
+	  break;
+	case '\v':
+	  obstack_grow (&obstack, "\\v", 2);
+	  break;
+	default:
+	  if (isprint (argv[0][i]))
+	    obstack_grow (&obstack, argv[0] + i, 1);
+	  else
+	    {
+	      char tmp[5];
+
+	      sprintf (tmp, "\\%o", (unsigned char) argv[0][i]);
+	      obstack_grow (&obstack, tmp, strlen (tmp));
+	    }
+	  break;
+	}
+    }
+  obstack_1grow (&obstack, '\"');
+
+  /* Apply other argv to FORMAT.  */
+  for (i = 1; i < argc; i++)
+    {
+      obstack_1grow (&obstack, ',');
+      obstack_grow (&obstack, argv[i], strlen (argv[i]));
+    }
+  obstack_1grow (&obstack, '\0');
+
+  ret = xstrdup (obstack_finish (&obstack));
+  obstack_free (&obstack, NULL);
+
+  return ret;
+}
+
+/* Insert breakpoint.
+   If dprintf is true, it will insert dprintf.
+   If not, it will insert other type breakpoint.  */
+
+static void
+mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
 {
   char *address = NULL;
   int hardware = 0;
@@ -99,9 +173,10 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
-  struct cleanup *back_to;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
@@ -163,35 +238,79 @@ mi_cmd_break_insert (char *command, char
     }
 
   if (oind >= argc)
-    error (_("-break-insert: Missing <location>"));
-  if (oind < argc - 1)
-    error (_("-break-insert: Garbage following <location>"));
+    error (_("-%s-insert: Missing <location>"),
+	   dprintf ? "dprintf" : "break");
   address = argv[oind];
+  if (dprintf)
+    {
+      int format_num = oind + 1;
+
+      if (hardware || tracepoint)
+	error (_("-dprintf-insert: does not support -h or -a"));
+      if (format_num >= argc)
+	error (_("-dprintf-insert: Missing <format>"));
+
+      extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
+      make_cleanup (xfree, extra_string);
+    }
+  else
+    {
+      if (oind < argc - 1)
+	error (_("-break-insert: Garbage following <location>"));
+    }
 
   /* Now we have what we need, let's insert the breakpoint!  */
-  back_to = setup_breakpoint_reporting ();
+  setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
   do_cleanups (back_to);
+}
+
+/* Implements the -break-insert command.
+   See the MI manual for the list of possible options.  */
+
+void
+mi_cmd_break_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (0, command, argv, argc);
+}
+
+/* Implements the -dprintf-insert command.
+   See the MI manual for the list of possible options.  */
 
+void
+mi_cmd_dprintf_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (1, command, argv, argc);
 }
 
 enum wp_type
--- a/mi/mi-cmds.c
+++ b/mi/mi-cmds.c
@@ -61,6 +61,8 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
   DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
 		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("dprintf-insert", mi_cmd_dprintf_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
   DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
 		   &mi_suppress_notification.breakpoint),
--- a/mi/mi-cmds.h
+++ b/mi/mi-cmds.h
@@ -39,6 +39,7 @@ typedef void (mi_cmd_argv_ftype) (char *
 extern mi_cmd_argv_ftype mi_cmd_ada_task_info;
 extern mi_cmd_argv_ftype mi_cmd_add_inferior;
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
+extern mi_cmd_argv_ftype mi_cmd_dprintf_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
 extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 2808 bytes --]

--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,8 @@ show debug nios2
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
+  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28971,6 +28971,84 @@ times="0"@}]@}
 @c (gdb)
 @end smallexample
 
+@subheading The @code{-dprintf-insert} Command
+@findex -dprintf-insert
+
+@subsubheading Synopsis
+
+@smallexample
+ -dprintf-insert [ -t ] [ -f ] [ -d ]
+    [ -c @var{condition} ] [ -i @var{ignore-count} ]
+    [ -p @var{thread-id} ] [ @var{location} ] [ @var{format} ]
+    [ @var{argument} ]
+@end smallexample
+
+@noindent
+If specified, @var{location}, can be one of:
+
+@itemize @bullet
+@item @var{function}
+@c @item +offset
+@c @item -offset
+@c @item @var{linenum}
+@item @var{filename}:@var{linenum}
+@item @var{filename}:function
+@item *@var{address}
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary breakpoint.
+@item -f
+If @var{location} cannot be parsed (for example if it
+refers to unknown files or functions), create a pending
+breakpoint.  Without this flag, @value{GDBN} will report
+an error, and won't create a breakpoint, if @var{location}
+cannot be parsed.
+@item -d
+Create a disabled breakpoint.
+@item -c @var{condition}
+Make the breakpoint conditional on @var{condition}.
+@item -i @var{ignore-count}
+Set the ignore count of the breakpoint (@pxref{Conditions, ignore count})
+to @var{ignore-count}.
+@item -p @var{thread-id}
+Restrict the breakpoint to the specified @var{thread-id}.
+@end table
+
+@subsubheading Result
+
+@xref{GDB/MI Breakpoint Information}, for details on the format of the
+resulting breakpoint.
+
+@c An out-of-band breakpoint instead of part of the result?
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{dprintf}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+4-dprintf-insert foo "At foo entry\n"
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-dprintf-insert 26 "arg=%d, g=%d\n" arg g
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @subheading The @code{-break-list} Command
 @findex -break-list
 

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7444 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,145 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-dprintf-insert" \
+    "1\\^error,msg=\"-dprintf-insert: Missing <location>\"" "mi insert without location"
+
+mi_gdb_test "2-dprintf-insert foo" \
+    "2\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-dprintf-insert 29" \
+    "3\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-dprintf-insert foo \"At foo entry\\n\"" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-dprintf-insert $dp_location1 \"arg=%d, g=%d\\n\" arg g" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	global mi_gdb_prompt
+
+	mi_run_cmd
+	set msg "mi 1st dprintf"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1234, g=1234.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"
+
+	set msg "mi 2nd dprintf"
+	mi_send_resuming_command "exec-continue" "$msg continue"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1235, g=2222.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Mon, 13 May 2013 11:38:40 +0800
> Cc: tromey@redhat.com, palves@redhat.com, hui_zhu@mentor.com, 
> 	gdb-patches@sourceware.org, marc.khouzam@ericsson.com
> 
> >> +If @var{location} cannot be parsed (for example if it
> >                                                   ^
> > A comma is missing.
> 
> The comma is after the right parenthesis: refers to unknown files or functions),
> Do I need change this part to following format?
> If @var{location} cannot be parsed, (for example if it
> refers to unknown files or functions) create a pending

No, I meant the comma after "for example", inside the parentheses.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

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

On Mon, May 13, 2013 at 11:55 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Hui Zhu <teawater@gmail.com>
>> Date: Mon, 13 May 2013 11:38:40 +0800
>> Cc: tromey@redhat.com, palves@redhat.com, hui_zhu@mentor.com,
>>       gdb-patches@sourceware.org, marc.khouzam@ericsson.com
>>
>> >> +If @var{location} cannot be parsed (for example if it
>> >                                                   ^
>> > A comma is missing.
>>
>> The comma is after the right parenthesis: refers to unknown files or functions),
>> Do I need change this part to following format?
>> If @var{location} cannot be parsed, (for example if it
>> refers to unknown files or functions) create a pending
>
> No, I meant the comma after "for example", inside the parentheses.

Fixed.

Post a new version for it.

Thanks,
Hui

2013-05-14  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
	* mi/mi-cmd-break.c (ctype.h): New include.
	(gdb_obstack.h): New include.
	(mi_argv_to_format, mi_cmd_break_insert_1): New.
	(mi_cmd_break_insert): Call mi_cmd_break_insert_1.
	(mi_cmd_dprintf_insert): New.
	* mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
	* mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.

2013-05-14  Hui Zhu  <hui@codesourcery.com>

	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
	"-dprintf-insert" command.

2013-05-14  Hui Zhu  <hui@codesourcery.com>

	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
	* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

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

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -301,7 +301,7 @@ struct breakpoint_ops bkpt_breakpoint_op
 static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 
 /* Dynamic printf class type.  */
-static struct breakpoint_ops dprintf_breakpoint_ops;
+struct breakpoint_ops dprintf_breakpoint_ops;
 
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
--- a/breakpoint.h
+++ b/breakpoint.h
@@ -1212,6 +1212,7 @@ extern void tbreak_command (char *, int)
 extern struct breakpoint_ops base_breakpoint_ops;
 extern struct breakpoint_ops bkpt_breakpoint_ops;
 extern struct breakpoint_ops tracepoint_breakpoint_ops;
+extern struct breakpoint_ops dprintf_breakpoint_ops;
 
 extern void initialize_breakpoint_ops (void);
 
--- a/mi/mi-cmd-break.c
+++ b/mi/mi-cmd-break.c
@@ -30,6 +30,8 @@
 #include "observer.h"
 #include "mi-main.h"
 #include "mi-cmd-break.h"
+#include "gdb_obstack.h"
+#include <ctype.h>
 
 enum
   {
@@ -84,11 +86,83 @@ setup_breakpoint_reporting (void)
 }
 
 
-/* Implements the -break-insert command.
-   See the MI manual for the list of possible options.  */
+/* Convert arguments in ARGV to the string in "format",argv,argv...
+   and return it.  */
 
-void
-mi_cmd_break_insert (char *command, char **argv, int argc)
+static char *
+mi_argv_to_format (char **argv, int argc)
+{
+  int i;
+  struct obstack obstack;
+  char *ret;
+
+  obstack_init (&obstack);
+
+  /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
+  obstack_1grow (&obstack, '\"');
+  for (i = 0; i < strlen (argv[0]); i++)
+    {
+      switch (argv[0][i])
+	{
+	case '\\':
+	  obstack_grow (&obstack, "\\\\", 2);
+	  break;
+	case '\a':
+	  obstack_grow (&obstack, "\\a", 2);
+	  break;
+	case '\b':
+	  obstack_grow (&obstack, "\\b", 2);
+	  break;
+	case '\f':
+	  obstack_grow (&obstack, "\\f", 2);
+	  break;
+	case '\n':
+	  obstack_grow (&obstack, "\\n", 2);
+	  break;
+	case '\r':
+	  obstack_grow (&obstack, "\\r", 2);
+	  break;
+	case '\t':
+	  obstack_grow (&obstack, "\\t", 2);
+	  break;
+	case '\v':
+	  obstack_grow (&obstack, "\\v", 2);
+	  break;
+	default:
+	  if (isprint (argv[0][i]))
+	    obstack_grow (&obstack, argv[0] + i, 1);
+	  else
+	    {
+	      char tmp[5];
+
+	      sprintf (tmp, "\\%o", (unsigned char) argv[0][i]);
+	      obstack_grow (&obstack, tmp, strlen (tmp));
+	    }
+	  break;
+	}
+    }
+  obstack_1grow (&obstack, '\"');
+
+  /* Apply other argv to FORMAT.  */
+  for (i = 1; i < argc; i++)
+    {
+      obstack_1grow (&obstack, ',');
+      obstack_grow (&obstack, argv[i], strlen (argv[i]));
+    }
+  obstack_1grow (&obstack, '\0');
+
+  ret = xstrdup (obstack_finish (&obstack));
+  obstack_free (&obstack, NULL);
+
+  return ret;
+}
+
+/* Insert breakpoint.
+   If dprintf is true, it will insert dprintf.
+   If not, it will insert other type breakpoint.  */
+
+static void
+mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc)
 {
   char *address = NULL;
   int hardware = 0;
@@ -99,9 +173,10 @@ mi_cmd_break_insert (char *command, char
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
-  struct cleanup *back_to;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   enum bptype type_wanted;
   struct breakpoint_ops *ops;
+  char *extra_string = NULL;
 
   enum opt
     {
@@ -163,35 +238,79 @@ mi_cmd_break_insert (char *command, char
     }
 
   if (oind >= argc)
-    error (_("-break-insert: Missing <location>"));
-  if (oind < argc - 1)
-    error (_("-break-insert: Garbage following <location>"));
+    error (_("-%s-insert: Missing <location>"),
+	   dprintf ? "dprintf" : "break");
   address = argv[oind];
+  if (dprintf)
+    {
+      int format_num = oind + 1;
+
+      if (hardware || tracepoint)
+	error (_("-dprintf-insert: does not support -h or -a"));
+      if (format_num >= argc)
+	error (_("-dprintf-insert: Missing <format>"));
+
+      extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
+      make_cleanup (xfree, extra_string);
+    }
+  else
+    {
+      if (oind < argc - 1)
+	error (_("-break-insert: Garbage following <location>"));
+    }
 
   /* Now we have what we need, let's insert the breakpoint!  */
-  back_to = setup_breakpoint_reporting ();
+  setup_breakpoint_reporting ();
 
-  /* Note that to request a fast tracepoint, the client uses the
-     "hardware" flag, although there's nothing of hardware related to
-     fast tracepoints -- one can implement slow tracepoints with
-     hardware breakpoints, but fast tracepoints are always software.
-     "fast" is a misnomer, actually, "jump" would be more appropriate.
-     A simulator or an emulator could conceivably implement fast
-     regular non-jump based tracepoints.  */
-  type_wanted = (tracepoint
-		 ? (hardware ? bp_fast_tracepoint : bp_tracepoint)
-		 : (hardware ? bp_hardware_breakpoint : bp_breakpoint));
-  ops = tracepoint ? &tracepoint_breakpoint_ops : &bkpt_breakpoint_ops;
+  if (tracepoint)
+    {
+      /* Note that to request a fast tracepoint, the client uses the
+	 "hardware" flag, although there's nothing of hardware related to
+	 fast tracepoints -- one can implement slow tracepoints with
+	 hardware breakpoints, but fast tracepoints are always software.
+	 "fast" is a misnomer, actually, "jump" would be more appropriate.
+	 A simulator or an emulator could conceivably implement fast
+	 regular non-jump based tracepoints.  */
+      type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint;
+      ops = &tracepoint_breakpoint_ops;
+    }
+  else if (dprintf)
+    {
+      type_wanted = bp_dprintf;
+      ops = &dprintf_breakpoint_ops;
+    }
+  else
+    {
+      type_wanted = hardware ? bp_hardware_breakpoint : bp_breakpoint;
+      ops = &bkpt_breakpoint_ops;
+    }
 
   create_breakpoint (get_current_arch (), address, condition, thread,
-		     NULL,
+		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
   do_cleanups (back_to);
+}
+
+/* Implements the -break-insert command.
+   See the MI manual for the list of possible options.  */
+
+void
+mi_cmd_break_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (0, command, argv, argc);
+}
+
+/* Implements the -dprintf-insert command.
+   See the MI manual for the list of possible options.  */
 
+void
+mi_cmd_dprintf_insert (char *command, char **argv, int argc)
+{
+  mi_cmd_break_insert_1 (1, command, argv, argc);
 }
 
 enum wp_type
--- a/mi/mi-cmds.c
+++ b/mi/mi-cmds.c
@@ -61,6 +61,8 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_CLI ("break-info", "info break", 1),
   DEF_MI_CMD_MI_1 ("break-insert", mi_cmd_break_insert,
 		   &mi_suppress_notification.breakpoint),
+  DEF_MI_CMD_MI_1 ("dprintf-insert", mi_cmd_dprintf_insert,
+		   &mi_suppress_notification.breakpoint),
   DEF_MI_CMD_CLI ("break-list", "info break", 0),
   DEF_MI_CMD_MI_1 ("break-passcount", mi_cmd_break_passcount,
 		   &mi_suppress_notification.breakpoint),
--- a/mi/mi-cmds.h
+++ b/mi/mi-cmds.h
@@ -39,6 +39,7 @@ typedef void (mi_cmd_argv_ftype) (char *
 extern mi_cmd_argv_ftype mi_cmd_ada_task_info;
 extern mi_cmd_argv_ftype mi_cmd_add_inferior;
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
+extern mi_cmd_argv_ftype mi_cmd_dprintf_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
 extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;

[-- Attachment #3: mi-dprintf-doc.txt --]
[-- Type: text/plain, Size: 2809 bytes --]

--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,8 @@ show debug nios2
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
+  ** The new command -dprintf-insert sets a dynamic printf breakpoint.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -28971,6 +28971,84 @@ times="0"@}]@}
 @c (gdb)
 @end smallexample
 
+@subheading The @code{-dprintf-insert} Command
+@findex -dprintf-insert
+
+@subsubheading Synopsis
+
+@smallexample
+ -dprintf-insert [ -t ] [ -f ] [ -d ]
+    [ -c @var{condition} ] [ -i @var{ignore-count} ]
+    [ -p @var{thread-id} ] [ @var{location} ] [ @var{format} ]
+    [ @var{argument} ]
+@end smallexample
+
+@noindent
+If specified, @var{location}, can be one of:
+
+@itemize @bullet
+@item @var{function}
+@c @item +offset
+@c @item -offset
+@c @item @var{linenum}
+@item @var{filename}:@var{linenum}
+@item @var{filename}:function
+@item *@var{address}
+@end itemize
+
+The possible optional parameters of this command are:
+
+@table @samp
+@item -t
+Insert a temporary breakpoint.
+@item -f
+If @var{location} cannot be parsed (for example, if it
+refers to unknown files or functions), create a pending
+breakpoint.  Without this flag, @value{GDBN} will report
+an error, and won't create a breakpoint, if @var{location}
+cannot be parsed.
+@item -d
+Create a disabled breakpoint.
+@item -c @var{condition}
+Make the breakpoint conditional on @var{condition}.
+@item -i @var{ignore-count}
+Set the ignore count of the breakpoint (@pxref{Conditions, ignore count})
+to @var{ignore-count}.
+@item -p @var{thread-id}
+Restrict the breakpoint to the specified @var{thread-id}.
+@end table
+
+@subsubheading Result
+
+@xref{GDB/MI Breakpoint Information}, for details on the format of the
+resulting breakpoint.
+
+@c An out-of-band breakpoint instead of part of the result?
+
+@subsubheading @value{GDBN} Command
+
+The corresponding @value{GDBN} command is @samp{dprintf}.
+
+@subsubheading Example
+
+@smallexample
+(gdb)
+4-dprintf-insert foo "At foo entry\n"
+4^done,bkpt=@{number="1",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040061b",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="25",thread-groups=["i1"],
+times="0",script=@{"printf \"At foo entry\\n\"","continue"@},
+original-location="foo"@}
+(gdb)
+5-dprintf-insert 26 "arg=%d, g=%d\n" arg g
+5^done,bkpt=@{number="2",type="dprintf",disp="keep",enabled="y",
+addr="0x000000000040062a",func="foo",file="mi-dprintf.c",
+fullname="mi-dprintf.c",line="26",thread-groups=["i1"],
+times="0",script=@{"printf \"arg=%d, g=%d\\n\", arg, g","continue"@},
+original-location="mi-dprintf.c:26"@}
+(gdb)
+@end smallexample
+
 @subheading The @code{-break-list} Command
 @findex -break-list
 

[-- Attachment #4: mi-dprintf-test.txt --]
[-- Type: text/plain, Size: 7444 bytes --]

--- a/testsuite/gdb.mi/Makefile.in
+++ b/testsuite/gdb.mi/Makefile.in
@@ -3,7 +3,7 @@ srcdir = @srcdir@
 
 PROGS = basics c_variable cpp_variable var-cmd dw2-ref-missing-frame	\
 	gdb669-pthreads gdb701 gdb792 mi-async mi-basics mi-break	\
-	mi-cli mi-console mi-disassemble mi-eval mi-file		\
+	mi-cli mi-console mi-disassemble mi-dprintf mi-eval mi-file	\
 	mi-file-transfer mi-non-stop mi-non-stop-exit			\
 	mi-ns-stale-regcache mi-nsintrall mi-nsmoribund mi-nsthrexec	\
 	mi-pending mi-pthreads mi-read-memory mi-regs mi-return		\
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+   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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static int g;
+
+void
+foo (int arg)
+{
+  g += arg;
+  g *= 2; /* set dprintf 1 here */
+  g /= 2.5; /* set breakpoint 1 here */
+}
+
+int
+main (int argc, char *argv[])
+{
+  int loc = 1234;
+
+  /* Ensure these functions are available.  */
+  printf ("kickoff %d\n", loc);
+  fprintf (stderr, "also to stderr %d\n", loc);
+
+  foo (loc++);
+  foo (loc++);
+  foo (loc++);
+  return g;
+}
+
+/* Make sure function 'malloc' is linked into program.  On some bare-metal
+   port, if we don't use 'malloc', it will not be linked in program.  'malloc'
+   is needed, otherwise we'll see such error message
+   evaluation of this expression requires the program to have a function
+   "malloc".  */
+
+void
+bar (void)
+{
+  void *p = malloc (16);
+
+  free (p);
+}
--- /dev/null
+++ b/testsuite/gdb.mi/mi-dprintf.exp
@@ -0,0 +1,145 @@
+#   Copyright (C) 2013 Free Software Foundation, Inc.
+#   Contributed by Hui Zhu  <hui@codesourcery.com>
+
+# 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/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile
+
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    untested "failed to compile $testfile"
+    return -1
+}
+
+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"]
+
+mi_run_to_main
+
+mi_gdb_test "1-dprintf-insert" \
+    "1\\^error,msg=\"-dprintf-insert: Missing <location>\"" "mi insert without location"
+
+mi_gdb_test "2-dprintf-insert foo" \
+    "2\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert breakpoint without format string"
+
+mi_gdb_test "3-dprintf-insert 29" \
+    "3\\^error,msg=\"-dprintf-insert: Missing <format>\"" "mi insert second breakpoint without format string"
+
+mi_gdb_test "-break-insert main" ".*" "mi insert breakpoint main"
+mi_delete_breakpoints
+
+mi_gdb_test "4-dprintf-insert foo \"At foo entry\\n\"" \
+    "4\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*" "mi insert dprintf foo"
+
+mi_gdb_test "5-dprintf-insert $dp_location1 \"arg=%d, g=%d\\n\" arg g" \
+    "5\\^done,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi insert dprintf dp_location1"
+
+mi_gdb_test "6-break-info" \
+    "6\\^done,BreakpointTable=\{nr_rows=\".\",nr_cols=\".\",hdr=\\\[\{width=\".*\",alignment=\".*\",col_name=\"number\",colhdr=\"Num\"\},\{width=\".*\",alignment=\".*\",col_name=\"type\",colhdr=\"Type\"\},\{width=\".*\",alignment=\".*\",col_name=\"disp\",colhdr=\"Disp\"\},\{width=\".*\",alignment=\".*\",col_name=\"enabled\",colhdr=\"Enb\"\},\{width=\".*\",alignment=\".*\",col_name=\"addr\",colhdr=\"Address\"\},\{width=\".*\",alignment=\".*\",col_name=\"what\",colhdr=\"What\"\}\\\],body=\\\[bkpt=\{number=\"3\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\".*\".*,bkpt=\{number=\".*\",type=\"dprintf\".*func=\"foo\",file=\".*mi-dprintf.c\",fullname=\".*mi-dprintf.c\",line=\"$dp_location1\".*" \
+    "mi info dprintf"
+
+mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1"
+
+proc mi_continue_dprintf {args} {
+    with_test_prefix $args {
+	global mi_gdb_prompt
+
+	mi_run_cmd
+	set msg "mi 1st dprintf"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1234, g=1234.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop"
+
+	set msg "mi 2nd dprintf"
+	mi_send_resuming_command "exec-continue" "$msg continue"
+	gdb_expect {
+            -re ".*At foo entry.*arg=1235, g=2222.*" {
+		pass $msg
+	    }
+	    -re ".*$mi_gdb_prompt$" {
+		fail $msg
+	    }
+            timeout {
+		fail $msg
+            }
+	}
+    }
+}
+
+mi_continue_dprintf "gdb"
+
+# The "call" style depends on having I/O functions available, so test.
+
+if ![target_info exists gdb,noinferiorio] {
+
+    # Now switch styles and rerun; in the absence of redirection the
+    # output should be the same.
+
+    mi_gdb_test "set dprintf-style call" ".*" "mi set dprintf style to call"
+    mi_continue_dprintf "call"
+
+    mi_gdb_test "set dprintf-function fprintf" ".*" "mi set dprintf-channel stderr"
+    mi_gdb_test "set dprintf-channel stderr" ".*" "mi set dprintf channel"
+    mi_continue_dprintf "fprintf"
+}
+
+set target_can_dprintf 0
+set msg "set dprintf style to agent"
+send_gdb "set dprintf-style agent\n"
+gdb_expect {
+    -re "warning: Target cannot run dprintf commands, falling back to GDB printf.*$mi_gdb_prompt$" {
+	unsupported "$msg"
+    }
+    -re ".*done.*$mi_gdb_prompt$" {
+	set target_can_dprintf 1
+	pass "$msg"
+    }
+    -re ".*$mi_gdb_prompt$" {
+	fail "$msg"
+    }
+    timeout {
+	fail "$msg"
+    }
+}
+
+if $target_can_dprintf {
+    mi_run_cmd
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+
+    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+
+    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+}
+
+mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

Ping http://sourceware.org/ml/gdb-patches/2013-05/msg00460.html

Thanks,
Hui

On Tue, May 14, 2013 at 12:56 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Mon, May 13, 2013 at 11:55 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Hui Zhu <teawater@gmail.com>
>>> Date: Mon, 13 May 2013 11:38:40 +0800
>>> Cc: tromey@redhat.com, palves@redhat.com, hui_zhu@mentor.com,
>>>       gdb-patches@sourceware.org, marc.khouzam@ericsson.com
>>>
>>> >> +If @var{location} cannot be parsed (for example if it
>>> >                                                   ^
>>> > A comma is missing.
>>>
>>> The comma is after the right parenthesis: refers to unknown files or functions),
>>> Do I need change this part to following format?
>>> If @var{location} cannot be parsed, (for example if it
>>> refers to unknown files or functions) create a pending
>>
>> No, I meant the comma after "for example", inside the parentheses.
>
> Fixed.
>
> Post a new version for it.
>
> Thanks,
> Hui
>
> 2013-05-14  Hui Zhu  <hui@codesourcery.com>
>
>         * breakpoint.c (dprintf_breakpoint_ops): Remove its static.
>         * breakpoint.h (dprintf_breakpoint_ops): Add extern.
>         * mi/mi-cmd-break.c (ctype.h): New include.
>         (gdb_obstack.h): New include.
>         (mi_argv_to_format, mi_cmd_break_insert_1): New.
>         (mi_cmd_break_insert): Call mi_cmd_break_insert_1.
>         (mi_cmd_dprintf_insert): New.
>         * mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
>         * mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.
>
> 2013-05-14  Hui Zhu  <hui@codesourcery.com>
>
>         * gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
>         "-dprintf-insert" command.
>
> 2013-05-14  Hui Zhu  <hui@codesourcery.com>
>
>         * gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
>         * gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Eli Zaretskii]

> From: Hui Zhu <teawater@gmail.com>
> Date: Mon, 20 May 2013 15:30:44 +0800
> Cc: tromey@redhat.com, palves@redhat.com, hui_zhu@mentor.com, 
> 	gdb-patches@sourceware.org, marc.khouzam@ericsson.com
> 
> Ping http://sourceware.org/ml/gdb-patches/2013-05/msg00460.html

OK.

Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu]

On Mon, May 20, 2013 at 11:44 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Hui Zhu <teawater@gmail.com>
>> Date: Mon, 20 May 2013 15:30:44 +0800
>> Cc: tromey@redhat.com, palves@redhat.com, hui_zhu@mentor.com,
>>       gdb-patches@sourceware.org, marc.khouzam@ericsson.com
>>
>> Ping http://sourceware.org/ml/gdb-patches/2013-05/msg00460.html
>
> OK.

Checked in http://sourceware.org/ml/gdb-cvs/2013-05/msg00193.html

Thanks,
Hui

[patch] Fix racy FAILs due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Jan Kratochvil]

On Tue, 21 May 2013 06:24:16 +0200, Hui Zhu wrote:
> Checked in http://sourceware.org/ml/gdb-cvs/2013-05/msg00193.html

It has random FAILs during normal run:

+PASS: gdb.mi/mi-dprintf.exp: fprintf: mi 1st dprintf
+FAIL: gdb.mi/mi-dprintf.exp: fprintf: mi 1st dprintf stop (timeout)
+PASS: gdb.mi/mi-dprintf.exp: fprintf: mi 2nd dprintf

and when I tested it with:
	reproducer for races of expect incomplete reads
	http://sourceware.org/bugzilla/show_bug.cgi?id=12649

it FAILs reproducibly, could you review the proposed fix?


Thanks,
Jan


gdb/testsuite/
2013-05-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR testsuite/12649
	* gdb.mi/mi-dprintf.exp (mi_continue_dprintf): Fix expect strings for
	racy matches.

diff --git a/gdb/testsuite/gdb.mi/mi-dprintf.exp b/gdb/testsuite/gdb.mi/mi-dprintf.exp
index 457f332..ea8b3a8 100644
--- a/gdb/testsuite/gdb.mi/mi-dprintf.exp
+++ b/gdb/testsuite/gdb.mi/mi-dprintf.exp
@@ -68,7 +68,7 @@ proc mi_continue_dprintf {args} {
 	mi_run_cmd
 	set msg "mi 1st dprintf"
 	gdb_expect {
-            -re ".*At foo entry.*arg=1234, g=1234.*" {
+            -re ".*At foo entry.*arg=1234, g=1234" {
 		pass $msg
 	    }
 	    -re ".*$mi_gdb_prompt$" {
@@ -83,7 +83,7 @@ proc mi_continue_dprintf {args} {
 	set msg "mi 2nd dprintf"
 	mi_send_resuming_command "exec-continue" "$msg continue"
 	gdb_expect {
-            -re ".*At foo entry.*arg=1235, g=2222.*" {
+            -re ".*At foo entry.*arg=1235, g=2222.*$mi_gdb_prompt$" {
 		pass $msg
 	    }
 	    -re ".*$mi_gdb_prompt$" {

Re: [patch] Fix racy FAILs due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Hui Zhu]

On Tue, May 21, 2013 at 4:10 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 21 May 2013 06:24:16 +0200, Hui Zhu wrote:
>> Checked in http://sourceware.org/ml/gdb-cvs/2013-05/msg00193.html
>
> It has random FAILs during normal run:
>
> +PASS: gdb.mi/mi-dprintf.exp: fprintf: mi 1st dprintf
> +FAIL: gdb.mi/mi-dprintf.exp: fprintf: mi 1st dprintf stop (timeout)
> +PASS: gdb.mi/mi-dprintf.exp: fprintf: mi 2nd dprintf
>
> and when I tested it with:
>         reproducer for races of expect incomplete reads
>         http://sourceware.org/bugzilla/show_bug.cgi?id=12649
>
> it FAILs reproducibly, could you review the proposed fix?

This patch fixed the issue in my part.

Thanks,
Hui

>
>
> Thanks,
> Jan
>
>
> gdb/testsuite/
> 2013-05-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         PR testsuite/12649
>         * gdb.mi/mi-dprintf.exp (mi_continue_dprintf): Fix expect strings for
>         racy matches.
>
> diff --git a/gdb/testsuite/gdb.mi/mi-dprintf.exp b/gdb/testsuite/gdb.mi/mi-dprintf.exp
> index 457f332..ea8b3a8 100644
> --- a/gdb/testsuite/gdb.mi/mi-dprintf.exp
> +++ b/gdb/testsuite/gdb.mi/mi-dprintf.exp
> @@ -68,7 +68,7 @@ proc mi_continue_dprintf {args} {
>         mi_run_cmd
>         set msg "mi 1st dprintf"
>         gdb_expect {
> -            -re ".*At foo entry.*arg=1234, g=1234.*" {
> +            -re ".*At foo entry.*arg=1234, g=1234" {
>                 pass $msg
>             }
>             -re ".*$mi_gdb_prompt$" {
> @@ -83,7 +83,7 @@ proc mi_continue_dprintf {args} {
>         set msg "mi 2nd dprintf"
>         mi_send_resuming_command "exec-continue" "$msg continue"
>         gdb_expect {
> -            -re ".*At foo entry.*arg=1235, g=2222.*" {
> +            -re ".*At foo entry.*arg=1235, g=2222.*$mi_gdb_prompt$" {
>                 pass $msg
>             }
>             -re ".*$mi_gdb_prompt$" {

[commit] [patch] Fix racy FAILs due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Jan Kratochvil]

On Tue, 21 May 2013 11:29:14 +0200, Hui Zhu wrote:
> This patch fixed the issue in my part.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-05/msg00199.html


Jan

Re: [commit] [patch] Fix racy FAILs due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Hui Zhu]

On Tue, May 21, 2013 at 11:01 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 21 May 2013 11:29:14 +0200, Hui Zhu wrote:
>> This patch fixed the issue in my part.
>
> Checked in:
>         http://sourceware.org/ml/gdb-cvs/2013-05/msg00199.html
>
>
> Jan

Thanks for this work.  :)

Best,
Hui

[patch] Fix racy FAILs #2 due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Jan Kratochvil]

On Tue, 21 May 2013 06:24:16 +0200, Hui Zhu wrote:
> Checked in http://sourceware.org/ml/gdb-cvs/2013-05/msg00193.html

I got another racy FAIL today:

FAIL: gdb.mi/mi-dprintf.exp: mi info dprintf second time

I found I forgot to run the "read1" reproducer with gdbserver yesterday, which
revealed more races.

I will check it in today.


Regards,
Jan


gdb/testsuite/
2013-05-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR testsuite/12649
	* gdb.mi/mi-dprintf.exp (mi_continue_dprintf) (mi 2nd dprintf): Replace
	$mi_gdb_prompt expectation by mi_expect_stop.
	(mi 1st dprintf, agent, mi 2nd dprintf, agent)
	(mi info dprintf second time): Replace them by mi_send_resuming_command
	and mi_expect_stop.

diff --git a/gdb/testsuite/gdb.mi/mi-dprintf.exp b/gdb/testsuite/gdb.mi/mi-dprintf.exp
index ea8b3a8..3509963 100644
--- a/gdb/testsuite/gdb.mi/mi-dprintf.exp
+++ b/gdb/testsuite/gdb.mi/mi-dprintf.exp
@@ -83,7 +83,7 @@ proc mi_continue_dprintf {args} {
 	set msg "mi 2nd dprintf"
 	mi_send_resuming_command "exec-continue" "$msg continue"
 	gdb_expect {
-            -re ".*At foo entry.*arg=1235, g=2222.*$mi_gdb_prompt$" {
+            -re ".*At foo entry.*arg=1235, g=2222" {
 		pass $msg
 	    }
 	    -re ".*$mi_gdb_prompt$" {
@@ -93,6 +93,7 @@ proc mi_continue_dprintf {args} {
 		fail $msg
             }
 	}
+	mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg 2nd stop"
     }
 }
 
@@ -135,11 +136,28 @@ gdb_expect {
 if $target_can_dprintf {
     mi_run_cmd
 
-    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 1st dprintf, agent"
+    mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "mi expect stop"
 
-    mi_gdb_test "continue" ".*breakpoint-hit.*func=\"foo\".*" "mi 2nd dprintf, agent"
+    mi_send_resuming_command "exec-continue" "mi 1st dprintf continue, agent"
+    mi_expect_stop ".*" "foo" ".*" ".*" ".*" "" "mi 1st dprintf, agent"
 
-    mi_gdb_test "6-break-info" ".*modified.*" "mi info dprintf second time"
+    mi_send_resuming_command "exec-continue" "mi 2nd dprintf continue, agent"
+
+    # The =breakpoint-modified text is a part of the "-exec-continue" output.
+    set msg "mi info dprintf second time"
+    gdb_expect {
+	-re "=breakpoint-modified," {
+	    pass $msg
+	}
+	-re ".*$mi_gdb_prompt$" {
+	    fail "$msg"
+	}
+	timeout {
+	    fail "$msg"
+	}
+    }
+
+    mi_expect_stop ".*" "foo" ".*" ".*" ".*" "" "mi 2nd dprintf, agent"
 }
 
 mi_gdb_test "set dprintf-style foobar" ".*error.*" "mi set dprintf style to an unrecognized type"

[commit] [patch] Fix racy FAILs #2 due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Jan Kratochvil]

On Thu, 23 May 2013 16:03:32 +0200, Jan Kratochvil wrote:
> gdb/testsuite/
> 2013-05-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	PR testsuite/12649
> 	* gdb.mi/mi-dprintf.exp (mi_continue_dprintf) (mi 2nd dprintf): Replace
> 	$mi_gdb_prompt expectation by mi_expect_stop.
> 	(mi 1st dprintf, agent, mi 2nd dprintf, agent)
> 	(mi info dprintf second time): Replace them by mi_send_resuming_command
> 	and mi_expect_stop.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-05/msg00237.html


Jan

Re: [commit] [patch] Fix racy FAILs #2 due to "read1" [Re: [PATCH] add -s option to make -break-insert support dprintf]

[Hui Zhu]

On Fri, May 24, 2013 at 11:37 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Thu, 23 May 2013 16:03:32 +0200, Jan Kratochvil wrote:
>> gdb/testsuite/
>> 2013-05-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
>>
>>       PR testsuite/12649
>>       * gdb.mi/mi-dprintf.exp (mi_continue_dprintf) (mi 2nd dprintf): Replace
>>       $mi_gdb_prompt expectation by mi_expect_stop.
>>       (mi 1st dprintf, agent, mi 2nd dprintf, agent)
>>       (mi info dprintf second time): Replace them by mi_send_resuming_command
>>       and mi_expect_stop.
>
> Checked in:
>         http://sourceware.org/ml/gdb-cvs/2013-05/msg00237.html
>
>
> Jan

Thanks.

Best,
Hui

Re: [PATCH] add -s option to make -break-insert support dprintf

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:

Hui> 2013-05-11  Hui Zhu  <hui@codesourcery.com>
Hui> 	* breakpoint.c (dprintf_breakpoint_ops): Remove its static.
Hui> 	* breakpoint.h (dprintf_breakpoint_ops): Add extern.
Hui> 	* mi/mi-cmd-break.c (ctype.h): New include.
Hui> 	(gdb_obstack.h): New include.
Hui> 	(mi_argv_to_format, mi_cmd_break_insert_1): New.
Hui> 	(mi_cmd_break_insert): Call mi_cmd_break_insert_1.
Hui> 	(mi_cmd_dprintf_insert): New.
Hui> 	* mi/mi-cmds.c (mi_cmds): Add "dprintf-insert".
Hui> 	* mi/mi-cmds.h (mi_cmd_dprintf_insert): New extern.

Hui> 2013-05-11  Hui Zhu  <hui@codesourcery.com>

Hui> 	* gdb.texinfo (GDB/MI Breakpoint Commands): Describe the
Hui> 	"-dprintf-insert" command.

Hui> 2013-05-11  Hui Zhu  <hui@codesourcery.com>

Hui> 	* gdb.mi/Makefile.in (PROGS): Add "mi-dprintf".
Hui> 	* gdb.mi/mi-dprintf.exp, gdb.mi/mi-dprintf.c: New.

The code bits are ok.
Thanks.

Tom