There's code in annotate.c and breakpoint.c that is supposed to
authorPedro Alves <palves@redhat.com>
Tue, 22 Jan 2013 20:08:30 +0000 (20:08 +0000)
committerPedro Alves <palves@redhat.com>
Tue, 22 Jan 2013 20:08:30 +0000 (20:08 +0000)
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/
2013-01-22  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/
2013-01-22  Pedro Alves  <palves@redhat.com>

* gdb.base/annota1.exp (annotate ignore count change): Add
expected output for failure case.

gdb/ChangeLog
gdb/annotate.c
gdb/annotate.h
gdb/breakpoint.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/annota1.exp

index eb02b37d01c67e54f5868ba03bb01e2be597cc9a..fd4e37f3925ae59f4b4b205ddbd4218075f5d22b 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-22  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.
+
 2013-01-22  Tom Tromey  <tromey@redhat.com>
 
        * dwarf2loc.c (dwarf2_compile_expr_to_ax) <DW_OP_fbreg>: Only
index 1919467e74387514944fb011d93ea8267a249130..25f765986c61da6966709bf6a6d65002ba0944f9 100644 (file)
@@ -37,8 +37,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)
 {
@@ -55,24 +53,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)
 {
@@ -106,11 +89,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
index 2ea465aa1abda396340c482f1377948dd3f096bc..ea8ad15c8f75982600f1b4063b4ae90c4351c1ea 100644 (file)
@@ -21,7 +21,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);
index fd4132f49f63e5c935cc601bb8fae9d27c00ef9a..35848b4a8b336a7f396d786e156a6898661c9d2b 100644 (file)
@@ -5137,7 +5137,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);
index 1010c998455f537fe17a57b44d82a215d555e8b1..5d3ed122a525fc27c4475c3522c4fe38ca43ccbc 100644 (file)
@@ -1,3 +1,8 @@
+2013-01-22  Pedro Alves  <palves@redhat.com>
+
+       * gdb.base/annota1.exp (annotate ignore count change): Add
+       expected output for failure case.
+
 2013-01-22  Tom Tromey  <tromey@redhat.com>
 
        * gdb.gdb/selftest.exp (do_steps_and_nexts): Handle bfd_init
index 89fbd94727649b3725a88e49be37afa759e91f3d..776738a65909a6106440d4eebbd90ae69c789a85 100644 (file)
@@ -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"
     }
 }