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

Hui Zhu
2013-03-29T08:02:21Z

[-- 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"