[PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

This patch provides hardware breakpoint/watchpoint support for
FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.

gdb/ChangeLog:
2011-12-05  Valery Khromov  <valery.khromov@gmail.com>

	PR gdb/12953
	* Makefile.in (HFILES_NO_SRCDIR): Add amd64bsd-nat.h.
	* amd64bsd-nat.c: Add support for debug registers (adapted from
	i386bsd-nat.c).
	[HAVE_PT_GETDBREGS] (DBREG_DRX): Define if not already defined.
	[HAVE_PT_GETDBREGS] (amd64bsd_dr_set, amd64bsd_dr_set_control,
	amd64bsd_dr_set_addr, amd64bsd_dr_reset_addr, amd64bsd_dr_get_status):
	New functions.
	* amd64bsd-nat.h: New file (adapted from i386bsd-nat.h).
	* amd64fbsd-nat.c: Include "amd64bsd-nat.h", "i386-nat.h".
	[HAVE_PT_GETDBREGS] (_initialize_amd64fbsd_nat): Add hardware
	watchpoints initialization.
	* config/i386/fbsd64.mh (NATDEPFILES): Add i386-nat.o.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 7db23a7..a0d3010 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -758,7 +758,7 @@ osf-share/cma_stack_int.h osf-share/cma_init.h \
 osf-share/cma_deb_core.h osf-share/AT386/cma_thread_io.h \
 osf-share/cma_sched.h proc-utils.h arm-tdep.h ax-gdb.h ppcnbsd-tdep.h \
 cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
-exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h \
+exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
 i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
 ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
 c-lang.h d-lang.h frame.h event-loop.h block.h cli/cli-setshow.h	\
diff --git a/gdb/amd64bsd-nat.c b/gdb/amd64bsd-nat.c
index 7821f2b..8b2f678 100644
--- a/gdb/amd64bsd-nat.c
+++ b/gdb/amd64bsd-nat.c
@@ -126,3 +126,80 @@ amd64bsd_target (void)
   t->to_store_registers = amd64bsd_store_inferior_registers;
   return t;
 }
