[PATCH] Rename breakpoints_changed to annotate_breakpoints_changed.

[PATCH] Rename breakpoints_changed to annotate_breakpoints_changed.

[Pedro Alves]

This is the only extern function in annotate.c that doesn't have the
"annotate_" prefix.

This function should probably be replaced by use of
observer_notify_breakpoint_modified, but meanwhile, this makes the
fact that this is annotation-specific stand out more.

Checked in.

2012-11-20  Pedro Alves  <palves@redhat.com>

	* annotate.c (breakpoints_changed): Rename to ...
	(annotate_breakpoints_changed): ... this.
	(annotate_stopped, breakpoint_changed): Adjust caller.
	* annotate.h (breakpoints_changed): Rename to ...
	(annotate_breakpoints_changed): ... this.
	* breakpoint.c (set_breakpoint_condition, breakpoint_set_commands)
	(do_map_commands_command, init_raw_breakpoint, clear_command)
	(set_ignore_count, enable_breakpoint_disp): Adjust callers.
---
 gdb/annotate.c   |    6 +++---
 gdb/annotate.h   |    2 +-
 gdb/breakpoint.c |   14 +++++++-------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gdb/annotate.c b/gdb/annotate.c
index cd0a94a..96902cb 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -50,7 +50,7 @@ print_value_flags (struct type *t)
 }
 \f
 void
-breakpoints_changed (void)
+annotate_breakpoints_changed (void)
 {
   if (annotation_level == 2)
     {
@@ -110,7 +110,7 @@ annotate_stopped (void)
   if (annotation_level > 1 && ignore_count_changed)
     {
       ignore_count_changed = 0;
-      breakpoints_changed ();
+      annotate_breakpoints_changed ();
     }
 }
 
@@ -563,7 +563,7 @@ annotate_array_section_end (void)
 static void
 breakpoint_changed (struct breakpoint *b)
 {
-  breakpoints_changed ();
+  annotate_breakpoints_changed ();
 }
 
 void
diff --git a/gdb/annotate.h b/gdb/annotate.h
index 0eae524..0f7b320 100644
--- a/gdb/annotate.h
+++ b/gdb/annotate.h
@@ -20,7 +20,7 @@
 #include "symtab.h"
 #include "gdbtypes.h"
 
-extern void breakpoints_changed (void);
+extern void annotate_breakpoints_changed (void);
 
 extern void annotate_ignore_count_change (void);
 extern void annotate_breakpoint (int);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ccf7ee1..387ddb8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -983,7 +983,7 @@ set_breakpoint_condition (struct breakpoint *b, char *exp,
     }
   mark_breakpoint_modified (b);
 
-  breakpoints_changed ();
+  annotate_breakpoints_changed ();
   observer_notify_breakpoint_modified (b);
 }
 
@@ -1214,7 +1214,7 @@ breakpoint_set_commands (struct breakpoint *b,
 
   decref_counted_command_line (&b->commands);
   b->commands = alloc_counted_command_line (commands);
-  breakpoints_changed ();
+  annotate_breakpoints_changed ();
   observer_notify_breakpoint_modified (b);
 }
 
@@ -1331,7 +1331,7 @@ do_map_commands_command (struct breakpoint *b, void *data)
       incref_counted_command_line (info->cmd);
       decref_counted_command_line (&b->commands);
       b->commands = info->cmd;
-      breakpoints_changed ();
+      annotate_breakpoints_changed ();
       observer_notify_breakpoint_modified (b);
     }
 }
