Bug 15075 - dprintf inteferes with "next"
Summary: dprintf inteferes with "next"
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: breakpoints (show other bugs)
Version: unknown
: P2 normal
Target Milestone: 7.6.1
Assignee: teawater
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-28 18:49 UTC by Tom Tromey
Modified: 2014-01-10 16:11 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Tromey 2013-01-28 18:49:55 UTC
Debug this simple program:

barimba. nl -ba q.c
     1	int main()
     2	{
     3	  int x = 5;
     4	
     5	  ++x;
     6	  ++x;
     7	
     8	  return x - 7;
     9	}


Now put a dprintf on line 6:

(gdb) dprintf q.c:6, "%d\n", x
Dprintf 1 at 0x400483: file q.c, line 6.


Now "start" and try "next"ing through the program.
Once the dprintf is hit, "next" will act like "continue".
This is a bug in how dprintf is implemented.

(gdb) start
Temporary breakpoint 2 at 0x400478: file q.c, line 3.
Starting program: /tmp/q 

Temporary breakpoint 2, main () at q.c:3
3	  int x = 5;
(gdb) n
5	  ++x;
(gdb) 
6
[Inferior 1 (process 15434) exited normally]
[Inferior 15434 exited]
Comment 1 Yao Qi 2013-02-02 18:43:22 UTC
We need add a new user-invisible "continue without clearing proceed status" command, and let dprintf to use it.  I'll post patches for this.
Comment 2 Tom Tromey 2013-02-02 20:34:29 UTC
(In reply to comment #1)
> We need add a new user-invisible "continue without clearing proceed status"
> command, and let dprintf to use it.  I'll post patches for this.

The existing Python "Breakpoint.stop" API needed this kind
of thing already, so the infrastructure exists.
The error was making dprintf use the breakpoint commands instead
of just having internal actions; the original review pointed this out.

If you do implement a command like this, don't make it user-invisible.
It might be useful for other things.
However, be sure to look at the other bugs related to this before
embarking on that course.
Comment 3 Yao Qi 2013-02-04 02:52:12 UTC
> The existing Python "Breakpoint.stop" API needed this kind
> of thing already, so the infrastructure exists.

Thanks for pointing this out.  I'll have a look.

> The error was making dprintf use the breakpoint commands instead
> of just having internal actions; the original review pointed this out.
> 

Right, for the record, it was pointed out here http://sourceware.org/ml/gdb-patches/2012-05/msg00211.html

> If you do implement a command like this, don't make it user-invisible.
> It might be useful for other things.

OK, no problem.

> However, be sure to look at the other bugs related to this before
> embarking on that course.

What do you mean by "the other bugs related to this"?  bugs related to dprintf or the new command we try to add?
Comment 4 Tom Tromey 2013-02-04 17:49:23 UTC
(In reply to comment #3)

> What do you mean by "the other bugs related to this"?  bugs related to dprintf
> or the new command we try to add?

I think there may be other bugs along the lines of "commands interfere
with next".  I may be confusing this with "finish doesn't work in commands"
though.

I don't understand the attraction of trying to keep dprintf working
via "commands".
Comment 5 Pedro Alves 2013-02-04 19:34:03 UTC
Related problems:

http://sourceware.org/ml/gdb-patches/2012-05/msg00690.html
Comment 6 teawater 2013-04-23 07:25:34 UTC
(gdb) b 6
Breakpoint 1 at 0x4004c3: file 3.c, line 6.
(gdb) commands 
Type commands for breakpoint(s) 1, one per line.
End with a line saying just "end".
>continue 
>end
(gdb) start 
Temporary breakpoint 2 at 0x4004b8: file 3.c, line 3.
Starting program: /home/teawater/gdb/bgdbno/gdb/a.out 

Temporary breakpoint 2, main () at 3.c:3
3	int x = 5;
(gdb) n
5	++x;
(gdb) n

Breakpoint 1, main () at 3.c:6
6	++x;
[Inferior 1 (process 17056) exited normally]
Comment 9 cvs-commit@gcc.gnu.org 2013-06-25 11:37:50 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	teawater@sourceware.org	2013-06-25 11:37:48

Modified files:
	gdb            : ChangeLog breakpoint.c breakpoint.h 
	gdb/testsuite  : ChangeLog 
	gdb/testsuite/gdb.base: dprintf.exp 
	gdb/testsuite/gdb.mi: mi-breakpoint-changed.exp 
Added files:
	gdb/testsuite/gdb.base: dprintf-next.c dprintf-next.exp 
	                        dprintf-non-stop.c dprintf-non-stop.exp 

Log message:
	2013-06-25  Yao Qi  <yao@codesourcery.com>
	Hui Zhu  <hui@codesourcery.com>
	Pedro Alves  <palves@redhat.com>
	
	PR breakpoints/15075
	PR breakpoints/15434
	* breakpoint.c (bpstat_stop_status): Call
	b->ops->after_condition_true.
	(update_dprintf_command_list): Don't append "continue" command
	to the command list of dprintf breakpoint.
	(base_breakpoint_after_condition_true): New function.
	(base_breakpoint_ops): Add base_breakpoint_after_condition_true.
	(dprintf_after_condition_true): New function.
	(initialize_breakpoint_ops): Set dprintf_after_condition_true.
	* breakpoint.h (breakpoint_ops): Add after_condition_true.
	
	2013-06-25  Yao Qi  <yao@codesourcery.com>
	Hui Zhu  <hui@codesourcery.com>
	Pedro Alves  <palves@redhat.com>
	
	PR breakpoints/15075
	PR breakpoints/15434
	* gdb.base/dprintf-next.c: New file.
	* gdb.base/dprintf-next.exp: New file.
	* gdb.base/dprintf-non-stop.c: New file.
	* gdb.base/dprintf-non-stop.exp: New file.
	* gdb.base/dprintf.exp: Don't check "continue" in the output
	of "info breakpoints".
	* gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify):
	Don't check "continue" in script field.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15735&r2=1.15736
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/breakpoint.c.diff?cvsroot=src&r1=1.769&r2=1.770
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/breakpoint.h.diff?cvsroot=src&r1=1.199&r2=1.200
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&r1=1.3704&r2=1.3705
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-next.c.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-next.exp.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-non-stop.c.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-non-stop.exp.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf.exp.diff?cvsroot=src&r1=1.10&r2=1.11
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp.diff?cvsroot=src&r1=1.6&r2=1.7
Comment 10 teawater 2013-06-25 11:42:36 UTC
Fixed.
Comment 11 cvs-commit@gcc.gnu.org 2013-06-26 02:32:00 UTC
CVSROOT:	/cvs/src
Module name:	src
Branch: 	gdb_7_6-branch
Changes by:	teawater@sourceware.org	2013-06-26 02:31:58

Modified files:
	gdb            : breakpoint.c breakpoint.h 
	gdb/testsuite/gdb.base: dprintf.exp 
	gdb/testsuite/gdb.mi: mi-breakpoint-changed.exp 

Log message:
	2013-06-26  Yao Qi  <yao@codesourcery.com>
	Hui Zhu  <hui@codesourcery.com>
	Pedro Alves  <palves@redhat.com>
	
	PR breakpoints/15075
	PR breakpoints/15434
	* breakpoint.c (bpstat_stop_status): Call
	b->ops->after_condition_true.
	(update_dprintf_command_list): Don't append "continue" command
	to the command list of dprintf breakpoint.
	(base_breakpoint_after_condition_true): New function.
	(base_breakpoint_ops): Add base_breakpoint_after_condition_true.
	(dprintf_after_condition_true): New function.
	(initialize_breakpoint_ops): Set dprintf_after_condition_true.
	* breakpoint.h (breakpoint_ops): Add after_condition_true.
	
	2013-06-26  Yao Qi  <yao@codesourcery.com>
	Hui Zhu  <hui@codesourcery.com>
	Pedro Alves  <palves@redhat.com>
	
	PR breakpoints/15075
	PR breakpoints/15434
	* gdb.base/dprintf-next.c: New file.
	* gdb.base/dprintf-next.exp: New file.
	* gdb.base/dprintf-non-stop.c: New file.
	* gdb.base/dprintf-non-stop.exp: New file.
	* gdb.base/dprintf.exp: Don't check "continue" in the output
	of "info breakpoints".
	* gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify):
	Don't check "continue" in script field.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/breakpoint.c.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=1.745.2.6&r2=1.745.2.7
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/breakpoint.h.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=1.193&r2=1.193.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf.exp.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=1.8.2.1&r2=1.8.2.2
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.mi/mi-breakpoint-changed.exp.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=1.5&r2=1.5.2.1
Comment 12 cvs-commit@gcc.gnu.org 2013-06-26 02:32:54 UTC
CVSROOT:	/cvs/src
Module name:	src
Branch: 	gdb_7_6-branch
Changes by:	teawater@sourceware.org	2013-06-26 02:32:53

