This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC-v2] ARI fixes: Remove NAT_FILE for solaris


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 <http://sourceware.org/ml/gdb-patches/2010-04/msg00278.html>:

> "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  <muller@ics.u-strasbg.fr>
> 
> 	* 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 <sys/proc.h> 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 <sys/proc.h> 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 <http://www.gnu.org/licenses/>.
> */
> -
> -#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
> 
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]