This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix dprintf work not right if it is pending
- From: Hui Zhu <teawater at gmail dot com>
- To: Keith Seitz <keiths at redhat dot com>
- Cc: Hui Zhu <hui_zhu at mentor dot com>, gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Fri, 29 Mar 2013 15:42:57 +0800
- Subject: Re: [PATCH] Fix dprintf work not right if it is pending
- References: <514BF736 dot 3070706 at mentor dot com> <514C3C85 dot 4000704 at codesourcery dot com> <514EEBFF dot 8090705 at redhat dot com> <CANFwon3D77yDiB_bQ0iZeg=KpkwoGiKnu=_7+kfcVV547M_cfg at mail dot gmail dot com> <5154378D dot 60302 at redhat dot com>
Hi Keith,
Thanks for your review.
On Thu, Mar 28, 2013 at 8:29 PM, Keith Seitz <keiths@redhat.com> wrote:
> On 03/25/2013 11:41 PM, Hui Zhu wrote:
>>
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -12963,6 +12963,15 @@ bkpt_re_set (struct breakpoint *b)
>> breakpoint_re_set_default (b);
>> }
>>
>> +static void
>> +dprintf_re_set (struct breakpoint *b)
>> +{
>
>
> This function needs a comment, even if it just mentions that this function
> is the re_set method for dprintf breakpoints.
Fixed.
>
>> + breakpoint_re_set_default (b);
>> +
>> + if (b->extra_string != NULL)
>> + update_dprintf_command_list (b);
>> +}
>> +
>
>
> This will update the command list every time breakpoints are reset and could
> be limited to only those needing updating. Is there perhaps a reason to
> always do this?
I think it need, because it need to generate different commands with
different status for example:
if (target_can_run_breakpoint_commands ())
printf_line = xstrprintf ("agent-printf %s", dprintf_args);
>
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/dprintf-pending.c
>> @@ -0,0 +1,31 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> + Copyright 2004-2013 Free Software Foundation, Inc.
>
> ^^^^^^^^^
> I think you meant only 2013. :-)
Fixed.
>
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/dprintf-pending.exp
>> +
>> +if [get_compiler_info] {
>> + return -1
>> +}
>
>
> Can you please add braces around the expression here? [Yeah, that's a nit,
> but it is one of my Tcl pet peeves, and this is a new file. Time to stomp
> out the old crud.]
Fixed.
Post the new version according to your comments.
Best,
Hui
>
> Keith
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12963,6 +12963,17 @@ bkpt_re_set (struct breakpoint *b)
breakpoint_re_set_default (b);
}
+/* Dprintf breakpoint_ops methods. */
+
+static void
+dprintf_re_set (struct breakpoint *b)
+{
+ breakpoint_re_set_default (b);
+
+ if (b->extra_string != NULL)
+ update_dprintf_command_list (b);
+}
+
static int
bkpt_insert_location (struct bp_location *bl)
{
@@ -16001,7 +16012,7 @@ initialize_breakpoint_ops (void)
ops = &dprintf_breakpoint_ops;
*ops = bkpt_base_breakpoint_ops;
- ops->re_set = bkpt_re_set;
+ ops->re_set = dprintf_re_set;
ops->resources_needed = bkpt_resources_needed;
ops->print_it = bkpt_print_it;
ops->print_mention = bkpt_print_mention;
--- a/gdb/testsuite/gdb.base/Makefile.in
+++ b/gdb/testsuite/gdb.base/Makefile.in
@@ -10,7 +10,8 @@ EXECUTABLES = a2-run advance all-types a
call-strs callexit callfuncs callfwmall charset checkpoint \
chng-syms code_elim1 code_elim2 commands compiler complex \
condbreak consecutive constvars coremaker cursal cvexpr \
- dbx-test del disasm-end-cu display dump dup-sect dup-sect.debug \
+ dbx-test del disasm-end-cu display dprintf-pending dump dup-sect \
+ dup-sect.debug \
dup-sect.stripped ending-run execd-prog expand-psymtabs exprs \
fileio find finish fixsection float foll-exec foll-fork foll-vfork \
frame-args freebpcmd fullname funcargs gcore \
@@ -44,7 +45,7 @@ EXECUTABLES = a2-run advance all-types a
wchar whatis whatis-exp catch-syscall \
pr10179 gnu_vector
-MISCELLANEOUS = coremmap.data ../foobar.baz fixsectshr.sl \
+MISCELLANEOUS = coremmap.data dprintf-pendshr.sl ../foobar.baz fixsectshr.sl \
pendshr.sl shreloc1.sl shreloc2.sl twice-tmp.c \
shr1.sl shr2.sl solib_sl.sl solib1.sl solib2.sl \
unloadshr.sl unloadshr2.sl watchpoint-solib-shr.sl \
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-pending.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+
+int k = 0;
+
+extern void pendfunc (int x);
+
+int main()
+{
+ pendfunc (3); /* break main here */
+ pendfunc (4);
+ k = 1;
+ pendfunc (3);
+ return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-pending.exp
@@ -0,0 +1,91 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+
+if {[skip_shlib_tests]} {
+ return 0
+}
+
+set testfile "dprintf-pending"
+set libfile "dprintf-pendshr"
+set srcfile $testfile.c
+set libsrc $srcdir/$subdir/$libfile.c
+set binfile $objdir/$subdir/$testfile
+set lib_sl $objdir/$subdir/$libfile.sl
+
+set lib_opts debug
+set exec_opts [list debug shlib=$lib_sl]
+
+if { [get_compiler_info] } {
+ return -1
+}
+
+if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+ || [gdb_compile $srcdir/$subdir/$srcfile $binfile executable $exec_opts] != ""} {
+ untested "Could not compile either $libsrc or $srcdir/$subdir/$srcfile."
+ return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+gdb_test_multiple "dprintf pendfunc1, \"x=%d\\n\", x" "set pending dprintf" {
+ -re ".*Make dprintf pending.*y or \\\[n\\\]. $" {
+ gdb_test "y" "Dprintf.*pendfunc1.*pending." "set pending dprintf (without symbols)"
+ }
+}
+
+gdb_test "info break" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+dprintf.*keep y.*PENDING.*pendfunc1.*" \
+"single pending dprintf info (without symbols)"
+
+gdb_load ${binfile}
+gdb_load_shlibs $lib_sl
+
+gdb_run_cmd
+
+gdb_test "" ".*x=3.*x=4.*x=3.*" "run to resolved dprintf (without symbols)"
+
+# Restart with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+gdb_load ${binfile}
+gdb_load_shlibs $lib_sl
+
+#
+# Test setting, querying, and modifying pending breakpoints
+#
+
+gdb_test_multiple "dprintf pendfunc1, \"x=%d\\n\", x" "set pending dprintf" {
+ -re ".*Make dprintf pending.*y or \\\[n\\\]. $" {
+ gdb_test "y" "Dprintf.*pendfunc1.*pending." "set pending dprintf"
+ }
+}
+
+gdb_test "info break" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+dprintf.*keep y.*PENDING.*pendfunc1.*" \
+"single pending dprintf info (without symbols)"
+
+gdb_run_cmd
+
+gdb_test "" ".*x=3.*x=4.*x=3.*" "run to resolved dprintf"
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dprintf-pendshr.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+
+void pendfunc1 (int x)
+{
+ int y = x + 4;
+}
+
+void pendfunc (int x)
+{
+ pendfunc1 (x);
+}