@@ -6996,7 +6996,7 @@ init_raw_breakpoint (struct breakpoint *b, struct gdbarch *gdbarch,
   if (bptype != bp_breakpoint && bptype != bp_hardware_breakpoint)
     b->pspace = sal.pspace;
 
-  breakpoints_changed ();
+  annotate_breakpoints_changed ();
 }
 
 /* set_raw_breakpoint is a low level routine for allocating and
@@ -11941,7 +11941,7 @@ clear_command (char *arg, int from_tty)
       else
 	printf_unfiltered (_("Deleted breakpoints "));
     }
-  breakpoints_changed ();
+  annotate_breakpoints_changed ();
 
   for (ix = 0; VEC_iterate(breakpoint_p, found, ix, b); ix++)
     {
@@ -14319,7 +14319,7 @@ set_ignore_count (int bptnum, int count, int from_tty)
 			       "crossings of breakpoint %d."),
 			     count, bptnum);
 	}
-      breakpoints_changed ();
+      annotate_breakpoints_changed ();
       observer_notify_breakpoint_modified (b);
       return;
     }
@@ -14584,7 +14584,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
   bpt->disposition = disposition;
   bpt->enable_count = count;
   update_global_location_list (1);
-  breakpoints_changed ();
+  annotate_breakpoints_changed ();
   
   observer_notify_breakpoint_modified (bpt);
 }

Re: [PATCH] Rename breakpoints_changed to annotate_breakpoints_changed.

[Pedro Alves]

On 11/20/2012 05:54 PM, Pedro Alves wrote:
> This is the only extern function in annotate.c that doesn't have the
> "annotate_" prefix.
> 
> This function should probably be replaced by use of
> observer_notify_breakpoint_modified, but meanwhile, 

FYI:

The reason I'm looking at annotation code is that I made a change
that puts software single-step breakpoints in the regular breakpoint
chain, and then that triggers breakpoints-invalid annotations for
each sss breakpoint that didn't happen before, and in turn that
leads to the gdb.log getting filled up with useless notifications
and the tests timing out.  I had a small patch to handle this, but it turns
out incomplete.  Further looking at this issue, I've decided to go ahead
and fully convert the breakpoints-invalid annotation to observers.

-- 
Pedro Alves

[PATCH 4/4] Skip breakpoints-invalid annotations for internal|momentary breakpoints.

[Pedro Alves]

If a breakpoint is not user visible, then there's no point in
bothering the frontend about it...  This is the exact same check MI
does.

I also smoke tested emacs in gud-gdb mode.  I didn't notice anything break.

gdb/
2012-11-21  Pedro Alves  <palves@redhat.com>

	* annotate.c (breakpoint_changed): Skip if breakpoint is not
	user-visible.

gdb/testsuite/
2012-11-21  Pedro Alves  <palves@redhat.com>

	* gdb.base/annota1.exp (signal sent): No longer expect
	breakpoints-invalid.
	* gdb.cp/annota2.exp (continue until exit)
	(watch triggered on a.x): Ditto.
---
 gdb/annotate.c                     |    3 +++
 gdb/testsuite/gdb.base/annota1.exp |    2 +-
 gdb/testsuite/gdb.cp/annota2.exp   |    6 +++---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/annotate.c b/gdb/annotate.c
index aa8b2a6..95d3ead 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -561,6 +561,9 @@ annotate_display_prompt (void)
 static void
 breakpoint_changed (struct breakpoint *b)
 {
+  if (b->number <= 0)
+    return;
+
   annotate_breakpoints_invalid ();
 }
 
diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp
index 75a7130..a16525c 100644
--- a/gdb/testsuite/gdb.base/annota1.exp
+++ b/gdb/testsuite/gdb.base/annota1.exp
@@ -425,7 +425,7 @@ if [target_info exists gdb,nosignals] {
 } else {
     setup_xfail hppa*-*-hpux11*
     gdb_test_multiple "signal SIGTRAP" "signal sent" {
-	-re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n${breakpoints_invalid}\r\n\032\032stopped\r\n$gdb_prompt$" {
+	-re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
 	    pass "signal sent"
 	}
     }
diff --git a/gdb/testsuite/gdb.cp/annota2.exp b/gdb/testsuite/gdb.cp/annota2.exp
index 2a1c4d3..fce5e69 100644
--- a/gdb/testsuite/gdb.cp/annota2.exp
+++ b/gdb/testsuite/gdb.cp/annota2.exp
@@ -102,7 +102,7 @@ gdb_test_multiple "print a" "print class" {
 # `a.x is 1' is asynchronous regarding to `frames-invalid'.
 #
 gdb_test_multiple "continue" "continue until exit" {
-    -re "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)*\r\na.x is 1\r\n\(\r\n\032\032frames-invalid\r\n\)*\r\n\032\032exited 0\r\n$inferior_exited_re normally.\r\n${breakpoints_invalid}\r\n\032\032stopped\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)*\r\na.x is 1\r\n\(\r\n\032\032frames-invalid\r\n\)*\r\n\032\032exited 0\r\n$inferior_exited_re normally.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
 	pass "continue until exit"
     }
 }
@@ -162,10 +162,10 @@ gdb_test_multiple "watch a.x" "set watch on a.x" {
 # annotate-watchpoint
 #
 gdb_test_multiple "next" "watch triggered on a.x" {
-    -re "\r\n\032\032post-prompt\r\n\032\032starting\r\n${frames_invalid}${breakpoints_invalid}\r\n\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n${breakpoints_invalid}.*$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n\032\032starting\r\n${frames_invalid}${breakpoints_invalid}\r\n\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n.*$gdb_prompt$" {
 	pass "watch triggered on a.x"
     }
-    -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n${frames_invalid}\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n${breakpoints_invalid}$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n${frames_invalid}\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
 	kfail "gdb/38" "watch triggered on a.x"
     }
 }

[PATCH 3/4] Fully move the breakpoints-invalid annotation to observers.

[Pedro Alves]

All annotate_breakpoints_changed calls are along-side
observer_notify_breakpoints_changed calls.  All, except the
init_raw_breakpoint one.  But that one is actually wrong.  The
breakpoint is being constructed at that point, and hasn't been placed
on the breakpoint chain yet.  It would be better placed in
install_breakpoint, and I actually started out that way.  But once the
annotate_breakpoints_changed are parallel to the observer calls, we
can fully move annotations to observers too.

One issue is that this changes the order of annotations a bit.
Before, we'd emit the annotation, and after call "mention()" on the
breakpoint (which prints the breakpoint number, etc.).  But, we call
the observers _after_ mention is called, so the annotation output will
change a little:

void
install_breakpoint (int internal, struct breakpoint *b, int update_gll)
{
  add_to_breakpoint_chain (b);
  set_breakpoint_number (internal, b);
  if (is_tracepoint (b))
    set_tracepoint_count (breakpoint_count);
  if (!internal)
    mention (b);
  observer_notify_breakpoint_created (b);

  if (update_gll)
    update_global_location_list (1);
}

I believe this order doesn't really matter (the frontend needs to wait
for the prompt anyway), so I just adjust the expected output in the
tests.  Emacs in annotations mode doesn't seem to complain.  Couple
that with the previous patch that suppressed duplicated annotations,
and, the fact that some annotations calls were actually missing (were
we do have observer calls), more changes to the tests are needed
anyway.

Tested on x86_64 Fedora 17.

gdb/
2012-11-21  Pedro Alves  <palves@redhat.com>

	* annotate.c (annotate_breakpoints_changed): Rename to ...
	(annotate_breakpoints_invalid): ... this.  Make static.
	(breakpoint_changed): Adjust.
	(_initialize_annotate): Always install the observers.  Install a
	"breakpoint_created" observer.
	* annotate.h (annotate_breakpoints_changed): Delete declaration.
	* breakpoint.c (set_breakpoint_condition)
	(breakpoint_set_commands, do_map_commands_command)
	(init_raw_breakpoint, clear_command, set_ignore_count)
	(enable_breakpoint_disp): No longer call
	annotate_breakpoints_changed.

gdb/testsuite/
2012-11-21  Pedro Alves  <palves@redhat.com>

	* gdb.base/annota1.exp (breakpoints_invalid): New variable.
	Adjust tests to breakpoints-invalid changes.
	* gdb.cp/annota2.exp (breakpoints_invalid, frames_invalid): New
	variables.
	Adjust tests to breakpoints-invalid changes.
---
 gdb/annotate.c                     |   15 +++++++--------
 gdb/annotate.h                     |    2 --
 gdb/breakpoint.c                   |   10 +---------
 gdb/testsuite/gdb.base/annota1.exp |   19 ++++++++++---------
 gdb/testsuite/gdb.cp/annota2.exp   |   15 +++++++++------
 5 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/gdb/annotate.c b/gdb/annotate.c
index 3c7acb9..aa8b2a6 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -53,8 +53,9 @@ print_value_flags (struct type *t)
     printf_filtered (("-"));
 }
 \f
-void
-annotate_breakpoints_changed (void)
+
+static void
+annotate_breakpoints_invalid ()
 {
   if (annotation_level == 2 && !breakpoints_invalid_emitted)
     {
@@ -560,15 +561,13 @@ annotate_display_prompt (void)
 static void
 breakpoint_changed (struct breakpoint *b)
 {
-  annotate_breakpoints_changed ();
+  annotate_breakpoints_invalid ();
 }
 
 void
 _initialize_annotate (void)
 {
-  if (annotation_level == 2)
-    {
-      observer_attach_breakpoint_deleted (breakpoint_changed);
-      observer_attach_breakpoint_modified (breakpoint_changed);
-    }
+  observer_attach_breakpoint_created (breakpoint_changed);
+  observer_attach_breakpoint_deleted (breakpoint_changed);
+  observer_attach_breakpoint_modified (breakpoint_changed);
 }
diff --git a/gdb/annotate.h b/gdb/annotate.h
index 15064b1..87f6077 100644
--- a/gdb/annotate.h
+++ b/gdb/annotate.h
@@ -20,8 +20,6 @@
 #include "symtab.h"
 #include "gdbtypes.h"
 
-extern void annotate_breakpoints_changed (void);
-
 extern void annotate_breakpoint (int);
 extern void annotate_catchpoint (int);
 extern void annotate_watchpoint (int);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0003240..7ba4a58 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -983,7 +983,6 @@ set_breakpoint_condition (struct breakpoint *b, char *exp,
     }
   mark_breakpoint_modified (b);
 
-  annotate_breakpoints_changed ();
   observer_notify_breakpoint_modified (b);
 }
 
@@ -1215,7 +1214,6 @@ breakpoint_set_commands (struct breakpoint *b,
 
   decref_counted_command_line (&b->commands);
   b->commands = alloc_counted_command_line (commands);
-  annotate_breakpoints_changed ();
   observer_notify_breakpoint_modified (b);
 }
 
@@ -1332,7 +1330,6 @@ do_map_commands_command (struct breakpoint *b, void *data)
       incref_counted_command_line (info->cmd);
       decref_counted_command_line (&b->commands);
       b->commands = info->cmd;
-      annotate_breakpoints_changed ();
       observer_notify_breakpoint_modified (b);
     }
 }
@@ -6995,8 +6992,6 @@ init_raw_breakpoint (struct breakpoint *b, struct gdbarch *gdbarch,
      program space.  */
   if (bptype != bp_breakpoint && bptype != bp_hardware_breakpoint)
     b->pspace = sal.pspace;
