From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25979 invoked by alias); 23 Apr 2008 11:16:18 -0000 Received: (qmail 25966 invoked by uid 22791); 23 Apr 2008 11:16:15 -0000 X-Spam-Check-By: sourceware.org Received: from ns2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 23 Apr 2008 11:15:51 +0000 Received: from Relay2.suse.de (mail2.suse.de [195.135.221.8]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 468FC4543C for ; Wed, 23 Apr 2008 13:15:48 +0200 (CEST) From: Andreas Schwab To: gdb-patches@sourceware.org Subject: Re: [rfc, rfa/doc] Multi-threaded watchpoint improvements References: <20070916183949.GA23966@caradoc.them.org> <20071001002015.GA15835@caradoc.them.org> <20080416224910.GA3716@caradoc.them.org> <20080416231831.GB6274@caradoc.them.org> <20080417123455.GA25679@caradoc.them.org> X-Yow: .. ich bin in einem dusenjet ins jahr 53 vor chr... ich lande im antiken Rom... einige gladiatoren spielen scrabble... ich rieche PIZZA... Date: Wed, 23 Apr 2008 11:55:00 -0000 In-Reply-To: (Andreas Schwab's message of "Sat, 19 Apr 2008 18:03:00 +0200") Message-ID: User-Agent: Gnus/5.110008 (No Gnus v0.8) Emacs/22.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit 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: 2008-04/txt/msg00511.txt.bz2 Andreas Schwab writes: > Daniel Jacobowitz writes: > >> On Thu, Apr 17, 2008 at 11:52:31AM +0200, Andreas Schwab wrote: >>> Looking closer, it is actually a kernel bug. PTRACE_GETSIGINFO is not >>> emulated for 32-bit processes, so that si_addr is set to the upper half >>> of the address, which is of course zero. >> >> Glad you could track that down. Yes, my patch made GDB less tolerant >> of targets which claim they can report the stopped data address, but >> actually fail. It will only report watchpoints when the target >> doesn't know what address has changed, or report a changed address >> that falls on a particular watchpoint. This lets us keep track of >> which thread hit each watchpoint. > > There is still a problem with that: if the watchpoint granularity is > bigger than the size of the data then gdb can still get this wrong. Here's a patch. OK? Andreas. 2008-04-23 Andreas Schwab * target.h (struct target_ops): Add to_watchpoint_addr_within_range. (target_watchpoint_addr_within_range): New function. * target.c (update_current_target): Inherit to_watchpoint_addr_within_range, defaulting to default_watchpoint_addr_within_range. (default_watchpoint_addr_within_range): New function. (debug_to_watchpoint_addr_within_range): New function. (setup_target_debug): Set to_watchpoint_addr_within_range. * ppc-linux-nat.c (ppc_linux_watchpoint_addr_within_range): New function. (_initialize_ppc_linux_nat): Set to_watchpoint_addr_within_range. * breakpoint.c (watchpoints_triggered): Use target_watchpoint_addr_within_range. doc/: * gdbint.texinfo (Algorithms): Describe target_watchpoint_addr_within_range. Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.310 diff -u -a -p -a -u -p -r1.310 breakpoint.c --- breakpoint.c 18 Apr 2008 00:41:28 -0000 1.310 +++ breakpoint.c 23 Apr 2008 11:00:51 -0000 @@ -2552,8 +2552,9 @@ watchpoints_triggered (struct target_wai for (loc = b->loc; loc; loc = loc->next) /* Exact match not required. Within range is sufficient. */ - if (addr >= loc->address - && addr < loc->address + loc->length) + if (target_watchpoint_addr_within_range (¤t_target, + addr, loc->address, + loc->length)) { b->watchpoint_triggered = watch_triggered_yes; break; Index: ppc-linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/ppc-linux-nat.c,v retrieving revision 1.78 diff -u -a -p -a -u -p -r1.78 ppc-linux-nat.c --- ppc-linux-nat.c 16 Jan 2008 04:48:55 -0000 1.78 +++ ppc-linux-nat.c 23 Apr 2008 11:00:52 -0000 @@ -889,6 +889,16 @@ ppc_linux_stopped_by_watchpoint (void) return ppc_linux_stopped_data_address (¤t_target, &addr); } +static int +ppc_linux_watchpoint_addr_within_range (struct target_ops *target, + CORE_ADDR addr, + CORE_ADDR start, int length) +{ + addr &= ~7; + /* Check whether [start, start+length-1] intersects [addr, addr+7]. */ + return start <= addr + 7 && start + length - 1 >= addr; +} + static void ppc_linux_store_inferior_registers (struct regcache *regcache, int regno) { @@ -997,6 +1007,7 @@ _initialize_ppc_linux_nat (void) t->to_remove_watchpoint = ppc_linux_remove_watchpoint; t->to_stopped_by_watchpoint = ppc_linux_stopped_by_watchpoint; t->to_stopped_data_address = ppc_linux_stopped_data_address; + t->to_watchpoint_addr_within_range = ppc_linux_watchpoint_addr_within_range; t->to_read_description = ppc_linux_read_description; Index: target.c =================================================================== RCS file: /cvs/src/src/gdb/target.c,v retrieving revision 1.159 diff -u -a -p -a -u -p -r1.159 target.c --- target.c 28 Mar 2008 16:37:08 -0000 1.159 +++ target.c 23 Apr 2008 11:00:52 -0000 @@ -49,6 +49,9 @@ static void kill_or_be_killed (int); static void default_terminal_info (char *, int); +static int default_watchpoint_addr_within_range (struct target_ops *, + CORE_ADDR, CORE_ADDR, int); + static int default_region_ok_for_hw_watchpoint (CORE_ADDR, int); static int nosymbol (char *, CORE_ADDR *); @@ -131,6 +134,9 @@ static int debug_to_stopped_by_watchpoin static int debug_to_stopped_data_address (struct target_ops *, CORE_ADDR *); +static int debug_to_watchpoint_addr_within_range (struct target_ops *, + CORE_ADDR, CORE_ADDR, int); + static int debug_to_region_ok_for_hw_watchpoint (CORE_ADDR, int); static void debug_to_terminal_init (void); @@ -416,9 +422,10 @@ update_current_target (void) INHERIT (to_insert_watchpoint, t); INHERIT (to_remove_watchpoint, t); INHERIT (to_stopped_data_address, t); - INHERIT (to_stopped_by_watchpoint, t); INHERIT (to_have_steppable_watchpoint, t); INHERIT (to_have_continuable_watchpoint, t); + INHERIT (to_stopped_by_watchpoint, t); + INHERIT (to_watchpoint_addr_within_range, t); INHERIT (to_region_ok_for_hw_watchpoint, t); INHERIT (to_terminal_init, t); INHERIT (to_terminal_inferior, t); @@ -544,6 +551,8 @@ update_current_target (void) de_fault (to_stopped_data_address, (int (*) (struct target_ops *, CORE_ADDR *)) return_zero); + de_fault (to_watchpoint_addr_within_range, + default_watchpoint_addr_within_range); de_fault (to_region_ok_for_hw_watchpoint, default_region_ok_for_hw_watchpoint); de_fault (to_terminal_init, @@ -1873,6 +1882,14 @@ default_region_ok_for_hw_watchpoint (COR } static int +default_watchpoint_addr_within_range (struct target_ops *target, + CORE_ADDR addr, + CORE_ADDR start, int length) +{ + return addr >= start && addr < start + length; +} + +static int return_zero (void) { return 0; @@ -2440,6 +2457,23 @@ debug_to_stopped_data_address (struct ta } static int +debug_to_watchpoint_addr_within_range (struct target_ops *target, + CORE_ADDR addr, + CORE_ADDR start, int length) +{ + int retval; + + retval = debug_target.to_watchpoint_addr_within_range (target, addr, + start, length); + + fprintf_filtered (gdb_stdlog, + "target_watchpoint_addr_within_range (0x%lx, 0x%lx, %d) = %d\n", + (unsigned long) addr, (unsigned long) start, length, + retval); + return retval; +} + +static int debug_to_insert_hw_breakpoint (struct bp_target_info *bp_tgt) { int retval; @@ -2782,6 +2816,7 @@ setup_target_debug (void) current_target.to_remove_watchpoint = debug_to_remove_watchpoint; current_target.to_stopped_by_watchpoint = debug_to_stopped_by_watchpoint; current_target.to_stopped_data_address = debug_to_stopped_data_address; + current_target.to_watchpoint_addr_within_range = debug_to_watchpoint_addr_within_range; current_target.to_region_ok_for_hw_watchpoint = debug_to_region_ok_for_hw_watchpoint; current_target.to_terminal_init = debug_to_terminal_init; current_target.to_terminal_inferior = debug_to_terminal_inferior; Index: target.h =================================================================== RCS file: /cvs/src/src/gdb/target.h,v retrieving revision 1.116 diff -u -a -p -a -u -p -r1.116 target.h --- target.h 8 Apr 2008 17:02:23 -0000 1.116 +++ target.h 23 Apr 2008 11:00:52 -0000 @@ -367,6 +367,8 @@ struct target_ops int to_have_steppable_watchpoint; int to_have_continuable_watchpoint; int (*to_stopped_data_address) (struct target_ops *, CORE_ADDR *); + int (*to_watchpoint_addr_within_range) (struct target_ops *, + CORE_ADDR, CORE_ADDR, int); int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR, int); void (*to_terminal_init) (void); void (*to_terminal_inferior) (void); @@ -1093,6 +1095,9 @@ extern int target_stopped_data_address_p #define target_stopped_data_address_p(CURRENT_TARGET) (1) #endif +#define target_watchpoint_addr_within_range(target, addr, start, length) \ + (*target.to_watchpoint_addr_within_range) (target, addr, start, length) + extern const struct target_desc *target_read_description (struct target_ops *); /* Command logging facility. */ --- doc/gdbint.texinfo.~1.281.~ 2008-04-22 11:46:53.000000000 +0200 +++ doc/gdbint.texinfo 2008-04-23 12:01:04.000000000 +0200 @@ -9,7 +9,7 @@ @ifinfo This file documents the internals of the GNU debugger @value{GDBN}. Copyright (C) 1990, 1991, 1992, 1993, 1994, 1996, 1998, 1999, 2000, 2001, - 2002, 2003, 2004, 2005, 2006 + 2002, 2003, 2004, 2005, 2006, 2008 Free Software Foundation, Inc. Contributed by Cygnus Solutions. Written by John Gilmore. Second Edition by Stan Shebs. @@ -743,10 +743,19 @@ target's watchpoint indication is sticky resuming, this method should clear it. For instance, the x86 debug control register has sticky triggered flags. +@findex target_watchpoint_addr_within_range +@item target_watchpoint_addr_within_range (@var{target}, @var{addr}, @var{start}, @var{length}) +Check whether @var{addr} (as returned by target_stopped_data_address) +lies within the hardware-defined watchpoint region described by +@var{start} and @var{length}. This only needs to be provided if the +granularity of a watchpoint is greater than one byte, i.e., if the +watchpoint can also trigger on nearby addresses outside of the watched +region. + @findex HAVE_STEPPABLE_WATCHPOINT @item HAVE_STEPPABLE_WATCHPOINT If defined to a non-zero value, it is not necessary to disable a -watchpoint to step over it. Like @code{gdbarch_have_nonsteppable_watchpoint}, +watchpoint to step over it. Like @code{gdbarch_have_nonsteppable_watchpoint}, this is usually set when watchpoints trigger at the instruction which will perform an interesting read or write. It should be set if there is a temporary disable bit which allows the processor -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."