+\f
+
+/* Support for debug registers.  */
+
+#ifdef HAVE_PT_GETDBREGS
+
+/* Not all versions of FreeBSD/amd64 that support the debug registers
+   have this macro.  */
+#ifndef DBREG_DRX
+#define DBREG_DRX(d, x) ((&d->dr0)[x])
+#endif
+
+static void
+amd64bsd_dr_set (int regnum, unsigned long value)
+{
+  struct dbreg dbregs;
+
+  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
+              (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+    perror_with_name (_("Couldn't get debug registers"));
+
+  /* For some mysterious reason, some of the reserved bits in the
+     debug control register get set.  Mask these off, otherwise the
+     ptrace call below will fail.  */
+  DBREG_DRX ((&dbregs), 7) &= ~(0xffffffff0000fc00);
+
+  DBREG_DRX ((&dbregs), regnum) = value;
+
+  if (ptrace (PT_SETDBREGS, PIDGET (inferior_ptid),
+              (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+    perror_with_name (_("Couldn't write debug registers"));
+}
+
+void
+amd64bsd_dr_set_control (unsigned long control)
+{
+  amd64bsd_dr_set (7, control);
+}
+
+void
+amd64bsd_dr_set_addr (int regnum, CORE_ADDR addr)
+{
+  gdb_assert (regnum >= 0 && regnum <= 4);
+
+  amd64bsd_dr_set (regnum, addr);
+}
+
+void
+amd64bsd_dr_reset_addr (int regnum)
+{
+  gdb_assert (regnum >= 0 && regnum <= 4);
+
+  amd64bsd_dr_set (regnum, 0);
+}
+
+unsigned long
+amd64bsd_dr_get_status (void)
+{
+  struct dbreg dbregs;
+
+  /* FIXME: kettenis/2001-03-31: Calling perror_with_name if the
+     ptrace call fails breaks debugging remote targets.  The correct
+     way to fix this is to add the hardware breakpoint and watchpoint
+     stuff to the target vector.  For now, just return zero if the
+     ptrace call fails.  */
+  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
+	      (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+#if 0
+    perror_with_name (_("Couldn't read debug registers"));
+#else
+    return 0;
+#endif
+
+  return DBREG_DRX ((&dbregs), 6);
+}
+
+#endif /* PT_GETDBREGS */
diff --git a/gdb/amd64bsd-nat.h b/gdb/amd64bsd-nat.h
new file mode 100644
index 0000000..0048f54
--- /dev/null
+++ b/gdb/amd64bsd-nat.h
@@ -0,0 +1,35 @@
+/* Native-dependent code for modern AMD64 BSD's.
+
+   Copyright (C) 2011
+   Free Software Foundation, Inc.
+   Contributed by Valery Khromov, Yandex.
+
+   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/>.  */
+
+#ifndef AMD64BSD_NAT_H
+#define AMD64BSD_NAT_H
+
+/* low level amd64 debug register functions used in amd64fbsd-nat.c.  */
+
+extern void amd64bsd_dr_set_control (unsigned long control);
+
+extern void amd64bsd_dr_set_addr (int regnum, CORE_ADDR addr);
+
+extern void amd64bsd_dr_reset_addr (int regnum);
+
+extern unsigned long amd64bsd_dr_get_status (void);
+
+#endif /* amd64bsd-nat.h */
diff --git a/gdb/amd64fbsd-nat.c b/gdb/amd64fbsd-nat.c
index 1663abc..8ffd42b 100644
--- a/gdb/amd64fbsd-nat.c
+++ b/gdb/amd64fbsd-nat.c
@@ -35,6 +35,8 @@
 #include "fbsd-nat.h"
 #include "amd64-tdep.h"
 #include "amd64-nat.h"
+#include "amd64bsd-nat.h"
+#include "i386-nat.h"
 \f
 
 /* Offset in `struct reg' where MEMBER is stored.  */
@@ -197,6 +199,19 @@ _initialize_amd64fbsd_nat (void)
 
   /* Add some extra features to the common *BSD/i386 target.  */
   t = amd64bsd_target ();
+
+#ifdef HAVE_PT_GETDBREGS
+
+  i386_use_watchpoints (t);
+
+  i386_dr_low.set_control = amd64bsd_dr_set_control;
+  i386_dr_low.set_addr = amd64bsd_dr_set_addr;
+  i386_dr_low.reset_addr = amd64bsd_dr_reset_addr;
+  i386_dr_low.get_status = amd64bsd_dr_get_status;
+  i386_set_debug_register_length (8);
+
+#endif /* HAVE_PT_GETDBREGS */
+
   t->to_pid_to_exec_file = fbsd_pid_to_exec_file;
   t->to_find_memory_regions = fbsd_find_memory_regions;
   t->to_make_corefile_notes = fbsd_make_corefile_notes;
diff --git a/gdb/config/i386/fbsd64.mh b/gdb/config/i386/fbsd64.mh
index 2266def..24a87bf 100644
--- a/gdb/config/i386/fbsd64.mh
+++ b/gdb/config/i386/fbsd64.mh
@@ -1,6 +1,6 @@
 # Host: FreeBSD/amd64
 NATDEPFILES= fork-child.o inf-ptrace.o \
-	fbsd-nat.o amd64-nat.o amd64bsd-nat.o amd64fbsd-nat.o \
+	fbsd-nat.o i386-nat.o amd64-nat.o amd64bsd-nat.o amd64fbsd-nat.o \
 	bsd-kvm.o
 
 LOADLIBES= -lkvm
-- 
WBR,
Valery Khromov

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Tom Tromey]

>>>>> "Valery" == Valery Khromov <valery.khromov@gmail.com> writes:

Valery> This patch provides hardware breakpoint/watchpoint support for
Valery> FreeBSD/AMD64.  Most of the code is borrowed from the i386
Valery> implementation.

I'm sorry nobody has replied to this yet.

I'm not really the right person to review it.

Do you have copyright assignment paperwork filed with the FSF?

Tom

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Mark Kettenis]

> From: Valery Khromov <valery.khromov@gmail.com>
> Date: Tue,  6 Dec 2011 04:54:24 +0400
> 
> This patch provides hardware breakpoint/watchpoint support for
> FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.

I fear that since the recent changes that Pedro made, the code needs
to be resycnhed with the i386 version :(.  Can you do that?

While you're at it, here are a few comments.

> gdb/ChangeLog:
> 2011-12-05  Valery Khromov  <valery.khromov@gmail.com>
> 
> 	PR gdb/12953
> 	* Makefile.in (HFILES_NO_SRCDIR): Add amd64bsd-nat.h.
> 	* amd64bsd-nat.c: Add support for debug registers (adapted from
> 	i386bsd-nat.c).
> 	[HAVE_PT_GETDBREGS] (DBREG_DRX): Define if not already defined.
> 	[HAVE_PT_GETDBREGS] (amd64bsd_dr_set, amd64bsd_dr_set_control,
> 	amd64bsd_dr_set_addr, amd64bsd_dr_reset_addr, amd64bsd_dr_get_status):
> 	New functions.
> 	* amd64bsd-nat.h: New file (adapted from i386bsd-nat.h).
> 	* amd64fbsd-nat.c: Include "amd64bsd-nat.h", "i386-nat.h".
> 	[HAVE_PT_GETDBREGS] (_initialize_amd64fbsd_nat): Add hardware
> 	watchpoints initialization.
> 	* config/i386/fbsd64.mh (NATDEPFILES): Add i386-nat.o.
> 
> --- a/gdb/amd64bsd-nat.c
> +++ b/gdb/amd64bsd-nat.c
> @@ -126,3 +126,80 @@ amd64bsd_target (void)
>    t->to_store_registers = amd64bsd_store_inferior_registers;
>    return t;
>  }
> +\f
> +
> +/* Support for debug registers.  */
> +
> +#ifdef HAVE_PT_GETDBREGS
> +
> +/* Not all versions of FreeBSD/amd64 that support the debug registers
> +   have this macro.  */
> +#ifndef DBREG_DRX
> +#define DBREG_DRX(d, x) ((&d->dr0)[x])
> +#endif

That bit can go.  FreeBSD/amd64 has always had the DBREG_DRX macro, at
least ever since support for debug registers was added.

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Pedro Alves]

On Tuesday 06 December 2011 00:54:24, Valery Khromov wrote:
> +  /* FIXME: kettenis/2001-03-31: Calling perror_with_name if the
> +     ptrace call fails breaks debugging remote targets.  The correct
> +     way to fix this is to add the hardware breakpoint and watchpoint
> +     stuff to the target vector.  For now, just return zero if the
> +     ptrace call fails.  */

This is no longer necessary, and was recently removed everywhere.  See:

<http://sourceware.org/ml/gdb-patches/2011-12/msg00139.html>

> +  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
> +             (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
> +#if 0
> +    perror_with_name (_("Couldn't read debug registers"));
> +#else
> +    return 0;
> +#endif

-- 
Pedro Alves

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Pedro Alves]

On Tuesday 20 December 2011 15:03:09, Mark Kettenis wrote:
> > From: Valery Khromov <valery.khromov@gmail.com>
> > Date: Tue,  6 Dec 2011 04:54:24 +0400
> > 
> > This patch provides hardware breakpoint/watchpoint support for
> > FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.
> 
> I fear that since the recent changes that Pedro made, the code needs
> to be resycnhed with the i386 version :(.  Can you do that?

Yeah, sorry about that.  I was actually planning on taking the
hit myself and updating Valery's patch.  Let me know if you'd
like me to do it.

> 
> While you're at it, here are a few comments.
> 
> > gdb/ChangeLog:
> > 2011-12-05  Valery Khromov  <valery.khromov@gmail.com>
> > 
> > 	PR gdb/12953
> > 	* Makefile.in (HFILES_NO_SRCDIR): Add amd64bsd-nat.h.
> > 	* amd64bsd-nat.c: Add support for debug registers (adapted from
> > 	i386bsd-nat.c).
> > 	[HAVE_PT_GETDBREGS] (DBREG_DRX): Define if not already defined.
> > 	[HAVE_PT_GETDBREGS] (amd64bsd_dr_set, amd64bsd_dr_set_control,
> > 	amd64bsd_dr_set_addr, amd64bsd_dr_reset_addr, amd64bsd_dr_get_status):
> > 	New functions.
> > 	* amd64bsd-nat.h: New file (adapted from i386bsd-nat.h).
> > 	* amd64fbsd-nat.c: Include "amd64bsd-nat.h", "i386-nat.h".
> > 	[HAVE_PT_GETDBREGS] (_initialize_amd64fbsd_nat): Add hardware
> > 	watchpoints initialization.
> > 	* config/i386/fbsd64.mh (NATDEPFILES): Add i386-nat.o.
> > 
> > --- a/gdb/amd64bsd-nat.c
> > +++ b/gdb/amd64bsd-nat.c
> > @@ -126,3 +126,80 @@ amd64bsd_target (void)
> >    t->to_store_registers = amd64bsd_store_inferior_registers;
> >    return t;
> >  }
> > +\f
> > +
> > +/* Support for debug registers.  */
> > +
> > +#ifdef HAVE_PT_GETDBREGS
> > +
> > +/* Not all versions of FreeBSD/amd64 that support the debug registers
> > +   have this macro.  */
> > +#ifndef DBREG_DRX
> > +#define DBREG_DRX(d, x) ((&d->dr0)[x])
> > +#endif
> 
> That bit can go.  FreeBSD/amd64 has always had the DBREG_DRX macro, at
> least ever since support for debug registers was added.
> 

-- 
Pedro Alves

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

On 20.12.2011, at 19:05, Pedro Alves wrote:

> On Tuesday 20 December 2011 15:03:09, Mark Kettenis wrote:
>>> From: Valery Khromov <valery.khromov@gmail.com>
>>> Date: Tue,  6 Dec 2011 04:54:24 +0400
>>> 
>>> This patch provides hardware breakpoint/watchpoint support for
>>> FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.
>> 
>> I fear that since the recent changes that Pedro made, the code needs
>> to be resycnhed with the i386 version :(.  Can you do that?
> 
> Yeah, sorry about that.  I was actually planning on taking the
> hit myself and updating Valery's patch.  Let me know if you'd
> like me to do it.

See the following e-mails for fixes.

I prefer not to change my original patch because FreeBSD still uses gdb 7.3.1 in ports and this fixes incompatible with it.

-- 
WBR,
Valery Khromov

[PATCH 1/2] FreeBSD/amd64: Updates for the recent changes that Pedro Alves made.

[Valery Khromov]

2011-12-20  Valery Khromov  <valery.khromov@gmail.com>

    * amd64bsd-nat.c (amd64bsd_dr_get): New.
    (amd64bsd_dr_reset_addr): Delete.
    (amd64bsd_dr_get_addr): New.
    (amd64bsd_dr_get_status): Use amd64bsd_dr_get.
    (amd64bsd_dr_get_control): New.
    * amd64bsd-nat.h (amd64bsd_dr_reset_addr): Delete.
    (amd64bsd_dr_get_addr): New.
    (amd64bsd_dr_get_control): New.
    * amd64fbsd-nat.c (_initialize_amd64fbsd_nat): No longer install
    i386_dr_low.reset_addr and i386_dr_low.unset_status.  Install
    amd64bsd_dr_get_control as i386_dr_low.get_control.  Install
    amd64bsd_dr_get_addr as i386_dr_low.get_addr.
---
 gdb/amd64bsd-nat.c  |   41 +++++++++++++++++++++--------------------
 gdb/amd64bsd-nat.h  |    4 +++-
 gdb/amd64fbsd-nat.c |    3 ++-
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/gdb/amd64bsd-nat.c b/gdb/amd64bsd-nat.c
index 8b2f678..b31002f 100644
--- a/gdb/amd64bsd-nat.c
+++ b/gdb/amd64bsd-nat.c
@@ -138,6 +138,18 @@ amd64bsd_target (void)
 #define DBREG_DRX(d, x) ((&d->dr0)[x])
 #endif
 
+static unsigned long
+amd64bsd_dr_get (ptid_t ptid, int regnum)
+{
+  struct dbreg dbregs;
+
+  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
+	      (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+    perror_with_name (_("Couldn't read debug registers"));
+
+  return DBREG_DRX ((&dbregs), regnum);
+}
+
 static void
 amd64bsd_dr_set (int regnum, unsigned long value)
 {
@@ -173,33 +185,22 @@ amd64bsd_dr_set_addr (int regnum, CORE_ADDR addr)
   amd64bsd_dr_set (regnum, addr);
 }
 
-void
-amd64bsd_dr_reset_addr (int regnum)
+CORE_ADDR
+amd64bsd_dr_get_addr (int regnum)
 {
-  gdb_assert (regnum >= 0 && regnum <= 4);
-
-  amd64bsd_dr_set (regnum, 0);
+  return amd64bsd_dr_get (inferior_ptid, regnum);
 }
 
 unsigned long
 amd64bsd_dr_get_status (void)
 {
-  struct dbreg dbregs;
-
-  /* FIXME: kettenis/2001-03-31: Calling perror_with_name if the
-     ptrace call fails breaks debugging remote targets.  The correct
-     way to fix this is to add the hardware breakpoint and watchpoint
-     stuff to the target vector.  For now, just return zero if the
-     ptrace call fails.  */
-  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
-	      (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
-#if 0
-    perror_with_name (_("Couldn't read debug registers"));
-#else
-    return 0;
-#endif
+  return amd64bsd_dr_get (inferior_ptid, 6);
+}
 
-  return DBREG_DRX ((&dbregs), 6);
+unsigned long
+amd64bsd_dr_get_control (void)
+{
+  return amd64bsd_dr_get (inferior_ptid, 7);
 }
 
 #endif /* PT_GETDBREGS */
diff --git a/gdb/amd64bsd-nat.h b/gdb/amd64bsd-nat.h
index 0048f54..25eb8d4 100644
--- a/gdb/amd64bsd-nat.h
+++ b/gdb/amd64bsd-nat.h
@@ -28,8 +28,10 @@ extern void amd64bsd_dr_set_control (unsigned long control);
 
 extern void amd64bsd_dr_set_addr (int regnum, CORE_ADDR addr);
 
-extern void amd64bsd_dr_reset_addr (int regnum);
+extern CORE_ADDR amd64bsd_dr_get_addr (int regnum);
 
 extern unsigned long amd64bsd_dr_get_status (void);
 
+extern unsigned long amd64bsd_dr_get_control (void);
+
 #endif /* amd64bsd-nat.h */
diff --git a/gdb/amd64fbsd-nat.c b/gdb/amd64fbsd-nat.c
index 8ffd42b..a0ff739 100644
--- a/gdb/amd64fbsd-nat.c
+++ b/gdb/amd64fbsd-nat.c
@@ -206,8 +206,9 @@ _initialize_amd64fbsd_nat (void)
 
   i386_dr_low.set_control = amd64bsd_dr_set_control;
   i386_dr_low.set_addr = amd64bsd_dr_set_addr;
-  i386_dr_low.reset_addr = amd64bsd_dr_reset_addr;
+  i386_dr_low.get_addr = amd64bsd_dr_get_addr;
   i386_dr_low.get_status = amd64bsd_dr_get_status;
+  i386_dr_low.get_control = amd64bsd_dr_get_control;
   i386_set_debug_register_length (8);
 
 #endif /* HAVE_PT_GETDBREGS */
-- 
WBR,
Valery Khromov

[PATCH 2/2] FreeBSD: Removed definition of DBREG_DRX macro because it is already in FreeBSD.

[Valery Khromov]

2011-12-20  Valery Khromov  <valery.khromov@gmail.com>

    * i386bsd-nat.c (DBREG_DRX): Delete.
    * amd64bsd-nat.c (DBREG_DRX): Delete.
---
 gdb/amd64bsd-nat.c |    6 ------
 gdb/i386bsd-nat.c  |    6 ------
 2 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/gdb/amd64bsd-nat.c b/gdb/amd64bsd-nat.c
index b31002f..21ab3ea 100644
--- a/gdb/amd64bsd-nat.c
+++ b/gdb/amd64bsd-nat.c
@@ -132,12 +132,6 @@ amd64bsd_target (void)
 
 #ifdef HAVE_PT_GETDBREGS
 
-/* Not all versions of FreeBSD/amd64 that support the debug registers
-   have this macro.  */
-#ifndef DBREG_DRX
-#define DBREG_DRX(d, x) ((&d->dr0)[x])
-#endif
-
 static unsigned long
 amd64bsd_dr_get (ptid_t ptid, int regnum)
 {
diff --git a/gdb/i386bsd-nat.c b/gdb/i386bsd-nat.c
index 2301c69..b727466 100644
--- a/gdb/i386bsd-nat.c
+++ b/gdb/i386bsd-nat.c
@@ -258,10 +258,4 @@ i386bsd_target (void)
 
 #ifdef HAVE_PT_GETDBREGS
 
-/* Not all versions of FreeBSD/i386 that support the debug registers
-   have this macro.  */
-#ifndef DBREG_DRX
-#define DBREG_DRX(d, x) ((&d->dr0)[x])
-#endif
-
 static void
-- 
WBR,
Valery Khromov

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

On 20.12.2011, at 17:42, Tom Tromey wrote:

>>>>>> "Valery" == Valery Khromov <valery.khromov@gmail.com> writes:
> 
> Valery> This patch provides hardware breakpoint/watchpoint support for
> Valery> FreeBSD/AMD64.  Most of the code is borrowed from the i386
> Valery> implementation.
> 
> I'm sorry nobody has replied to this yet.
> 
> I'm not really the right person to review it.
> 
> Do you have copyright assignment paperwork filed with the FSF?


No, I don't.
How can I do it?

-- 
WBR,
Valery Khromov

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Mark Kettenis]

> From: Valery Khromov <valery.khromov@gmail.com>
> Date: Tue, 20 Dec 2011 20:47:53 +0300
> 
> On 20.12.2011, at 19:05, Pedro Alves wrote:
> 
> > On Tuesday 20 December 2011 15:03:09, Mark Kettenis wrote:
> >>> From: Valery Khromov <valery.khromov@gmail.com>
> >>> Date: Tue,  6 Dec 2011 04:54:24 +0400
> >>> 
> >>> This patch provides hardware breakpoint/watchpoint support for
> >>> FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.
> >> 
> >> I fear that since the recent changes that Pedro made, the code needs
> >> to be resycnhed with the i386 version :(.  Can you do that?
> > 
> > Yeah, sorry about that.  I was actually planning on taking the
> > hit myself and updating Valery's patch.  Let me know if you'd
> > like me to do it.
> 
> See the following e-mails for fixes.
> 
> I prefer not to change my original patch because FreeBSD still uses
> gdb 7.3.1 in ports and this fixes incompatible with it.

Sorry, you'll have to send a single diff against the current GDB CVS
sources for this, with a single Changelog entry.  Diffs on top of
diffs are hard to review, and there is no point in having
intermediate/broken steps in our CVS repository.

[PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

This patch provides hardware breakpoint/watchpoint support for
FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.

Also removed not necessary DBREG_DRX macro definition.

gdb/ChangeLog:
2011-12-20  Valery Khromov  <valery.khromov@gmail.com>

	PR gdb/12953
	* Makefile.in (HFILES_NO_SRCDIR): Add amd64bsd-nat.h.
	* amd64bsd-nat.c: Add support for debug registers (adapted from
	i386bsd-nat.c).
	[HAVE_PT_GETDBREGS] (amd64bsd_dr_get, amd64bsd_dr_set,
	amd64bsd_dr_set_control, amd64bsd_dr_set_addr, amd64bsd_dr_get_addr,
	amd64bsd_dr_get_status, amd64bsd_dr_get_control): New functions.
	* amd64bsd-nat.h: New file (adapted from i386bsd-nat.h).
	* amd64fbsd-nat.c: Include "amd64bsd-nat.h", "i386-nat.h".
	[HAVE_PT_GETDBREGS] (_initialize_amd64fbsd_nat): Add hardware
	watchpoints initialization.
	* config/i386/fbsd64.mh (NATDEPFILES): Add i386-nat.o.
	* i386bsd-nat.c: [HAVE_PT_GETDBREGS] (DBREG_DRX): Delete.
---
 gdb/Makefile.in           |    2 +-
 gdb/amd64bsd-nat.c        |   72 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/amd64bsd-nat.h        |   37 +++++++++++++++++++++++
 gdb/amd64fbsd-nat.c       |   16 ++++++++++
 gdb/config/i386/fbsd64.mh |    2 +-
 gdb/i386bsd-nat.c         |    6 ----
 6 files changed, 127 insertions(+), 8 deletions(-)
 create mode 100644 gdb/amd64bsd-nat.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b71db33..ada8585 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -772,7 +772,7 @@ common/gdb_signals.h common/gdb_thread_db.h common/i386-xstate.h \
 common/linux-ptrace.h \
 proc-utils.h arm-tdep.h ax-gdb.h ppcnbsd-tdep.h	\
 cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
-exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h \
+exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
 i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
 ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
 c-lang.h d-lang.h frame.h event-loop.h block.h cli/cli-setshow.h	\
diff --git a/gdb/amd64bsd-nat.c b/gdb/amd64bsd-nat.c
index 7821f2b..21ab3ea 100644
--- a/gdb/amd64bsd-nat.c
+++ b/gdb/amd64bsd-nat.c
@@ -126,3 +126,75 @@ amd64bsd_target (void)
   t->to_store_registers = amd64bsd_store_inferior_registers;
   return t;
 }
+\f
+
+/* Support for debug registers.  */
+
+#ifdef HAVE_PT_GETDBREGS
+
+static unsigned long
+amd64bsd_dr_get (ptid_t ptid, int regnum)
+{
+  struct dbreg dbregs;
+
+  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
+	      (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+    perror_with_name (_("Couldn't read debug registers"));
+
+  return DBREG_DRX ((&dbregs), regnum);
+}
+
+static void
+amd64bsd_dr_set (int regnum, unsigned long value)
+{
+  struct dbreg dbregs;
+
+  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
+              (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+    perror_with_name (_("Couldn't get debug registers"));
+
+  /* For some mysterious reason, some of the reserved bits in the
+     debug control register get set.  Mask these off, otherwise the
+     ptrace call below will fail.  */
+  DBREG_DRX ((&dbregs), 7) &= ~(0xffffffff0000fc00);
+
+  DBREG_DRX ((&dbregs), regnum) = value;
+
+  if (ptrace (PT_SETDBREGS, PIDGET (inferior_ptid),
+              (PTRACE_TYPE_ARG3) &dbregs, 0) == -1)
+    perror_with_name (_("Couldn't write debug registers"));
+}
+
+void
+amd64bsd_dr_set_control (unsigned long control)
+{
+  amd64bsd_dr_set (7, control);
+}
+
+void
+amd64bsd_dr_set_addr (int regnum, CORE_ADDR addr)
+{
+  gdb_assert (regnum >= 0 && regnum <= 4);
+
+  amd64bsd_dr_set (regnum, addr);
+}
+
+CORE_ADDR
+amd64bsd_dr_get_addr (int regnum)
+{
+  return amd64bsd_dr_get (inferior_ptid, regnum);
+}
+
+unsigned long
+amd64bsd_dr_get_status (void)
+{
+  return amd64bsd_dr_get (inferior_ptid, 6);
+}
+
+unsigned long
+amd64bsd_dr_get_control (void)
+{
+  return amd64bsd_dr_get (inferior_ptid, 7);
+}
+
+#endif /* PT_GETDBREGS */
diff --git a/gdb/amd64bsd-nat.h b/gdb/amd64bsd-nat.h
new file mode 100644
index 0000000..25eb8d4
--- /dev/null
+++ b/gdb/amd64bsd-nat.h
@@ -0,0 +1,37 @@
+/* Native-dependent code for modern AMD64 BSD's.
+
+   Copyright (C) 2011
+   Free Software Foundation, Inc.
+   Contributed by Valery Khromov, Yandex.
+
+   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/>.  */
+
+#ifndef AMD64BSD_NAT_H
+#define AMD64BSD_NAT_H
+
+/* low level amd64 debug register functions used in amd64fbsd-nat.c.  */
+
+extern void amd64bsd_dr_set_control (unsigned long control);
+
+extern void amd64bsd_dr_set_addr (int regnum, CORE_ADDR addr);
+
+extern CORE_ADDR amd64bsd_dr_get_addr (int regnum);
+
+extern unsigned long amd64bsd_dr_get_status (void);
+
+extern unsigned long amd64bsd_dr_get_control (void);
+
+#endif /* amd64bsd-nat.h */
diff --git a/gdb/amd64fbsd-nat.c b/gdb/amd64fbsd-nat.c
index 359138d..ec6d4a4 100644
--- a/gdb/amd64fbsd-nat.c
+++ b/gdb/amd64fbsd-nat.c
@@ -34,6 +34,8 @@
 #include "fbsd-nat.h"
 #include "amd64-tdep.h"
 #include "amd64-nat.h"
+#include "amd64bsd-nat.h"
+#include "i386-nat.h"
 \f
 
 /* Offset in `struct reg' where MEMBER is stored.  */
@@ -155,6 +157,20 @@ _initialize_amd64fbsd_nat (void)
 
   /* Add some extra features to the common *BSD/i386 target.  */
   t = amd64bsd_target ();
+
+#ifdef HAVE_PT_GETDBREGS
+
+  i386_use_watchpoints (t);
+
+  i386_dr_low.set_control = amd64bsd_dr_set_control;
+  i386_dr_low.set_addr = amd64bsd_dr_set_addr;
+  i386_dr_low.get_addr = amd64bsd_dr_get_addr;
+  i386_dr_low.get_status = amd64bsd_dr_get_status;
+  i386_dr_low.get_control = amd64bsd_dr_get_control;
+  i386_set_debug_register_length (8);
+
+#endif /* HAVE_PT_GETDBREGS */
+
   t->to_pid_to_exec_file = fbsd_pid_to_exec_file;
   t->to_find_memory_regions = fbsd_find_memory_regions;
   t->to_make_corefile_notes = fbsd_make_corefile_notes;
diff --git a/gdb/config/i386/fbsd64.mh b/gdb/config/i386/fbsd64.mh
index 2266def..24a87bf 100644
--- a/gdb/config/i386/fbsd64.mh
+++ b/gdb/config/i386/fbsd64.mh
@@ -1,6 +1,6 @@
 # Host: FreeBSD/amd64
 NATDEPFILES= fork-child.o inf-ptrace.o \
-	fbsd-nat.o amd64-nat.o amd64bsd-nat.o amd64fbsd-nat.o \
+	fbsd-nat.o i386-nat.o amd64-nat.o amd64bsd-nat.o amd64fbsd-nat.o \
 	bsd-kvm.o
 
 LOADLIBES= -lkvm
diff --git a/gdb/i386bsd-nat.c b/gdb/i386bsd-nat.c
index 22c79e2..661a07f 100644
--- a/gdb/i386bsd-nat.c
+++ b/gdb/i386bsd-nat.c
@@ -258,12 +258,6 @@ i386bsd_target (void)
 
 #ifdef HAVE_PT_GETDBREGS
 
-/* Not all versions of FreeBSD/i386 that support the debug registers
-   have this macro.  */
-#ifndef DBREG_DRX
-#define DBREG_DRX(d, x) ((&d->dr0)[x])
-#endif
-
 static unsigned long
 i386bsd_dr_get (ptid_t ptid, int regnum)
 {
-- 
WBR,
Valery Khromov

Re: [PATCH] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Joel Brobecker]

> > Do you have copyright assignment paperwork filed with the FSF?
> 
> No, I don't.
> How can I do it?

Let me send you the paperwork separately.

-- 
Joel

Re: [PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

Hi,

I have copyright assignment paperwork finished.
What should I do next?

On 20.12.2011, at 23:10, Valery Khromov wrote:

> This patch provides hardware breakpoint/watchpoint support for
> FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.

-- 
WBR,
Valery Khromov

Re: [PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Tom Tromey]

>>>>> "Valery" == Valery Khromov <valery.khromov@gmail.com> writes:

Valery> I have copyright assignment paperwork finished.
Valery> What should I do next?

I re-read the thread and I think there is still an unfulfilled request:

http://sourceware.org/ml/gdb-patches/2011-12/msg00695.html

Other than that, wait for review.

Feel free to ping a complete patch weekly until someone responds.

Tom

Re: [PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

On 01.02.2012, at 23:27, Tom Tromey wrote:

>>>>>> "Valery" == Valery Khromov <valery.khromov@gmail.com> writes:
> 
> Valery> I have copyright assignment paperwork finished.
> Valery> What should I do next?
> 
> I re-read the thread and I think there is still an unfulfilled request:
> 
> http://sourceware.org/ml/gdb-patches/2011-12/msg00695.html
> 

"[PATCH v2]" is the latest version of the original patch which contains a single diff agains the current GDB CVS
sources (as for December 2011).
So, I believe the request from the link above has been fulfilled.

> Other than that, wait for review.
> 
> Feel free to ping a complete patch weekly until someone responds.
> 
> Tom

-- 
WBR,
Valery Khromov

Re: [PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Pedro Alves]

On 12/20/2011 08:10 PM, Valery Khromov wrote:
> This patch provides hardware breakpoint/watchpoint support for
> FreeBSD/AMD64.  Most of the code is borrowed from the i386 implementation.

Thanks.  It looks good to me.  I've applied the patch now.

> Also removed not necessary DBREG_DRX macro definition.

> 2011-12-20  Valery Khromov  <valery.khromov@gmail.com>
> 
> 	* i386bsd-nat.c: [HAVE_PT_GETDBREGS] (DBREG_DRX): Delete.

But without this bit though.  Mark only mentioned that the macro always
exists on amd64, not i386:

> That bit can go.  FreeBSD/amd64 has always had the DBREG_DRX macro, at
> least ever since support for debug registers was added.

The code does mention that some versions may need it:

/* Not all versions of FreeBSD/i386 that support the debug registers
   have this macro.  */
#ifndef DBREG_DRX
#define DBREG_DRX(d, x) ((&d->dr0)[x])
#endif

So if this can also be removed, let's do that separately.

-- 
Pedro Alves

Re: [PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Mark Kettenis]

> From: Valery Khromov <valery.khromov@gmail.com>
> Date: Thu, 2 Feb 2012 12:03:23 +0300
> 
> On 01.02.2012, at 23:27, Tom Tromey wrote:
> 
> >>>>>> "Valery" == Valery Khromov <valery.khromov@gmail.com> writes:
> > 
> > Valery> I have copyright assignment paperwork finished.
> > Valery> What should I do next?
> > 
> > I re-read the thread and I think there is still an unfulfilled request:
> > 
> > http://sourceware.org/ml/gdb-patches/2011-12/msg00695.html
> > 
> 
> "[PATCH v2]" is the latest version of the original patch which contains a single diff agains the current GDB CVS
> sources (as for December 2011).
> So, I believe the request from the link above has been fulfilled.

Yes.  I think the "Contributed by ..." bit in amd64bsd-nat.h is a bit
silly given that this file only contains a couple of function
prototypes, but that's hardly worth arguing about.

Re: [PATCH v2] Fix PR gdb/12953: No hardware watchpoints on FreeBSD amd64

[Valery Khromov]

On 11.02.2012, at 21:51, Mark Kettenis wrote:

>> From: Valery Khromov <valery.khromov@gmail.com>
>> Date: Thu, 2 Feb 2012 12:03:23 +0300
>> 
>> On 01.02.2012, at 23:27, Tom Tromey wrote:
>> 
>>>>>>>> "Valery" == Valery Khromov <valery.khromov@gmail.com> writes:
>>> 
>>> Valery> I have copyright assignment paperwork finished.
>>> Valery> What should I do next?
>>> 
>>> I re-read the thread and I think there is still an unfulfilled request:
>>> 
>>> http://sourceware.org/ml/gdb-patches/2011-12/msg00695.html
>>> 
>> 
>> "[PATCH v2]" is the latest version of the original patch which contains a single diff agains the current GDB CVS
>> sources (as for December 2011).
>> So, I believe the request from the link above has been fulfilled.
> 
> Yes.  I think the "Contributed by ..." bit in amd64bsd-nat.h is a bit
> silly given that this file only contains a couple of function
> prototypes, but that's hardly worth arguing about.


It's from ages when this patch was used internal-only in my company. 
No problem, feel free to remove it.

-- 
WBR,
Valery Khromov