From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8941 invoked by alias); 8 Oct 2003 18:26:51 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 8934 invoked from network); 8 Oct 2003 18:26:50 -0000 Received: from unknown (HELO localhost.redhat.com) (207.219.125.105) by sources.redhat.com with SMTP; 8 Oct 2003 18:26:50 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id BB8412B89; Wed, 8 Oct 2003 14:26:48 -0400 (EDT) Message-ID: <3F8456E8.6090005@redhat.com> Date: Wed, 08 Oct 2003 18:26:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030820 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Mark Kettenis , kevinb@redhat.com Cc: gdb-patches@sources.redhat.com Subject: Re: [RFC] TARGET_ADJUST_BREAKPOINT_ADDRESS - patch 1 of 4 References: <1031004002813.ZM24546@localhost.localdomain> <200310061249.h96CnP6f000466@elgar.kettenis.dyndns.org> <1031006153657.ZM11197@localhost.localdomain> <200310062109.h96L9Waq000481@elgar.kettenis.dyndns.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-10/txt/msg00234.txt.bz2 > Date: Mon, 6 Oct 2003 08:36:58 -0700 > From: Kevin Buettner > > On Oct 6, 2:49pm, Mark Kettenis wrote: > > > The patch below should be non-controversial. It merely adds the > > TARGET_ADJUST_BREAKPOINT_ADDRESS method. Code which uses it (the > > possibly controversial bit) will come in patch #3. Documentation will > > be in the next patch, #2. Finally, target specific code which > > requires TARGET_ADJUST_BREAKPOINT_ADDRESS will be posted in patch #4. > > > > Do we really need the "TARGET" in the name of the new method? It made > > me think that this was something that was going to be added to the > > target-vector instead of the architecture vector. > > How about just "ADJUST_BREAKPOINT_ADDRESS" ? > > Sounds fine to me :-). Thanks, Then ADJUST_BREAKPOINT_ADDRESS is the name. [snip] > To review the criticism of the previous TARGET_ADJUST_BREAKPOINT > patch (from three years ago!), see: Since then, the architecture vector has gained "m" and "M" - i.e., strictly multi-arch - and the new method will need to be one of those. Looking at: +/* Call TARGET_ADJUST_BREAKPOINT_ADDRESS and print warning if an address + adjustment was made. Returns the adjusted address. */ + +static CORE_ADDR +adjust_breakpoint_address (CORE_ADDR bpaddr) +{ + CORE_ADDR adjusted_bpaddr; + + adjusted_bpaddr = TARGET_ADJUST_BREAKPOINT_ADDRESS (bpaddr); + + if (adjusted_bpaddr != bpaddr) + breakpoint_adjustment_warning (bpaddr, adjusted_bpaddr, 0, 0); + + return adjusted_bpaddr; +} + I've a strong preference for "M" - method with predicate - since, for everyone but an frv developer, this code is irrelevant. Having a predicate would let adjust_breakpoint_address be written in a style that reflects this atypical code path: if (gdbarch_adjust_..._p()) adjusted = ... if (changed) warning ... and the average (non-frv) developer wouldn't be distracted by the change. Other than that, the architect method is ok. Andrew PS: I wonder if the other parts of your change slam straight into DanielJ's two teer breakpoint effect (assuming I remember his long ago intent) vis: - the high-level user specified breakpoint (requested_address?) - the low-level corresponding list of real breakpoints (address?) I also wonder if PPC64 GDB could use this to do the descriptor -> function address transformation that I described in: http://sources.redhat.com/ml/gdb-patches/2003-09/msg00415.html Andrew