From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5260 invoked by alias); 28 Nov 2007 20:04:55 -0000 Received: (qmail 5076 invoked by uid 22791); 28 Nov 2007 20:04:37 -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; Wed, 28 Nov 2007 20:04:23 +0000 Received: (qmail 19854 invoked from network); 28 Nov 2007 20:04:21 -0000 Received: from unknown (HELO wind.local) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Nov 2007 20:04:21 -0000 From: Vladimir Prus To: Jim Blandy Subject: Re: [RFA] Don't reset watchpoint block on solib load. Date: Wed, 28 Nov 2007 20:04:00 -0000 User-Agent: KMail/1.9.6 References: <200711202013.47537.vladimir@codesourcery.com> <200711281858.51145.vladimir@codesourcery.com> In-Reply-To: Cc: gdb-patches@sources.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200711282304.16403.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/msg00529.txt.bz2 On Wednesday 28 November 2007 22:45:58 you wrote: > > Vladimir Prus writes: > > On Wednesday 28 November 2007 02:00:15 Jim Blandy wrote: > >> > >> Vladimir Prus writes: > >> > There's code inside breakpoint_re_set_one to refresh watchpoints, > >> > which seems suspicious to me. > >> > > >> > First problem with that code is that it always resets watchpoint's > >> > block to NULL. So, if we have a local watchpoint, and you do > >> > dlopen (without exiting the scope where watchpoint is valid), > >> > then the watchpoint's block will be reset to NULL, and > >> > watchpoint's expression will be reparsed in global block -- > >> > which will surely break the watchpoint. > >> > >> Is that right? We set innermost_block to NULL, but then we call > >> parse_expression, which should set innermost_block to the innermost > >> block containing a symbol actually used by the expression. > > > > Except that parse_expression is called while we're inside shared lib > > machinery code -- so neither original block nor original frame is > > used, and parse_expression is likely to fail. > > > >> We also call breakpoint_re_set_one when we've unloaded a shared > >> library. At that point, b->exp_valid_block could be a dangling > >> pointer; we can't use it to re-parse the expression. > >> > >> I think the bug is that we re-parse the expression with > >> parse_expression, which leaves the scope unspecified, defaulting to > >> the currently selected frame. We should: > >> > >> 1) Verify that the frame given by b->watchpoint_frame is still valid, > >> and delete the watchpoint if it isn't. > >> > >> 2) Call get_frame_block (b->watchpoint_frame) to see if we have a > >> block for the frame's location, and deleting the watchpoint if we > >> don't (saying we don't have the symbolic info available to update > >> it), and > >> > >> 3) Call parse_exp_1 (..., watchpoint frame's block, ...) to reparse > >> the watchpoint's expression in the proper block. > > > > Let me try thinking out loud. > > > > 1. Suppose I have a watchpoint on a global variable. Such a watchpoint > > can reasonably survive program restart, and given that it can be set on > > a global variable (including static variables of C++ classes) defined > > in a shared library, it makes sense recompute such watchpoint > > when a shared library is loaded, or unloaded. > > 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. 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. > At the moment, a watchpoint has two interesting fields: > > - b->exp_valid_block, which is set only when the watchpoint refers to > frame-local variables. Frustratingly, it's NULL if the watchpoint > refers to block-scoped static variables, even though we have no way > of finding those variables again if we re-parse the expression. Right, that's rather nasty. > - b->watchpoint_frame, which establishes which instances of the local > variables the watchpoint refers to. > > The interesting thing is that exp_valid_block is almost always used as > a binary value. The only exception is when we use it as a sanity > check on a frame we've found that matches b->watchpoint_frame. Yes, I noticed that too. > Should we instead be saving the PC in whose scope we parsed the > expression? That's something we could legitimately look up in > breakpoint_re_set_one after a symbol table change. The test in > watchpoint_check could look up the PC's function and compare it to the > frame's function. Well, it does not work with address randomization. And I think that for global watchpoints, just re-parsing the expression works good enough. > (There's also some terminology that's bugging me: the way the code > calls watchpoints "in scope" or "out of scope" is misleading: it's not > about scope, it's about what the ISO C standard calls "lifetime". For > example, a static variable local to some block is "in scope" from the > point of its declaration to the end of its block, but the variable's > lifetime is the execution of the entire program (or until the shared > library that contains the function is dlclosed). A watchpoint should > be deleted when the lifetime of any of the objects it refers to ends, > regardless of whether they are in scope or not.) Similar problem exists in MI varobjs -- where 'in scope' means something unclear. - Volodya