-
-  annotate_breakpoints_changed ();
 }
 
 /* set_raw_breakpoint is a low level routine for allocating and
@@ -11941,7 +11936,6 @@ clear_command (char *arg, int from_tty)
       else
 	printf_unfiltered (_("Deleted breakpoints "));
     }
-  annotate_breakpoints_changed ();
 
   for (ix = 0; VEC_iterate(breakpoint_p, found, ix, b); ix++)
     {
@@ -14319,7 +14313,6 @@ set_ignore_count (int bptnum, int count, int from_tty)
 			       "crossings of breakpoint %d."),
 			     count, bptnum);
 	}
-      annotate_breakpoints_changed ();
       observer_notify_breakpoint_modified (b);
       return;
     }
@@ -14584,8 +14577,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
   bpt->disposition = disposition;
   bpt->enable_count = count;
   update_global_location_list (1);
-  annotate_breakpoints_changed ();
-  
+
   observer_notify_breakpoint_modified (bpt);
 }
 
diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp
index 92a9674..75a7130 100644
--- a/gdb/testsuite/gdb.base/annota1.exp
+++ b/gdb/testsuite/gdb.base/annota1.exp
@@ -25,6 +25,7 @@ if [is_remote target] then {
     return 0
 }
 
+set breakpoints_invalid "\r\n\032\032breakpoints-invalid\r\n"
 
 #
 # test running programs
@@ -190,7 +191,7 @@ gdb_test_multiple "print non_existent_value" "print non_existent_value" {
 # we can test annotate-signal-handler-caller
 #
 gdb_test_multiple "break handle_USR1" "break handle_USR1" {
-    -re  "\r\n\032\032post-prompt\r\n\r\n\032\032breakpoints-invalid\r\nBreakpoint.*at $hex: file.*$srcfile, line.*\r\n$gdb_prompt$" {
+    -re  "\r\n\032\032post-prompt\r\nBreakpoint.*at $hex: file.*$srcfile, line.*\r\n\032\032breakpoints-invalid\r\n.*$gdb_prompt$" {
 	pass "break handle_USR1"
     }
 }
@@ -199,7 +200,7 @@ gdb_test_multiple "break handle_USR1" "break handle_USR1" {
 # break at printf. When we are stopped at printf, we can test 
 #
 gdb_test_multiple "break printf" "break printf" {
-    -re  "\r\n\032\032post-prompt\r\n\r\n\032\032breakpoints-invalid\r\nBreakpoint.*at $hex.*$gdb_prompt$" {
+    -re  "\r\n\032\032post-prompt\r\nBreakpoint.*at $hex.*\032\032breakpoints-invalid\r\n.*$gdb_prompt$" {
 	pass "break printf" 
     }
     -re  "\r\n\032\032post-prompt\r\nwarning: Breakpoint address adjusted from $hex to $hex.\r\n\r\n\032\032breakpoints-invalid\r\nBreakpoint.*at $hex.*$gdb_prompt$" {
@@ -210,7 +211,7 @@ gdb_test_multiple "break printf" "break printf" {
 #
 # get to printf
 #
-set pat_begin "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n"
+set pat_begin "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\n${breakpoints_invalid}"
 set pat_adjust "warning: Breakpoint 3 address previously adjusted from $hex to $hex.\r\n"
 set pat_end "\r\n\032\032breakpoint 3\r\n\r\nBreakpoint 3, \r\n\032\032frame-begin 0 $hex\r\n\r\n(\032\032frame-address\r\n$hex\r\n\032\032frame-address-end\r\n in \r\n)*.*\032\032frame-function-name\r\n.*printf(@.*)?\r\n\032\032frame-args\r\n.*\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$"
 
@@ -279,19 +280,19 @@ if [target_info exists gdb,nosignals] {
 # delete all the breakpoints
 #
 gdb_test_multiple "delete 1" "delete bp 1" {
-    -re "\r\n\032\032post-prompt\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n${breakpoints_invalid}$gdb_prompt$" {
 	pass "delete bp 1"
     }
 }
 
 gdb_test_multiple "delete 2" "delete bp 2" {
-    -re "\r\n\032\032post-prompt\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n${breakpoints_invalid}$gdb_prompt$" {
 	pass "delete bp 2"
     }
 }
 
 gdb_test_multiple "delete 3" "delete bp 3" {
-    -re "\r\n\032\032post-prompt\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n${breakpoints_invalid}$gdb_prompt$" {
 	pass "delete bp 3"
     }
 }
@@ -301,10 +302,10 @@ gdb_test_multiple "delete 3" "delete bp 3" {
 # to test the annotate output for the display command.
 #
 gdb_test_multiple "break main" "break at main" {
-    -re "post-prompt.*\032\032breakpoints-invalid.*Breakpoint 4 at $hex: file ${escapedsrcfile}, line $main_line.*$gdb_prompt$" {
+    -re "post-prompt.*Breakpoint 4 at $hex: file ${escapedsrcfile}, line $main_line.*\032\032breakpoints-invalid.*$gdb_prompt$" {
 	pass "break at main"
     }
-    -re "post-prompt.*\032\032breakpoints-invalid.*Breakpoint 4 at $hex: file .*${srcfile}, line $main_line.*$gdb_prompt$" {
+    -re "post-prompt.*Breakpoint 4 at $hex: file .*${srcfile}, line $main_line.*\032\032breakpoints-invalid.*$gdb_prompt$" {
 	setup_xfail "*-*-*" 1270
 	fail "break at main"
     }
@@ -424,7 +425,7 @@ if [target_info exists gdb,nosignals] {
 } else {
     setup_xfail hppa*-*-hpux11*
     gdb_test_multiple "signal SIGTRAP" "signal sent" {
-	-re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+	-re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n${breakpoints_invalid}\r\n\032\032stopped\r\n$gdb_prompt$" {
 	    pass "signal sent"
 	}
     }
diff --git a/gdb/testsuite/gdb.cp/annota2.exp b/gdb/testsuite/gdb.cp/annota2.exp
index 9a54fac..2a1c4d3 100644
--- a/gdb/testsuite/gdb.cp/annota2.exp
+++ b/gdb/testsuite/gdb.cp/annota2.exp
@@ -38,6 +38,9 @@ if [is_remote target] then {
     return 0
 }
 
+set breakpoints_invalid "\r\n\032\032breakpoints-invalid\r\n"
+set frames_invalid "\r\n\032\032frames-invalid\r\n"
+
 #
 # line number where we need to stop in main
 #
@@ -99,7 +102,7 @@ gdb_test_multiple "print a" "print class" {
 # `a.x is 1' is asynchronous regarding to `frames-invalid'.
 #
 gdb_test_multiple "continue" "continue until exit" {
-    -re "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)*\r\na.x is 1\r\n\(\r\n\032\032frames-invalid\r\n\)*\r\n\032\032exited 0\r\n$inferior_exited_re normally.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)*\r\na.x is 1\r\n\(\r\n\032\032frames-invalid\r\n\)*\r\n\032\032exited 0\r\n$inferior_exited_re normally.\r\n${breakpoints_invalid}\r\n\032\032stopped\r\n$gdb_prompt$" {
 	pass "continue until exit"
     }
 }
@@ -112,7 +115,7 @@ gdb_expect {
   -re ".*Delete all breakpoints. \\(y or n\\) \r\n\032\032query.*$" {
            send_gdb "y\n"
            gdb_expect {
-	     -re "\r\n\032\032post-query\r\n$gdb_prompt$" { pass "delete bps" }
+	     -re "\r\n\032\032post-query\r\n${breakpoints_invalid}$gdb_prompt$" { pass "delete bps" }
 	     -re ".*$gdb_prompt$" { fail "delete bps" }
 	     timeout  { fail "delete bps (timeout)" }
 	 }
@@ -125,7 +128,7 @@ gdb_expect {
 # break at first line of main. 
 #
 gdb_test_multiple "break 22" "break at main" {
-    -re  "\r\n\032\032post-prompt\r\n\r\n\032\032breakpoints-invalid\r\nBreakpoint.*at $hex: file.*$srcfile, line.*\r\n$gdb_prompt$" {
+    -re  "\r\n\032\032post-prompt\r\nBreakpoint.*at $hex: file.*$srcfile, line.*\r\n\032\032breakpoints-invalid.*\r\n$gdb_prompt$" {
 	pass "breakpoint at main"
     }
 }
@@ -149,7 +152,7 @@ gdb_test_multiple "run" "run until main breakpoint" {
 # set up a watch point on a.x 
 #
 gdb_test_multiple "watch a.x" "set watch on a.x" {
-    -re "\r\n\032\032post-prompt\r\n\r\n\032\032breakpoints-invalid\r\n.*atchpoint 3: a.x\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n.*atchpoint 3: a.x.*\r\n\032\032breakpoints-invalid\r\n.*$gdb_prompt$" {
 	pass "set watch on a.x"
     }
 }
@@ -159,10 +162,10 @@ gdb_test_multiple "watch a.x" "set watch on a.x" {
 # annotate-watchpoint
 #
 gdb_test_multiple "next" "watch triggered on a.x" {
-    -re "\r\n\032\032post-prompt\r\n\r\n(\032\032breakpoints-invalid\r\n\r\n)*\032\032starting\r\n\r\n(\032\032frames-invalid\r\n\r\n)*\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n.*$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n\032\032starting\r\n${frames_invalid}${breakpoints_invalid}\r\n\032\032watchpoint 3\r\n.*atchpoint 3: a.x\r\n\r\nOld value = 0\r\nNew value = 1\r\n\r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*$srcfile\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$decimal\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n${breakpoints_invalid}.*$gdb_prompt$" {
 	pass "watch triggered on a.x"
     }
-    -re "\r\n\032\032post-prompt\r\n\r\n(\032\032breakpoints-invalid\r\n\r\n)*\032\032starting\r\n\r\n(\032\032frames-invalid\r\n\r\n)*\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\n\r\n\032\032starting\r\n${frames_invalid}\r\n\032\032source .*$srcfile.*beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped\r\n${breakpoints_invalid}$gdb_prompt$" {
 	kfail "gdb/38" "watch triggered on a.x"
     }
 }

[PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Pedro Alves]

With some changes to how software single-step (SSS) breakpoints are
handled, one of those being to place SSS breakpoints on the breakpoint
chain as all other breakpoints, annota1.exp times out with lots and
lots of breakpoint-invalid and frame-changed annotations.  All those
extra annotations are actually unnecessary.  For one, SSS breakpoints
are internal breakpoints, so the frontend shouldn't care if they were
added, removed or changed.  Then, there's really no point in emitting
"breakpoints-invalid" or "frames-invalid" more than once between times
the frontend/user can actually issues GDB commands; the frontend will
have to wait for the GDB prompt to refresh its state, so emitting
those annotations at most once between prompts is enough.  Non-stop or
async would complicate this, but no frontend will be using annotations
in those modes (one of goes of emacs switching to MI was non-stop mode
support, AFAIK).  The previous patch reveals there has been an
intention in the past to suppress multiple breakpoints-invalid
annotations caused by ignore count changes.  As the previous patch
shows, that's always been broken, but in any case, this patch actually
makes it work.  The next patch will remove several annotation-specific
calls in breakpoint.c in favor of always using the breakpoint modified
& friends observers, and that causes yet more of these annotations,
because several calls to the corresponding annotate_* functions in
breakpoint.c are missing, particularly in newer code.

So all in all, here's a simple mechanism that avoids sending the same
annotation to the frontend more than once until gdb is ready to accept
further commands.

Tested on x86_64 Fedora 17.

2012-11-21  Pedro Alves  <palves@redhat.com>

	* annotate.c (frames_invalid_emitted)
	(breakpoints_invalid_emitted): New globals.
	(annotate_breakpoints_changed, annotate_frames_invalid): Skip
	emitting the annotation if it has already been emitted.
	(annotate_display_prompt): New function.
	* annotate.h (annotate_display_prompt): New declaration.
	* event-top.c: Include annotate.h.
	(display_gdb_prompt): Call annotate_display_prompt.
---
 gdb/annotate.c  |   23 +++++++++++++++++++++--
 gdb/annotate.h  |    2 ++
 gdb/event-top.c |    3 +++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/gdb/annotate.c b/gdb/annotate.c
index a222736..3c7acb9 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -38,6 +38,12 @@ static void breakpoint_changed (struct breakpoint *b);
 void (*deprecated_annotate_signalled_hook) (void);
 void (*deprecated_annotate_signal_hook) (void);
 
+/* Booleans indicating whether we've emitted certain notifications.
+   Used to suppress useless repeated notifications until the next time
+   we're ready to accept more commands.  */
+static int frames_invalid_emitted;
+static int breakpoints_invalid_emitted;
+
 static void
 print_value_flags (struct type *t)
 {
@@ -50,10 +56,11 @@ print_value_flags (struct type *t)
 void
 annotate_breakpoints_changed (void)
 {
-  if (annotation_level == 2)
+  if (annotation_level == 2 && !breakpoints_invalid_emitted)
     {
       target_terminal_ours ();
       printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
+      breakpoints_invalid_emitted = 1;
     }
 }
 
@@ -185,10 +192,11 @@ annotate_breakpoints_table_end (void)
 void
 annotate_frames_invalid (void)
 {
-  if (annotation_level == 2)
+  if (annotation_level == 2 && !frames_invalid_emitted)
     {
       target_terminal_ours ();
       printf_unfiltered (("\n\032\032frames-invalid\n"));
+      frames_invalid_emitted = 1;
     }
 }
 
@@ -538,6 +546,17 @@ annotate_array_section_end (void)
     printf_filtered (("\n\032\032array-section-end\n"));
 }
 
