From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31650 invoked by alias); 2 Oct 2009 23:01:36 -0000 Received: (qmail 31279 invoked by uid 22791); 2 Oct 2009 23:01:35 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 02 Oct 2009 23:01:31 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 9D3762BABEF; Fri, 2 Oct 2009 19:01:29 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id yx76ljVEes1f; Fri, 2 Oct 2009 19:01:29 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 5B2CC2BABE8; Fri, 2 Oct 2009 19:01:29 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id EC440F5906; Fri, 2 Oct 2009 16:01:24 -0700 (PDT) Date: Fri, 02 Oct 2009 23:01:00 -0000 From: Joel Brobecker To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: [patch 2/4] Fix hw watchpoints: reordered / simultaneously hit [fixup #1] Message-ID: <20091002230124.GG10338@adacore.com> References: <20090817194612.GC10694@host0.dyn.jankratochvil.net> <20091002221254.GA7767@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091002221254.GA7767@host0.dyn.jankratochvil.net> User-Agent: Mutt/1.5.18 (2008-05-17) 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/msg00079.txt.bz2 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