From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22912 invoked by alias); 29 Nov 2007 06:09:25 -0000 Received: (qmail 22903 invoked by uid 22791); 29 Nov 2007 06:09:24 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 29 Nov 2007 06:09:18 +0000 Received: (qmail 15338 invoked from network); 29 Nov 2007 06:09:16 -0000 Received: from unknown (HELO wind.local) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 29 Nov 2007 06:09:16 -0000 From: Vladimir Prus To: Jim Blandy Subject: Re: [RFA] Don't reset watchpoint block on solib load. Date: Thu, 29 Nov 2007 06:09:00 -0000 User-Agent: KMail/1.9.6 Cc: gdb-patches@sources.redhat.com References: <200711202013.47537.vladimir@codesourcery.com> <200711282304.16403.vladimir@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200711290909.05858.vladimir@codesourcery.com> 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: 2007-11/txt/msg00536.txt.bz2 On Thursday 29 November 2007 01:36:35 Jim Blandy wrote: > > Vladimir Prus writes: > >> The steps I wrote don't address the case of watchpoints that aren't > >> frame-relative. I wonder how we should be dealing with those. > >> > >> If we have a watchpoint on a static variable local to some file or > >> block, then I don't honestly see how we can possibly re-set it > >> correctly after a symbol reload with the data we have now. We can't > >> tell whether b->exp_valid_block is a dangling pointer or not, and > >> b->watchpoint_frame will be null on such watchpoints. > > > > I haven't run into any case when b->exp_valid_block is not-NULL, > > but b->watchpoint_frame is NULL. > > By 'dangling pointer', I mean that it's referring to a block that was > freed when we freed the objfile it belongs to. My point was just that > neither of those fields would be useful in addressing the problem I > described. Yes, the 'dangling pointer' is a potential problem; I even have a half-finished patch for that. Except, that it's not easy to get such dangling pointer. For global watchpoint, exp_valid_block is NULL. For watchpoint on local var, to get dangling pointer you need to unloaded shlib from within shlib code, which is likely to crash before we get back. > > > In fact, we don't need to care about blocks for global watchpoints -- > > just like ordinary breakpoint on 'foo' does not care which shared library > > 'foo' comes from, global watchpoint on 'important_data_structure' need > > not care about where that variable comes from. If we re-parse watchpoint > > expression on each solib load, things are fine. > > Our messages crossed paths: I think we should worry about re-parsing > global watchpoints in the proper scope. > > For example, we do take care to remember which function breakpoints > refer to when there is more than one possible referent. In the > transcript below, note that the breakpoint set on the static 'foo' in > t2.c, as opposed to the global 'foo' in t1.c, remains there even when > we re-read the main executable. > > Watchpoints should behave the same way. Not that breakpoints in shared libraries do not behave this way, exactly because shared library can be loaded at random address. > This isn't what GDB does now, but I think this behavior is not hard to > obtain. When we set a watchpoint, instead of b->exp_valid_block, we > should record the PC in whose scope we parsed the expression. When we > check a watchpoint's condition, we should look up the PC's block: if > we can't find one, then we've unloaded symbolic information for that > watchpoint, and we should delete it. > > The tricky part is deciding what to do when the main executable file > is modified. It's hard to see what exactly the watchpoint should > stick to in this case, since the PC isn't a reliable identifier across > such changes. Perhaps it would be enough to also record the name of > the function and source file the PC falls in, re-parse if they have > remained stable, and delete the watchpoint if they have changed. The question is how useful this is. Program changes all the time, and PC can easily end up in a different function. And I think a change can also cause PC to end up in the same function and source file, but different block. And we don't have any way to persistently identify blocks. Anyway, the point of my two patches was to change watchpoints to use multiple locations internally, without breaking behaviour that GDB *now* tries to implement. Now, gdb tries to reparse global watchpoints, my patches keep that, except that now GDB does not crash doing so. You'd note my second patch has 'update_watchpoint' function, which is the single place to update watchpoint if something changes. Should we come up with a smarter approach of refreshing breakpoints, we can just rename the 'reparse' parameter to that function into 'symbols_changed', and let that function have any smart logic we'll come up. There will be no need to redo anything else of my patches. - Volodya