* [PATCH] Fix MI dprintf-insert not printing when a location is pending. @ 2015-03-26 16:47 Antoine Tremblay 2015-03-26 19:11 ` Keith Seitz 0 siblings, 1 reply; 11+ messages in thread From: Antoine Tremblay @ 2015-03-26 16:47 UTC (permalink / raw) To: gdb-patches; +Cc: Antoine Tremblay This patch fixes the "Format string required" error when trying to print a dprintf on a pending location when set via the MI interface even if the format string is entered correctly. This patch also adds a test case to check that issue called mi-dprintf-pending.exp. gdb/ChangeLog: PR breakpoints/16465 * breakpoint.c (create_breakpoint): Fix missing extra_string. gdb/testsuite/ChangeLog: PR breakpoints/16465 * gdb.mi/mi-dprintf-pending.c: New file. * gdb.mi/mi-dprintf-pending.exp: New test. * gdb.mi/mi-dprintf-pendshr.c: New file. --- gdb/breakpoint.c | 13 ++++- gdb/testsuite/gdb.mi/mi-dprintf-pending.c | 24 +++++++++ gdb/testsuite/gdb.mi/mi-dprintf-pending.exp | 77 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c | 27 ++++++++++ 4 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pending.c create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pending.exp create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0724a72..60f8213 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -9774,7 +9774,10 @@ create_breakpoint (struct gdbarch *gdbarch, b->addr_string = copy_arg; if (parse_arg) - b->cond_string = NULL; + { + b->cond_string = NULL; + b->extra_string = NULL; + } else { /* Create a private copy of condition string. */ @@ -9783,10 +9786,16 @@ create_breakpoint (struct gdbarch *gdbarch, cond_string = xstrdup (cond_string); make_cleanup (xfree, cond_string); } + /* Create a private copy of any extra string. */ + if (extra_string) + { + extra_string = xstrdup (extra_string); + make_cleanup (xfree, extra_string); + } b->cond_string = cond_string; + b->extra_string = extra_string; b->thread = thread; } - b->extra_string = NULL; b->ignore_count = ignore_count; b->disposition = tempflag ? disp_del : disp_donttouch; b->condition_not_parsed = 1; diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.c b/gdb/testsuite/gdb.mi/mi-dprintf-pending.c new file mode 100644 index 0000000..d6d8aac --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +extern void pendfunc (); + +int main() +{ + pendfunc (); + return 0; /* set breakpoint 1 here */ +} diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp new file mode 100644 index 0000000..6832f1d --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp @@ -0,0 +1,77 @@ +# Copyright 2015 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/>. + +# This test checks if dprintf prints correctly when it's pending. +# See PR breakpoints/16465. + +load_lib mi-support.exp + +if {[skip_shlib_tests]} { + return 0 +} + +standard_testfile mi-dprintf-pending.c + +set libfile1 "mi-dprintf-pendshr" +set libsrc1 $srcdir/$subdir/$libfile1.c +set lib_sl1 [standard_output_file $libfile1.sl] +set lib_opts debug +set exec_opts [list debug shlib=$lib_sl1 shlib_load] + +if [get_compiler_info] { + return -1 +} + +if { [gdb_compile_shlib $libsrc1 $lib_sl1 $lib_opts] != "" } { + untested "Could not compile $libsrc1" + return -1 +} + +if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable $exec_opts] != ""} { + untested "Could not compile $srcdir/$subdir/$srcfile" + return -1 +} + +# Start with a fresh gdb. +gdb_exit +mi_gdb_start +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load ${binfile} +mi_load_shlibs $lib_sl1 + +set bp_location1 [gdb_get_line_number "set breakpoint 1 here"] + +# Set pending dprintf via MI. +mi_gdb_test "-dprintf-insert -f pendfunc1 \"hello\"" \ + ".*\\^done,bkpt={number=\"1\",type=\"dprintf\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"}" \ + "mi set dprintf" + +mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1" + +mi_run_cmd + +set msg "mi dprintf" +gdb_expect { + -re ".*~\"hello\"" { + pass $msg + } + -re ".*$mi_gdb_prompt$" { + fail $msg + } + timeout { + fail $msg + } +} +mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop" diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c new file mode 100644 index 0000000..fe49a8d --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +void +pendfunc1 () +{ +} + +void +pendfunc () +{ + pendfunc1(); +} -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-26 16:47 [PATCH] Fix MI dprintf-insert not printing when a location is pending Antoine Tremblay @ 2015-03-26 19:11 ` Keith Seitz 2015-03-27 12:34 ` Antoine Tremblay 0 siblings, 1 reply; 11+ messages in thread From: Keith Seitz @ 2015-03-26 19:11 UTC (permalink / raw) To: Antoine Tremblay, gdb-patches On 03/26/2015 09:47 AM, Antoine Tremblay wrote: > gdb/ChangeLog: > > PR breakpoints/16465 > * breakpoint.c (create_breakpoint): Fix missing extra_string. This is a real nit, so please don't go making any changes here unless a maintainer requests it, but this changelog entry doesn't really explain the change you've made. ["Save `extra_string' for pending breakpoints." is much more descriptive/helpful.] > @@ -9783,10 +9786,16 @@ create_breakpoint (struct gdbarch *gdbarch, > cond_string = xstrdup (cond_string); > make_cleanup (xfree, cond_string); > } > + /* Create a private copy of any extra string. */ > + if (extra_string) We explicitly test against NULL for pointers. [I know there are quite a few violations of this in this function. All are awaiting an easy/obvious separate cleanup. :-)] > + { > + extra_string = xstrdup (extra_string); > + make_cleanup (xfree, extra_string); > + } > b->cond_string = cond_string; > + b->extra_string = extra_string; > b->thread = thread; > } > - b->extra_string = NULL; > b->ignore_count = ignore_count; > b->disposition = tempflag ? disp_del : disp_donttouch; > b->condition_not_parsed = 1; FWIW, I have pretty much an identical change in my locations API refactor, where I ran across this problem (and more) during testing. > diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp > new file mode 100644 > index 0000000..6832f1d > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp > @@ -0,0 +1,77 @@ > +if {[skip_shlib_tests]} { > +if [get_compiler_info] { > +if { [gdb_compile_shlib $libsrc1 $lib_sl1 $lib_opts] != "" } { > +if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable $exec_opts] != ""} { This is a /big/ nitpick, but it's something that consistently irritates me: compare the coding style of the four statements above. While there is no "rule" governing which is most correct/desired, I always use the first. I would ask you to choose one style and be consistent, but I am not asking you to make any changes right now. Just please keep this in mind in future patches. > +# Set pending dprintf via MI. > +mi_gdb_test "-dprintf-insert -f pendfunc1 \"hello\"" \ > + ".*\\^done,bkpt={number=\"1\",type=\"dprintf\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"}" \ > + "mi set dprintf" > + > +mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint bp_location1" Is it possible to use mi_make_breakpoint for these tests? > + > +mi_run_cmd > + > +set msg "mi dprintf" > +gdb_expect { > + -re ".*~\"hello\"" { > + pass $msg > + } > + -re ".*$mi_gdb_prompt$" { > + fail $msg > + } > + timeout { > + fail $msg > + } > +} This a pretty common test suite idiom, I think. Can mi_gdb_test be used instead of gdb_expect? > +mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop" > diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c > new file mode 100644 > index 0000000..fe49a8d > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c > @@ -0,0 +1,27 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2015 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/>. */ > + > +void > +pendfunc1 () > +{ > +} > + > +void > +pendfunc () > +{ > + pendfunc1(); > +} > IIRC, we are now requiring test case conformance to the coding standard. [At least, that's what I've been told in the past.] So, "(void)" in the function decls and spaces between function names and '('. [I don't think we're requiring function comments for trivial stuff like this, though.] Keith ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-26 19:11 ` Keith Seitz @ 2015-03-27 12:34 ` Antoine Tremblay 2015-03-27 12:37 ` Antoine Tremblay 2015-03-27 16:46 ` [PATCH] Fix MI dprintf-insert not printing when a location is pending Keith Seitz 0 siblings, 2 replies; 11+ messages in thread From: Antoine Tremblay @ 2015-03-27 12:34 UTC (permalink / raw) To: Keith Seitz, gdb-patches On 03/26/2015 03:10 PM, Keith Seitz wrote: > On 03/26/2015 09:47 AM, Antoine Tremblay wrote: >> gdb/ChangeLog: >> >> PR breakpoints/16465 >> * breakpoint.c (create_breakpoint): Fix missing extra_string. > > This is a real nit, so please don't go making any changes here unless a > maintainer requests it, but this changelog entry doesn't really explain > the change you've made. ["Save `extra_string' for pending breakpoints." > is much more descriptive/helpful.] > Indeed why not :) >> @@ -9783,10 +9786,16 @@ create_breakpoint (struct gdbarch *gdbarch, >> cond_string = xstrdup (cond_string); >> make_cleanup (xfree, cond_string); >> } >> + /* Create a private copy of any extra string. */ >> + if (extra_string) > > We explicitly test against NULL for pointers. [I know there are quite a > few violations of this in this function. All are awaiting an > easy/obvious separate cleanup. :-)] > Yes, it will look weird to just change the one I added though... That's what I did anyway so that the others can be part of a cleanup patch. >> + { >> + extra_string = xstrdup (extra_string); >> + make_cleanup (xfree, extra_string); >> + } >> b->cond_string = cond_string; >> + b->extra_string = extra_string; >> b->thread = thread; >> } >> - b->extra_string = NULL; >> b->ignore_count = ignore_count; >> b->disposition = tempflag ? disp_del : disp_donttouch; >> b->condition_not_parsed = 1; > > FWIW, I have pretty much an identical change in my locations API > refactor, where I ran across this problem (and more) during testing. :) Glad it can remove unrelated stuff from your patch > >> diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp >> b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp >> new file mode 100644 >> index 0000000..6832f1d >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp >> @@ -0,0 +1,77 @@ >> +if {[skip_shlib_tests]} { >> +if [get_compiler_info] { >> +if { [gdb_compile_shlib $libsrc1 $lib_sl1 $lib_opts] != "" } { >> +if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable >> $exec_opts] != ""} { > > This is a /big/ nitpick, but it's something that consistently irritates > me: compare the coding style of the four statements above. While there > is no "rule" governing which is most correct/desired, I always use the > first. I would ask you to choose one style and be consistent, but I am > not asking you to make any changes right now. Just please keep this in > mind in future patches. > I really had not noticed that, thanks for making me notice! I must confess I do a lot of copy & paste when writing a test. The if {[func]} seems indeed the one used in all the tcl docs so I'll use that. >> +# Set pending dprintf via MI. >> +mi_gdb_test "-dprintf-insert -f pendfunc1 \"hello\"" \ >> + >> ".*\\^done,bkpt={number=\"1\",type=\"dprintf\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"}" >> \ >> + "mi set dprintf" >> + >> +mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint >> bp_location1" > > Is it possible to use mi_make_breakpoint for these tests? Unfortunately for the dprintf one mi_make_breakpoint doesn't support pending breakpoints, it creates something like : bkpt={number="2",type=".*",disp=".*",enabled=".*",addr=".*",func=".*", file=".*/myfile.c",fullname=".*",line="3",thread-groups=\[.*\], times="0".*original-location=".*"} But with pending funcs it should be pending= ... It could be the subject of another patch to add that support. I used mi_create_breakpoint for the other breakpoint now > >> + >> +mi_run_cmd >> + >> +set msg "mi dprintf" >> +gdb_expect { >> + -re ".*~\"hello\"" { >> + pass $msg >> + } >> + -re ".*$mi_gdb_prompt$" { >> + fail $msg >> + } >> + timeout { >> + fail $msg >> + } >> +} > > This a pretty common test suite idiom, I think. Can mi_gdb_test be used > instead of gdb_expect? That I can't since mi_gdb_test requires a command and in this case I'm just doing expect on that comes after mi_run_cmd, there's no command associated with it.. > >> +mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop" >> diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c >> b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c >> new file mode 100644 >> index 0000000..fe49a8d >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c >> @@ -0,0 +1,27 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2015 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/>. */ >> + >> +void >> +pendfunc1 () >> +{ >> +} >> + >> +void >> +pendfunc () >> +{ >> + pendfunc1(); >> +} >> > > IIRC, we are now requiring test case conformance to the coding standard. > [At least, that's what I've been told in the past.] So, "(void)" in the > function decls and spaces between function names and '('. [I don't think > we're requiring function comments for trivial stuff like this, though.] > Indeed that's really my old habits dying hard... fixed sorry about that. Thanks for the review ! , Patch v2 is coming up in a minute as a separate email... Antoine ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-27 12:34 ` Antoine Tremblay @ 2015-03-27 12:37 ` Antoine Tremblay 2015-03-27 16:52 ` Keith Seitz 2015-03-27 16:46 ` [PATCH] Fix MI dprintf-insert not printing when a location is pending Keith Seitz 1 sibling, 1 reply; 11+ messages in thread From: Antoine Tremblay @ 2015-03-27 12:37 UTC (permalink / raw) To: keiths, gdb-patches; +Cc: Antoine Tremblay This patch fixes the "Format string required" error when trying to print a dprintf on a pending location when set via the MI interface even if the format string is entered correctly. This patch also adds a test case to check that issue called mi-dprintf-pending.exp. gdb/ChangeLog: PR breakpoints/16465 * breakpoint.c (create_breakpoint): Save extra_string for pending breakpoints. gdb/testsuite/ChangeLog: PR breakpoints/16465 * gdb.mi/mi-dprintf-pending.c: New file. * gdb.mi/mi-dprintf-pending.exp: New test. * gdb.mi/mi-dprintf-pendshr.c: New file. --- gdb/breakpoint.c | 13 ++++- gdb/testsuite/gdb.mi/mi-dprintf-pending.c | 24 +++++++++ gdb/testsuite/gdb.mi/mi-dprintf-pending.exp | 78 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c | 27 ++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pending.c create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pending.exp create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0724a72..e59df9a 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -9774,7 +9774,10 @@ create_breakpoint (struct gdbarch *gdbarch, b->addr_string = copy_arg; if (parse_arg) - b->cond_string = NULL; + { + b->cond_string = NULL; + b->extra_string = NULL; + } else { /* Create a private copy of condition string. */ @@ -9783,10 +9786,16 @@ create_breakpoint (struct gdbarch *gdbarch, cond_string = xstrdup (cond_string); make_cleanup (xfree, cond_string); } + /* Create a private copy of any extra string. */ + if (extra_string != NULL) + { + extra_string = xstrdup (extra_string); + make_cleanup (xfree, extra_string); + } b->cond_string = cond_string; + b->extra_string = extra_string; b->thread = thread; } - b->extra_string = NULL; b->ignore_count = ignore_count; b->disposition = tempflag ? disp_del : disp_donttouch; b->condition_not_parsed = 1; diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.c b/gdb/testsuite/gdb.mi/mi-dprintf-pending.c new file mode 100644 index 0000000..678c307 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +extern void pendfunc (); + +int main (void) +{ + pendfunc (); + return 0; /* set breakpoint 1 here */ +} diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp new file mode 100644 index 0000000..6185329 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp @@ -0,0 +1,78 @@ +# Copyright 2015 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/>. + +# This test checks if dprintf prints correctly when it's pending. +# See PR breakpoints/16465. + +load_lib mi-support.exp + +if {[skip_shlib_tests]} { + return 0 +} + +standard_testfile mi-dprintf-pending.c + +set libfile1 "mi-dprintf-pendshr" +set libsrc1 $srcdir/$subdir/$libfile1.c +set lib_sl1 [standard_output_file $libfile1.sl] +set lib_opts debug +set exec_opts [list debug shlib=$lib_sl1 shlib_load] + +if {[get_compiler_info]} { + return -1 +} + +if {[gdb_compile_shlib $libsrc1 $lib_sl1 $lib_opts] != ""} { + untested "Could not compile $libsrc1" + return -1 +} + +if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable $exec_opts] != ""} { + untested "Could not compile $srcdir/$subdir/$srcfile" + return -1 +} + +# Start with a fresh gdb. +gdb_exit +mi_gdb_start +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load ${binfile} +mi_load_shlibs $lib_sl1 + +set bp_location1 [gdb_get_line_number "set breakpoint 1 here"] + +# Set pending dprintf via MI. +mi_gdb_test "-dprintf-insert -f pendfunc1 \"hello\"" \ + ".*\\^done,bkpt={number=\"1\",type=\"dprintf\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"}" \ + "mi set dprintf" + +mi_create_breakpoint $bp_location1 "mi insert breakpoint bp_location1" \ + -type "breakpoint" -line $bp_location1 -file $srcdir/$subdir/$srcfile + +mi_run_cmd + +set msg "mi dprintf" +gdb_expect { + -re ".*~\"hello\"" { + pass $msg + } + -re ".*$mi_gdb_prompt$" { + fail $msg + } + timeout { + fail $msg + } +} +mi_expect_stop ".*" ".*" ".*" ".*" ".*" "" "$msg stop" diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c new file mode 100644 index 0000000..05d188c --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +void +pendfunc1 (void) +{ +} + +void +pendfunc (void) +{ + pendfunc1 (); +} -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-27 12:37 ` Antoine Tremblay @ 2015-03-27 16:52 ` Keith Seitz 2015-03-27 17:13 ` Antoine Tremblay 0 siblings, 1 reply; 11+ messages in thread From: Keith Seitz @ 2015-03-27 16:52 UTC (permalink / raw) To: Antoine Tremblay, gdb-patches On 03/27/2015 05:36 AM, Antoine Tremblay wrote: > diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp > new file mode 100644 > index 0000000..6185329 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp ... > +set msg "mi dprintf" > +gdb_expect { > + -re ".*~\"hello\"" { > + pass $msg > + } > + -re ".*$mi_gdb_prompt$" { > + fail $msg > + } > + timeout { > + fail $msg > + } > +} Pending a previous query about using mi_gdb_test here, I recommend a maintainer approve this patch. Thank you for your patch! Keith ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-27 16:52 ` Keith Seitz @ 2015-03-27 17:13 ` Antoine Tremblay 2015-03-27 17:34 ` Keith Seitz 0 siblings, 1 reply; 11+ messages in thread From: Antoine Tremblay @ 2015-03-27 17:13 UTC (permalink / raw) To: Keith Seitz, gdb-patches On 03/27/2015 12:51 PM, Keith Seitz wrote: > On 03/27/2015 05:36 AM, Antoine Tremblay wrote: >> diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp >> b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp >> new file mode 100644 >> index 0000000..6185329 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp > ... > >> +set msg "mi dprintf" >> +gdb_expect { >> + -re ".*~\"hello\"" { >> + pass $msg >> + } >> + -re ".*$mi_gdb_prompt$" { >> + fail $msg >> + } >> + timeout { >> + fail $msg >> + } >> +} > > Pending a previous query about using mi_gdb_test here, I recommend a > maintainer approve this patch. > Indeed I had missed that you can call it without a command, my mind must have skipped that line, thanks :) However there is still a problem forcing me to use gdb_expect I think, namely mi_gdb_test waits for a command prompt like so : -re "^($string_regex\[\r\n\]+)?($pattern\[\r\n\]+$mi_gdb_prompt\[ \]*)" However dprintf does mean you will get a prompt printed... You will get a prompt only when hitting the breakpoint I placed. This means that if for some reason the breakpoint would not hit, the dprintf test would fail, when it's the breakpoint that should fail. I could remove mi_expect_stop at the end and assume that breakpoint will not fail but I think it's not right ? also mi-dprintf.exp is using the same mi_run_cmd / gdb_expect / mi_expect_stop ... What do you think ? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-27 17:13 ` Antoine Tremblay @ 2015-03-27 17:34 ` Keith Seitz 2015-04-20 12:14 ` Antoine Tremblay 0 siblings, 1 reply; 11+ messages in thread From: Keith Seitz @ 2015-03-27 17:34 UTC (permalink / raw) To: Antoine Tremblay, gdb-patches On 03/27/2015 10:12 AM, Antoine Tremblay wrote: > However there is still a problem forcing me to use gdb_expect I think, > namely mi_gdb_test waits for a command prompt like so : > > -re "^($string_regex\[\r\n\]+)?($pattern\[\r\n\]+$mi_gdb_prompt\[ \]*)" > > However dprintf does mean you will get a prompt printed... > > You will get a prompt only when hitting the breakpoint I placed. Ick. Yeah, you are right. My bad. > I could remove mi_expect_stop at the end and assume that breakpoint will > not fail but I think it's not right ? also mi-dprintf.exp is using the > same mi_run_cmd / gdb_expect / mi_expect_stop ... > > What do you think ? I think everything is fine as it is. Sorry about the noise. So, once again, while I am not a maintainer, I recommend a maintainer approve this patch. Thank you for your patience, Keith ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-27 17:34 ` Keith Seitz @ 2015-04-20 12:14 ` Antoine Tremblay 2015-06-12 12:27 ` [PATCH v3] Fix MI dprintf-insert not printing on a resolved pending location Antoine Tremblay 0 siblings, 1 reply; 11+ messages in thread From: Antoine Tremblay @ 2015-04-20 12:14 UTC (permalink / raw) To: gdb-patches ping... On 03/27/2015 01:34 PM, Keith Seitz wrote: > On 03/27/2015 10:12 AM, Antoine Tremblay wrote: >> However there is still a problem forcing me to use gdb_expect I think, >> namely mi_gdb_test waits for a command prompt like so : >> >> -re "^($string_regex\[\r\n\]+)?($pattern\[\r\n\]+$mi_gdb_prompt\[ \]*)" >> >> However dprintf does mean you will get a prompt printed... >> >> You will get a prompt only when hitting the breakpoint I placed. > > Ick. Yeah, you are right. My bad. > >> I could remove mi_expect_stop at the end and assume that breakpoint will >> not fail but I think it's not right ? also mi-dprintf.exp is using the >> same mi_run_cmd / gdb_expect / mi_expect_stop ... >> >> What do you think ? > > I think everything is fine as it is. Sorry about the noise. > > So, once again, while I am not a maintainer, I recommend a maintainer > approve this patch. > > Thank you for your patience, > Keith > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] Fix MI dprintf-insert not printing on a resolved pending location. 2015-04-20 12:14 ` Antoine Tremblay @ 2015-06-12 12:27 ` Antoine Tremblay 2015-06-12 12:33 ` Pedro Alves 0 siblings, 1 reply; 11+ messages in thread From: Antoine Tremblay @ 2015-06-12 12:27 UTC (permalink / raw) To: gdb-patches; +Cc: Antoine Tremblay --- Diffs in v3 : - some syntax formating for int main - wording changes to make it more clear that the pending location is now resolved - more explicit mi_expect_stop --- This patch fixes the "Format string required" error when trying to print a dprintf on a now resolved, pending location when set via the MI interface even if the format string is entered correctly. This patch also adds a test case to check that issue called mi-dprintf-pending.exp. gdb/ChangeLog: PR breakpoints/16465 * breakpoint.c (create_breakpoint): Save extra_string for pending breakpoints. gdb/testsuite/ChangeLog: PR breakpoints/16465 * gdb.mi/mi-dprintf-pending.c: New file. * gdb.mi/mi-dprintf-pending.exp: New test. * gdb.mi/mi-dprintf-pendshr.c: New file. --- gdb/breakpoint.c | 13 ++++- gdb/testsuite/gdb.mi/mi-dprintf-pending.c | 25 +++++++++ gdb/testsuite/gdb.mi/mi-dprintf-pending.exp | 79 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c | 27 ++++++++++ 4 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pending.c create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pending.exp create mode 100644 gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 657c58e..0a960f8 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -9773,7 +9773,10 @@ create_breakpoint (struct gdbarch *gdbarch, b->addr_string = copy_arg; if (parse_arg) - b->cond_string = NULL; + { + b->cond_string = NULL; + b->extra_string = NULL; + } else { /* Create a private copy of condition string. */ @@ -9782,10 +9785,16 @@ create_breakpoint (struct gdbarch *gdbarch, cond_string = xstrdup (cond_string); make_cleanup (xfree, cond_string); } + /* Create a private copy of any extra string. */ + if (extra_string != NULL) + { + extra_string = xstrdup (extra_string); + make_cleanup (xfree, extra_string); + } b->cond_string = cond_string; + b->extra_string = extra_string; b->thread = thread; } - b->extra_string = NULL; b->ignore_count = ignore_count; b->disposition = tempflag ? disp_del : disp_donttouch; b->condition_not_parsed = 1; diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.c b/gdb/testsuite/gdb.mi/mi-dprintf-pending.c new file mode 100644 index 0000000..dcb9a0a --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.c @@ -0,0 +1,25 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +extern void pendfunc (); + +int +main (void) +{ + pendfunc (); + return 0; /* set breakpoint 1 here */ +} diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp new file mode 100644 index 0000000..5bb4a1b --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pending.exp @@ -0,0 +1,79 @@ +# Copyright 2015 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/>. + +# This test checks if dprintf prints correctly when it's resolved from +# pending state. +# See PR breakpoints/16465. + +load_lib mi-support.exp + +if {[skip_shlib_tests]} { + return 0 +} + +standard_testfile mi-dprintf-pending.c + +set libfile1 "mi-dprintf-pendshr" +set libsrc1 $srcdir/$subdir/$libfile1.c +set lib_sl1 [standard_output_file $libfile1.sl] +set lib_opts debug +set exec_opts [list debug shlib=$lib_sl1 shlib_load] + +if {[get_compiler_info]} { + return -1 +} + +if {[gdb_compile_shlib $libsrc1 $lib_sl1 $lib_opts] != ""} { + untested "Could not compile $libsrc1" + return -1 +} + +if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable $exec_opts] != ""} { + untested "Could not compile $srcdir/$subdir/$srcfile" + return -1 +} + +# Start with a fresh gdb. +gdb_exit +mi_gdb_start +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load ${binfile} +mi_load_shlibs $lib_sl1 + +set bp_location1 [gdb_get_line_number "set breakpoint 1 here"] + +# Set pending dprintf via MI. +mi_gdb_test "-dprintf-insert -f pendfunc1 \"hello\"" \ + ".*\\^done,bkpt={number=\"1\",type=\"dprintf\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"}" \ + "mi set dprintf" + +mi_create_breakpoint $bp_location1 "mi insert breakpoint bp_location1" \ + -type "breakpoint" -line $bp_location1 -file $srcdir/$subdir/$srcfile + +mi_run_cmd + +set msg "mi dprintf" +gdb_expect { + -re ".*~\"hello\"" { + pass $msg + } + -re ".*$mi_gdb_prompt$" { + fail $msg + } + timeout { + fail $msg + } +} +mi_expect_stop ".*" "main" ".*" ".*$srcfile" "$bp_location1" "" "$msg stop" diff --git a/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c new file mode 100644 index 0000000..05d188c --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-dprintf-pendshr.c @@ -0,0 +1,27 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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/>. */ + +void +pendfunc1 (void) +{ +} + +void +pendfunc (void) +{ + pendfunc1 (); +} -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] Fix MI dprintf-insert not printing on a resolved pending location. 2015-06-12 12:27 ` [PATCH v3] Fix MI dprintf-insert not printing on a resolved pending location Antoine Tremblay @ 2015-06-12 12:33 ` Pedro Alves 0 siblings, 0 replies; 11+ messages in thread From: Pedro Alves @ 2015-06-12 12:33 UTC (permalink / raw) To: Antoine Tremblay, gdb-patches On 06/12/2015 01:27 PM, Antoine Tremblay wrote: > --- > Diffs in v3 : > - some syntax formating for int main > - wording changes to make it more clear that the pending location is now resolved > - more explicit mi_expect_stop Thanks. That does make it more obvious what is going on here. > This patch fixes the "Format string required" error when trying to print > a dprintf on a now resolved, pending location when set via the MI interface > even if the format string is entered correctly. > > This patch also adds a test case to check that issue called > mi-dprintf-pending.exp. > OK with: > +extern void pendfunc (); (void) here too. Thank you and thanks Keith too. -- Pedro Alves ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix MI dprintf-insert not printing when a location is pending. 2015-03-27 12:34 ` Antoine Tremblay 2015-03-27 12:37 ` Antoine Tremblay @ 2015-03-27 16:46 ` Keith Seitz 1 sibling, 0 replies; 11+ messages in thread From: Keith Seitz @ 2015-03-27 16:46 UTC (permalink / raw) To: Antoine Tremblay, gdb-patches On 03/27/2015 05:34 AM, Antoine Tremblay wrote: > On 03/26/2015 03:10 PM, Keith Seitz wrote: >> On 03/26/2015 09:47 AM, Antoine Tremblay wrote: >>> +# Set pending dprintf via MI. >>> +mi_gdb_test "-dprintf-insert -f pendfunc1 \"hello\"" \ >>> + >>> ".*\\^done,bkpt={number=\"1\",type=\"dprintf\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc1\",times=\"0\",original-location=\"pendfunc1\"}" >>> >>> \ >>> + "mi set dprintf" >>> + >>> +mi_gdb_test "-break-insert $bp_location1" ".*" "mi insert breakpoint >>> bp_location1" >> >> Is it possible to use mi_make_breakpoint for these tests? > > Unfortunately for the dprintf one mi_make_breakpoint doesn't support > pending breakpoints, it creates something like : > bkpt={number="2",type=".*",disp=".*",enabled=".*",addr=".*",func=".*", > file=".*/myfile.c",fullname=".*",line="3",thread-groups=\[.*\], > times="0".*original-location=".*"} > > But with pending funcs it should be pending= ... Ah, I see. Thank you for pointing that out. I *thought* that I did that already! > It could be the subject of another patch to add that support. > Indeed. > I used mi_create_breakpoint for the other breakpoint now > Excellent. >>> +mi_run_cmd >>> + >>> +set msg "mi dprintf" >>> +gdb_expect { >>> + -re ".*~\"hello\"" { >>> + pass $msg >>> + } >>> + -re ".*$mi_gdb_prompt$" { >>> + fail $msg >>> + } >>> + timeout { >>> + fail $msg >>> + } >>> +} >> >> This a pretty common test suite idiom, I think. Can mi_gdb_test be used >> instead of gdb_expect? > > That I can't since mi_gdb_test requires a command and in this case I'm > just doing expect on that comes after mi_run_cmd, there's no command > associated with it.. While not common in gdb.mi (with mi_gdb_test), the rest of gdb.* uses the idiom: gdb_test "" ... all the time. Looking at the description of mi_gdb_test: # mi_gdb_test COMMAND PATTERN MESSAGE [IPATTERN] -- send a command to gdb; # test the result. # # COMMAND is the command to execute, send to GDB with send_gdb. If # this is the null string no command is sent. [Very quickly] Browsing through the code of that function, it seems like it should work. If it does not, would you please file a bug? It *definitely* should work. Keith ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-06-12 12:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-26 16:47 [PATCH] Fix MI dprintf-insert not printing when a location is pending Antoine Tremblay 2015-03-26 19:11 ` Keith Seitz 2015-03-27 12:34 ` Antoine Tremblay 2015-03-27 12:37 ` Antoine Tremblay 2015-03-27 16:52 ` Keith Seitz 2015-03-27 17:13 ` Antoine Tremblay 2015-03-27 17:34 ` Keith Seitz 2015-04-20 12:14 ` Antoine Tremblay 2015-06-12 12:27 ` [PATCH v3] Fix MI dprintf-insert not printing on a resolved pending location Antoine Tremblay 2015-06-12 12:33 ` Pedro Alves 2015-03-27 16:46 ` [PATCH] Fix MI dprintf-insert not printing when a location is pending Keith Seitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).