From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14830 invoked by alias); 27 Nov 2007 23:00:43 -0000 Received: (qmail 14759 invoked by uid 22791); 27 Nov 2007 23:00: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; Tue, 27 Nov 2007 23:00:31 +0000 Received: (qmail 9449 invoked from network); 27 Nov 2007 23:00:28 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Nov 2007 23:00:28 -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> From: Jim Blandy Date: Tue, 27 Nov 2007 23:00:00 -0000 In-Reply-To: <200711202013.47537.vladimir@codesourcery.com> (Vladimir Prus's message of "Tue, 20 Nov 2007 20:13:46 +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/msg00518.txt.bz2 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. 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. > Second problem is that this code reevalautes the expression, > and given that insert_breakpoints does that too, we can just > reset breakpoints value to NULL, and have insert_breakpoints to the > work. I think it's an invariant that b->val may be NULL only when we have just started the inferior, and know that insert_breakpoints will be called. In other contexts, we don't always call insert_breakpoints before letting the program run. Wouldn't leaving the value NULL cause a problem in that case? > Finally, this code reevaluates condition. Re-parses, you mean? > While this is probably > correct way to handle case where meaning of condition changes due to > loading of shared library, there's no code to match for the > case when a shared library is unloaded. I think a more robust > approach if to reevaluate condition inside insert_bp_location. I agree. > This patch is prompted by the following problem: > > void some_function() { > > g = 10; > .... > dlopen("whatever", ...); > .... > g = 15; > } > > If you set watchpoint on 'g', and continue over dlopen, the watchpoint is never hit. > The exact mode of failure differs. I actually have a testcase for this, and it > passes for me locally, and I would have liked to provide it, but there are two > issues for which I don't have yet a complete solution: > > - if we have no debug information for ld.so, then when we stop in > ld.so, we cannot find the frame associated with watchpoint, and delete > watchpoint. Does this case arise in normal usage? I'm not saying it doesn't; I'm just not sure how to work around it either, so I'm wondering how serious a problem it is. > - if we have debug information for ld.so, then when we stop in > ld.so, gdb tries to reevaluate 'g'. Unfortunately, it does that in > wrong block, does not find 'g', and dies with internal error. My suggestion above should avoid this.