From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19487 invoked by alias); 20 Jan 2012 16:31:52 -0000 Received: (qmail 19468 invoked by uid 22791); 20 Jan 2012 16:31:49 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD 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; Fri, 20 Jan 2012 16:31:36 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0KGVZdj000646 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 20 Jan 2012 11:31:36 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0KGVYop026605; Fri, 20 Jan 2012 11:31:35 -0500 Message-ID: <4F1996E6.2030707@redhat.com> Date: Fri, 20 Jan 2012 17:40:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0 MIME-Version: 1.0 To: Tom Tromey CC: gdb-patches@sourceware.org Subject: Re: [4/4] RFC: implement catch load and catch unload References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 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-01/txt/msg00736.txt.bz2 Handling this piecemeal. On 01/19/2012 08:35 PM, Tom Tromey wrote: > * There is a change in internal_bkpt_check_status that is somewhat > hacky. This was the simplest way I could find to make it so that > "catch load" reported the catchpoint; without this, bpstat_print would > find the internal bp_shlib_event bpstat and print it instead, even if > stop-on-solib-events was not set. Hmm. Should we ever print if the breakpoint does not cause a stop? IOW, can't we make that unconditional if we're also clearing bs->stop? Related, it looks like there's a bug here: bpstat_stop_status: if (bs->stop) { ... /* Print nothing for this entry if we don't stop or don't print. */ if (bs->stop == 0 || bs->print == 0) bs->print_it = print_it_noop; } That "Print nothing" only triggers if we think to stop, and then the conditions evaluate false, or the breakpoint is silent. That should probably be moved out of the `if (bs->stop) condition' above. Having said that, I've put it in patch form, on top of mainline. WDYT? * breakpoint.c (bpstat_stop_status): Moving clearing print_it outside `bs->stop' block. (bpstat_what): Rework bp_shlib_event handling. (internal_bkpt_check_status): If the breakpoint is a bp_shlib_event, then set bs->stop and bs->print if stop_on_solib_events is set. --- gdb/breakpoint.c | 37 ++++++++++++++++++++----------------- 1 files changed, 20 insertions(+), 17 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index f6a0276..3f3a417 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4257,10 +4257,12 @@ bpstat_stop_status (struct address_space *aspace, bs->print = 0; } - /* Print nothing for this entry if we don't stop or don't print. */ - if (bs->stop == 0 || bs->print == 0) - bs->print_it = print_it_noop; } + + /* Print nothing for this entry if we don't stop or don't + print. */ + if (!bs->stop || !bs->print) + bs->print_it = print_it_noop; } /* If we aren't stopping, the value of some hardware watchpoint may @@ -4341,6 +4343,9 @@ bpstat_what (bpstat bs_head) else bptype = bs->breakpoint_at->type; + if (bptype == bp_shlib_event) + shlib_event = 1; + switch (bptype) { case bp_none: @@ -4349,6 +4354,7 @@ bpstat_what (bpstat bs_head) case bp_hardware_breakpoint: case bp_until: case bp_finish: + case bp_shlib_event: if (bs->stop) { if (bs->print) @@ -4426,18 +4432,6 @@ bpstat_what (bpstat bs_head) This requires no further action. */ } break; - case bp_shlib_event: - shlib_event = 1; - - /* If requested, stop when the dynamic linker notifies GDB - of events. This allows the user to get control and place - breakpoints in initializer routines for dynamically - loaded objects (among other things). */ - if (stop_on_solib_events) - this_action = BPSTAT_WHAT_STOP_NOISY; - else - this_action = BPSTAT_WHAT_SINGLE; - break; case bp_jit_event: jit_event = 1; this_action = BPSTAT_WHAT_SINGLE; @@ -11144,8 +11138,17 @@ internal_bkpt_re_set (struct breakpoint *b) static void internal_bkpt_check_status (bpstat bs) { - /* We do not stop for these. */ - bs->stop = 0; + if (bs->breakpoint_at->type == bp_shlib_event) + { + /* If requested, stop when the dynamic linker notifies GDB of + events. This allows the user to get control and place + breakpoints in initializer routines for dynamically loaded + objects (among other things). */ + bs->stop = stop_on_solib_events; + bs->print = stop_on_solib_events; + } + else + bs->stop = 0; } static enum print_stop_action