From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26586 invoked by alias); 18 Feb 2010 18:41:48 -0000 Received: (qmail 26577 invoked by uid 22791); 18 Feb 2010 18:41:47 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_SORBS_WEB,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout22.012.net.il (HELO mtaout22.012.net.il) (80.179.55.172) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 18 Feb 2010 18:41:39 +0000 Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0KY100100VV82800@a-mtaout22.012.net.il> for gdb-patches@sourceware.org; Thu, 18 Feb 2010 20:41:29 +0200 (IST) Received: from HOME-C4E4A596F7 ([84.228.213.68]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0KY100KFNVX3VL90@a-mtaout22.012.net.il>; Thu, 18 Feb 2010 20:41:29 +0200 (IST) Date: Thu, 18 Feb 2010 18:41:00 -0000 From: Eli Zaretskii Subject: Re: read watchpoints don't work on targets that support read watchpoints In-reply-to: <201002180111.31520.pedro@codesourcery.com> To: Pedro Alves Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83aav6xuag.fsf@gnu.org> References: <201002180111.31520.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: 2010-02/txt/msg00459.txt.bz2 > From: Pedro Alves > Date: Thu, 18 Feb 2010 01:11:31 +0000 > > The problem is that bpstat_check_watchpoint logic for read watchpoints > is bogus for targets that do support read watchpoints properly: I'd say "bogus" is a little exaggerated here, to say the least. For x86, it's the best we can do, and works reasonably well even under contrived conditions (see below for one such example). And at the time this was written, the number of targets that did support read-only watchpoints was very small. > case WP_VALUE_CHANGED: > if (b->type == bp_read_watchpoint) > { > /* Don't stop: read watchpoints shouldn't fire if > the value has changed. This is for targets > which cannot set read-only watchpoints. */ > bs->print_it = print_it_noop; > bs->stop = 0; > } > > If we don't get a memory write trap, then when the next read traps, > we'll see that the watched value changes, and consequently we just > ignore the watchpoint! Right, but I could argue that this situation is rarely of practical importance: why didn't the user use a data-write watchpoint for the program you've shown? > As Jeremy says on the PR: "over-enthusiastic reporting is less > of an issue than under-enthusiastic reporting". I don't agree with this philosophy, but I don't want to start an academic argument, either. What worries me is this: what will happen on x86 under your suggested change if the same address is watched with 2 different watchpoints: a read one and a write one? (If you wonder why would that be useful, then the answer is that each one could use a different condition.) Before the changes which added this logic, the results were a total mess. > The logic of not reporting read watchpoints if the memory > changes could be reinstated, if conditionalized on the target > telling the core that it can't do read watchpoints, but it > can do access watchpoints instead. Or the target itself could > do that (filtering read-write traps from gdb if the memory > watched doesn't change), which is more efficient, transparent > and flexible. Or such targets should just refuse > to install read watchpoints, and GDB should try to fallback > to trying access watchpoints, and knowing it should apply > the "only-if-not-changed" logic then. This is never perfect, > because we'll report reads when the program writes the same value > as the memory already had, but then again, this is what already > happens today. The latter option is a bit problematic with > the current interfaces, as we don't know _why_ is the target > refusing a read watchpoint. In any case, I think that > targets allowing read watchpoints to be inserted, and > then trapping on writes should get what they deserve. > > I think we should apply this, and work from here on if needed. > Any objection or better ideas? I would suggest to fix the problem in a more fundamental manner, instead of degrading the watchpoint support on one target for the sake of another. I like this suggestion the best: The logic of not reporting read watchpoints if the memory changes could be reinstated, if conditionalized on the target telling the core that it can't do read watchpoints, but it can do access watchpoints instead. With this setup, we can properly support both x86 and targets that support real read-only watchpoints.