+/* Called when GDB is about to display the prompt.  Used to reset
+   annotation suppression whenever we're ready to accept new
+   frontend/user commands.  */
+
+void
+annotate_display_prompt (void)
+{
+  frames_invalid_emitted = 0;
+  breakpoints_invalid_emitted = 0;
+}
+
 static void
 breakpoint_changed (struct breakpoint *b)
 {
diff --git a/gdb/annotate.h b/gdb/annotate.h
index 443d892..15064b1 100644
--- a/gdb/annotate.h
+++ b/gdb/annotate.h
@@ -45,6 +45,8 @@ extern void annotate_frames_invalid (void);
 extern void annotate_new_thread (void);
 extern void annotate_thread_changed (void);
 
+extern void annotate_display_prompt (void);
+
 struct type;
 
 extern void annotate_field_begin (struct type *);
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 5c533e4..2f0bbf7 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -36,6 +36,7 @@
 #include "observer.h"
 #include "continuations.h"
 #include "gdbcmd.h"		/* for dont_repeat() */
+#include "annotate.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -232,6 +233,8 @@ display_gdb_prompt (char *new_prompt)
   char *actual_gdb_prompt = NULL;
   struct cleanup *old_chain;
 
+  annotate_display_prompt ();
+
   /* Reset the nesting depth used when trace-commands is set.  */
   reset_command_nest_depth ();
 

[PATCH 1/4] Remove (alleged) "breakpoints-changed" annotation suppression on ignore count changes.

[Pedro Alves]

There's code in annotate.c and breakpoint.c that is supposed to
suppress multiple breakpoints-invalid annotations when the ignore
count of a breakpoint changes, up until the target actually stops.

But, the code is bogus:

void
annotate_breakpoints_changed (void)
{
  if (annotation_level == 2)
    {
      target_terminal_ours ();
      printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
      if (ignore_count_changed)
	ignore_count_changed = 0;   /* Avoid multiple break annotations.  */
    }
}

The "ignore_count_changed" flag isn't actually guarding the output of
the annotation at all.  It would have been better written something
like:

void
annotate_breakpoints_changed (void)
{
  if (annotation_level == 2 && !ignore_count_changed)
    {
      target_terminal_ours ();
      printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
      ignore_count_changed = 0;   /* Avoid multiple break annotations.  */
    }
}

but, it wasn't.  AFAICS, that goes all the way back to the original
patch'es submission and check in, at
<http://sourceware.org/ml/gdb-patches/1999-q4/msg00106.html>.  I
looked a tar of HP's wdb from 1999, and even though that contains
local changes in the annotate code, this suppression seems borked
there too to me.

The original patch added a test to supposedly exercise this
suppression, but, it actually doesn't.  It merely tests that
"breakpoints-invalid" is output after "stopped", but doesn't check
whether the duplicates supression actually works (IOW, check that only
_one_ annotation is seen).  I was going to simply delete the tests
too, but a following patch will eliminate the duplicates in a
different way (which I needed for a different reason), so instead, I'm
making the tests actually fail if a duplicate annotation is seen.

Worry not, the test doesn't actually fail!  The reason is that
breakpoint.c does:

      else if (b->ignore_count > 0)
	{
	  b->ignore_count--;
	  annotate_ignore_count_change ();
	  bs->stop = 0;
	  /* Increase the hit count even though we don't stop.  */
	  ++(b->hit_count);
	  observer_notify_breakpoint_modified (b);
	}

where the annotate_ignore_count_change call is meant to inform the
"breakpoint_modified" annotation observer to ignore the notification.
All sounds good.  But, the trouble is that nowadays annotate.c only
installs the observers if GDB is started with annotations enabled with
a command line option (gdb --annotate=2):

void
_initialize_annotate (void)
{
  if (annotation_level == 2)
    {
      observer_attach_breakpoint_deleted (breakpoint_changed);
      observer_attach_breakpoint_modified (breakpoint_changed);
    }
}

and annota1.exp, to enable annotations, starts GDB normally, and
afterwards does "set annotate 2", so the observers aren't installed
when annota1.exp is run, and therefore changing the ignore count isn't
triggering any annotation at all...

gdb/
2012-11-21  Pedro Alves  <palves@redhat.com>

	* annotate.c (ignore_count_changed): Delete.
	(annotate_breakpoints_changed): Don't clear ignore_count_changed.
	(annotate_ignore_count_change): Delete.
	(annotate_stopped): Don't emit a delayed breakpoints-changed
	annotation.
	* annotate.h (annotate_ignore_count_change): Delete.
	* breakpoint.c (bpstat_check_breakpoint_conditions): Don't call
	annotate_ignore_count_change.

gdb/testsuite/
2012-11-21  Pedro Alves  <palves@redhat.com>

	* gdb.base/annota1.exp (annotate ignore count change): Add
	expected output for failure case.
---
 gdb/annotate.c                     |   22 ----------------------
 gdb/annotate.h                     |    1 -
 gdb/breakpoint.c                   |    1 -
 gdb/testsuite/gdb.base/annota1.exp |    5 ++++-
 4 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/gdb/annotate.c b/gdb/annotate.c
index 96902cb..a222736 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -38,8 +38,6 @@ static void breakpoint_changed (struct breakpoint *b);
 void (*deprecated_annotate_signalled_hook) (void);
 void (*deprecated_annotate_signal_hook) (void);
 
-static int ignore_count_changed = 0;
-
 static void
 print_value_flags (struct type *t)
 {
@@ -56,24 +54,9 @@ annotate_breakpoints_changed (void)
     {
       target_terminal_ours ();
       printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
-      if (ignore_count_changed)
-	ignore_count_changed = 0;   /* Avoid multiple break annotations.  */
     }
 }
 
-/* The GUI needs to be informed of ignore_count changes, but we don't
-   want to provide successive multiple breakpoints-invalid messages
-   that are all caused by the fact that the ignore count is changing
-   (which could keep the GUI very busy).  One is enough, after the
-   target actually "stops".  */
-
-void
-annotate_ignore_count_change (void)
-{
-  if (annotation_level > 1)
-    ignore_count_changed = 1;
-}
-
 void
 annotate_breakpoint (int num)
 {
@@ -107,11 +90,6 @@ annotate_stopped (void)
 {
   if (annotation_level > 1)
     printf_filtered (("\n\032\032stopped\n"));
-  if (annotation_level > 1 && ignore_count_changed)
-    {
-      ignore_count_changed = 0;
-      annotate_breakpoints_changed ();
-    }
 }
 
 void
diff --git a/gdb/annotate.h b/gdb/annotate.h
index 0f7b320..443d892 100644
--- a/gdb/annotate.h
+++ b/gdb/annotate.h
@@ -22,7 +22,6 @@
 
 extern void annotate_breakpoints_changed (void);
 
-extern void annotate_ignore_count_change (void);
 extern void annotate_breakpoint (int);
 extern void annotate_catchpoint (int);
 extern void annotate_watchpoint (int);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 5749fa7..0003240 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5112,7 +5112,6 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
       else if (b->ignore_count > 0)
 	{
 	  b->ignore_count--;
-	  annotate_ignore_count_change ();
 	  bs->stop = 0;
 	  /* Increase the hit count even though we don't stop.  */
 	  ++(b->hit_count);
diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp
index 98b4650..92a9674 100644
--- a/gdb/testsuite/gdb.base/annota1.exp
+++ b/gdb/testsuite/gdb.base/annota1.exp
@@ -365,7 +365,10 @@ gdb_test_multiple "ignore 5 4" "ignore 5 4" {
 }
 
 gdb_test_multiple "continue" "annotate ignore count change" {
-    -re ".*$srcfile:$value_inc_line:.*\032\032stopped\r\n\r\n\032\032breakpoints-invalid\r\n$gdb_prompt$" {
+    -re ".*breakpoints-invalid.*breakpoints-invalid.*$gdb_prompt$" {
+	fail "annotate ignore count change"
+    }
+    -re ".*$srcfile:$value_inc_line:.*\032\032stopped\r\n$gdb_prompt$" {
 	pass "annotate ignore count change"
     }
 }

[PATCH 0/4] Misc annotations changes.

[Pedro Alves]

On 11/21/2012 03:42 PM, Pedro Alves wrote:
> The reason I'm looking at annotation code is that I made a change
> that puts software single-step breakpoints in the regular breakpoint
> chain, and then that triggers breakpoints-invalid annotations for
> each sss breakpoint that didn't happen before, and in turn that
> leads to the gdb.log getting filled up with useless notifications
> and the tests timing out.  I had a small patch to handle this, but it turns
> out incomplete.  Further looking at this issue, I've decided to go ahead
> and fully convert the breakpoints-invalid annotation to observers.

And there are the changes.  The changes are sort of independent, but
the each builds on the previous.  This order was chosen to avoid
introducing failures in the tree at each stage.

I've tested this on X86_64 Fedora 17, and smoke tested it with emacs
24.1.1 in gud-gdb mode.

Let me know if you see an issue with this.

---

Pedro Alves (4):
      Remove (alledged) "breakpoints-changed" annotation suppression on ignore count changes.
      Suppress repeated annotations until GDB is ready to accept input.
      Fully move the breakpoints-invalid annotation to observers.
      Skip breakpoints-invalid annotations for internal|momentary breakpoints.


 gdb/annotate.c                     |   61 ++++++++++++++++++------------------
 gdb/annotate.h                     |    5 +--
 gdb/breakpoint.c                   |   11 +-----
 gdb/event-top.c                    |    3 ++
 gdb/testsuite/gdb.base/annota1.exp |   22 ++++++++-----
 gdb/testsuite/gdb.cp/annota2.exp   |   13 +++++---
 6 files changed, 57 insertions(+), 58 deletions(-)

-- 
Pedro Alves

Re: [PATCH 3/4] Fully move the breakpoints-invalid annotation to observers.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> 2012-11-21  Pedro Alves  <palves@redhat.com>

Pedro> 	* annotate.c (annotate_breakpoints_changed): Rename to ...
Pedro> 	(annotate_breakpoints_invalid): ... this.  Make static.

Pedro> +static void
Pedro> +annotate_breakpoints_invalid ()

I think it should read '(void)'.

Tom

Re: [PATCH 0/4] Misc annotations changes.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Let me know if you see an issue with this.

No issues, I think this approach is clearly better.

Starting with Emacs 24, only --fullname is used.  So, all the level >= 2
annotations will be dead at some point.

Tom

Re: [PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Non-stop or async would complicate this, but no frontend will be
Pedro> using annotations in those modes (one of goes of emacs switching
Pedro> to MI was non-stop mode support, AFAIK).

I've used these modes in Emacs (using M-x gud-gdb) before, and it has
worked ok-enough.  Can you say if these patches make it noticeably
worse, or will it be pretty much like it was before?

Tom

Re: [PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Pedro Alves]

On 11/26/2012 03:50 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Non-stop or async would complicate this, but no frontend will be
> Pedro> using annotations in those modes (one of goes of emacs switching
> Pedro> to MI was non-stop mode support, AFAIK).
> 
> I've used these modes in Emacs (using M-x gud-gdb) before, and it has
> worked ok-enough.  Can you say if these patches make it noticeably
> worse, or will it be pretty much like it was before?

Hmm.  There's potential for making it worse.  The patch suppresses duplicate
annotations until the next prompt is displayed.  With background commands,
events can be reported without re-displaying a prompt [*].
In that case, emacs might miss annotations.

With this version, we don't do annotation suppression if a background
command is in progress.  This means that

 define twobreaks
 >   b foo
 >   b bar
 >end
 twobreaks

triggers two annotations in async mode if the target is not running, or
isn't running the foreground, while always just one in sync mode.  I don't
think that's a real problem.

If it turns out more complicated than this, I can just drop this one.
This is most useful for the case of forcing single-stepping for
everything (even "continue").  The same as installing a software watchpoint,
but without the software watchpoint.   That triggers a huge number of
-frames-invalid annotations.  I could work around that in the
tests themselves.  The repeated breakpoints-invalid annotations for
sss breakpoints go away with patch 4.

[*] - though that could itself be considered a bug, the CLI output is less than
ideal here.  We should be able to keep the bottom line reserved for the prompt,
and scroll the rest of the output without visually interfering with the prompt line.
E.g., we could be able to unwind the cursor to column 0, print whatever while handling
the event, and then redisplay the prompt as it was, without the user noticing.  Or
perhaps there's cleaner ways even.

Is there any way to force emacs 24 to do full annotations?  It actually
hadn't realized that "--fullname" was annotate=1, not 2, so my previous
emacs testing was useless, as these notifications only happen with
annotate=2...

-- 
Pedro Alves

2012-11-26  Pedro Alves  <palves@redhat.com>

	* annotate.c: Include "inferior.h".
	(frames_invalid_emitted)
	(breakpoints_invalid_emitted): New globals.
	(async_background_execution_p): New function.
	(annotate_breakpoints_changed, annotate_frames_invalid): Skip
	emitting the annotation if it has already been emitted.
	(annotate_display_prompt): New function.
	* annotate.h (annotate_display_prompt): New declaration.
	* event-top.c: Include annotate.h.
	(display_gdb_prompt): Call annotate_display_prompt.
---

 gdb/annotate.c  |   40 ++++++++++++++++++++++++++++++++++++++--
 gdb/annotate.h  |    2 ++
 gdb/event-top.c |    3 +++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/gdb/annotate.c b/gdb/annotate.c
index a222736..a3b70da 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -24,6 +24,7 @@
 #include "gdbtypes.h"
 #include "breakpoint.h"
 #include "observer.h"
+#include "inferior.h"
 \f

 /* Prototypes for local functions.  */
@@ -38,6 +39,23 @@ static void breakpoint_changed (struct breakpoint *b);
 void (*deprecated_annotate_signalled_hook) (void);
 void (*deprecated_annotate_signal_hook) (void);

+/* Booleans indicating whether we've emitted certain notifications.
+   Used to suppress useless repeated notifications until the next time
+   we're ready to accept more commands.  Reset whenever a prompt is
+   displayed.  */
+static int frames_invalid_emitted;
+static int breakpoints_invalid_emitted;
+
+/* True if the target can async, and a synchronous execution command
+   is not in progress.  If true, input is accepted, so don't suppress
+   annotations.  */
+
+static int
+async_background_execution_p (void)
+{
+  return (target_can_async_p () && !sync_execution);
+}
+
 static void
 print_value_flags (struct type *t)
 {
@@ -47,13 +65,17 @@ print_value_flags (struct type *t)
     printf_filtered (("-"));
 }
 \f
+
 void
 annotate_breakpoints_changed (void)
 {
-  if (annotation_level == 2)
+  if (annotation_level == 2
+      && (!breakpoints_invalid_emitted
+	  || async_background_execution_p ()))
     {
       target_terminal_ours ();
       printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
+      breakpoints_invalid_emitted = 1;
     }
 }

@@ -185,10 +207,13 @@ annotate_breakpoints_table_end (void)
 void
 annotate_frames_invalid (void)
 {
-  if (annotation_level == 2)
+  if (annotation_level == 2
+      && (!frames_invalid_emitted
+	  || async_background_execution_p ()))
     {
       target_terminal_ours ();
       printf_unfiltered (("\n\032\032frames-invalid\n"));
+      frames_invalid_emitted = 1;
     }
 }

@@ -538,6 +563,17 @@ annotate_array_section_end (void)
     printf_filtered (("\n\032\032array-section-end\n"));
 }

+/* Called when GDB is about to display the prompt.  Used to reset
+   annotation suppression whenever we're ready to accept new
+   frontend/user commands.  */
+
+void
+annotate_display_prompt (void)
+{
+  frames_invalid_emitted = 0;
+  breakpoints_invalid_emitted = 0;
+}
+
 static void
 breakpoint_changed (struct breakpoint *b)
 {
diff --git a/gdb/annotate.h b/gdb/annotate.h
index 443d892..15064b1 100644
--- a/gdb/annotate.h
+++ b/gdb/annotate.h
@@ -45,6 +45,8 @@ extern void annotate_frames_invalid (void);
 extern void annotate_new_thread (void);
 extern void annotate_thread_changed (void);

+extern void annotate_display_prompt (void);
+
 struct type;

 extern void annotate_field_begin (struct type *);
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 5c533e4..2f0bbf7 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -36,6 +36,7 @@
 #include "observer.h"
 #include "continuations.h"
 #include "gdbcmd.h"		/* for dont_repeat() */
+#include "annotate.h"

 /* readline include files.  */
 #include "readline/readline.h"
@@ -232,6 +233,8 @@ display_gdb_prompt (char *new_prompt)
   char *actual_gdb_prompt = NULL;
   struct cleanup *old_chain;

+  annotate_display_prompt ();
+
   /* Reset the nesting depth used when trace-commands is set.  */
   reset_command_nest_depth ();

Re: [PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Hmm.  There's potential for making it worse.  The patch
Pedro> suppresses duplicate annotations until the next prompt is
Pedro> displayed.  With background commands, events can be reported
Pedro> without re-displaying a prompt [*].  In that case, emacs might
Pedro> miss annotations.

FWIW, I am not sure this is so very bad.
Async is still an advanced user thing.
And, if Emacs gets out of sync it is simple to do "up;down" or the like
to get back in sync.

Pedro> With this version, we don't do annotation suppression if a background
Pedro> command is in progress.  This means that

Pedro>  define twobreaks
Pedro> b foo
Pedro> b bar
Pedro> end
Pedro>  twobreaks

Pedro> triggers two annotations in async mode if the target is not
Pedro> running, or isn't running the foreground, while always just one
Pedro> in sync mode.  I don't think that's a real problem.

Me neither.  It isn't even clear Emacs does anything with those
annotations at all.

Pedro> [*] - though that could itself be considered a bug, the CLI
Pedro> output is less than ideal here.  We should be able to keep the
Pedro> bottom line reserved for the prompt, and scroll the rest of the
Pedro> output without visually interfering with the prompt line.  E.g.,
Pedro> we could be able to unwind the cursor to column 0, print whatever
Pedro> while handling the event, and then redisplay the prompt as it
Pedro> was, without the user noticing.  Or perhaps there's cleaner ways
Pedro> even.

Do you mean, do this in Emacs or for the CLI itself?
Either way I think it would be nice.  I've often wished we could replace
the "Reading..." messages, which are often illegible, with some kind of
nicer hash progress meter thing ...

Pedro> Is there any way to force emacs 24 to do full annotations?  It actually
Pedro> hadn't realized that "--fullname" was annotate=1, not 2, so my previous
Pedro> emacs testing was useless, as these notifications only happen with
Pedro> annotate=2...

I don't think there is.  When I tried just editing the gdb command line
I ended up with a lot of "^Z^Z" in the gud buffer.  I guess you could
dig up Emacs 23 (still in F16...) or try to downgrade the gud code.

Tom

Re: [PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Eli Zaretskii]

> Date: Mon, 26 Nov 2012 18:28:12 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> Is there any way to force emacs 24 to do full annotations?

Emacs 24 switched to MI, and keeps the old code that uses annotations
only as fallback and for backward compatibility.

Re: [PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Pedro Alves]

On 11/27/2012 06:48 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> Pedro> [*] - though that could itself be considered a bug, the CLI
> Pedro> output is less than ideal here.  We should be able to keep the
> Pedro> bottom line reserved for the prompt, and scroll the rest of the
> Pedro> output without visually interfering with the prompt line.  E.g.,
> Pedro> we could be able to unwind the cursor to column 0, print whatever
> Pedro> while handling the event, and then redisplay the prompt as it
> Pedro> was, without the user noticing.  Or perhaps there's cleaner ways
> Pedro> even.
> 
> Do you mean, do this in Emacs or for the CLI itself?
> Either way I think it would be nice.

For the CLI itself.  I found an old prototype patch I had that still works.

Try e.g., a test program that has several threads, debug with target-async/non-stop
on, and do "interrupt -a".  With mainline, you get:

(gdb) c -a&
Continuing.
(gdb) [New Thread 0x7ffff7fd0700 (LWP 19273)]
[New Thread 0x7ffff77cf700 (LWP 19274)]
interrupt -a
(gdb)
[Thread 0x7ffff7fd1740 (LWP 19263)] #1 stopped.
0x0000003d26a08e60 in pthread_join (threadid=140737353942784, thread_return=0x7fffffffdb90) at pthread_join.c:93
93          lll_wait_tid (pd->tid);

[Thread 0x7ffff77cf700 (LWP 19274)] #3 stopped.
0x0000003d25eba6ed in nanosleep () at ../sysdeps/unix/syscall-template.S:82
82      T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)

[Thread 0x7ffff7fd0700 (LWP 19273)] #2 stopped.
0x0000003d25eba6ed in nanosleep () at ../sysdeps/unix/syscall-template.S:82
82      T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)


With the patch, you get:

(gdb) c -a&
Continuing.
[New Thread 0x7ffff7fd0700 (LWP 22367)]
During symbol reading, cannot get low and high bounds for subprogram DIE at 29519.
During symbol reading, Child DIE 0x7885 and its abstract origin 0x734f have different parents.
During symbol reading, Child DIE 0x7a01 and its abstract origin 0x7801 have different tags.
[New Thread 0x7ffff77cf700 (LWP 22368)]
(gdb) interrupt -a

[Thread 0x7ffff7fd0700 (LWP 22367)] #2 stopped.
0x0000003d25eba6ed in nanosleep () at ../sysdeps/unix/syscall-template.S:82
82      T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)

[Thread 0x7ffff77cf700 (LWP 22368)] #3 stopped.
0x0000003d25eba6ed in nanosleep () at ../sysdeps/unix/syscall-template.S:82
82      T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)

[Thread 0x7ffff7fd1740 (LWP 22357)] #1 stopped.
0x0000003d26a08e60 in pthread_join (threadid=140737353942784, thread_return=0x7fffffffdb90) at pthread_join.c:93
93          lll_wait_tid (pd->tid);
(gdb)

The difference above is in where "(gdb)" appears.  The latter is much neater.
But you really have to try it to and see for yourself in action.

The issue with this implementation is that it still flickers the prompt.
Not noticeable usually, unless you have a stream of gdb output while
the prompt is visible.  E.g., try a program that has a thread looping,
set a break in the loop, with condition 0 ("break someline if 0"), and
enable "set debug infrun 1".  Ideally, the prompt line would stay
always clearly visible, without flicker, with the other gdb output
scrolling starting at the line above the prompt.

So I'm not really sure of what's the proper way to handle this with
readline.  Maybe we need to handle this at a lower layer, say, make
the uiout level aware of the prompt buffer contents.

-- 
Pedro Alves

 gdb/infrun.c     |   28 ++++++++++++++++++++++++++++
 gdb/tui/tui-io.c |   11 +----------
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index e7c20e1..391ce12 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -59,6 +59,7 @@
 #include "objfiles.h"
 #include "completer.h"
 #include "target-descriptions.h"
+#include "readline/readline.h"

 /* Prototypes for local functions */

@@ -2743,6 +2744,29 @@ wait_for_inferior (void)
   do_cleanups (old_cleanups);
 }

+static void
+redisplay_prompt (void *old_prompt)
+{
+  rl_set_prompt (old_prompt);
+  xfree (old_prompt);
+  (*rl_redisplay_function) ();
+}
+
+static void
+hide_prompt_and_make_cleanup (void)
+{
+  char *prompt;
+
+  /* Don't rely on rl_save_prompt, as that doesn't handle nesting.  */
+
+  prompt = xstrdup (rl_prompt);
+
+  rl_set_prompt ("");
+  (*rl_redisplay_function) ();
+
+  make_cleanup (redisplay_prompt, prompt);
+}
+
 /* Asynchronous version of wait_for_inferior.  It is called by the
    event loop whenever a change of state is detected on the file
    descriptor corresponding to the target.  It can be called more than
@@ -2762,6 +2786,10 @@ fetch_inferior_event (void *client_data)
   int was_sync = sync_execution;
   int cmd_done = 0;

+  /* Hide the prompt while here, so that any message printed while
+     handling the event doesn't mess up with the prompt.  */
+  hide_prompt_and_make_cleanup ();
+
   memset (ecs, 0, sizeof (*ecs));

   /* We're handling a live event, so make sure we're doing live
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index fe88f1a..77e8875 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -142,10 +142,6 @@ static FILE *tui_old_rl_outstream;
 static int tui_readline_pipe[2];
 #endif

-/* The last gdb prompt that was registered in readline.
-   This may be the main gdb prompt or a secondary prompt.  */
-static char *tui_rl_saved_prompt;
-
 static unsigned int tui_handle_resize_during_io (unsigned int);

 static void
@@ -221,7 +217,7 @@ tui_redisplay_readline (void)
   if (tui_current_key_mode == TUI_SINGLE_KEY_MODE)
     prompt = "";
   else
-    prompt = tui_rl_saved_prompt;
+    prompt = rl_display_prompt;

   c_pos = -1;
   c_line = -1;
@@ -288,11 +284,6 @@ tui_redisplay_readline (void)
 static void
 tui_prep_terminal (int notused1)
 {
-  /* Save the prompt registered in readline to correctly display it.
-     (we can't use gdb_prompt() due to secondary prompts and can't use
-     rl_prompt because it points to an alloca buffer).  */
-  xfree (tui_rl_saved_prompt);
-  tui_rl_saved_prompt = xstrdup (rl_prompt);
 }

 /* Readline callback to restore the terminal.  It is called once each

Re: [PATCH 2/4] Suppress repeated annotations until GDB is ready to accept input.

[Pedro Alves]

On 11/27/2012 06:48 PM, Tom Tromey wrote:
> 
> I don't think there is.  When I tried just editing the gdb command line
> I ended up with a lot of "^Z^Z" in the gud buffer.  I guess you could
> dig up Emacs 23 (still in F16...) or try to downgrade the gud code.

While everyone is racing to install F18, I've set up a F16 VM instead to
test this with Emacs 23.  I've ran a few manual tests, trying to exercise
the code paths, with both --annotate=3 (which is what gud on E23 wants to
use by default), and --annotate=2.  I didn't see anything go wrong.  I also
tried some background/async commands, and it didn't look like Emacs got
any more confused than with the stock gdb.

Regression testing on x86_64 Fedora 17 didn't show any regression.

I'm putting the series in now.

Thanks,
-- 
Pedro Alves