From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23497 invoked by alias); 24 Jan 2006 21:20:53 -0000 Received: (qmail 23487 invoked by uid 22791); 24 Jan 2006 21:20:52 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Tue, 24 Jan 2006 21:20:51 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1F1Vao-00078K-8O; Tue, 24 Jan 2006 16:20:46 -0500 Date: Tue, 24 Jan 2006 21:20:00 -0000 From: Daniel Jacobowitz To: Wu Zhou Cc: Eli Zaretskii , gdb-patches@sources.redhat.com, bje@au1.ibm.com, anton@au1.ibm.com, pgilliam@us.ibm.com Subject: Re: [RFC] GDB patches for hw watchpoints - revised Message-ID: <20060124212046.GC26974@nevyn.them.org> Mail-Followup-To: Wu Zhou , Eli Zaretskii , gdb-patches@sources.redhat.com, bje@au1.ibm.com, anton@au1.ibm.com, pgilliam@us.ibm.com References: <20060122205641.GF27224@nevyn.them.org> <20060124034304.GA4719@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.8i X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-01/txt/msg00373.txt.bz2 On Tue, Jan 24, 2006 at 06:59:53PM +0800, Wu Zhou wrote: > Hi Daniel, > > Because there are quite a few places in different arch/target of GDB which > use region_size_ok_for_hw_watchpoint, so I am prefering to get this done > in two steps: first add to_region_ok_for_hw_watchpoint into struct > target_ops, then replace these to_region_size_ok_for_hw_watchpoint > reference with to_region_ok_for_hw_watchpoint ones. IMHO, it is easier to > not confuse the original intention of this patch with this replacement. > What is your thought on this? Sure. Couple small things but we're almost done. > (ppc_linux_check_watch_resources): New function to check whether > the target has available hardware watchpoint resource. > (ppc_linux_region_ok_for_hw_watchpoint): New function to check if > the region is ok for hardware watchpoint. > (ppc_linux_insert_watchpoint): New function to insert a hardware > watchpoint. > (ppc_linux_remove_watchpoint): New function to remove a hardware > watchpoint. > (ppc_linux_stopped_data_address): New function to get the stopped > data address of the watchpoint. > (ppc_linux_stopped_by_watchpoint): New function to see if the > inferior is stopped by watchpoint. "New function" is sufficient; if you feel that the function needs an explanation, it should go in the code, not in the changelog. > target.h (struct target_ops): Add a new target vector Missed a "* " there. > + /* P630 has nonsteppable watchpoint. So we are assuming that all PowerPC > + targets have nonsteppable watchpoint. */ > + set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1); Please skip this comment. > @@ -1575,6 +1582,12 @@ find_default_create_inferior (char *exec > } > > static int > +default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) > +{ > + return (len <= TYPE_LENGTH (builtin_type_void_data_ptr)); > +} TARGET_REGION_OK_FOR_HW_WATCHPOINT will now always be defined, because of the #ifdef in target.h. Therefore this won't trigger (from breakpoint.c): #if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT) #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR,LEN) \ (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(LEN)) #endif So you need to call TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len) here, in case the target has overridden that. -- Daniel Jacobowitz CodeSourcery