Added files:
	gdb/testsuite/gdb.base: dprintf-next.c dprintf-next.exp 
	                        dprintf-non-stop.c dprintf-non-stop.exp 

Log message:
	2013-06-26  Yao Qi  <yao@codesourcery.com>
	Hui Zhu  <hui@codesourcery.com>
	Pedro Alves  <palves@redhat.com>
	
	PR breakpoints/15075
	PR breakpoints/15434
	* breakpoint.c (bpstat_stop_status): Call
	b->ops->after_condition_true.
	(update_dprintf_command_list): Don't append "continue" command
	to the command list of dprintf breakpoint.
	(base_breakpoint_after_condition_true): New function.
	(base_breakpoint_ops): Add base_breakpoint_after_condition_true.
	(dprintf_after_condition_true): New function.
	(initialize_breakpoint_ops): Set dprintf_after_condition_true.
	* breakpoint.h (breakpoint_ops): Add after_condition_true.
	
	2013-06-26  Yao Qi  <yao@codesourcery.com>
	Hui Zhu  <hui@codesourcery.com>
	Pedro Alves  <palves@redhat.com>
	
	PR breakpoints/15075
	PR breakpoints/15434
	* gdb.base/dprintf-next.c: New file.
	* gdb.base/dprintf-next.exp: New file.
	* gdb.base/dprintf-non-stop.c: New file.
	* gdb.base/dprintf-non-stop.exp: New file.
	* gdb.base/dprintf.exp: Don't check "continue" in the output
	of "info breakpoints".
	* gdb.mi/mi-breakpoint-changed.exp (test_insert_delete_modify):
	Don't check "continue" in script field.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-next.c.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=NONE&r2=1.1.2.2
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-next.exp.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=NONE&r2=1.1.2.2
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-non-stop.c.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=NONE&r2=1.1.2.2
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.base/dprintf-non-stop.exp.diff?cvsroot=src&only_with_tag=gdb_7_6-branch&r1=NONE&r2=1.1.2.2
Comment 13 Marc Khouzam 2014-01-10 16:11:56 UTC
This is an awesome fix.  Thanks!
I won't mark as Verified since I didn't open it myself.