From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29291 invoked by alias); 23 Apr 2010 01:13:05 -0000 Received: (qmail 29278 invoked by uid 22791); 23 Apr 2010 01:13:04 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=BAYES_00,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 23 Apr 2010 01:12:58 +0000 Received: (qmail 22014 invoked from network); 23 Apr 2010 01:12:57 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 23 Apr 2010 01:12:57 -0000 From: Pedro Alves To: "Pierre Muller" Subject: Re: [RFC-v2] ARI fixes: Remove NAT_FILE for solaris Date: Fri, 23 Apr 2010 01:13:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: gdb-patches@sourceware.org, Peter.Schauer@regent.e-technik.tu-muenchen.de, "'Joel Brobecker'" References: <001501cad7ff$3950cf30$abf26d90$@muller@ics-cnrs.unistra.fr> <201004092042.05112.pedro@codesourcery.com> <000001cae274$41cfedb0$c56fc910$@muller@ics-cnrs.unistra.fr> In-Reply-To: <000001cae274$41cfedb0$c56fc910$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201004230212.54941.pedro@codesourcery.com> 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: 2010-04/txt/msg00776.txt.bz2 1st, the email subject is a bit misleading. The main issue here is to remove the solaris hack, not pacifying ARI. 2nd, I sayd in : > "First off, we get to decide if the special casing on solaris is > still needed in the first place. It may not be needed anymore. > What does it affect? What do the comments around it suggest? > For what versions of solaris was it relevant. Etc.." I'd still like to hear if we care if we're still supporting Solaris <= 2.7 (released 1998). > Second, if we still need the workaround, can it be done all > within the target side? I'm not convinced we can't take that approach instead. Can't we make procfs.c do the workaround itself, when it gets a single-step request? The new gdbserver Solaris port could do that same, if it cares for such old Solaris. I also made a third comment in that email; it can be wrong, but, please don't just ignore it. Lastly, I'd prefer to keep a patch that removes the workaround separate from a patch that finaly removes the empty NAT_FILE for solaris, and the associated glue. -- Pedro Alves On Friday 23 April 2010 00:33:46, Pierre Muller wrote: > Thanks to Peter's answer to gdb mailing list, > http://sourceware.org/ml/gdb/2010-04/msg00082.html > I was able to write a new patch. > > The main idea is to add a to_cannot_step_hw_watchpoints > filed to target_ops, create a macro called > target_cannot_step_hw_watchpoints and you that > inside infrun.c instead of the CANNOT_STEP_HW_WATCHPOINTS macro. > > The macro still gets defined for solaris 2.6 and 2.7 versions > and triggers solaris_cannot_step_hw_watchpoints > to return 1 instead of zero. > > The reason for using a function rather than an integer > field was to be able later (once Solaris has a gdbserver) > to also set this for remote targets. > > Comments? > > Pierre > > > 2010-04-23 Pierre Muller > > * configure.ac (Solaris): Set CANNOT_STEP_HW_WATCHPOINTS macro > for i386 cpu family and OS version <= 2.7. > * config.in: Regenerate. > * configure: Idem. > * i386-sol2-nat.c (solaris_cannot_step_hw_watchpoints): New > function. > (_initialize_amd64_sol2_nat): Set target > to_cannot_step_hw_watchpoints > to solaris_cannot_step_hw_watchpoints. > * infrun.c (show_debug_infrun): Remove redefinition of > CANNOT_STEP_HW_WATCHPOINTS macro. > (resume): Replace CANNOT_STEP_HW_WATCHPOINTS macro with > target_cannot_step_hw_watchpoints macro. > * target.h (target_ops): Add to_cannot_step_hw_watchpoints > field. > (target_cannot_step_hw_watchpoints): New macro. > * target.c (update_current_target): Inherited > to_cannot_step_hw_watchpoints. > Set default to return_zero function. > * config/i386/nm-i386sol2.h: Delete. > * config/i386/i386sol2.mh: Remove NAT_FILE. > * config/i386/sol2-64.mh: Idem. > > Index: src/gdb/config.in > =================================================================== > RCS file: /cvs/src/src/gdb/config.in,v > retrieving revision 1.116 > diff -u -p -r1.116 config.in > --- src/gdb/config.in 10 Mar 2010 18:37:22 -0000 1.116 > +++ src/gdb/config.in 22 Apr 2010 23:08:24 -0000 > @@ -18,6 +18,9 @@ > /* Define to the number of bits in type 'wint_t'. */ > #undef BITSIZEOF_WINT_T > > +/* Define for Solaris Kernel bug. */ > +#undef CANNOT_STEP_HW_WATCHPOINTS > + > /* Define to 1 if the compiler supports long long. */ > #undef CC_HAS_LONG_LONG > > Index: src/gdb/configure > =================================================================== > RCS file: /cvs/src/src/gdb/configure,v > retrieving revision 1.301 > diff -u -p -r1.301 configure > --- src/gdb/configure 15 Mar 2010 17:03:01 -0000 1.301 > +++ src/gdb/configure 22 Apr 2010 23:08:29 -0000 > @@ -11882,6 +11882,15 @@ $as_echo "#define NEW_PROC_API 1" >>conf > > $as_echo "#define NEW_PROC_API 1" >>confdefs.h > > +# This bug might have been solved, but for which version of solaris system? > + if test "${gdb_host_cpu}" = "i386"; then > + case "${host}" in > + *-*-solaris2.[67] ) > + > +$as_echo "#define CANNOT_STEP_HW_WATCHPOINTS 1" >>confdefs.h > + > + esac > + fi > ;; > mips-sgi-irix5*) > # Work around needing _KMEMUSER problem on IRIX 5. > Index: src/gdb/configure.ac > =================================================================== > RCS file: /cvs/src/src/gdb/configure.ac,v > retrieving revision 1.116 > diff -u -p -r1.116 configure.ac > --- src/gdb/configure.ac 15 Mar 2010 17:03:00 -0000 1.116 > +++ src/gdb/configure.ac 22 Apr 2010 23:08:30 -0000 > @@ -1035,6 +1035,14 @@ if test "${target}" = "${host}"; then > AC_DEFINE(NEW_PROC_API, 1, > [Define if you want to use new multi-fd /proc interface > (replaces HAVE_MULTIPLE_PROC_FDS as well as other macros).]) > +# This bug might have been solved, but for which version of solaris system? > + if test "${gdb_host_cpu}" = "i386"; then > + case "${host}" in > + *-*-solaris2.[[67]] ) > + AC_DEFINE(CANNOT_STEP_HW_WATCHPOINTS, 1, > + [Define for Solaris Kernel bug.]) > + esac > + fi > ;; > mips-sgi-irix5*) > # Work around needing _KMEMUSER problem on IRIX 5. > Index: src/gdb/i386-sol2-nat.c > =================================================================== > RCS file: /cvs/src/src/gdb/i386-sol2-nat.c,v > retrieving revision 1.11 > diff -u -p -r1.11 i386-sol2-nat.c > --- src/gdb/i386-sol2-nat.c 1 Jan 2010 07:31:34 -0000 1.11 > +++ src/gdb/i386-sol2-nat.c 22 Apr 2010 23:08:30 -0000 > @@ -130,6 +130,16 @@ fill_fpregset (const struct regcache *re > > #endif > > +static int > +solaris_cannot_step_hw_watchpoints (void) > +{ > +#ifdef CANNOT_STEP_HW_WATCHPOINTS > + return 1; > +#else > + return 0; > +#endif > +} > + > /* Provide a prototype to silence -Wmissing-prototypes. */ > extern void _initialize_amd64_sol2_nat (void); > > @@ -145,6 +155,8 @@ _initialize_amd64_sol2_nat (void) > procfs_use_watchpoints (t); > #endif > > + t->to_cannot_step_hw_watchpoints = solaris_cannot_step_hw_watchpoints; > + > #if defined (PR_MODEL_NATIVE) && (PR_MODEL_NATIVE == PR_MODEL_LP64) > amd64_native_gregset32_reg_offset = amd64_sol2_gregset32_reg_offset; > amd64_native_gregset32_num_regs = > Index: src/gdb/infrun.c > =================================================================== > RCS file: /cvs/src/src/gdb/infrun.c,v > retrieving revision 1.435 > diff -u -p -r1.435 infrun.c > --- src/gdb/infrun.c 25 Mar 2010 20:48:53 -0000 1.435 > +++ src/gdb/infrun.c 22 Apr 2010 23:08:32 -0000 > @@ -179,16 +179,6 @@ show_debug_infrun (struct ui_file *file, > #endif > > > -/* Convert the #defines into values. This is temporary until wfi control > - flow is completely sorted out. */ > - > -#ifndef CANNOT_STEP_HW_WATCHPOINTS > -#define CANNOT_STEP_HW_WATCHPOINTS 0 > -#else > -#undef CANNOT_STEP_HW_WATCHPOINTS > -#define CANNOT_STEP_HW_WATCHPOINTS 1 > -#endif > - > /* Tables of how to react to signals; the user sets them. */ > > static unsigned char *signal_stop; > @@ -1492,7 +1482,7 @@ resume (int step, enum target_signal sig > Work around the problem by removing hardware watchpoints if a step is > requested, GDB will check for a hardware watchpoint trigger after the > step anyway. */ > - if (CANNOT_STEP_HW_WATCHPOINTS && step) > + if (target_cannot_step_hw_watchpoints () && step) > remove_hw_watchpoints (); > > > Index: src/gdb/target.c > =================================================================== > RCS file: /cvs/src/src/gdb/target.c,v > retrieving revision 1.247 > diff -u -p -r1.247 target.c > --- src/gdb/target.c 19 Apr 2010 22:06:17 -0000 1.247 > +++ src/gdb/target.c 22 Apr 2010 23:08:33 -0000 > @@ -661,6 +661,7 @@ update_current_target (void) > INHERIT (to_set_disconnected_tracing, t); > INHERIT (to_set_circular_trace_buffer, t); > INHERIT (to_get_tib_address, t); > + INHERIT (to_cannot_step_hw_watchpoints, t); > INHERIT (to_magic, t); > /* Do not inherit to_memory_map. */ > /* Do not inherit to_flash_erase. */ > @@ -857,6 +858,9 @@ update_current_target (void) > de_fault (to_get_tib_address, > (int (*) (ptid_t, CORE_ADDR *)) > tcomplain); > + de_fault (to_cannot_step_hw_watchpoints, > + (int (*) (void)) > + return_zero); > #undef de_fault > > /* Finally, position the target-stack beneath the squashed > Index: src/gdb/target.h > =================================================================== > RCS file: /cvs/src/src/gdb/target.h,v > retrieving revision 1.178 > diff -u -p -r1.178 target.h > --- src/gdb/target.h 16 Apr 2010 07:49:35 -0000 1.178 > +++ src/gdb/target.h 22 Apr 2010 23:08:35 -0000 > @@ -686,6 +686,9 @@ struct target_ops > a Windows OS specific feature. */ > int (*to_get_tib_address) (ptid_t ptid, CORE_ADDR *addr); > > + /* Returns 1 if target needs to disable watchpoints before stepping. > */ > + int (*to_cannot_step_hw_watchpoints) (void); > + > int to_magic; > /* Need sub-structure for target machine related rather than comm > related? > */ > @@ -1378,6 +1381,9 @@ extern int target_search_memory (CORE_AD > #define target_get_tib_address(ptid, addr) \ > (*current_target.to_get_tib_address) ((ptid), (addr)) > > +#define target_cannot_step_hw_watchpoints() \ > + (*current_target.to_cannot_step_hw_watchpoints) () > + > /* Command logging facility. */ > > #define target_log_command(p) \ > Index: src/gdb/config/i386/i386sol2.mh > =================================================================== > RCS file: /cvs/src/src/gdb/config/i386/i386sol2.mh,v > retrieving revision 1.12 > diff -u -p -r1.12 i386sol2.mh > --- src/gdb/config/i386/i386sol2.mh 26 Oct 2009 18:28:13 -0000 1.12 > +++ src/gdb/config/i386/i386sol2.mh 22 Apr 2010 23:08:36 -0000 > @@ -1,4 +1,3 @@ > # Host: Solaris x86 > NATDEPFILES= fork-child.o i386v4-nat.o i386-sol2-nat.o \ > procfs.o proc-api.o proc-events.o proc-flags.o proc-why.o > -NAT_FILE= nm-i386sol2.h > Index: src/gdb/config/i386/nm-i386sol2.h > =================================================================== > RCS file: /cvs/src/src/gdb/config/i386/nm-i386sol2.h,v > retrieving revision 1.19 > diff -u -p -r1.19 nm-i386sol2.h > --- src/gdb/config/i386/nm-i386sol2.h 1 Jan 2010 07:31:48 -0000 1.19 > +++ src/gdb/config/i386/nm-i386sol2.h 22 Apr 2010 23:08:36 -0000 > @@ -1,32 +0,0 @@ > -/* Native support for i386 running Solaris 2. > - Copyright 1998, 1999, 2000, 2007, 2008, 2009, 2010 > - Free Software Foundation, Inc. > - > - This file is part of GDB. > - > - This program is free software; you can redistribute it and/or modify > - it under the terms of the GNU General Public License as published by > - the Free Software Foundation; either version 3 of the License, or > - (at your option) any later version. > - > - This program is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - GNU General Public License for more details. > - > - You should have received a copy of the GNU General Public License > - along with this program. If not, see . > */ > - > -#ifdef NEW_PROC_API /* Solaris 6 and above can do HW watchpoints */ > - > -/* Solaris x86 2.6 and 2.7 targets have a kernel bug when stepping > - over an instruction that causes a page fault without triggering > - a hardware watchpoint. The kernel properly notices that it shouldn't > - stop, because the hardware watchpoint is not triggered, but it forgets > - the step request and continues the program normally. > - Work around the problem by removing hardware watchpoints if a step is > - requested, GDB will check for a hardware watchpoint trigger after the > - step anyway. */ > -#define CANNOT_STEP_HW_WATCHPOINTS > - > -#endif /* NEW_PROC_API */ > Index: src/gdb/config/i386/sol2-64.mh > =================================================================== > RCS file: /cvs/src/src/gdb/config/i386/sol2-64.mh,v > retrieving revision 1.2 > diff -u -p -r1.2 sol2-64.mh > --- src/gdb/config/i386/sol2-64.mh 26 Oct 2009 18:28:13 -0000 1.2 > +++ src/gdb/config/i386/sol2-64.mh 22 Apr 2010 23:08:36 -0000 > @@ -1,4 +1,3 @@ > # Host: Solaris x86_64 > NATDEPFILES= fork-child.o amd64-nat.o i386v4-nat.o i386-sol2-nat.o \ > procfs.o proc-api.o proc-events.o proc-flags.o proc-why.o > -NAT_FILE= nm-i386sol2.h > >