From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6897 invoked by alias); 21 Nov 2012 20:14:32 -0000 Received: (qmail 6777 invoked by uid 22791); 21 Nov 2012 20:14:31 -0000 X-SWARE-Spam-Status: No, hits=-7.8 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Nov 2012 20:14:24 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qALKEO1i025759 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 21 Nov 2012 15:14:24 -0500 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qALKENVc010972 for ; Wed, 21 Nov 2012 15:14:23 -0500 Subject: [PATCH 1/4] Remove (alleged) "breakpoints-changed" annotation suppression on ignore count changes. To: gdb-patches@sourceware.org From: Pedro Alves Date: Wed, 21 Nov 2012 20:14:00 -0000 Message-ID: <20121121201422.1015.45168.stgit@brno.lan> In-Reply-To: <20121121201416.1015.36832.stgit@brno.lan> References: <20121121201416.1015.36832.stgit@brno.lan> User-Agent: StGit/0.16 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-11/txt/msg00586.txt.bz2 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 . 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 * 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 * 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" } }