From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31586 invoked by alias); 9 Feb 2006 05:44:33 -0000 Received: (qmail 31576 invoked by uid 22791); 9 Feb 2006 05:44:31 -0000 X-Spam-Check-By: sourceware.org Received: from ausmtp04.au.ibm.com (HELO ausmtp04.au.ibm.com) (202.81.18.152) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 09 Feb 2006 05:44:30 +0000 Received: from sd0112e0.au.ibm.com (d23rh903.au.ibm.com [202.81.18.201]) by ausmtp04.au.ibm.com (8.12.10/8.12.10) with ESMTP id k195oevE342248 for ; Thu, 9 Feb 2006 16:50:44 +1100 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.250.237]) by sd0112e0.au.ibm.com (8.12.10/NCO/VERS6.8) with ESMTP id k195lZjX241720 for ; Thu, 9 Feb 2006 16:47:35 +1100 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.12.11/8.13.3) with ESMTP id k195iIgS001365 for ; Thu, 9 Feb 2006 16:44:19 +1100 Received: from wks190294wss.cn.ibm.com (wks190294wss.cn.ibm.com [9.181.133.242] (may be forged)) by d23av04.au.ibm.com (8.12.11/8.12.11) with ESMTP id k195iDi0001229; Thu, 9 Feb 2006 16:44:15 +1100 Date: Thu, 09 Feb 2006 05:44:00 -0000 From: Wu Zhou To: Daniel Jacobowitz 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 In-Reply-To: Message-ID: References: <20060122205641.GF27224@nevyn.them.org> <20060124034304.GA4719@nevyn.them.org> <20060124212046.GC26974@nevyn.them.org> <20060202014308.GA19507@nevyn.them.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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-02/txt/msg00200.txt.bz2 Daniel, I committed the two patches you approved. Here is a seperate patch for gdbint.texinfo: 2006-02-08 Wu Zhou * gdbint.texinfo (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. Index: gdbint.texinfo =================================================================== RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v retrieving revision 1.238 diff -c -p -r1.238 gdbint.texinfo *** gdbint.texinfo 6 Feb 2006 22:14:31 -0000 1.238 --- gdbint.texinfo 9 Feb 2006 05:32:27 -0000 *************** the same time). *** 465,477 **** Return non-zero if hardware watchpoints can be used to watch a region whose address is @var{addr} and whose length in bytes is @var{len}. - @findex TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT - @item TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (@var{size}) - Return non-zero if hardware watchpoints can be used to watch a region - whose size is @var{size}. @value{GDBN} only uses this macro as a - fall-back, in case @code{TARGET_REGION_OK_FOR_HW_WATCHPOINT} is not - defined. - @cindex insert or remove hardware watchpoint @findex target_insert_watchpoint @findex target_remove_watchpoint --- 465,470 ---- Besides this, I am also thinking of a little more cleanup work in config/i386/nm-i386.h: macro TARGET_REGION_OK_FOR_HW_WATCHPOINT can be replaced by a target vector initialization in i386-nat.c or somewhere else. Maybe some other macros can also be replaced, such as STOPPED_BY_WATCHPOINT, target_stopped_data_address(target, x) and so on... What is your thought on this? Regards - Wu Zhou On Wed, 8 Feb 2006, Wu Zhou wrote: > Hi Daniel, > > Sorry for the delayed reply. I am just back from a vacation. > > On Wed, 1 Feb 2006, Daniel Jacobowitz wrote: > > > On Wed, Jan 25, 2006 at 04:34:14PM +0800, Wu Zhou wrote: > > > I am also thinking of replace the macro > > > TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (SIZE) with > > > TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE). Thus the code will seem > > > to be more clean: we will only have one macro to see if the target region > > > is ok for watchpoint monitoring. Following this way, function > > > default_region_ok_for_hw_watchpoint will also return back to its original > > > implementation. What is your thought on this? > > > > I'm not sure what you mean. After these patches, the only reference to > > TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT is in gdbint.texinfo (which I'd > > appreciate if you fixed, in a separate patch - thanks in advance). > > What are the two ways now? > > What I mean is the second patch below, which you had said ok. :-) > That is the only way I can thought of at that time. Now that you had said > ok, I don't need to find a second way. :-) > > BTW. I will fix the reference to TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT > in gdbint.texinfo in a separate patch after commiting this. > > > > > On Wed, Jan 25, 2006 at 11:18:49AM +0800, Wu Zhou wrote: > > > OK to commit? > > > > > > 2006-01-22 Ben Elliston > > > Wu Zhou > > > > > > * ppc-linux-nat.c (PTRACE_GET_DEBUGREG, PTRACE_SET_DEBUGREG, > > > PTRACE_GETSIGINFO): Define. > > > (last_stopped_data_address): New. > > > (ppc_linux_check_watch_resources): New function. > > > (ppc_linux_region_ok_for_hw_watchpoint): New function. > > > (ppc_linux_insert_watchpoint): New function. > > > (ppc_linux_remove_watchpoint): New function. > > > (ppc_linux_stopped_data_address): New function. > > > (ppc_linux_stopped_by_watchpoint): New function. > > > (_initialize_ppc_linux_nat): Set the above hardware watchpoint > > > related target vectors. > > > * rs6000-tdep.c (rs6000_gdbarch_init): Set PPC architectures > > > to have nonsteppable watchpoint. > > > * target.c (default_region_ok_for_hw_watchpoint, > > > debug_to_region_ok_for_hw_watchpoint): New prototypes. > > > (update_current_target): Inherit to_region_ok_for_hw_watchpoint > > > and set default to_region_ok_for_hw_watchpoint. > > > (default_region_ok_for_hw_watchpoint): New function. > > > (debug_to_region_ok_for_hw_watchpoint): New function. > > > (setup_target_debug): Set to_region_ok_for_hw_watchpoint of > > > debug_target. > > > * target.h (struct target_ops): Add a new target vector > > > to_region_ok_for_hw_watchpoint. > > > (TARGET_REGION_OK_FOR_HW_WATCHPOINT): Define this if it is not > > > defined anyplace else. > > > > On Wed, Jan 25, 2006 at 04:34:14PM +0800, Wu Zhou wrote: > > > 2006-01-25 Wu Zhou > > > > > > * breakpoint.c (TARGET_REGION_OK_FOR_HW_WATCHPOINT): Delete. > > > * config/i386/nm-i386sol2.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New. > > > (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. > > > * config/mips/nm-irix5.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New. > > > (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. > > > * config/sparc/nm-sol2.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New. > > > (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. > > > * inf-ttrace.c (inf_ttrace_region_ok_for_hw_watchpoint): New. > > > (inf_ttrace_region_size_ok_for_hw_watchpoint): Delete. > > > (inf_ttrace_target): Delete to_region_size_ok_for_hw_watchpoint and > > > add to_region_ok_for_hw_watchpoint. > > > * s390-nat.c (s390_region_size_ok_for_hw_watchpoint): Delete. > > > (s390_region_ok_for_hw_watchpoint): New. > > > (_initialize_s390_nat): Delete to_region_size_ok_for_hw_watchpoint > > > and add to_region_ok_for_hw_watchpoint. > > > * target.c (default_region_size_ok_for_hw_watchpoint, > > > debug_to_region_size_ok_for_hw_watchpoint): Delete prototype. > > > (update_current_target): Delete to_region_size_ok_for_hw_watchpoint > > > inheritance and default_region_size_ok_for_hw_watchpoint. > > > (default_region_ok_for_hw_watchpoint): If len is less than or equal > > > the length of void pointer, return ok. > > > (default_region_size_ok_for_hw_watchpoint): Delete. > > > (debug_to_region_size_ok_for_hw_watchpoint): Delete. > > > (setup_target_debug): Delete to_region_size_ok_for_hw_watchpoint. > > > * target.h (struct target_ops): Delete > > > to_region_size_ok_for_hw_watchpoint. > > > (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete. > > > > These patches are both OK. You might want to combine them - it's a > > much smaller diff :-) But it doesn't matter since you've already got > > them separated out. > > Thanks for reviewing that. I would like to commit them one by one, thus I > don't need to think about how to re-describe them. :-) > > I also think that it is clearer to differentiate the purpose of these two > patches. Combining them together seems a little confusing to me. > > Best Regards > - Wu Zhou