From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21767 invoked by alias); 3 Oct 2009 17:23:38 -0000 Received: (qmail 21724 invoked by uid 22791); 3 Oct 2009 17:23:37 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 03 Oct 2009 17:23:30 +0000 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n93HN6lr032125; Sat, 3 Oct 2009 13:23:06 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx04.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n93HN3V5024713 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 3 Oct 2009 13:23:06 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n93HN2am017934; Sat, 3 Oct 2009 19:23:02 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id n93HN2be017933; Sat, 3 Oct 2009 19:23:02 +0200 Date: Sat, 03 Oct 2009 17:23:00 -0000 From: Jan Kratochvil To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [patch 2/4] Fix hw watchpoints: reordered / simultaneously hit [fixup #1] Message-ID: <20091003172302.GD26203@host0.dyn.jankratochvil.net> References: <20090817194612.GC10694@host0.dyn.jankratochvil.net> <20091002221254.GA7767@host0.dyn.jankratochvil.net> <20091002230124.GG10338@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091002230124.GG10338@adacore.com> User-Agent: Mutt/1.5.20 (2009-08-17) 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: 2009-10/txt/msg00090.txt.bz2 Hi Joel, I fully agree with you (I started to code the new parameter there then I did reset --hard it and provided the save_inferior_ptid hack there instead). Still I would like to get accepted first the watchpoints set and the cleanups can get in later, the watchpoints were first posted for FSF GDB on 14 Oct 2007. There are currently/still 31 usages of save_inferior_ptid() in FSF GDB HEAD. Unfortunately it is unavoidable to patch it in/before the patch 3/4 because gdb_assert() in linux_nat_stopped_data_address() triggers otherwise (GDB just harmlessly reported wrong debug output before). Removing that gdb_assert() is inappropriate as it is a correct assumption at that point. Or do you suggest different patch schedule? Thanks, Jan On Sat, 03 Oct 2009 01:01:24 +0200, Joel Brobecker wrote: > Jan, > > > the patch got extended by this incremental change (fixing a regression): > > > > gdb/ > > Fix assertion failure with `set debug infrun 1'. > > * infrun.c (handle_inferior_event ): New variable > > old_chain. Temporarily switch INFERIOR_PTID. > > * target.h (target_stopped_by_watchpoint): Extend the comment. > > (target_stopped_data_address): New comment. Rename X as ADDR_P. > > Can we treat this part independently from the rest? I assume that > this is a latent issue that only gets uncovered by your watchpoint > patch? I think the bug is there regardless, and even if we can't test > it now, it's going to simplify a bit my task if we treat this piece > independently. > > I think that your fix is not optimal. Here is what I suggest instead: > Make the ptid an explicit parameter in the call to > target_stopped_by_watchpoint. There are two parts to this change, > and I think we can find a way of making the change in two steps: > > - First step: Change the profile of target_stopped_by_watchpoint > to add a ptid (say as the second argument). We're also trying > to get rid of these target_... macros, so now is the time to > turn that macro into a function. We update all the callers > to pass the ptid (could be either inferior_ptid or ecs-ptid > in your infrun case). > > We hold off the update of all the to_stopped_data_address > callbacks in struct target_ops for now. This means that we need > to keep the current interface as is, and that we need to temporarily > change the inferior_ptid before calling the callback. > > - Second step: Change the callbacks interface, and update all > the callbacks. This is trickier. For the platforms that we can test, > I suggest trying to fix the code accordingly. For the others, do > the same as before: Start by changing the inferior_ptid during > the life of the callback. > > This second step might or might not be worth it, as the code > on which the callbacks depend might not entirely ready. But > at least we pushed the mess to the target code. > > Since I'm dumping some cleanup work on you, I can take care of the > second part if you are willing to take care of the first one. > > -- > Joel