From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27279 invoked by alias); 28 Nov 2007 19:46:33 -0000 Received: (qmail 27061 invoked by uid 22791); 28 Nov 2007 19:46:32 -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 19:46:26 +0000 Received: (qmail 12798 invoked from network); 28 Nov 2007 19:46:24 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Nov 2007 19:46:24 -0000 To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Don't reset watchpoint block on solib load. References: <200711202013.47537.vladimir@codesourcery.com> <200711281858.51145.vladimir@codesourcery.com> From: Jim Blandy Date: Wed, 28 Nov 2007 19:46:00 -0000 In-Reply-To: <200711281858.51145.vladimir@codesourcery.com> (Vladimir Prus's message of "Wed, 28 Nov 2007 18:58:50 +0300") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2007-11/txt/msg00528.txt.bz2 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. 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. - 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. 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. (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.) (I need to eat lunch, but I'll get to the rest when I get back.)