From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3312 invoked by alias); 3 Nov 2011 16:02:19 -0000 Received: (qmail 3262 invoked by uid 22791); 3 Nov 2011 16:02:18 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 03 Nov 2011 16:01:59 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=EU1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RLzjo-0006ER-Lc from pedro_alves@mentor.com ; Thu, 03 Nov 2011 09:01:56 -0700 Received: from scottsdale.localnet ([172.16.63.104]) by EU1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Thu, 3 Nov 2011 16:01:53 +0000 From: Pedro Alves To: Tom Tromey Subject: Re: RFC: don't set the pspace on ordinary breakpoints Date: Thu, 03 Nov 2011 16:02:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-12-generic; KDE/4.7.2; x86_64; ; ) Cc: gdb-patches@sourceware.org, Ulrich Weigand References: <201111022021.03286.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201111031601.51221.pedro@codesourcery.com> X-IsSubscribed: yes 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: 2011-11/txt/msg00074.txt.bz2 On Thursday 03 November 2011 14:01:19, Tom Tromey wrote: > >>>>> "Pedro" == Pedro Alves writes: > > Pedro> Thanks. IMO, the executing_startup change IMO could go in > Pedro> separately, as soon as it is correct, it looks quite independent. > Pedro> What prompted it, BTW? > > It seemed wrong to mark an entire breakpoint as "startup disabled" in > the case where the breakpoint has locations in multiple inferiors. Ah, indeed. > static void > bkpt_re_set (struct breakpoint *b) > { > - /* Do not attempt to re-set breakpoints disabled during startup. */ > - if (b->enable_state == bp_startup_disabled) > - return; > - I'm looking at this one, and thinking that if we simply remove the guard, we'll allow re-set to go through while we can't trust symbols/sections in the program space, because they haven't been relocated yet, but this is wrong (as opposed to just inneficient) because GDB will actually read memory from the wrong addresses (prologue skipping, breakpoint shadow, at least). We can't just make bp_startup_disabled be per-location, because a re-set could find new locations, defeating the guard. So I think we'll maybe want this: static void bkpt_re_set (struct breakpoint *b) { - /* Do not attempt to re-set breakpoints disabled during startup. */ - if (b->enable_state == bp_startup_disabled) - return; + if (current_program_space->executing_startup) + return 0; And adjust the `current_program_space' reference in the ambiguous linespec patch to whatever will be necessary then. and: @@ -1578,6 +1578,9 @@ should_be_inserted (struct bp_location *bl) if (!bl->enabled || bl->shlib_disabled || bl->duplicate) return 0; + if (!bl->owner->ops->should_be_inserted (bl)) + return 0; With the regular breakpoint's (or any other breakpoint that uses linespecs) implementation of the new should_be_inserted hook being: static int bkpt_should_be_inserted (struct bp_location *bl) { return !bl->pspace->executing_startup; } It seems to me that watchpoints shouldn't be inserted during startup either (can't trust symbols for the watchpoint's expression either), so maybe my previous suggestion is indeed good enough: @@ -1578,6 +1578,9 @@ should_be_inserted (struct bp_location *bl) if (!bl->enabled || bl->shlib_disabled || bl->duplicate) return 0; -+ if (bl->pspace->executing_startup) ++ if (user_breakpoint_p (bl->owner) && bl->pspace->executing_startup) + return 0; Unless we have some user breakpoint kind that would want to be inserted in the startup phase. One such example may simply be a breakpoint set at a specific address, to debug exactly the startup phase. So we'd need: static int bkpt_should_be_inserted (struct bp_location *bl) { if (!bl->pspace->executing_startup) { struct breakpoint *b = bl->owner; if (!linespec_is_address (b->addr_string)) return 0; if (b->addr_string_range_end != NULL && !linespec_is_address (b->addr_string_range_end)) return 0; } return 1; } linespec_is_address would return true to linespecs of "*NUMBER" form. (or if we have a linespec struct, we'd record address-ness there). -- Pedro Alves