[RFC] ARI fixes: Remove NAT_FILE for solaris

[RFC] ARI fixes: Remove NAT_FILE for solaris

[Pierre Muller]

  In the series of efforts to get rid of
native headers.

This patch simply removes i386/nm-i386sol2.h

The only remaining code was
#ifdef NEW_PROC_API
#define CANNOT_STEP_HW_WATCHPOINTS
#endif

  I moved that into gdb/configure.ac.
The most difficult part was to get all the tools
(in the correct versions) to be able to regenerate
config.in and configure on a solaris machine.

  One question might be if this bug is now fixed in
Solaris kernel and if we should not only define this for older kernels.
  Does anyone know of some simple code to test
if this bug is still present?


Pierre


2010-04-09  Pierre Muller  <muller@ics.u-strasbg.fr>

	* configure.ac (solaris): Define CANNOT_STEP_HW_WATCHPOINTS
	for i386 processor family.
	* config.in: Regenerate.
	* configure: Regenerate.
	* config/i386/i386sol2.mh: Suppress NAT_FILE.
	* config/i386/sol2-64.mh: Suppress NAT_FILE.
	* config/i386/nm-i386sol2.h: Delete file.


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	9 Apr 2010 15:13:37 -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	9 Apr 2010 15:13:39 -0000
@@ -11882,6 +11882,12 @@ $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
+
+$as_echo "#define CANNOT_STEP_HW_WATCHPOINTS 1" >>confdefs.h
+
+      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	9 Apr 2010 15:13:39 -0000
@@ -1035,6 +1035,11 @@ 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
+        AC_DEFINE(CANNOT_STEP_HW_WATCHPOINTS, 1,
+        [Define for Solaris Kernel bug.])
+      fi
       ;;
   mips-sgi-irix5*)
       # Work around <sys/proc.h> needing _KMEMUSER problem on IRIX 5.
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	9 Apr 2010 15:13:40 -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	9 Apr 2010 15:13:40 -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	9 Apr 2010 15:13:40 -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

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

[Pedro Alves]

On Friday 09 April 2010 17:10:50, Pierre Muller wrote:
> This patch simply removes i386/nm-i386sol2.h
> 
> The only remaining code was
> #ifdef NEW_PROC_API
> #define CANNOT_STEP_HW_WATCHPOINTS
> #endif
> 
>   I moved that into gdb/configure.ac.
> The most difficult part was to get all the tools
> (in the correct versions) to be able to regenerate
> config.in and configure on a solaris machine.

This fixes nothing.  There's a reason we want to get rid
of macros defined in nm files and used in common code.  As is,
a cross debugger hosted on solaris behaves differently from
a cross debugger hosted on all other hosts.

-- 
Pedro Alves

RE: [RFC] ARI fixes: Remove NAT_FILE for solaris

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Friday, April 09, 2010 6:13 PM
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFC] ARI fixes: Remove NAT_FILE for solaris
> 
> On Friday 09 April 2010 17:10:50, Pierre Muller wrote:
> > This patch simply removes i386/nm-i386sol2.h
> >
> > The only remaining code was
> > #ifdef NEW_PROC_API
> > #define CANNOT_STEP_HW_WATCHPOINTS
> > #endif
> >
> >   I moved that into gdb/configure.ac.
> > The most difficult part was to get all the tools
> > (in the correct versions) to be able to regenerate
> > config.in and configure on a solaris machine.
> 
> This fixes nothing.  There's a reason we want to get rid
> of macros defined in nm files and used in common code.  As is,
> a cross debugger hosted on solaris behaves differently from
> a cross debugger hosted on all other hosts.

  I agree, but I thought that doing this step by step would be easier...
My big patches take ages to get in, so I prefer to do it bitwise!

  How could we change this so that this
would only trigger if we are debugging native and not some other 
target?
  
  Could we change the macro
into a test that the current target is native solaris?
I don't know enough on gdbarch and tdep structures
to have an idea how to write such a test.

  Advices welcome.

Pierre

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

[Pedro Alves]

On Friday 09 April 2010 19:43:53, Pierre Muller wrote:
> > -----Message d'origine-----
> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] De la part de Pedro Alves
> > Envoyé : Friday, April 09, 2010 6:13 PM
> > À : gdb-patches@sourceware.org
> > Cc : Pierre Muller
> > Objet : Re: [RFC] ARI fixes: Remove NAT_FILE for solaris
> > 
> > On Friday 09 April 2010 17:10:50, Pierre Muller wrote:
> > > This patch simply removes i386/nm-i386sol2.h
> > >
> > > The only remaining code was
> > > #ifdef NEW_PROC_API
> > > #define CANNOT_STEP_HW_WATCHPOINTS
> > > #endif
> > >
> > >   I moved that into gdb/configure.ac.
> > > The most difficult part was to get all the tools
> > > (in the correct versions) to be able to regenerate
> > > config.in and configure on a solaris machine.
> > 
> > This fixes nothing.  There's a reason we want to get rid
> > of macros defined in nm files and used in common code.  As is,
> > a cross debugger hosted on solaris behaves differently from
> > a cross debugger hosted on all other hosts.
> 
>   I agree, but I thought that doing this step by step would be easier...

In principle that's fine.  But, the only effect of your patch
is pacifying the ARI; it's a step backwards.  ARI is tool with
a purpose --- remind us of things we need to fix, not to have
clean results by hiding bugs!

> My big patches take ages to get in, so I prefer to do it bitwise!

Yeah, sorry, I still have one of those in my queue.

> 
>   How could we change this so that this
> would only trigger if we are debugging native and not some other 
> target?

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..

Second, if we still need the workaround, can it be done all
within the target side?

>   Could we change the macro
> into a test that the current target is native solaris?

If we can't do the fix in the target side, then I don't
think that would be correct.  What if we're cross
debugging solaris from a non-solaris host (recently someone
mentioned they had a solaris gdbserver port)?  That would 
suggest a gdbarch flag instead (target_gdbarch).

But I'd first try to check if the workaround is still
relevant at all.

> I don't know enough on gdbarch and tdep structures
> to have an idea how to write such a test.
> 
>   Advices welcome.

-- 
Pedro Alves

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

[Pierre Muller]

  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

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

[Pedro Alves]

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
> 
>