public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c
@ 2014-06-27 14:12 Gary Benson
  2014-06-27 14:12 ` [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description Gary Benson
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

Hi all,

This series is an updated version of the series I posted this morning.
Mark Kettenis rightly pointed out that x86_linux_read_description was
a mess of #ifdef spaghetti so I have rewritten it.

Patch 2 has changed because that's where the x86_linux_read_description
merge is.  Patch 6 has changed because that's where the code is pulled
from {i386,amd64}-linux-nat.c into x86-linux-nat.c.  The remaining
patches are the same.

I've inlined the original description of the series below.

Is this ok to commit?

Thanks,
Gary


--
This series refactors the shared code in {i386,amd64}-linux-nat.c into
the new files x86-linux-nat.[ch] and i386-linux-nat.h.  The first five
patches remove spurious changes between the files so that the code to
be shared is identical:

  1/7 - Rename identical functions
  2/7 - Merge {i386,amd64}_linux_read_description
  3/7 - Merge ps_get_thread_area
  4/7 - Pull out common parts of _initialize_{i386,amd64}_linux_nat
  5/7 - Comment and whitespace changes

The sixth patch moves the shared code into the new files and does all
the makefile and build system updates:

  6/7 - Move duplicated code into new files

The seventh patch tidies the blocks of #include directives at the
tops of {i386,amd64}-linux-nat.c:

  7/7 - Tidy #include lists

This final patch is not strictly necessary for the series, but it
makes things tidier.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 3/7 v2] Merge ps_get_thread_area
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
  2014-06-27 14:12 ` [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description Gary Benson
  2014-06-27 14:12 ` [PATCH 1/7 v2] Rename identical functions Gary Benson
@ 2014-06-27 14:12 ` Gary Benson
  2014-07-09 13:08   ` Pedro Alves
  2014-07-09 13:08   ` Pedro Alves
  2014-06-27 14:28 ` [PATCH 6/7 v2] Move duplicated code into new files Gary Benson
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

This commit adds a new helper, x86_linux_get_thread_area, to
hold the common parts of the ps_get_thread_area functions in
i386-linux-nat.c and amd64-linux-nat.c.

This patch is unchanged from the original version in this series.

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* amd64-linux-nat.c (x86_linux_get_thread_area): New function.
	(ps_get_thread_area): Delegate to the above in 32-bit mode.
	* i386-linux-nat.c (x86_linux_get_thread_area): New function.
	(ps_get_thread_area): Delegate to the above.
---
 gdb/ChangeLog         |    7 +++++
 gdb/amd64-linux-nat.c |   73 +++++++++++++++++++++++++++++++++++-------------
 gdb/i386-linux-nat.c  |   36 +++++++++++++++++++-----
 3 files changed, 88 insertions(+), 28 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index c442878..57e5c51 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -490,6 +490,47 @@ x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
 
 \f
 
+/* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
+   the thread local storage (or its descriptor) and returns PS_OK
+   on success.  Returns PS_ERR on failure.  */
+
+static ps_err_e
+x86_linux_get_thread_area (pid_t pid, void *addr, unsigned int *base_addr)
+{
+  /* NOTE: cagney/2003-08-26: The definition of this buffer is found
+     in the kernel header <asm-i386/ldt.h>.  It, after padding, is 4 x
+     4 byte integers in size: `entry_number', `base_addr', `limit',
+     and a bunch of status bits.
+
+     The values returned by this ptrace call should be part of the
+     regcache buffer, and ps_get_thread_area should channel its
+     request through the regcache.  That way remote targets could
+     provide the value using the remote protocol and not this direct
+     call.
+
+     Is this function needed?  I'm guessing that the `base' is the
+     address of a descriptor that libthread_db uses to find the
+     thread local address base that GDB needs.  Perhaps that
+     descriptor is defined by the ABI.  Anyway, given that
+     libthread_db calls this function without prompting (gdb
+     requesting tls base) I guess it needs info in there anyway.  */
+  unsigned int desc[4];
+
+  /* This code assumes that "int" is 32 bits and that
+     GET_THREAD_AREA returns no more than 4 int values.  */
+  gdb_assert (sizeof (int) == 4);
+
+#ifndef PTRACE_GET_THREAD_AREA
+#define PTRACE_GET_THREAD_AREA 25
+#endif
+
+  if (ptrace (PTRACE_GET_THREAD_AREA, pid, addr, &desc) < 0)
+    return PS_ERR;
+
+  *base_addr = desc[1];
+  return PS_OK;
+}
+
 /* This function is called by libthread_db as part of its handling of
    a request for a thread's local storage address.  */
 
@@ -499,26 +540,18 @@ ps_get_thread_area (const struct ps_prochandle *ph,
 {
   if (gdbarch_bfd_arch_info (target_gdbarch ())->bits_per_word == 32)
     {
-      /* The full structure is found in <asm-i386/ldt.h>.  The second
-	 integer is the LDT's base_address and that is used to locate
-	 the thread's local storage.  See i386-linux-nat.c more
-	 info.  */
-      unsigned int desc[4];
-
-      /* This code assumes that "int" is 32 bits and that
-	 GET_THREAD_AREA returns no more than 4 int values.  */
-      gdb_assert (sizeof (int) == 4);	
-#ifndef PTRACE_GET_THREAD_AREA
-#define PTRACE_GET_THREAD_AREA 25
-#endif
-      if  (ptrace (PTRACE_GET_THREAD_AREA, 
-		   lwpid, (void *) (long) idx, (unsigned long) &desc) < 0)
-	return PS_ERR;
-      
-      /* Extend the value to 64 bits.  Here it's assumed that a "long"
-	 and a "void *" are the same.  */
-      (*base) = (void *) (long) desc[1];
-      return PS_OK;
+      unsigned int base_addr;
+      ps_err_e result;
+
+      result = x86_linux_get_thread_area (lwpid, (void *) (long) idx,
+					  &base_addr);
+      if (result == PS_OK)
+	{
+	  /* Extend the value to 64 bits.  Here it's assumed that
+	     a "long" and a "void *" are the same.  */
+	  (*base) = (void *) (long) base_addr;
+	}
+      return result;
     }
   else
     {
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 62ad29f..d647c3d 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -848,12 +848,12 @@ x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
 
 \f
 
-/* Called by libthread_db.  Returns a pointer to the thread local
-   storage (or its descriptor).  */
+/* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
+   the thread local storage (or its descriptor) and returns PS_OK
+   on success.  Returns PS_ERR on failure.  */
 
-ps_err_e
-ps_get_thread_area (const struct ps_prochandle *ph, 
-		    lwpid_t lwpid, int idx, void **base)
+static ps_err_e
+x86_linux_get_thread_area (pid_t pid, void *addr, unsigned int *base_addr)
 {
   /* NOTE: cagney/2003-08-26: The definition of this buffer is found
      in the kernel header <asm-i386/ldt.h>.  It, after padding, is 4 x
@@ -873,19 +873,39 @@ ps_get_thread_area (const struct ps_prochandle *ph,
      libthread_db calls this function without prompting (gdb
      requesting tls base) I guess it needs info in there anyway.  */
   unsigned int desc[4];
+
+  /* This code assumes that "int" is 32 bits and that
+     GET_THREAD_AREA returns no more than 4 int values.  */
   gdb_assert (sizeof (int) == 4);
 
 #ifndef PTRACE_GET_THREAD_AREA
 #define PTRACE_GET_THREAD_AREA 25
 #endif
 
-  if (ptrace (PTRACE_GET_THREAD_AREA, lwpid,
-	      (void *) idx, (unsigned long) &desc) < 0)
+  if (ptrace (PTRACE_GET_THREAD_AREA, pid, addr, &desc) < 0)
     return PS_ERR;
 
-  *(int *)base = desc[1];
+  *base_addr = desc[1];
   return PS_OK;
 }
+
+/* Called by libthread_db.  Returns a pointer to the thread local
+   storage (or its descriptor).  */
+
+ps_err_e
+ps_get_thread_area (const struct ps_prochandle *ph,
+		    lwpid_t lwpid, int idx, void **base)
+{
+  unsigned int base_addr;
+  ps_err_e result;
+
+  result = x86_linux_get_thread_area (lwpid, (void *) idx, &base_addr);
+
+  if (result == PS_OK)
+    *(int *) base = base_addr;
+
+  return result;
+}
 \f
 
 /* The instruction for a GNU/Linux system call is:
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/7 v2] Rename identical functions
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
  2014-06-27 14:12 ` [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description Gary Benson
@ 2014-06-27 14:12 ` Gary Benson
  2014-07-09 13:04   ` Pedro Alves
  2014-06-27 14:12 ` [PATCH 3/7 v2] Merge ps_get_thread_area Gary Benson
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

amd64-linux-nat.c and i386-linux-nat.c contain a number of functions
which are identical but for prefix on their names.  This commit
renames all such functions to have the prefix x86_ instead of the
prefixes amd64_ or i386_ and updates all uses of those functions.

This patch is unchanged from the original version in this series.

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* amd64-linux-nat.c (amd64_linux_dr_get): Renamed to
	x86_linux_dr_get.  All uses updated.
	(amd64_linux_dr_set): Renamed to
	x86_linux_dr_set.  All uses updated.
	(amd64_linux_dr_get_addr): Renamed to
	x86_linux_dr_get_addr.  All uses updated.
	(amd64_linux_dr_get_control): Renamed to
	x86_linux_dr_get_control.  All uses updated.
	(amd64_linux_dr_get_status): Renamed to
	x86_linux_dr_get_status.  All uses updated.
	(amd64_linux_dr_set_control): Renamed to
	x86_linux_dr_set_control.  All uses updated.
	(amd64_linux_dr_set_addr): Renamed to
	x86_linux_dr_set_addr.  All uses updated.
	(amd64_linux_prepare_to_resume): Renamed to
	x86_linux_prepare_to_resume.  All uses updated.
	(amd64_linux_new_thread): Renamed to
	x86_linux_new_thread.  All uses updated.
	(amd64_linux_new_fork): Renamed to
	x86_linux_new_fork.  All uses updated.
	(amd64_linux_child_post_startup_inferior): Renamed to
	x86_linux_child_post_startup_inferior.  All uses updated.
	(amd64_linux_enable_btrace): Renamed to
	x86_linux_enable_btrace.  All uses updated.
	(amd64_linux_disable_btrace): Renamed to
	x86_linux_disable_btrace.  All uses updated.
	(amd64_linux_teardown_btrace): Renamed to
	x86_linux_teardown_btrace.  All uses updated.
	(amd64_linux_read_btrace): Renamed to
	x86_linux_read_btrace.  All uses updated.
	* i386-linux-nat.c (i386_linux_dr_get): Renamed to
	x86_linux_dr_get.  All uses updated.
	(i386_linux_dr_set): Renamed to
	x86_linux_dr_set.  All uses updated.
	(i386_linux_dr_get_addr): Renamed to
	x86_linux_dr_get_addr.  All uses updated.
	(i386_linux_dr_get_control): Renamed to
	x86_linux_dr_get_control.  All uses updated.
	(i386_linux_dr_get_status): Renamed to
	x86_linux_dr_get_status.  All uses updated.
	(i386_linux_dr_set_control): Renamed to
	x86_linux_dr_set_control.  All uses updated.
	(i386_linux_dr_set_addr): Renamed to
	x86_linux_dr_set_addr.  All uses updated.
	(i386_linux_prepare_to_resume): Renamed to
	x86_linux_prepare_to_resume.  All uses updated.
	(i386_linux_new_thread): Renamed to
	x86_linux_new_thread.  All uses updated.
	(i386_linux_new_fork): Renamed to
	x86_linux_new_fork.  All uses updated.
	(i386_linux_child_post_startup_inferior): Renamed to
	x86_linux_child_post_startup_inferior.  All uses updated.
	(i386_linux_enable_btrace): Renamed to
	x86_linux_enable_btrace.  All uses updated.
	(i386_linux_disable_btrace): Renamed to
	x86_linux_disable_btrace.  All uses updated.
	(i386_linux_teardown_btrace): Renamed to
	x86_linux_teardown_btrace.  All uses updated.
	(i386_linux_read_btrace): Renamed to
	x86_linux_read_btrace.  All uses updated.
---
 gdb/ChangeLog         |   63 ++++++++++++++++++++++++++++++++++++++
 gdb/amd64-linux-nat.c |   80 ++++++++++++++++++++++++------------------------
 gdb/i386-linux-nat.c  |   80 ++++++++++++++++++++++++------------------------
 3 files changed, 143 insertions(+), 80 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index fa677c8..d8e424a 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -280,7 +280,7 @@ amd64_linux_store_inferior_registers (struct target_ops *ops,
 /* Support for debug registers.  */
 
 static unsigned long
-amd64_linux_dr_get (ptid_t ptid, int regnum)
+x86_linux_dr_get (ptid_t ptid, int regnum)
 {
   int tid;
   unsigned long value;
@@ -301,7 +301,7 @@ amd64_linux_dr_get (ptid_t ptid, int regnum)
 /* Set debug register REGNUM to VALUE in only the one LWP of PTID.  */
 
 static void
-amd64_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
+x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 {
   int tid;
 
@@ -319,28 +319,28 @@ amd64_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 /* Return the inferior's debug register REGNUM.  */
 
 static CORE_ADDR
-amd64_linux_dr_get_addr (int regnum)
+x86_linux_dr_get_addr (int regnum)
 {
   /* DR6 and DR7 are retrieved with some other way.  */
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
-  return amd64_linux_dr_get (inferior_ptid, regnum);
+  return x86_linux_dr_get (inferior_ptid, regnum);
 }
 
 /* Return the inferior's DR7 debug control register.  */
 
 static unsigned long
-amd64_linux_dr_get_control (void)
+x86_linux_dr_get_control (void)
 {
-  return amd64_linux_dr_get (inferior_ptid, DR_CONTROL);
+  return x86_linux_dr_get (inferior_ptid, DR_CONTROL);
 }
 
 /* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
 
 static unsigned long
-amd64_linux_dr_get_status (void)
+x86_linux_dr_get_status (void)
 {
-  return amd64_linux_dr_get (inferior_ptid, DR_STATUS);
+  return x86_linux_dr_get (inferior_ptid, DR_STATUS);
 }
 
 /* Callback for iterate_over_lwps.  Update the debug registers of
@@ -368,7 +368,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 /* Set DR_CONTROL to CONTROL in all LWPs of the current inferior.  */
 
 static void
-amd64_linux_dr_set_control (unsigned long control)
+x86_linux_dr_set_control (unsigned long control)
 {
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
 
@@ -379,7 +379,7 @@ amd64_linux_dr_set_control (unsigned long control)
    inferior.  */
 
 static void
-amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
+x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 {
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
 
@@ -392,7 +392,7 @@ amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
    If the debug regs have changed, update the thread's copies.  */
 
 static void
-amd64_linux_prepare_to_resume (struct lwp_info *lwp)
+x86_linux_prepare_to_resume (struct lwp_info *lwp)
 {
   int clear_status = 0;
 
@@ -418,12 +418,12 @@ amd64_linux_prepare_to_resume (struct lwp_info *lwp)
       /* Clear DR_CONTROL first.  In some cases, setting DR0-3 to a
 	 value that doesn't match what is enabled in DR_CONTROL
 	 results in EINVAL.  */
-      amd64_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
+      x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
 
       for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
 	if (state->dr_ref_count[i] > 0)
 	  {
-	    amd64_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
+	    x86_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
 
 	    /* If we're setting a watchpoint, any change the inferior
 	       had done itself to the debug registers needs to be
@@ -435,17 +435,17 @@ amd64_linux_prepare_to_resume (struct lwp_info *lwp)
       /* If DR_CONTROL is supposed to be zero, we've already set it
 	 above.  */
       if (state->dr_control_mirror != 0)
-	amd64_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
+	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
 
       lwp->arch_private->debug_registers_changed = 0;
     }
 
   if (clear_status || lwp->stopped_by_watchpoint)
-    amd64_linux_dr_set (lwp->ptid, DR_STATUS, 0);
+    x86_linux_dr_set (lwp->ptid, DR_STATUS, 0);
 }
 
 static void
-amd64_linux_new_thread (struct lwp_info *lp)
+x86_linux_new_thread (struct lwp_info *lp)
 {
   struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
 
@@ -457,7 +457,7 @@ amd64_linux_new_thread (struct lwp_info *lp)
 /* linux_nat_new_fork hook.   */
 
 static void
-amd64_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
+x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
 {
   pid_t parent_pid;
   struct i386_debug_reg_state *parent_state;
@@ -582,7 +582,7 @@ static void (*super_post_startup_inferior) (struct target_ops *self,
 					    ptid_t ptid);
 
 static void
-amd64_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
+x86_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
 {
   i386_cleanup_dregs ();
   super_post_startup_inferior (self, ptid);
@@ -1171,7 +1171,7 @@ amd64_linux_read_description (struct target_ops *ops)
 /* Enable branch tracing.  */
 
 static struct btrace_target_info *
-amd64_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
+x86_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
 {
   struct btrace_target_info *tinfo;
   struct gdbarch *gdbarch;
@@ -1193,8 +1193,8 @@ amd64_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
 /* Disable branch tracing.  */
 
 static void
-amd64_linux_disable_btrace (struct target_ops *self,
-			    struct btrace_target_info *tinfo)
+x86_linux_disable_btrace (struct target_ops *self,
+			  struct btrace_target_info *tinfo)
 {
   enum btrace_error errcode = linux_disable_btrace (tinfo);
 
@@ -1205,18 +1205,18 @@ amd64_linux_disable_btrace (struct target_ops *self,
 /* Teardown branch tracing.  */
 
 static void
-amd64_linux_teardown_btrace (struct target_ops *self,
-			     struct btrace_target_info *tinfo)
+x86_linux_teardown_btrace (struct target_ops *self,
+			   struct btrace_target_info *tinfo)
 {
   /* Ignore errors.  */
   linux_disable_btrace (tinfo);
 }
 
 static enum btrace_error
-amd64_linux_read_btrace (struct target_ops *self,
-			 VEC (btrace_block_s) **data,
-			 struct btrace_target_info *btinfo,
-			 enum btrace_read_type type)
+x86_linux_read_btrace (struct target_ops *self,
+		       VEC (btrace_block_s) **data,
+		       struct btrace_target_info *btinfo,
+		       enum btrace_read_type type)
 {
   return linux_read_btrace (data, btinfo, type);
 }
@@ -1242,16 +1242,16 @@ _initialize_amd64_linux_nat (void)
 
   i386_use_watchpoints (t);
 
-  i386_dr_low.set_control = amd64_linux_dr_set_control;
-  i386_dr_low.set_addr = amd64_linux_dr_set_addr;
-  i386_dr_low.get_addr = amd64_linux_dr_get_addr;
-  i386_dr_low.get_status = amd64_linux_dr_get_status;
-  i386_dr_low.get_control = amd64_linux_dr_get_control;
+  i386_dr_low.set_control = x86_linux_dr_set_control;
+  i386_dr_low.set_addr = x86_linux_dr_set_addr;
+  i386_dr_low.get_addr = x86_linux_dr_get_addr;
+  i386_dr_low.get_status = x86_linux_dr_get_status;
+  i386_dr_low.get_control = x86_linux_dr_get_control;
   i386_set_debug_register_length (8);
 
   /* Override the GNU/Linux inferior startup hook.  */
   super_post_startup_inferior = t->to_post_startup_inferior;
-  t->to_post_startup_inferior = amd64_linux_child_post_startup_inferior;
+  t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
 
   /* Add our register access methods.  */
   t->to_fetch_registers = amd64_linux_fetch_inferior_registers;
@@ -1261,16 +1261,16 @@ _initialize_amd64_linux_nat (void)
 
   /* Add btrace methods.  */
   t->to_supports_btrace = linux_supports_btrace;
-  t->to_enable_btrace = amd64_linux_enable_btrace;
-  t->to_disable_btrace = amd64_linux_disable_btrace;
-  t->to_teardown_btrace = amd64_linux_teardown_btrace;
-  t->to_read_btrace = amd64_linux_read_btrace;
+  t->to_enable_btrace = x86_linux_enable_btrace;
+  t->to_disable_btrace = x86_linux_disable_btrace;
+  t->to_teardown_btrace = x86_linux_teardown_btrace;
+  t->to_read_btrace = x86_linux_read_btrace;
 
   /* Register the target.  */
   linux_nat_add_target (t);
-  linux_nat_set_new_thread (t, amd64_linux_new_thread);
-  linux_nat_set_new_fork (t, amd64_linux_new_fork);
+  linux_nat_set_new_thread (t, x86_linux_new_thread);
+  linux_nat_set_new_fork (t, x86_linux_new_fork);
   linux_nat_set_forget_process (t, i386_forget_process);
   linux_nat_set_siginfo_fixup (t, amd64_linux_siginfo_fixup);
-  linux_nat_set_prepare_to_resume (t, amd64_linux_prepare_to_resume);
+  linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
 }
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index ff96501..24f9407 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -647,7 +647,7 @@ i386_linux_store_inferior_registers (struct target_ops *ops,
 /* Get debug register REGNUM value from only the one LWP of PTID.  */
 
 static unsigned long
-i386_linux_dr_get (ptid_t ptid, int regnum)
+x86_linux_dr_get (ptid_t ptid, int regnum)
 {
   int tid;
   unsigned long value;
@@ -668,7 +668,7 @@ i386_linux_dr_get (ptid_t ptid, int regnum)
 /* Set debug register REGNUM to VALUE in only the one LWP of PTID.  */
 
 static void
-i386_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
+x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 {
   int tid;
 
@@ -686,28 +686,28 @@ i386_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 /* Return the inferior's debug register REGNUM.  */
 
 static CORE_ADDR
-i386_linux_dr_get_addr (int regnum)
+x86_linux_dr_get_addr (int regnum)
 {
   /* DR6 and DR7 are retrieved with some other way.  */
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
-  return i386_linux_dr_get (inferior_ptid, regnum);
+  return x86_linux_dr_get (inferior_ptid, regnum);
 }
 
 /* Return the inferior's DR7 debug control register.  */
 
 static unsigned long
-i386_linux_dr_get_control (void)
+x86_linux_dr_get_control (void)
 {
-  return i386_linux_dr_get (inferior_ptid, DR_CONTROL);
+  return x86_linux_dr_get (inferior_ptid, DR_CONTROL);
 }
 
 /* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
 
 static unsigned long
-i386_linux_dr_get_status (void)
+x86_linux_dr_get_status (void)
 {
-  return i386_linux_dr_get (inferior_ptid, DR_STATUS);
+  return x86_linux_dr_get (inferior_ptid, DR_STATUS);
 }
 
 /* Callback for iterate_over_lwps.  Update the debug registers of
@@ -735,7 +735,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 /* Set DR_CONTROL to ADDR in all LWPs of the current inferior.  */
 
 static void
-i386_linux_dr_set_control (unsigned long control)
+x86_linux_dr_set_control (unsigned long control)
 {
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
 
@@ -746,7 +746,7 @@ i386_linux_dr_set_control (unsigned long control)
    inferior.  */
 
 static void
-i386_linux_dr_set_addr (int regnum, CORE_ADDR addr)
+x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 {
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
 
@@ -759,7 +759,7 @@ i386_linux_dr_set_addr (int regnum, CORE_ADDR addr)
    If the debug regs have changed, update the thread's copies.  */
 
 static void
-i386_linux_prepare_to_resume (struct lwp_info *lwp)
+x86_linux_prepare_to_resume (struct lwp_info *lwp)
 {
   int clear_status = 0;
 
@@ -778,12 +778,12 @@ i386_linux_prepare_to_resume (struct lwp_info *lwp)
       /* See amd64_linux_prepare_to_resume for Linux kernel note on
 	 i386_linux_dr_set calls ordering.  */
 
-      i386_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
+      x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
 
       for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
 	if (state->dr_ref_count[i] > 0)
 	  {
-	    i386_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
+	    x86_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
 
 	    /* If we're setting a watchpoint, any change the inferior
 	       had done itself to the debug registers needs to be
@@ -793,17 +793,17 @@ i386_linux_prepare_to_resume (struct lwp_info *lwp)
 	  }
 
       if (state->dr_control_mirror != 0)
-	i386_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
+	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
 
       lwp->arch_private->debug_registers_changed = 0;
     }
 
   if (clear_status || lwp->stopped_by_watchpoint)
-    i386_linux_dr_set (lwp->ptid, DR_STATUS, 0);
+    x86_linux_dr_set (lwp->ptid, DR_STATUS, 0);
 }
 
 static void
-i386_linux_new_thread (struct lwp_info *lp)
+x86_linux_new_thread (struct lwp_info *lp)
 {
   struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
 
@@ -815,7 +815,7 @@ i386_linux_new_thread (struct lwp_info *lp)
 /* linux_nat_new_fork hook.   */
 
 static void
-i386_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
+x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
 {
   pid_t parent_pid;
   struct i386_debug_reg_state *parent_state;
@@ -989,7 +989,7 @@ static void (*super_post_startup_inferior) (struct target_ops *self,
 					    ptid_t ptid);
 
 static void
-i386_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
+x86_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
 {
   i386_cleanup_dregs ();
   super_post_startup_inferior (self, ptid);
@@ -1067,7 +1067,7 @@ i386_linux_read_description (struct target_ops *ops)
 /* Enable branch tracing.  */
 
 static struct btrace_target_info *
-i386_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
+x86_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
 {
   struct btrace_target_info *tinfo;
   struct gdbarch *gdbarch;
@@ -1089,8 +1089,8 @@ i386_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
 /* Disable branch tracing.  */
 
 static void
-i386_linux_disable_btrace (struct target_ops *self,
-			   struct btrace_target_info *tinfo)
+x86_linux_disable_btrace (struct target_ops *self,
+			  struct btrace_target_info *tinfo)
 {
   enum btrace_error errcode = linux_disable_btrace (tinfo);
 
@@ -1101,18 +1101,18 @@ i386_linux_disable_btrace (struct target_ops *self,
 /* Teardown branch tracing.  */
 
 static void
-i386_linux_teardown_btrace (struct target_ops *self,
-			    struct btrace_target_info *tinfo)
+x86_linux_teardown_btrace (struct target_ops *self,
+			   struct btrace_target_info *tinfo)
 {
   /* Ignore errors.  */
   linux_disable_btrace (tinfo);
 }
 
 static enum btrace_error
-i386_linux_read_btrace (struct target_ops *self,
-			VEC (btrace_block_s) **data,
-			struct btrace_target_info *btinfo,
-			enum btrace_read_type type)
+x86_linux_read_btrace (struct target_ops *self,
+		       VEC (btrace_block_s) **data,
+		       struct btrace_target_info *btinfo,
+		       enum btrace_read_type type)
 {
   return linux_read_btrace (data, btinfo, type);
 }
@@ -1130,11 +1130,11 @@ _initialize_i386_linux_nat (void)
 
   i386_use_watchpoints (t);
 
-  i386_dr_low.set_control = i386_linux_dr_set_control;
-  i386_dr_low.set_addr = i386_linux_dr_set_addr;
-  i386_dr_low.get_addr = i386_linux_dr_get_addr;
-  i386_dr_low.get_status = i386_linux_dr_get_status;
-  i386_dr_low.get_control = i386_linux_dr_get_control;
+  i386_dr_low.set_control = x86_linux_dr_set_control;
+  i386_dr_low.set_addr = x86_linux_dr_set_addr;
+  i386_dr_low.get_addr = x86_linux_dr_get_addr;
+  i386_dr_low.get_status = x86_linux_dr_get_status;
+  i386_dr_low.get_control = x86_linux_dr_get_control;
   i386_set_debug_register_length (4);
 
   /* Override the default ptrace resume method.  */
@@ -1142,7 +1142,7 @@ _initialize_i386_linux_nat (void)
 
   /* Override the GNU/Linux inferior startup hook.  */
   super_post_startup_inferior = t->to_post_startup_inferior;
-  t->to_post_startup_inferior = i386_linux_child_post_startup_inferior;
+  t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
 
   /* Add our register access methods.  */
   t->to_fetch_registers = i386_linux_fetch_inferior_registers;
@@ -1152,15 +1152,15 @@ _initialize_i386_linux_nat (void)
 
   /* Add btrace methods.  */
   t->to_supports_btrace = linux_supports_btrace;
-  t->to_enable_btrace = i386_linux_enable_btrace;
-  t->to_disable_btrace = i386_linux_disable_btrace;
-  t->to_teardown_btrace = i386_linux_teardown_btrace;
-  t->to_read_btrace = i386_linux_read_btrace;
+  t->to_enable_btrace = x86_linux_enable_btrace;
+  t->to_disable_btrace = x86_linux_disable_btrace;
+  t->to_teardown_btrace = x86_linux_teardown_btrace;
+  t->to_read_btrace = x86_linux_read_btrace;
 
   /* Register the target.  */
   linux_nat_add_target (t);
-  linux_nat_set_new_thread (t, i386_linux_new_thread);
-  linux_nat_set_new_fork (t, i386_linux_new_fork);
+  linux_nat_set_new_thread (t, x86_linux_new_thread);
+  linux_nat_set_new_fork (t, x86_linux_new_fork);
   linux_nat_set_forget_process (t, i386_forget_process);
-  linux_nat_set_prepare_to_resume (t, i386_linux_prepare_to_resume);
+  linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
 }
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
@ 2014-06-27 14:12 ` Gary Benson
  2014-07-09 13:07   ` Pedro Alves
  2014-06-27 14:12 ` [PATCH 1/7 v2] Rename identical functions Gary Benson
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

This commit merges i386_ and amd64_linux_read_description, renaming
both to x86_linux_read_description.

This patch differs from the original version in this series
in that x86_linux_read_description is much cleaner, having
been rewritten to avoid "#ifdef spaghetti".

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* amd64-linux-nat.c (amd64_linux_read_description): Renamed to
	x86_linux_read_description.  All uses updated.  amd64-specific
	code conditionalized.  Conditionalized i386-specific code added.
	Redundant cast removed.
	* i386-linux-nat.c (i386_linux_read_description): Renamed to
	x86_linux_read_description.  All uses updated.  i386-specific
	code conditionalized.  Conditionalized amd64-specific code added.
	One sizeof replaced with the actual type it is describing.
---
 gdb/ChangeLog         |   11 +++
 gdb/amd64-linux-nat.c |  179 ++++++++++++++++++++++++++----------------------
 gdb/i386-linux-nat.c  |  100 +++++++++++++++++++++++++---
 3 files changed, 198 insertions(+), 92 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index d8e424a..c442878 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -1034,55 +1034,75 @@ amd64_linux_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction)
     return 0;
 }
 
-/* Get Linux/x86 target description from running target.
-
-   Value of CS segment register:
-     1. 64bit process: 0x33.
-     2. 32bit process: 0x23.
-
-   Value of DS segment register:
-     1. LP64 process: 0x0.
-     2. X32 process: 0x2b.
- */
+#ifdef __x86_64__
+/* Value of CS segment register:
+     64bit process: 0x33
+     32bit process: 0x23  */
+#define AMD64_LINUX_USER64_CS 0x33
+
+/* Value of DS segment register:
+     LP64 process: 0x0
+     X32 process: 0x2b  */
+#define AMD64_LINUX_X32_DS 0x2b
+#endif
 
-#define AMD64_LINUX_USER64_CS	0x33
-#define AMD64_LINUX_X32_DS	0x2b
+/* Get Linux/x86 target description from running target.  */
 
 static const struct target_desc *
-amd64_linux_read_description (struct target_ops *ops)
+x86_linux_read_description (struct target_ops *ops)
 {
-  unsigned long cs;
-  unsigned long ds;
   int tid;
-  int is_64bit;
+  int is_64bit = 0;
+#ifdef __x86_64__
   int is_x32;
+#endif
   static uint64_t xcr0;
+  uint64_t xcr0_features_bits;
 
   /* GNU/Linux LWP ID's are process ID's.  */
   tid = ptid_get_lwp (inferior_ptid);
   if (tid == 0)
     tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
 
-  /* Get CS register.  */
-  errno = 0;
-  cs = ptrace (PTRACE_PEEKUSER, tid,
-	       offsetof (struct user_regs_struct, cs), 0);
-  if (errno != 0)
-    perror_with_name (_("Couldn't get CS register"));
-
-  is_64bit = cs == AMD64_LINUX_USER64_CS;
-
-  /* Get DS register.  */
-  errno = 0;
-  ds = ptrace (PTRACE_PEEKUSER, tid,
-	       offsetof (struct user_regs_struct, ds), 0);
-  if (errno != 0)
-    perror_with_name (_("Couldn't get DS register"));
-
-  is_x32 = ds == AMD64_LINUX_X32_DS;
+#ifdef __x86_64__
+  {
+    unsigned long cs;
+    unsigned long ds;
+
+    /* Get CS register.  */
+    errno = 0;
+    cs = ptrace (PTRACE_PEEKUSER, tid,
+		 offsetof (struct user_regs_struct, cs), 0);
+    if (errno != 0)
+      perror_with_name (_("Couldn't get CS register"));
+
+    is_64bit = cs == AMD64_LINUX_USER64_CS;
+
+    /* Get DS register.  */
+    errno = 0;
+    ds = ptrace (PTRACE_PEEKUSER, tid,
+		 offsetof (struct user_regs_struct, ds), 0);
+    if (errno != 0)
+      perror_with_name (_("Couldn't get DS register"));
+
+    is_x32 = ds == AMD64_LINUX_X32_DS;
+
+    if (sizeof (void *) == 4 && is_64bit && !is_x32)
+      error (_("Can't debug 64-bit process with 32-bit GDB"));
+  }
+#elif HAVE_PTRACE_GETFPXREGS
+  if (have_ptrace_getfpxregs == -1)
+    {
+      elf_fpxregset_t fpxregs;
 
-  if (sizeof (void *) == 4 && is_64bit && !is_x32)
-    error (_("Can't debug 64-bit process with 32-bit GDB"));
+      if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
+	{
+	  have_ptrace_getfpxregs = 0;
+	  have_ptrace_getregset = 0;
+	  return tdesc_i386_mmx_linux;
+	}
+    }
+#endif
 
   if (have_ptrace_getregset == -1)
     {
@@ -1094,7 +1114,7 @@ amd64_linux_read_description (struct target_ops *ops)
 
       /* Check if PTRACE_GETREGSET works.  */
       if (ptrace (PTRACE_GETREGSET, tid,
-		  (unsigned int) NT_X86_XSTATE, (long) &iov) < 0)
+		  (unsigned int) NT_X86_XSTATE, &iov) < 0)
 	have_ptrace_getregset = 0;
       else
 	{
@@ -1106,66 +1126,61 @@ amd64_linux_read_description (struct target_ops *ops)
 	}
     }
 
-  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  */
-  if (have_ptrace_getregset && (xcr0 & I386_XSTATE_ALL_MASK))
+  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  If
+     PTRACE_GETREGSET is not available then set xcr0_features_bits to
+     zero so that the "no-features" descriptions are returned by the
+     switches below.  */
+  if (have_ptrace_getregset)
+    xcr0_features_bits = xcr0 & I386_XSTATE_ALL_MASK;
+  else
+    xcr0_features_bits = 0;
+
+  if (is_64bit)
     {
-      switch (xcr0 & I386_XSTATE_ALL_MASK)
+#ifdef __x86_64__
+      switch (xcr0_features_bits)
 	{
-      case I386_XSTATE_MPX_AVX512_MASK:
-      case I386_XSTATE_AVX512_MASK:
-	if (is_64bit)
-	  {
-	    if (is_x32)
-	      return tdesc_x32_avx512_linux;
-	    else
-	      return tdesc_amd64_avx512_linux;
-	  }
-	else
-	  return tdesc_i386_avx512_linux;
+	case I386_XSTATE_MPX_AVX512_MASK:
+	case I386_XSTATE_AVX512_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx512_linux;
+	  else
+	    return tdesc_amd64_avx512_linux;
 	case I386_XSTATE_MPX_MASK:
-	  if (is_64bit)
-	    {
-	      if (is_x32)
-		return tdesc_x32_avx_linux; /* No MPX on x32 using AVX.  */
-	      else
-		return tdesc_amd64_mpx_linux;
-	    }
+	  if (is_x32)
+	    return tdesc_x32_avx_linux; /* No MPX on x32 using AVX.  */
 	  else
-	    return tdesc_i386_mpx_linux;
+	    return tdesc_amd64_mpx_linux;
 	case I386_XSTATE_AVX_MASK:
-	  if (is_64bit)
-	    {
-	      if (is_x32)
-		return tdesc_x32_avx_linux;
-	      else
-		return tdesc_amd64_avx_linux;
-	    }
+	  if (is_x32)
+	    return tdesc_x32_avx_linux;
 	  else
-	    return tdesc_i386_avx_linux;
+	    return tdesc_amd64_avx_linux;
 	default:
-	  if (is_64bit)
-	    {
-	      if (is_x32)
-		return tdesc_x32_linux;
-	      else
-		return tdesc_amd64_linux;
-	    }
+	  if (is_x32)
+	    return tdesc_x32_linux;
 	  else
-	    return tdesc_i386_linux;
+	    return tdesc_amd64_linux;
 	}
+#endif
     }
   else
     {
-      if (is_64bit)
+      switch (xcr0_features_bits)
 	{
-	  if (is_x32)
-	    return tdesc_x32_linux;
-	  else
-	    return tdesc_amd64_linux;
+	case I386_XSTATE_MPX_AVX512_MASK:
+	case I386_XSTATE_AVX512_MASK:
+	  return tdesc_i386_avx512_linux;
+	case I386_XSTATE_MPX_MASK:
+	  return tdesc_i386_mpx_linux;
+	case I386_XSTATE_AVX_MASK:
+	  return tdesc_i386_avx_linux;
+	default:
+	  return tdesc_i386_linux;
 	}
-      else
-	return tdesc_i386_linux;
     }
+
+  gdb_assert_not_reached ("failed to return tdesc");
 }
 
 /* Enable branch tracing.  */
@@ -1257,7 +1272,7 @@ _initialize_amd64_linux_nat (void)
   t->to_fetch_registers = amd64_linux_fetch_inferior_registers;
   t->to_store_registers = amd64_linux_store_inferior_registers;
 
-  t->to_read_description = amd64_linux_read_description;
+  t->to_read_description = x86_linux_read_description;
 
   /* Add btrace methods.  */
   t->to_supports_btrace = linux_supports_btrace;
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 24f9407..62ad29f 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -995,20 +995,63 @@ x86_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
   super_post_startup_inferior (self, ptid);
 }
 
+#ifdef __x86_64__
+/* Value of CS segment register:
+     64bit process: 0x33
+     32bit process: 0x23  */
+#define AMD64_LINUX_USER64_CS 0x33
+
+/* Value of DS segment register:
+     LP64 process: 0x0
+     X32 process: 0x2b  */
+#define AMD64_LINUX_X32_DS 0x2b
+#endif
+
 /* Get Linux/x86 target description from running target.  */
 
 static const struct target_desc *
-i386_linux_read_description (struct target_ops *ops)
+x86_linux_read_description (struct target_ops *ops)
 {
   int tid;
+  int is_64bit = 0;
+#ifdef __x86_64__
+  int is_x32;
+#endif
   static uint64_t xcr0;
+  uint64_t xcr0_features_bits;
 
   /* GNU/Linux LWP ID's are process ID's.  */
   tid = ptid_get_lwp (inferior_ptid);
   if (tid == 0)
     tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
 
-#ifdef HAVE_PTRACE_GETFPXREGS
+#ifdef __x86_64__
+  {
+    unsigned long cs;
+    unsigned long ds;
+
+    /* Get CS register.  */
+    errno = 0;
+    cs = ptrace (PTRACE_PEEKUSER, tid,
+		 offsetof (struct user_regs_struct, cs), 0);
+    if (errno != 0)
+      perror_with_name (_("Couldn't get CS register"));
+
+    is_64bit = cs == AMD64_LINUX_USER64_CS;
+
+    /* Get DS register.  */
+    errno = 0;
+    ds = ptrace (PTRACE_PEEKUSER, tid,
+		 offsetof (struct user_regs_struct, ds), 0);
+    if (errno != 0)
+      perror_with_name (_("Couldn't get DS register"));
+
+    is_x32 = ds == AMD64_LINUX_X32_DS;
+
+    if (sizeof (void *) == 4 && is_64bit && !is_x32)
+      error (_("Can't debug 64-bit process with 32-bit GDB"));
+  }
+#elif HAVE_PTRACE_GETFPXREGS
   if (have_ptrace_getfpxregs == -1)
     {
       elf_fpxregset_t fpxregs;
@@ -1031,8 +1074,8 @@ i386_linux_read_description (struct target_ops *ops)
       iov.iov_len = sizeof (xstateregs);
 
       /* Check if PTRACE_GETREGSET works.  */
-      if (ptrace (PTRACE_GETREGSET, tid, (unsigned int) NT_X86_XSTATE,
-		  &iov) < 0)
+      if (ptrace (PTRACE_GETREGSET, tid,
+		  (unsigned int) NT_X86_XSTATE, &iov) < 0)
 	have_ptrace_getregset = 0;
       else
 	{
@@ -1040,14 +1083,51 @@ i386_linux_read_description (struct target_ops *ops)
 
 	  /* Get XCR0 from XSAVE extended state.  */
 	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
-			     / sizeof (long long))];
+			     / sizeof (uint64_t))];
 	}
     }
 
-  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  */
+  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  If
+     PTRACE_GETREGSET is not available then set xcr0_features_bits to
+     zero so that the "no-features" descriptions are returned by the
+     switches below.  */
   if (have_ptrace_getregset)
+    xcr0_features_bits = xcr0 & I386_XSTATE_ALL_MASK;
+  else
+    xcr0_features_bits = 0;
+
+  if (is_64bit)
     {
-      switch ((xcr0 & I386_XSTATE_ALL_MASK))
+#ifdef __x86_64__
+      switch (xcr0_features_bits)
+	{
+	case I386_XSTATE_MPX_AVX512_MASK:
+	case I386_XSTATE_AVX512_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx512_linux;
+	  else
+	    return tdesc_amd64_avx512_linux;
+	case I386_XSTATE_MPX_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx_linux; /* No MPX on x32 using AVX.  */
+	  else
+	    return tdesc_amd64_mpx_linux;
+	case I386_XSTATE_AVX_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx_linux;
+	  else
+	    return tdesc_amd64_avx_linux;
+	default:
+	  if (is_x32)
+	    return tdesc_x32_linux;
+	  else
+	    return tdesc_amd64_linux;
+	}
+#endif
+    }
+  else
+    {
+      switch (xcr0_features_bits)
 	{
 	case I386_XSTATE_MPX_AVX512_MASK:
 	case I386_XSTATE_AVX512_MASK:
@@ -1060,8 +1140,8 @@ i386_linux_read_description (struct target_ops *ops)
 	  return tdesc_i386_linux;
 	}
     }
-  else
-    return tdesc_i386_linux;
+
+  gdb_assert_not_reached ("failed to return tdesc");
 }
 
 /* Enable branch tracing.  */
@@ -1148,7 +1228,7 @@ _initialize_i386_linux_nat (void)
   t->to_fetch_registers = i386_linux_fetch_inferior_registers;
   t->to_store_registers = i386_linux_store_inferior_registers;
 
-  t->to_read_description = i386_linux_read_description;
+  t->to_read_description = x86_linux_read_description;
 
   /* Add btrace methods.  */
   t->to_supports_btrace = linux_supports_btrace;
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 6/7 v2] Move duplicated code into new files
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
                   ` (2 preceding siblings ...)
  2014-06-27 14:12 ` [PATCH 3/7 v2] Merge ps_get_thread_area Gary Benson
@ 2014-06-27 14:28 ` Gary Benson
  2014-07-09 13:12   ` Pedro Alves
  2014-06-27 14:40 ` [PATCH 5/7 v2] Comment and whitespace changes Gary Benson
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

This commit moves the shared code in {i386,amd64}-linux-nat.c
into the new files x86-linux-nat.[ch].  Additionally, a new
file i386-linux-nat.h was required to expose a value required
by the 32-bit code in x86-linux-nat.c.

This patch differs from the original version in this series
in that x86_linux_read_description has been rewritten in an
earlier patch.

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* i386-linux-nat.h: New file.
	* x86-linux-nat.h: Likewise.
	* x86-linux-nat.c: Likewise.
	* Makefile.in (HFILES_NO_SRCDIR): Add the above new headers.
	* config/i386/linux.mh (NATDEPFILES): Add x86-linux-nat.o.
	* config/i386/linux64.mh (NATDEPFILES): Likewise.
	* amd64-linux-nat.c (x86-linux-nat.h): New include.
	(PTRACE_GETREGSET): Now in x86-linux-nat.h.
	(PTRACE_SETREGSET): Likewise.
	(arch_lwp_info): Now in x86-linux-nat.c.
	(have_ptrace_getregset): Now in x86-linux-nat.h.
	(x86_linux_dr_get): Now in x86-linux-nat.c.
	(x86_linux_dr_set): Likewise.
	(x86_linux_dr_get_addr): Likewise.
	(x86_linux_dr_get_control) Likewise.:
	(x86_linux_dr_get_status): Likewise.
	(update_debug_registers_callback): Likewise.
	(x86_linux_dr_set_control): Likewise.
	(x86_linux_dr_set_addr): Likewise.
	(x86_linux_prepare_to_resume): Likewise.
	(x86_linux_new_thread): Likewise.
	(x86_linux_new_fork): Likewise.
	(x86_linux_get_thread_area): Likewise.
	(super_post_startup_inferior): Likewise.
	(x86_linux_child_post_startup_inferior): Likewise.
	(AMD64_LINUX_USER64_CS): Likewise.
	(AMD64_LINUX_X32_DS): Likewise.
	(x86_linux_read_description): Likewise.
	(x86_linux_enable_btrace): Likewise.
	(x86_linux_disable_btrace): Likewise.
	(x86_linux_teardown_btrace): Likewise.
	(x86_linux_read_btrace): Likewise.
	(x86_linux_create_target): Likewise.
	(x86_linux_register_target): Likewise.
	* i386-linux-nat.c (x86-linux-nat.h): New include.
	(PTRACE_GETREGSET): Now in x86-linux-nat.h.
	(PTRACE_SETREGSET): Likewise.
	(arch_lwp_info): Now in x86-linux-nat.c.
	(have_ptrace_getregset): Now in x86-linux-nat.h.
	(x86_linux_dr_get): Now in x86-linux-nat.c.
	(x86_linux_dr_set): Likewise.
	(x86_linux_dr_get_addr): Likewise.
	(x86_linux_dr_get_control) Likewise.:
	(x86_linux_dr_get_status): Likewise.
	(update_debug_registers_callback): Likewise.
	(x86_linux_dr_set_control): Likewise.
	(x86_linux_dr_set_addr): Likewise.
	(x86_linux_prepare_to_resume): Likewise.
	(x86_linux_new_thread): Likewise.
	(x86_linux_new_fork): Likewise.
	(x86_linux_get_thread_area): Likewise.
	(super_post_startup_inferior): Likewise.
	(x86_linux_child_post_startup_inferior): Likewise.
	(AMD64_LINUX_USER64_CS): Likewise.
	(AMD64_LINUX_X32_DS): Likewise.
	(x86_linux_read_description): Likewise.
	(x86_linux_enable_btrace): Likewise.
	(x86_linux_disable_btrace): Likewise.
	(x86_linux_teardown_btrace): Likewise.
	(x86_linux_read_btrace): Likewise.
	(x86_linux_create_target): Likewise.
	(x86_linux_register_target): Likewise.
---
 gdb/ChangeLog              |   65 +++++
 gdb/Makefile.in            |    3 +-
 gdb/amd64-linux-nat.c      |  535 +-----------------------------------------
 gdb/config/i386/linux.mh   |    2 +-
 gdb/config/i386/linux64.mh |    1 +
 gdb/i386-linux-nat.c       |  535 +-----------------------------------------
 gdb/i386-linux-nat.h       |   26 ++
 gdb/x86-linux-nat.c        |  568 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/x86-linux-nat.h        |   51 ++++
 9 files changed, 718 insertions(+), 1068 deletions(-)
 create mode 100644 gdb/i386-linux-nat.h
 create mode 100644 gdb/x86-linux-nat.c
 create mode 100644 gdb/x86-linux-nat.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8838e99..ce15501 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -934,7 +934,8 @@ nat/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
 ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
-common/print-utils.h common/rsp-low.h nat/i386-dregs.h
+common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
+i386-linux-nat.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index f29f8c7..bd03ea3 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -58,24 +58,7 @@
 #include "i386-nat.h"
 #include "i386-xstate.h"
 
-#ifndef PTRACE_GETREGSET
-#define PTRACE_GETREGSET	0x4204
-#endif
-
-#ifndef PTRACE_SETREGSET
-#define PTRACE_SETREGSET	0x4205
-#endif
-
-/* Per-thread arch-specific data we want to keep.  */
-
-struct arch_lwp_info
-{
-  /* Non-zero if our copy differs from what's recorded in the thread.  */
-  int debug_registers_changed;
-};
-
-/* Does the current host support PTRACE_GETREGSET?  */
-static int have_ptrace_getregset = -1;
+#include "x86-linux-nat.h"
 
 /* Mapping between the general-purpose registers in GNU/Linux x86-64
    `struct user' format and GDB's register cache layout for GNU/Linux
@@ -278,262 +261,6 @@ amd64_linux_store_inferior_registers (struct target_ops *ops,
 }
 \f
 
-/* Support for debug registers.  */
-
-/* Get debug register REGNUM value from only the one LWP of PTID.  */
-
-static unsigned long
-x86_linux_dr_get (ptid_t ptid, int regnum)
-{
-  int tid;
-  unsigned long value;
-
-  tid = ptid_get_lwp (ptid);
-  if (tid == 0)
-    tid = ptid_get_pid (ptid);
-
-  errno = 0;
-  value = ptrace (PTRACE_PEEKUSER, tid,
-		  offsetof (struct user, u_debugreg[regnum]), 0);
-  if (errno != 0)
-    perror_with_name (_("Couldn't read debug register"));
-
-  return value;
-}
-
-/* Set debug register REGNUM to VALUE in only the one LWP of PTID.  */
-
-static void
-x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
-{
-  int tid;
-
-  tid = ptid_get_lwp (ptid);
-  if (tid == 0)
-    tid = ptid_get_pid (ptid);
-
-  errno = 0;
-  ptrace (PTRACE_POKEUSER, tid,
-	  offsetof (struct user, u_debugreg[regnum]), value);
-  if (errno != 0)
-    perror_with_name (_("Couldn't write debug register"));
-}
-
-/* Return the inferior's debug register REGNUM.  */
-
-static CORE_ADDR
-x86_linux_dr_get_addr (int regnum)
-{
-  /* DR6 and DR7 are retrieved with some other way.  */
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
-
-  return x86_linux_dr_get (inferior_ptid, regnum);
-}
-
-/* Return the inferior's DR7 debug control register.  */
-
-static unsigned long
-x86_linux_dr_get_control (void)
-{
-  return x86_linux_dr_get (inferior_ptid, DR_CONTROL);
-}
-
-/* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
-
-static unsigned long
-x86_linux_dr_get_status (void)
-{
-  return x86_linux_dr_get (inferior_ptid, DR_STATUS);
-}
-
-/* Callback for iterate_over_lwps.  Update the debug registers of
-   LWP.  */
-
-static int
-update_debug_registers_callback (struct lwp_info *lwp, void *arg)
-{
-  if (lwp->arch_private == NULL)
-    lwp->arch_private = XCNEW (struct arch_lwp_info);
-
-  /* The actual update is done later just before resuming the lwp, we
-     just mark that the registers need updating.  */
-  lwp->arch_private->debug_registers_changed = 1;
-
-  /* If the lwp isn't stopped, force it to momentarily pause, so we
-     can update its debug registers.  */
-  if (!lwp->stopped)
-    linux_stop_lwp (lwp);
-
-  /* Continue the iteration.  */
-  return 0;
-}
-
-/* Set DR_CONTROL to CONTROL in all LWPs of the current inferior.  */
-
-static void
-x86_linux_dr_set_control (unsigned long control)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
-   inferior.  */
-
-static void
-x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
-  gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* Called when resuming a thread.
-   If the debug regs have changed, update the thread's copies.  */
-
-static void
-x86_linux_prepare_to_resume (struct lwp_info *lwp)
-{
-  int clear_status = 0;
-
-  /* NULL means this is the main thread still going through the shell,
-     or, no watchpoint has been set yet.  In that case, there's
-     nothing to do.  */
-  if (lwp->arch_private == NULL)
-    return;
-
-  if (lwp->arch_private->debug_registers_changed)
-    {
-      struct i386_debug_reg_state *state
-	= i386_debug_reg_state (ptid_get_pid (lwp->ptid));
-      int i;
-
-      /* On Linux kernel before 2.6.33 commit
-	 72f674d203cd230426437cdcf7dd6f681dad8b0d
-	 if you enable a breakpoint by the DR_CONTROL bits you need to have
-	 already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
-
-	 Ensure DR_CONTROL gets written as the very last register here.  */
-
-      /* Clear DR_CONTROL first.  In some cases, setting DR0-3 to a
-	 value that doesn't match what is enabled in DR_CONTROL
-	 results in EINVAL.  */
-      x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
-
-      for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
-	if (state->dr_ref_count[i] > 0)
-	  {
-	    x86_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
-
-	    /* If we're setting a watchpoint, any change the inferior
-	       had done itself to the debug registers needs to be
-	       discarded, otherwise, i386_stopped_data_address can get
-	       confused.  */
-	    clear_status = 1;
-	  }
-
-      /* If DR_CONTROL is supposed to be zero, we've already set it
-	 above.  */
-      if (state->dr_control_mirror != 0)
-	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
-
-      lwp->arch_private->debug_registers_changed = 0;
-    }
-
-  if (clear_status || lwp->stopped_by_watchpoint)
-    x86_linux_dr_set (lwp->ptid, DR_STATUS, 0);
-}
-
-static void
-x86_linux_new_thread (struct lwp_info *lp)
-{
-  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
-
-  info->debug_registers_changed = 1;
-
-  lp->arch_private = info;
-}
-
-/* linux_nat_new_fork hook.   */
-
-static void
-x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
-{
-  pid_t parent_pid;
-  struct i386_debug_reg_state *parent_state;
-  struct i386_debug_reg_state *child_state;
-
-  /* NULL means no watchpoint has ever been set in the parent.  In
-     that case, there's nothing to do.  */
-  if (parent->arch_private == NULL)
-    return;
-
-  /* Linux kernel before 2.6.33 commit
-     72f674d203cd230426437cdcf7dd6f681dad8b0d
-     will inherit hardware debug registers from parent
-     on fork/vfork/clone.  Newer Linux kernels create such tasks with
-     zeroed debug registers.
-
-     GDB core assumes the child inherits the watchpoints/hw
-     breakpoints of the parent, and will remove them all from the
-     forked off process.  Copy the debug registers mirrors into the
-     new process so that all breakpoints and watchpoints can be
-     removed together.  The debug registers mirror will become zeroed
-     in the end before detaching the forked off process, thus making
-     this compatible with older Linux kernels too.  */
-
-  parent_pid = ptid_get_pid (parent->ptid);
-  parent_state = i386_debug_reg_state (parent_pid);
-  child_state = i386_debug_reg_state (child_pid);
-  *child_state = *parent_state;
-}
-
-\f
-
-/* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
-   the thread local storage (or its descriptor) and returns PS_OK
-   on success.  Returns PS_ERR on failure.  */
-
-static ps_err_e
-x86_linux_get_thread_area (pid_t pid, void *addr, unsigned int *base_addr)
-{
-  /* NOTE: cagney/2003-08-26: The definition of this buffer is found
-     in the kernel header <asm-i386/ldt.h>.  It, after padding, is 4 x
-     4 byte integers in size: `entry_number', `base_addr', `limit',
-     and a bunch of status bits.
-
-     The values returned by this ptrace call should be part of the
-     regcache buffer, and ps_get_thread_area should channel its
-     request through the regcache.  That way remote targets could
-     provide the value using the remote protocol and not this direct
-     call.
-
-     Is this function needed?  I'm guessing that the `base' is the
-     address of a descriptor that libthread_db uses to find the
-     thread local address base that GDB needs.  Perhaps that
-     descriptor is defined by the ABI.  Anyway, given that
-     libthread_db calls this function without prompting (gdb
-     requesting tls base) I guess it needs info in there anyway.  */
-  unsigned int desc[4];
-
-  /* This code assumes that "int" is 32 bits and that
-     GET_THREAD_AREA returns no more than 4 int values.  */
-  gdb_assert (sizeof (int) == 4);
-
-#ifndef PTRACE_GET_THREAD_AREA
-#define PTRACE_GET_THREAD_AREA 25
-#endif
-
-  if (ptrace (PTRACE_GET_THREAD_AREA, pid, addr, &desc) < 0)
-    return PS_ERR;
-
-  *base_addr = desc[1];
-  return PS_OK;
-}
-
 /* This function is called by libthread_db as part of its handling of
    a request for a thread's local storage address.  */
 
@@ -614,17 +341,6 @@ ps_get_thread_area (const struct ps_prochandle *ph,
 }
 \f
 
-static void (*super_post_startup_inferior) (struct target_ops *self,
-					    ptid_t ptid);
-
-static void
-x86_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
-{
-  i386_cleanup_dregs ();
-  super_post_startup_inferior (self, ptid);
-}
-\f
-
 /* When GDB is built as a 64-bit application on linux, the
    PTRACE_GETSIGINFO data is always presented in 64-bit layout.  Since
    debugging a 32-bit inferior with a 64-bit GDB should look the same
@@ -1069,254 +785,7 @@ amd64_linux_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction)
   else
     return 0;
 }
-
-#ifdef __x86_64__
-/* Value of CS segment register:
-     64bit process: 0x33
-     32bit process: 0x23  */
-#define AMD64_LINUX_USER64_CS 0x33
-
-/* Value of DS segment register:
-     LP64 process: 0x0
-     X32 process: 0x2b  */
-#define AMD64_LINUX_X32_DS 0x2b
-#endif
-
-/* Get Linux/x86 target description from running target.  */
-
-static const struct target_desc *
-x86_linux_read_description (struct target_ops *ops)
-{
-  int tid;
-  int is_64bit = 0;
-#ifdef __x86_64__
-  int is_x32;
-#endif
-  static uint64_t xcr0;
-  uint64_t xcr0_features_bits;
-
-  /* GNU/Linux LWP ID's are process ID's.  */
-  tid = ptid_get_lwp (inferior_ptid);
-  if (tid == 0)
-    tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
-
-#ifdef __x86_64__
-  {
-    unsigned long cs;
-    unsigned long ds;
-
-    /* Get CS register.  */
-    errno = 0;
-    cs = ptrace (PTRACE_PEEKUSER, tid,
-		 offsetof (struct user_regs_struct, cs), 0);
-    if (errno != 0)
-      perror_with_name (_("Couldn't get CS register"));
-
-    is_64bit = cs == AMD64_LINUX_USER64_CS;
-
-    /* Get DS register.  */
-    errno = 0;
-    ds = ptrace (PTRACE_PEEKUSER, tid,
-		 offsetof (struct user_regs_struct, ds), 0);
-    if (errno != 0)
-      perror_with_name (_("Couldn't get DS register"));
-
-    is_x32 = ds == AMD64_LINUX_X32_DS;
-
-    if (sizeof (void *) == 4 && is_64bit && !is_x32)
-      error (_("Can't debug 64-bit process with 32-bit GDB"));
-  }
-#elif HAVE_PTRACE_GETFPXREGS
-  if (have_ptrace_getfpxregs == -1)
-    {
-      elf_fpxregset_t fpxregs;
-
-      if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
-	{
-	  have_ptrace_getfpxregs = 0;
-	  have_ptrace_getregset = 0;
-	  return tdesc_i386_mmx_linux;
-	}
-    }
-#endif
-
-  if (have_ptrace_getregset == -1)
-    {
-      uint64_t xstateregs[(I386_XSTATE_SSE_SIZE / sizeof (uint64_t))];
-      struct iovec iov;
-
-      iov.iov_base = xstateregs;
-      iov.iov_len = sizeof (xstateregs);
-
-      /* Check if PTRACE_GETREGSET works.  */
-      if (ptrace (PTRACE_GETREGSET, tid,
-		  (unsigned int) NT_X86_XSTATE, &iov) < 0)
-	have_ptrace_getregset = 0;
-      else
-	{
-	  have_ptrace_getregset = 1;
-
-	  /* Get XCR0 from XSAVE extended state.  */
-	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
-			     / sizeof (uint64_t))];
-	}
-    }
-
-  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  If
-     PTRACE_GETREGSET is not available then set xcr0_features_bits to
-     zero so that the "no-features" descriptions are returned by the
-     switches below.  */
-  if (have_ptrace_getregset)
-    xcr0_features_bits = xcr0 & I386_XSTATE_ALL_MASK;
-  else
-    xcr0_features_bits = 0;
-
-  if (is_64bit)
-    {
-#ifdef __x86_64__
-      switch (xcr0_features_bits)
-	{
-	case I386_XSTATE_MPX_AVX512_MASK:
-	case I386_XSTATE_AVX512_MASK:
-	  if (is_x32)
-	    return tdesc_x32_avx512_linux;
-	  else
-	    return tdesc_amd64_avx512_linux;
-	case I386_XSTATE_MPX_MASK:
-	  if (is_x32)
-	    return tdesc_x32_avx_linux; /* No MPX on x32 using AVX.  */
-	  else
-	    return tdesc_amd64_mpx_linux;
-	case I386_XSTATE_AVX_MASK:
-	  if (is_x32)
-	    return tdesc_x32_avx_linux;
-	  else
-	    return tdesc_amd64_avx_linux;
-	default:
-	  if (is_x32)
-	    return tdesc_x32_linux;
-	  else
-	    return tdesc_amd64_linux;
-	}
-#endif
-    }
-  else
-    {
-      switch (xcr0_features_bits)
-	{
-	case I386_XSTATE_MPX_AVX512_MASK:
-	case I386_XSTATE_AVX512_MASK:
-	  return tdesc_i386_avx512_linux;
-	case I386_XSTATE_MPX_MASK:
-	  return tdesc_i386_mpx_linux;
-	case I386_XSTATE_AVX_MASK:
-	  return tdesc_i386_avx_linux;
-	default:
-	  return tdesc_i386_linux;
-	}
-    }
-
-  gdb_assert_not_reached ("failed to return tdesc");
-}
-
-/* Enable branch tracing.  */
-
-static struct btrace_target_info *
-x86_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
-{
-  struct btrace_target_info *tinfo;
-  struct gdbarch *gdbarch;
-
-  errno = 0;
-  tinfo = linux_enable_btrace (ptid);
-
-  if (tinfo == NULL)
-    error (_("Could not enable branch tracing for %s: %s."),
-	   target_pid_to_str (ptid), safe_strerror (errno));
-
-  /* Fill in the size of a pointer in bits.  */
-  gdbarch = target_thread_architecture (ptid);
-  tinfo->ptr_bits = gdbarch_ptr_bit (gdbarch);
-
-  return tinfo;
-}
-
-/* Disable branch tracing.  */
-
-static void
-x86_linux_disable_btrace (struct target_ops *self,
-			  struct btrace_target_info *tinfo)
-{
-  enum btrace_error errcode = linux_disable_btrace (tinfo);
-
-  if (errcode != BTRACE_ERR_NONE)
-    error (_("Could not disable branch tracing."));
-}
-
-/* Teardown branch tracing.  */
-
-static void
-x86_linux_teardown_btrace (struct target_ops *self,
-			   struct btrace_target_info *tinfo)
-{
-  /* Ignore errors.  */
-  linux_disable_btrace (tinfo);
-}
-
-static enum btrace_error
-x86_linux_read_btrace (struct target_ops *self,
-		       VEC (btrace_block_s) **data,
-		       struct btrace_target_info *btinfo,
-		       enum btrace_read_type type)
-{
-  return linux_read_btrace (data, btinfo, type);
-}
-
-/* Create an x86 GNU/Linux target.  */
-
-static struct target_ops *
-x86_linux_create_target (void)
-{
-  /* Fill in the generic GNU/Linux methods.  */
-  struct target_ops *t = linux_target ();
-
-  /* Initialize the debug register function vectors.  */
-  i386_use_watchpoints (t);
-  i386_dr_low.set_control = x86_linux_dr_set_control;
-  i386_dr_low.set_addr = x86_linux_dr_set_addr;
-  i386_dr_low.get_addr = x86_linux_dr_get_addr;
-  i386_dr_low.get_status = x86_linux_dr_get_status;
-  i386_dr_low.get_control = x86_linux_dr_get_control;
-  i386_set_debug_register_length (sizeof (void *));
-
-  /* Override the GNU/Linux inferior startup hook.  */
-  super_post_startup_inferior = t->to_post_startup_inferior;
-  t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
-
-  /* Add the description reader.  */
-  t->to_read_description = x86_linux_read_description;
-
-  /* Add btrace methods.  */
-  t->to_supports_btrace = linux_supports_btrace;
-  t->to_enable_btrace = x86_linux_enable_btrace;
-  t->to_disable_btrace = x86_linux_disable_btrace;
-  t->to_teardown_btrace = x86_linux_teardown_btrace;
-  t->to_read_btrace = x86_linux_read_btrace;
-
-  return t;
-}
-
-/* Register an x86 GNU/Linux target.  */
-
-static void
-x86_linux_register_target (struct target_ops *t)
-{
-  linux_nat_add_target (t);
-  linux_nat_set_new_thread (t, x86_linux_new_thread);
-  linux_nat_set_new_fork (t, x86_linux_new_fork);
-  linux_nat_set_forget_process (t, i386_forget_process);
-  linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
-}
+\f
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_amd64_linux_nat (void);
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index be18dcf..536ed3d 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -2,7 +2,7 @@
 
 NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o \
-	i386-nat.o i386-dregs.o i386-linux-nat.o \
+	i386-nat.o i386-dregs.o i386-linux-nat.o x86-linux-nat.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
 	linux-btrace.o linux-waitpid.o
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index 3126b75..8faca42 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -1,6 +1,7 @@
 # Host: GNU/Linux x86-64
 NATDEPFILES= inf-ptrace.o fork-child.o \
 	i386-nat.o i386-dregs.o amd64-nat.o amd64-linux-nat.o \
+	x86-linux-nat.o \
 	linux-nat.o linux-osdata.o \
 	proc-service.o linux-thread-db.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-btrace.o \
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 51b3338..3778c77 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -60,25 +60,7 @@
 
 #include "i386-xstate.h"
 
-#ifndef PTRACE_GETREGSET
-#define PTRACE_GETREGSET	0x4204
-#endif
-
-#ifndef PTRACE_SETREGSET
-#define PTRACE_SETREGSET	0x4205
-#endif
-
-/* Per-thread arch-specific data we want to keep.  */
-
-struct arch_lwp_info
-{
-  /* Non-zero if our copy differs from what's recorded in the thread.  */
-  int debug_registers_changed;
-};
-
-/* Does the current host support PTRACE_GETREGSET?  */
-static int have_ptrace_getregset = -1;
-\f
+#include "x86-linux-nat.h"
 
 /* The register sets used in GNU/Linux ELF core-dumps are identical to
    the register sets in `struct user' that is used for a.out
@@ -642,262 +624,6 @@ i386_linux_store_inferior_registers (struct target_ops *ops,
 }
 \f
 
-/* Support for debug registers.  */
-
-/* Get debug register REGNUM value from only the one LWP of PTID.  */
-
-static unsigned long
-x86_linux_dr_get (ptid_t ptid, int regnum)
-{
-  int tid;
-  unsigned long value;
-
-  tid = ptid_get_lwp (ptid);
-  if (tid == 0)
-    tid = ptid_get_pid (ptid);
-
-  errno = 0;
-  value = ptrace (PTRACE_PEEKUSER, tid,
-		  offsetof (struct user, u_debugreg[regnum]), 0);
-  if (errno != 0)
-    perror_with_name (_("Couldn't read debug register"));
-
-  return value;
-}
-
-/* Set debug register REGNUM to VALUE in only the one LWP of PTID.  */
-
-static void
-x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
-{
-  int tid;
-
-  tid = ptid_get_lwp (ptid);
-  if (tid == 0)
-    tid = ptid_get_pid (ptid);
-
-  errno = 0;
-  ptrace (PTRACE_POKEUSER, tid,
-	  offsetof (struct user, u_debugreg[regnum]), value);
-  if (errno != 0)
-    perror_with_name (_("Couldn't write debug register"));
-}
-
-/* Return the inferior's debug register REGNUM.  */
-
-static CORE_ADDR
-x86_linux_dr_get_addr (int regnum)
-{
-  /* DR6 and DR7 are retrieved with some other way.  */
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
-
-  return x86_linux_dr_get (inferior_ptid, regnum);
-}
-
-/* Return the inferior's DR7 debug control register.  */
-
-static unsigned long
-x86_linux_dr_get_control (void)
-{
-  return x86_linux_dr_get (inferior_ptid, DR_CONTROL);
-}
-
-/* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
-
-static unsigned long
-x86_linux_dr_get_status (void)
-{
-  return x86_linux_dr_get (inferior_ptid, DR_STATUS);
-}
-
-/* Callback for iterate_over_lwps.  Update the debug registers of
-   LWP.  */
-
-static int
-update_debug_registers_callback (struct lwp_info *lwp, void *arg)
-{
-  if (lwp->arch_private == NULL)
-    lwp->arch_private = XCNEW (struct arch_lwp_info);
-
-  /* The actual update is done later just before resuming the lwp, we
-     just mark that the registers need updating.  */
-  lwp->arch_private->debug_registers_changed = 1;
-
-  /* If the lwp isn't stopped, force it to momentarily pause, so we
-     can update its debug registers.  */
-  if (!lwp->stopped)
-    linux_stop_lwp (lwp);
-
-  /* Continue the iteration.  */
-  return 0;
-}
-
-/* Set DR_CONTROL to CONTROL in all LWPs of the current inferior.  */
-
-static void
-x86_linux_dr_set_control (unsigned long control)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
-   inferior.  */
-
-static void
-x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
-  gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* Called when resuming a thread.
-   If the debug regs have changed, update the thread's copies.  */
-
-static void
-x86_linux_prepare_to_resume (struct lwp_info *lwp)
-{
-  int clear_status = 0;
-
-  /* NULL means this is the main thread still going through the shell,
-     or, no watchpoint has been set yet.  In that case, there's
-     nothing to do.  */
-  if (lwp->arch_private == NULL)
-    return;
-
-  if (lwp->arch_private->debug_registers_changed)
-    {
-      struct i386_debug_reg_state *state
-	= i386_debug_reg_state (ptid_get_pid (lwp->ptid));
-      int i;
-
-      /* On Linux kernel before 2.6.33 commit
-	 72f674d203cd230426437cdcf7dd6f681dad8b0d
-	 if you enable a breakpoint by the DR_CONTROL bits you need to have
-	 already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
-
-	 Ensure DR_CONTROL gets written as the very last register here.  */
-
-      /* Clear DR_CONTROL first.  In some cases, setting DR0-3 to a
-	 value that doesn't match what is enabled in DR_CONTROL
-	 results in EINVAL.  */
-      x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
-
-      for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
-	if (state->dr_ref_count[i] > 0)
-	  {
-	    x86_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
-
-	    /* If we're setting a watchpoint, any change the inferior
-	       had done itself to the debug registers needs to be
-	       discarded, otherwise, i386_stopped_data_address can get
-	       confused.  */
-	    clear_status = 1;
-	  }
-
-      /* If DR_CONTROL is supposed to be zero, we've already set it
-	 above.  */
-      if (state->dr_control_mirror != 0)
-	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
-
-      lwp->arch_private->debug_registers_changed = 0;
-    }
-
-  if (clear_status || lwp->stopped_by_watchpoint)
-    x86_linux_dr_set (lwp->ptid, DR_STATUS, 0);
-}
-
-static void
-x86_linux_new_thread (struct lwp_info *lp)
-{
-  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
-
-  info->debug_registers_changed = 1;
-
-  lp->arch_private = info;
-}
-
-/* linux_nat_new_fork hook.   */
-
-static void
-x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
-{
-  pid_t parent_pid;
-  struct i386_debug_reg_state *parent_state;
-  struct i386_debug_reg_state *child_state;
-
-  /* NULL means no watchpoint has ever been set in the parent.  In
-     that case, there's nothing to do.  */
-  if (parent->arch_private == NULL)
-    return;
-
-  /* Linux kernel before 2.6.33 commit
-     72f674d203cd230426437cdcf7dd6f681dad8b0d
-     will inherit hardware debug registers from parent
-     on fork/vfork/clone.  Newer Linux kernels create such tasks with
-     zeroed debug registers.
-
-     GDB core assumes the child inherits the watchpoints/hw
-     breakpoints of the parent, and will remove them all from the
-     forked off process.  Copy the debug registers mirrors into the
-     new process so that all breakpoints and watchpoints can be
-     removed together.  The debug registers mirror will become zeroed
-     in the end before detaching the forked off process, thus making
-     this compatible with older Linux kernels too.  */
-
-  parent_pid = ptid_get_pid (parent->ptid);
-  parent_state = i386_debug_reg_state (parent_pid);
-  child_state = i386_debug_reg_state (child_pid);
-  *child_state = *parent_state;
-}
-
-\f
-
-/* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
-   the thread local storage (or its descriptor) and returns PS_OK
-   on success.  Returns PS_ERR on failure.  */
-
-static ps_err_e
-x86_linux_get_thread_area (pid_t pid, void *addr, unsigned int *base_addr)
-{
-  /* NOTE: cagney/2003-08-26: The definition of this buffer is found
-     in the kernel header <asm-i386/ldt.h>.  It, after padding, is 4 x
-     4 byte integers in size: `entry_number', `base_addr', `limit',
-     and a bunch of status bits.
-
-     The values returned by this ptrace call should be part of the
-     regcache buffer, and ps_get_thread_area should channel its
-     request through the regcache.  That way remote targets could
-     provide the value using the remote protocol and not this direct
-     call.
-
-     Is this function needed?  I'm guessing that the `base' is the
-     address of a descriptor that libthread_db uses to find the
-     thread local address base that GDB needs.  Perhaps that
-     descriptor is defined by the ABI.  Anyway, given that
-     libthread_db calls this function without prompting (gdb
-     requesting tls base) I guess it needs info in there anyway.  */
-  unsigned int desc[4];
-
-  /* This code assumes that "int" is 32 bits and that
-     GET_THREAD_AREA returns no more than 4 int values.  */
-  gdb_assert (sizeof (int) == 4);
-
-#ifndef PTRACE_GET_THREAD_AREA
-#define PTRACE_GET_THREAD_AREA 25
-#endif
-
-  if (ptrace (PTRACE_GET_THREAD_AREA, pid, addr, &desc) < 0)
-    return PS_ERR;
-
-  *base_addr = desc[1];
-  return PS_OK;
-}
-
 /* Called by libthread_db.  Returns a pointer to the thread local
    storage (or its descriptor).  */
 
@@ -1013,264 +739,7 @@ i386_linux_resume (struct target_ops *ops,
   if (ptrace (request, pid, 0, gdb_signal_to_host (signal)) == -1)
     perror_with_name (("ptrace"));
 }
-
-static void (*super_post_startup_inferior) (struct target_ops *self,
-					    ptid_t ptid);
-
-static void
-x86_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
-{
-  i386_cleanup_dregs ();
-  super_post_startup_inferior (self, ptid);
-}
-
-#ifdef __x86_64__
-/* Value of CS segment register:
-     64bit process: 0x33
-     32bit process: 0x23  */
-#define AMD64_LINUX_USER64_CS 0x33
-
-/* Value of DS segment register:
-     LP64 process: 0x0
-     X32 process: 0x2b  */
-#define AMD64_LINUX_X32_DS 0x2b
-#endif
-
-/* Get Linux/x86 target description from running target.  */
-
-static const struct target_desc *
-x86_linux_read_description (struct target_ops *ops)
-{
-  int tid;
-  int is_64bit = 0;
-#ifdef __x86_64__
-  int is_x32;
-#endif
-  static uint64_t xcr0;
-  uint64_t xcr0_features_bits;
-
-  /* GNU/Linux LWP ID's are process ID's.  */
-  tid = ptid_get_lwp (inferior_ptid);
-  if (tid == 0)
-    tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
-
-#ifdef __x86_64__
-  {
-    unsigned long cs;
-    unsigned long ds;
-
-    /* Get CS register.  */
-    errno = 0;
-    cs = ptrace (PTRACE_PEEKUSER, tid,
-		 offsetof (struct user_regs_struct, cs), 0);
-    if (errno != 0)
-      perror_with_name (_("Couldn't get CS register"));
-
-    is_64bit = cs == AMD64_LINUX_USER64_CS;
-
-    /* Get DS register.  */
-    errno = 0;
-    ds = ptrace (PTRACE_PEEKUSER, tid,
-		 offsetof (struct user_regs_struct, ds), 0);
-    if (errno != 0)
-      perror_with_name (_("Couldn't get DS register"));
-
-    is_x32 = ds == AMD64_LINUX_X32_DS;
-
-    if (sizeof (void *) == 4 && is_64bit && !is_x32)
-      error (_("Can't debug 64-bit process with 32-bit GDB"));
-  }
-#elif HAVE_PTRACE_GETFPXREGS
-  if (have_ptrace_getfpxregs == -1)
-    {
-      elf_fpxregset_t fpxregs;
-
-      if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
-	{
-	  have_ptrace_getfpxregs = 0;
-	  have_ptrace_getregset = 0;
-	  return tdesc_i386_mmx_linux;
-	}
-    }
-#endif
-
-  if (have_ptrace_getregset == -1)
-    {
-      uint64_t xstateregs[(I386_XSTATE_SSE_SIZE / sizeof (uint64_t))];
-      struct iovec iov;
-
-      iov.iov_base = xstateregs;
-      iov.iov_len = sizeof (xstateregs);
-
-      /* Check if PTRACE_GETREGSET works.  */
-      if (ptrace (PTRACE_GETREGSET, tid,
-		  (unsigned int) NT_X86_XSTATE, &iov) < 0)
-	have_ptrace_getregset = 0;
-      else
-	{
-	  have_ptrace_getregset = 1;
-
-	  /* Get XCR0 from XSAVE extended state.  */
-	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
-			     / sizeof (uint64_t))];
-	}
-    }
-
-  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  If
-     PTRACE_GETREGSET is not available then set xcr0_features_bits to
-     zero so that the "no-features" descriptions are returned by the
-     switches below.  */
-  if (have_ptrace_getregset)
-    xcr0_features_bits = xcr0 & I386_XSTATE_ALL_MASK;
-  else
-    xcr0_features_bits = 0;
-
-  if (is_64bit)
-    {
-#ifdef __x86_64__
-      switch (xcr0_features_bits)
-	{
-	case I386_XSTATE_MPX_AVX512_MASK:
-	case I386_XSTATE_AVX512_MASK:
-	  if (is_x32)
-	    return tdesc_x32_avx512_linux;
-	  else
-	    return tdesc_amd64_avx512_linux;
-	case I386_XSTATE_MPX_MASK:
-	  if (is_x32)
-	    return tdesc_x32_avx_linux; /* No MPX on x32 using AVX.  */
-	  else
-	    return tdesc_amd64_mpx_linux;
-	case I386_XSTATE_AVX_MASK:
-	  if (is_x32)
-	    return tdesc_x32_avx_linux;
-	  else
-	    return tdesc_amd64_avx_linux;
-	default:
-	  if (is_x32)
-	    return tdesc_x32_linux;
-	  else
-	    return tdesc_amd64_linux;
-	}
-#endif
-    }
-  else
-    {
-      switch (xcr0_features_bits)
-	{
-	case I386_XSTATE_MPX_AVX512_MASK:
-	case I386_XSTATE_AVX512_MASK:
-	  return tdesc_i386_avx512_linux;
-	case I386_XSTATE_MPX_MASK:
-	  return tdesc_i386_mpx_linux;
-	case I386_XSTATE_AVX_MASK:
-	  return tdesc_i386_avx_linux;
-	default:
-	  return tdesc_i386_linux;
-	}
-    }
-
-  gdb_assert_not_reached ("failed to return tdesc");
-}
-
-/* Enable branch tracing.  */
-
-static struct btrace_target_info *
-x86_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
-{
-  struct btrace_target_info *tinfo;
-  struct gdbarch *gdbarch;
-
-  errno = 0;
-  tinfo = linux_enable_btrace (ptid);
-
-  if (tinfo == NULL)
-    error (_("Could not enable branch tracing for %s: %s."),
-	   target_pid_to_str (ptid), safe_strerror (errno));
-
-  /* Fill in the size of a pointer in bits.  */
-  gdbarch = target_thread_architecture (ptid);
-  tinfo->ptr_bits = gdbarch_ptr_bit (gdbarch);
-
-  return tinfo;
-}
-
-/* Disable branch tracing.  */
-
-static void
-x86_linux_disable_btrace (struct target_ops *self,
-			  struct btrace_target_info *tinfo)
-{
-  enum btrace_error errcode = linux_disable_btrace (tinfo);
-
-  if (errcode != BTRACE_ERR_NONE)
-    error (_("Could not disable branch tracing."));
-}
-
-/* Teardown branch tracing.  */
-
-static void
-x86_linux_teardown_btrace (struct target_ops *self,
-			   struct btrace_target_info *tinfo)
-{
-  /* Ignore errors.  */
-  linux_disable_btrace (tinfo);
-}
-
-static enum btrace_error
-x86_linux_read_btrace (struct target_ops *self,
-		       VEC (btrace_block_s) **data,
-		       struct btrace_target_info *btinfo,
-		       enum btrace_read_type type)
-{
-  return linux_read_btrace (data, btinfo, type);
-}
-
-/* Create an x86 GNU/Linux target.  */
-
-static struct target_ops *
-x86_linux_create_target (void)
-{
-  /* Fill in the generic GNU/Linux methods.  */
-  struct target_ops *t = linux_target ();
-
-  /* Initialize the debug register function vectors.  */
-  i386_use_watchpoints (t);
-  i386_dr_low.set_control = x86_linux_dr_set_control;
-  i386_dr_low.set_addr = x86_linux_dr_set_addr;
-  i386_dr_low.get_addr = x86_linux_dr_get_addr;
-  i386_dr_low.get_status = x86_linux_dr_get_status;
-  i386_dr_low.get_control = x86_linux_dr_get_control;
-  i386_set_debug_register_length (sizeof (void *));
-
-  /* Override the GNU/Linux inferior startup hook.  */
-  super_post_startup_inferior = t->to_post_startup_inferior;
-  t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
-
-  /* Add the description reader.  */
-  t->to_read_description = x86_linux_read_description;
-
-  /* Add btrace methods.  */
-  t->to_supports_btrace = linux_supports_btrace;
-  t->to_enable_btrace = x86_linux_enable_btrace;
-  t->to_disable_btrace = x86_linux_disable_btrace;
-  t->to_teardown_btrace = x86_linux_teardown_btrace;
-  t->to_read_btrace = x86_linux_read_btrace;
-
-  return t;
-}
-
-/* Register an x86 GNU/Linux target.  */
-
-static void
-x86_linux_register_target (struct target_ops *t)
-{
-  linux_nat_add_target (t);
-  linux_nat_set_new_thread (t, x86_linux_new_thread);
-  linux_nat_set_new_fork (t, x86_linux_new_fork);
-  linux_nat_set_forget_process (t, i386_forget_process);
-  linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
-}
+\f
 
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_i386_linux_nat;
diff --git a/gdb/i386-linux-nat.h b/gdb/i386-linux-nat.h
new file mode 100644
index 0000000..f2c29bd
--- /dev/null
+++ b/gdb/i386-linux-nat.h
@@ -0,0 +1,26 @@
+/* Native-dependent code for GNU/Linux i386.
+
+   Copyright (C) 1999-2014 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/>.  */
+
+#ifndef I386_LINUX_NAT_H
+#define I386_LINUX_NAT_H 1
+
+/* Does the current host support the GETFPXREGS request? */
+extern int have_ptrace_getfpxregs;
+
+#endif
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
new file mode 100644
index 0000000..dd5ee4c
--- /dev/null
+++ b/gdb/x86-linux-nat.c
@@ -0,0 +1,568 @@
+/* Native-dependent code for GNU/Linux x86 (i386 and x86-64).
+
+   Copyright (C) 1999-2014 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/>.  */
+
+#include "defs.h"
+#include "inferior.h"
+#include "elf/common.h"
+#include "gdb_proc_service.h"
+#include <sys/ptrace.h>
+#include <sys/user.h>
+#include <sys/procfs.h>
+
+#include "i386-nat.h"
+#include "linux-nat.h"
+#ifndef __x86_64__
+#include "i386-linux-nat.h"
+#endif
+#include "x86-linux-nat.h"
+#include "i386-linux-tdep.h"
+#ifdef __x86_64__
+#include "amd64-linux-tdep.h"
+#endif
+#include "i386-xstate.h"
+#include "nat/linux-btrace.h"
+
+/* Per-thread arch-specific data we want to keep.  */
+
+struct arch_lwp_info
+{
+  /* Non-zero if our copy differs from what's recorded in the thread.  */
+  int debug_registers_changed;
+};
+
+/* Does the current host support PTRACE_GETREGSET?  */
+int have_ptrace_getregset = -1;
+\f
+
+/* Support for debug registers.  */
+
+/* Get debug register REGNUM value from only the one LWP of PTID.  */
+
+static unsigned long
+x86_linux_dr_get (ptid_t ptid, int regnum)
+{
+  int tid;
+  unsigned long value;
+
+  tid = ptid_get_lwp (ptid);
+  if (tid == 0)
+    tid = ptid_get_pid (ptid);
+
+  errno = 0;
+  value = ptrace (PTRACE_PEEKUSER, tid,
+		  offsetof (struct user, u_debugreg[regnum]), 0);
+  if (errno != 0)
+    perror_with_name (_("Couldn't read debug register"));
+
+  return value;
+}
+
+/* Set debug register REGNUM to VALUE in only the one LWP of PTID.  */
+
+static void
+x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
+{
+  int tid;
+
+  tid = ptid_get_lwp (ptid);
+  if (tid == 0)
+    tid = ptid_get_pid (ptid);
+
+  errno = 0;
+  ptrace (PTRACE_POKEUSER, tid,
+	  offsetof (struct user, u_debugreg[regnum]), value);
+  if (errno != 0)
+    perror_with_name (_("Couldn't write debug register"));
+}
+
+/* Return the inferior's debug register REGNUM.  */
+
+static CORE_ADDR
+x86_linux_dr_get_addr (int regnum)
+{
+  /* DR6 and DR7 are retrieved with some other way.  */
+  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
+
+  return x86_linux_dr_get (inferior_ptid, regnum);
+}
+
+/* Return the inferior's DR7 debug control register.  */
+
+static unsigned long
+x86_linux_dr_get_control (void)
+{
+  return x86_linux_dr_get (inferior_ptid, DR_CONTROL);
+}
+
+/* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
+
+static unsigned long
+x86_linux_dr_get_status (void)
+{
+  return x86_linux_dr_get (inferior_ptid, DR_STATUS);
+}
+
+/* Callback for iterate_over_lwps.  Update the debug registers of
+   LWP.  */
+
+static int
+update_debug_registers_callback (struct lwp_info *lwp, void *arg)
+{
+  if (lwp->arch_private == NULL)
+    lwp->arch_private = XCNEW (struct arch_lwp_info);
+
+  /* The actual update is done later just before resuming the lwp, we
+     just mark that the registers need updating.  */
+  lwp->arch_private->debug_registers_changed = 1;
+
+  /* If the lwp isn't stopped, force it to momentarily pause, so we
+     can update its debug registers.  */
+  if (!lwp->stopped)
+    linux_stop_lwp (lwp);
+
+  /* Continue the iteration.  */
+  return 0;
+}
+
+/* Set DR_CONTROL to CONTROL in all LWPs of the current inferior.  */
+
+static void
+x86_linux_dr_set_control (unsigned long control)
+{
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
+
+  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+}
+
+/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
+   inferior.  */
+
+static void
+x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
+{
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
+
+  gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
+
+  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+}
+
+/* Called when resuming a thread.
+   If the debug regs have changed, update the thread's copies.  */
+
+static void
+x86_linux_prepare_to_resume (struct lwp_info *lwp)
+{
+  int clear_status = 0;
+
+  /* NULL means this is the main thread still going through the shell,
+     or, no watchpoint has been set yet.  In that case, there's
+     nothing to do.  */
+  if (lwp->arch_private == NULL)
+    return;
+
+  if (lwp->arch_private->debug_registers_changed)
+    {
+      struct i386_debug_reg_state *state
+	= i386_debug_reg_state (ptid_get_pid (lwp->ptid));
+      int i;
+
+      /* On Linux kernel before 2.6.33 commit
+	 72f674d203cd230426437cdcf7dd6f681dad8b0d
+	 if you enable a breakpoint by the DR_CONTROL bits you need to have
+	 already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
+
+	 Ensure DR_CONTROL gets written as the very last register here.  */
+
+      /* Clear DR_CONTROL first.  In some cases, setting DR0-3 to a
+	 value that doesn't match what is enabled in DR_CONTROL
+	 results in EINVAL.  */
+      x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
+
+      for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
+	if (state->dr_ref_count[i] > 0)
+	  {
+	    x86_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
+
+	    /* If we're setting a watchpoint, any change the inferior
+	       had done itself to the debug registers needs to be
+	       discarded, otherwise, i386_stopped_data_address can get
+	       confused.  */
+	    clear_status = 1;
+	  }
+
+      /* If DR_CONTROL is supposed to be zero, we've already set it
+	 above.  */
+      if (state->dr_control_mirror != 0)
+	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
+
+      lwp->arch_private->debug_registers_changed = 0;
+    }
+
+  if (clear_status || lwp->stopped_by_watchpoint)
+    x86_linux_dr_set (lwp->ptid, DR_STATUS, 0);
+}
+
+static void
+x86_linux_new_thread (struct lwp_info *lp)
+{
+  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
+
+  info->debug_registers_changed = 1;
+
+  lp->arch_private = info;
+}
+\f
+
+/* linux_nat_new_fork hook.   */
+
+static void
+x86_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
+{
+  pid_t parent_pid;
+  struct i386_debug_reg_state *parent_state;
+  struct i386_debug_reg_state *child_state;
+
+  /* NULL means no watchpoint has ever been set in the parent.  In
+     that case, there's nothing to do.  */
+  if (parent->arch_private == NULL)
+    return;
+
+  /* Linux kernel before 2.6.33 commit
+     72f674d203cd230426437cdcf7dd6f681dad8b0d
+     will inherit hardware debug registers from parent
+     on fork/vfork/clone.  Newer Linux kernels create such tasks with
+     zeroed debug registers.
+
+     GDB core assumes the child inherits the watchpoints/hw
+     breakpoints of the parent, and will remove them all from the
+     forked off process.  Copy the debug registers mirrors into the
+     new process so that all breakpoints and watchpoints can be
+     removed together.  The debug registers mirror will become zeroed
+     in the end before detaching the forked off process, thus making
+     this compatible with older Linux kernels too.  */
+
+  parent_pid = ptid_get_pid (parent->ptid);
+  parent_state = i386_debug_reg_state (parent_pid);
+  child_state = i386_debug_reg_state (child_pid);
+  *child_state = *parent_state;
+}
+\f
+
+static void (*super_post_startup_inferior) (struct target_ops *self,
+					    ptid_t ptid);
+
+static void
+x86_linux_child_post_startup_inferior (struct target_ops *self, ptid_t ptid)
+{
+  i386_cleanup_dregs ();
+  super_post_startup_inferior (self, ptid);
+}
+
+#ifdef __x86_64__
+/* Value of CS segment register:
+     64bit process: 0x33
+     32bit process: 0x23  */
+#define AMD64_LINUX_USER64_CS 0x33
+
+/* Value of DS segment register:
+     LP64 process: 0x0
+     X32 process: 0x2b  */
+#define AMD64_LINUX_X32_DS 0x2b
+#endif
+
+/* Get Linux/x86 target description from running target.  */
+
+static const struct target_desc *
+x86_linux_read_description (struct target_ops *ops)
+{
+  int tid;
+  int is_64bit = 0;
+#ifdef __x86_64__
+  int is_x32;
+#endif
+  static uint64_t xcr0;
+  uint64_t xcr0_features_bits;
+
+  /* GNU/Linux LWP ID's are process ID's.  */
+  tid = ptid_get_lwp (inferior_ptid);
+  if (tid == 0)
+    tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
+
+#ifdef __x86_64__
+  {
+    unsigned long cs;
+    unsigned long ds;
+
+    /* Get CS register.  */
+    errno = 0;
+    cs = ptrace (PTRACE_PEEKUSER, tid,
+		 offsetof (struct user_regs_struct, cs), 0);
+    if (errno != 0)
+      perror_with_name (_("Couldn't get CS register"));
+
+    is_64bit = cs == AMD64_LINUX_USER64_CS;
+
+    /* Get DS register.  */
+    errno = 0;
+    ds = ptrace (PTRACE_PEEKUSER, tid,
+		 offsetof (struct user_regs_struct, ds), 0);
+    if (errno != 0)
+      perror_with_name (_("Couldn't get DS register"));
+
+    is_x32 = ds == AMD64_LINUX_X32_DS;
+
+    if (sizeof (void *) == 4 && is_64bit && !is_x32)
+      error (_("Can't debug 64-bit process with 32-bit GDB"));
+  }
+#elif HAVE_PTRACE_GETFPXREGS
+  if (have_ptrace_getfpxregs == -1)
+    {
+      elf_fpxregset_t fpxregs;
+
+      if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
+	{
+	  have_ptrace_getfpxregs = 0;
+	  have_ptrace_getregset = 0;
+	  return tdesc_i386_mmx_linux;
+	}
+    }
+#endif
+
+  if (have_ptrace_getregset == -1)
+    {
+      uint64_t xstateregs[(I386_XSTATE_SSE_SIZE / sizeof (uint64_t))];
+      struct iovec iov;
+
+      iov.iov_base = xstateregs;
+      iov.iov_len = sizeof (xstateregs);
+
+      /* Check if PTRACE_GETREGSET works.  */
+      if (ptrace (PTRACE_GETREGSET, tid,
+		  (unsigned int) NT_X86_XSTATE, &iov) < 0)
+	have_ptrace_getregset = 0;
+      else
+	{
+	  have_ptrace_getregset = 1;
+
+	  /* Get XCR0 from XSAVE extended state.  */
+	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
+			     / sizeof (uint64_t))];
+	}
+    }
+
+  /* Check the native XCR0 only if PTRACE_GETREGSET is available.  If
+     PTRACE_GETREGSET is not available then set xcr0_features_bits to
+     zero so that the "no-features" descriptions are returned by the
+     switches below.  */
+  if (have_ptrace_getregset)
+    xcr0_features_bits = xcr0 & I386_XSTATE_ALL_MASK;
+  else
+    xcr0_features_bits = 0;
+
+  if (is_64bit)
+    {
+#ifdef __x86_64__
+      switch (xcr0_features_bits)
+	{
+	case I386_XSTATE_MPX_AVX512_MASK:
+	case I386_XSTATE_AVX512_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx512_linux;
+	  else
+	    return tdesc_amd64_avx512_linux;
+	case I386_XSTATE_MPX_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx_linux; /* No MPX on x32 using AVX.  */
+	  else
+	    return tdesc_amd64_mpx_linux;
+	case I386_XSTATE_AVX_MASK:
+	  if (is_x32)
+	    return tdesc_x32_avx_linux;
+	  else
+	    return tdesc_amd64_avx_linux;
+	default:
+	  if (is_x32)
+	    return tdesc_x32_linux;
+	  else
+	    return tdesc_amd64_linux;
+	}
+#endif
+    }
+  else
+    {
+      switch (xcr0_features_bits)
+	{
+	case I386_XSTATE_MPX_AVX512_MASK:
+	case I386_XSTATE_AVX512_MASK:
+	  return tdesc_i386_avx512_linux;
+	case I386_XSTATE_MPX_MASK:
+	  return tdesc_i386_mpx_linux;
+	case I386_XSTATE_AVX_MASK:
+	  return tdesc_i386_avx_linux;
+	default:
+	  return tdesc_i386_linux;
+	}
+    }
+
+  gdb_assert_not_reached ("failed to return tdesc");
+}
+\f
+
+/* Enable branch tracing.  */
+
+static struct btrace_target_info *
+x86_linux_enable_btrace (struct target_ops *self, ptid_t ptid)
+{
+  struct btrace_target_info *tinfo;
+  struct gdbarch *gdbarch;
+
+  errno = 0;
+  tinfo = linux_enable_btrace (ptid);
+
+  if (tinfo == NULL)
+    error (_("Could not enable branch tracing for %s: %s."),
+	   target_pid_to_str (ptid), safe_strerror (errno));
+
+  /* Fill in the size of a pointer in bits.  */
+  gdbarch = target_thread_architecture (ptid);
+  tinfo->ptr_bits = gdbarch_ptr_bit (gdbarch);
+
+  return tinfo;
+}
+
+/* Disable branch tracing.  */
+
+static void
+x86_linux_disable_btrace (struct target_ops *self,
+			  struct btrace_target_info *tinfo)
+{
+  enum btrace_error errcode = linux_disable_btrace (tinfo);
+
+  if (errcode != BTRACE_ERR_NONE)
+    error (_("Could not disable branch tracing."));
+}
+
+/* Teardown branch tracing.  */
+
+static void
+x86_linux_teardown_btrace (struct target_ops *self,
+			   struct btrace_target_info *tinfo)
+{
+  /* Ignore errors.  */
+  linux_disable_btrace (tinfo);
+}
+
+static enum btrace_error
+x86_linux_read_btrace (struct target_ops *self,
+		       VEC (btrace_block_s) **data,
+		       struct btrace_target_info *btinfo,
+		       enum btrace_read_type type)
+{
+  return linux_read_btrace (data, btinfo, type);
+}
+\f
+
+/* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
+   the thread local storage (or its descriptor) and returns PS_OK
+   on success.  Returns PS_ERR on failure.  */
+
+ps_err_e
+x86_linux_get_thread_area (pid_t pid, void *addr, unsigned int *base_addr)
+{
+  /* NOTE: cagney/2003-08-26: The definition of this buffer is found
+     in the kernel header <asm-i386/ldt.h>.  It, after padding, is 4 x
+     4 byte integers in size: `entry_number', `base_addr', `limit',
+     and a bunch of status bits.
+
+     The values returned by this ptrace call should be part of the
+     regcache buffer, and ps_get_thread_area should channel its
+     request through the regcache.  That way remote targets could
+     provide the value using the remote protocol and not this direct
+     call.
+
+     Is this function needed?  I'm guessing that the `base' is the
+     address of a descriptor that libthread_db uses to find the
+     thread local address base that GDB needs.  Perhaps that
+     descriptor is defined by the ABI.  Anyway, given that
+     libthread_db calls this function without prompting (gdb
+     requesting tls base) I guess it needs info in there anyway.  */
+  unsigned int desc[4];
+
+  /* This code assumes that "int" is 32 bits and that
+     GET_THREAD_AREA returns no more than 4 int values.  */
+  gdb_assert (sizeof (int) == 4);
+
+#ifndef PTRACE_GET_THREAD_AREA
+#define PTRACE_GET_THREAD_AREA 25
+#endif
+
+  if (ptrace (PTRACE_GET_THREAD_AREA, pid, addr, &desc) < 0)
+    return PS_ERR;
+
+  *base_addr = desc[1];
+  return PS_OK;
+}
+\f
+
+/* Create an x86 GNU/Linux target.  */
+
+struct target_ops *
+x86_linux_create_target (void)
+{
+  /* Fill in the generic GNU/Linux methods.  */
+  struct target_ops *t = linux_target ();
+
+  /* Initialize the debug register function vectors.  */
+  i386_use_watchpoints (t);
+  i386_dr_low.set_control = x86_linux_dr_set_control;
+  i386_dr_low.set_addr = x86_linux_dr_set_addr;
+  i386_dr_low.get_addr = x86_linux_dr_get_addr;
+  i386_dr_low.get_status = x86_linux_dr_get_status;
+  i386_dr_low.get_control = x86_linux_dr_get_control;
+  i386_set_debug_register_length (sizeof (void *));
+
+  /* Override the GNU/Linux inferior startup hook.  */
+  super_post_startup_inferior = t->to_post_startup_inferior;
+  t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
+
+  /* Add the description reader.  */
+  t->to_read_description = x86_linux_read_description;
+
+  /* Add btrace methods.  */
+  t->to_supports_btrace = linux_supports_btrace;
+  t->to_enable_btrace = x86_linux_enable_btrace;
+  t->to_disable_btrace = x86_linux_disable_btrace;
+  t->to_teardown_btrace = x86_linux_teardown_btrace;
+  t->to_read_btrace = x86_linux_read_btrace;
+
+  return t;
+}
+
+/* Register an x86 GNU/Linux target.  */
+
+void
+x86_linux_register_target (struct target_ops *t)
+{
+  linux_nat_add_target (t);
+  linux_nat_set_new_thread (t, x86_linux_new_thread);
+  linux_nat_set_new_fork (t, x86_linux_new_fork);
+  linux_nat_set_forget_process (t, i386_forget_process);
+  linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
+}
diff --git a/gdb/x86-linux-nat.h b/gdb/x86-linux-nat.h
new file mode 100644
index 0000000..8b92ca9
--- /dev/null
+++ b/gdb/x86-linux-nat.h
@@ -0,0 +1,51 @@
+/* Native-dependent code for GNU/Linux x86 (i386 and x86-64).
+
+   Copyright (C) 1999-2014 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/>.  */
+
+#ifndef X86_LINUX_NAT_H
+#define X86_LINUX_NAT_H 1
+
+#ifndef PTRACE_GETREGSET
+#define PTRACE_GETREGSET	0x4204
+#endif
+
+#ifndef PTRACE_SETREGSET
+#define PTRACE_SETREGSET	0x4205
+#endif
+
+/* Does the current host support PTRACE_GETREGSET?  */
+extern int have_ptrace_getregset;
+\f
+
+/* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
+   the thread local storage (or its descriptor) and returns PS_OK
+   on success.  Returns PS_ERR on failure.  */
+
+extern ps_err_e x86_linux_get_thread_area (pid_t pid, void *addr,
+					   unsigned int *base_addr);
+\f
+
+/* Create an x86 GNU/Linux target.  */
+
+extern struct target_ops *x86_linux_create_target (void);
+
+/* Register an x86 GNU/Linux target.  */
+
+extern void x86_linux_register_target (struct target_ops *t);
+
+#endif
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 5/7 v2] Comment and whitespace changes
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
                   ` (3 preceding siblings ...)
  2014-06-27 14:28 ` [PATCH 6/7 v2] Move duplicated code into new files Gary Benson
@ 2014-06-27 14:40 ` Gary Benson
  2014-07-09 13:11   ` Pedro Alves
  2014-06-27 14:52 ` [PATCH 7/7 v2] Tidy #include lists Gary Benson
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

This commit merges the comments and whitespace in the common
parts of i386-linux-nat.c and amd64-linux-nat.c.

This patch is unchanged from the original version in this series.

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* amd64-linux-nat.c: Comment and whitespace changes.
	* i386-linux-nat.c: Comment and whitespace changes.
---
 gdb/ChangeLog         |    5 +++++
 gdb/amd64-linux-nat.c |    3 +++
 gdb/i386-linux-nat.c  |   15 ++++++++++++---
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 2a291df..f29f8c7 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -277,8 +277,11 @@ amd64_linux_store_inferior_registers (struct target_ops *ops,
     }
 }
 \f
+
 /* Support for debug registers.  */
 
+/* Get debug register REGNUM value from only the one LWP of PTID.  */
+
 static unsigned long
 x86_linux_dr_get (ptid_t ptid, int regnum)
 {
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 5ef69ff..51b3338 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -732,7 +732,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
   return 0;
 }
 
-/* Set DR_CONTROL to ADDR in all LWPs of the current inferior.  */
+/* Set DR_CONTROL to CONTROL in all LWPs of the current inferior.  */
 
 static void
 x86_linux_dr_set_control (unsigned long control)
@@ -775,9 +775,16 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 	= i386_debug_reg_state (ptid_get_pid (lwp->ptid));
       int i;
 
-      /* See amd64_linux_prepare_to_resume for Linux kernel note on
-	 i386_linux_dr_set calls ordering.  */
+      /* On Linux kernel before 2.6.33 commit
+	 72f674d203cd230426437cdcf7dd6f681dad8b0d
+	 if you enable a breakpoint by the DR_CONTROL bits you need to have
+	 already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
 
+	 Ensure DR_CONTROL gets written as the very last register here.  */
+
+      /* Clear DR_CONTROL first.  In some cases, setting DR0-3 to a
+	 value that doesn't match what is enabled in DR_CONTROL
+	 results in EINVAL.  */
       x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
 
       for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
@@ -792,6 +799,8 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 	    clear_status = 1;
 	  }
 
+      /* If DR_CONTROL is supposed to be zero, we've already set it
+	 above.  */
       if (state->dr_control_mirror != 0)
 	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
 
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 7/7 v2] Tidy #include lists
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
                   ` (4 preceding siblings ...)
  2014-06-27 14:40 ` [PATCH 5/7 v2] Comment and whitespace changes Gary Benson
@ 2014-06-27 14:52 ` Gary Benson
  2014-07-09 13:13   ` Pedro Alves
                     ` (2 more replies)
  2014-06-27 14:53 ` [PATCH 4/7 v2] Pull out common parts of _initialize_{i386,amd64}_linux_nat Gary Benson
  2014-07-09 13:17 ` [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Pedro Alves
  7 siblings, 3 replies; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:52 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

This commit tidies up the #include lists in {i386,amd64}-linux-nat.c,
removing headers that are no longer required and reordering some lines
so that both files roughly match.  Additionally, an unused definition
was removed from the middle of the #include list in i386-linux-nat.c.

This patch is unchanged from the original version in this series.

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* amd64-linux-nat.c (gdbcore.h): Removed include.
	(regset.h): Likewise.
	(nat/linux-btrace.h): Likewise.
	(btrace.h): Likewise.
	(gdb_assert.h): Likewise.
	(string.h): Likewise.
	(sys/uio.h): Likewise.
	(sys/debugreg.h): Likewise.
	(sys/syscall.h): Likewise.
	(sys/procfs.h): Likewise.
	(sys/user.h): Likewise.
	(asm/ptrace.h): Likewise.
	(i386-nat.h): Likewise.
	* i386-linux-nat.c (i386-nat.h): Likewise.
	(regset.h): Likewise.
	(target.h): Likewise.
	(linux-nat.h): Likewise.
	(nat/linux-btrace.h): Likewise.
	(btrace.h): Likewise.
	(gdb_assert.h): Likewise.
	(string.h): Likewise.
	(sys/uio.h): Likewise.
	(sys/user.h): Likewise.
	(sys/procfs.h): Likewise.
	(sys/reg.h): Likewise.
	(sys/debugreg.h): Likewise.
	(ORIG_EAX): Removed definition.
---
 gdb/ChangeLog         |   30 ++++++++++++++++++++++++++++++
 gdb/amd64-linux-nat.c |   32 +++++---------------------------
 gdb/i386-linux-nat.c  |   33 +++------------------------------
 3 files changed, 38 insertions(+), 57 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index bd03ea3..5ac09c6 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -20,42 +20,20 @@
 
 #include "defs.h"
 #include "inferior.h"
-#include "gdbcore.h"
 #include "regcache.h"
-#include "regset.h"
-#include "linux-nat.h"
-#include "amd64-linux-tdep.h"
-#include "nat/linux-btrace.h"
-#include "btrace.h"
-
-#include "gdb_assert.h"
-#include <string.h>
 #include "elf/common.h"
-#include <sys/uio.h>
 #include <sys/ptrace.h>
-#include <sys/debugreg.h>
-#include <sys/syscall.h>
-#include <sys/procfs.h>
-#include <sys/user.h>
 #include <asm/prctl.h>
-/* FIXME ezannoni-2003-07-09: we need <sys/reg.h> to be included after
-   <asm/ptrace.h> because the latter redefines FS and GS for no apparent
-   reason, and those definitions don't match the ones that libpthread_db
-   uses, which come from <sys/reg.h>.  */
-/* ezannoni-2003-07-09: I think this is fixed.  The extraneous defs have
-   been removed from ptrace.h in the kernel.  However, better safe than
-   sorry.  */
-#include <asm/ptrace.h>
 #include <sys/reg.h>
-#include "gdb_proc_service.h"
-
-/* Prototypes for supply_gregset etc.  */
 #include "gregset.h"
+#include "gdb_proc_service.h"
 
+#include "amd64-nat.h"
+#include "linux-nat.h"
+#include "x86-linux-nat.h"
 #include "amd64-tdep.h"
+#include "amd64-linux-tdep.h"
 #include "i386-linux-tdep.h"
-#include "amd64-nat.h"
-#include "i386-nat.h"
 #include "i386-xstate.h"
 
 #include "x86-linux-nat.h"
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 3778c77..91c91a6 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -18,46 +18,19 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#include "i386-nat.h"
 #include "inferior.h"
 #include "gdbcore.h"
 #include "regcache.h"
-#include "regset.h"
-#include "target.h"
-#include "linux-nat.h"
-#include "nat/linux-btrace.h"
-#include "btrace.h"
-
-#include "gdb_assert.h"
-#include <string.h>
 #include "elf/common.h"
-#include <sys/uio.h>
 #include <sys/ptrace.h>
-#include <sys/user.h>
-#include <sys/procfs.h>
-
-#ifdef HAVE_SYS_REG_H
-#include <sys/reg.h>
-#endif
-
-#ifndef ORIG_EAX
-#define ORIG_EAX -1
-#endif
-
-#ifdef HAVE_SYS_DEBUGREG_H
-#include <sys/debugreg.h>
-#endif
-
-/* Prototypes for supply_gregset etc.  */
 #include "gregset.h"
+#include "gdb_proc_service.h"
 
+#include "x86-linux-nat.h"
+#include "i386-linux-nat.h"
 #include "i387-tdep.h"
 #include "i386-tdep.h"
 #include "i386-linux-tdep.h"
-
-/* Defines ps_err_e, struct ps_prochandle.  */
-#include "gdb_proc_service.h"
-
 #include "i386-xstate.h"
 
 #include "x86-linux-nat.h"
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 4/7 v2] Pull out common parts of _initialize_{i386,amd64}_linux_nat
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
                   ` (5 preceding siblings ...)
  2014-06-27 14:52 ` [PATCH 7/7 v2] Tidy #include lists Gary Benson
@ 2014-06-27 14:53 ` Gary Benson
  2014-07-09 13:09   ` Pedro Alves
  2014-07-09 13:17 ` [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Pedro Alves
  7 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-06-27 14:53 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mark Kettenis, Pedro Alves

This commit adds two new helpers, x86_linux_create_target
and x86_linux_register_target, to hold the parts of
_initialize_i386_linux_nat and _initialize_amd64_linux_nat
which are common.

This patch is unchanged from the original version in this series.

gdb/
2014-06-27  Gary Benson  <gbenson@redhat.com>

	* amd64-linux-nat.c (x86_linux_create_target): New function.
	(x86_linux_register_target): Likewise.
	(_initialize_amd64_linux_nat): Delegate to the above new functions.
	* i386-linux-nat.c (x86_linux_create_target): New function.
	(x86_linux_register_target): Likewise.
	(_initialize_i386_linux_nat): Delegate to the above new functions.
---
 gdb/ChangeLog         |    9 ++++++
 gdb/amd64-linux-nat.c |   68 ++++++++++++++++++++++++++++++++----------------
 gdb/i386-linux-nat.c  |   52 +++++++++++++++++++++++++------------
 3 files changed, 89 insertions(+), 40 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 57e5c51..2a291df 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -1269,42 +1269,28 @@ x86_linux_read_btrace (struct target_ops *self,
   return linux_read_btrace (data, btinfo, type);
 }
 
-/* Provide a prototype to silence -Wmissing-prototypes.  */
-void _initialize_amd64_linux_nat (void);
+/* Create an x86 GNU/Linux target.  */
 
-void
-_initialize_amd64_linux_nat (void)
+static struct target_ops *
+x86_linux_create_target (void)
 {
-  struct target_ops *t;
-
-  amd64_native_gregset32_reg_offset = amd64_linux_gregset32_reg_offset;
-  amd64_native_gregset32_num_regs = I386_LINUX_NUM_REGS;
-  amd64_native_gregset64_reg_offset = amd64_linux_gregset_reg_offset;
-  amd64_native_gregset64_num_regs = AMD64_LINUX_NUM_REGS;
-
-  gdb_assert (ARRAY_SIZE (amd64_linux_gregset32_reg_offset)
-	      == amd64_native_gregset32_num_regs);
-
   /* Fill in the generic GNU/Linux methods.  */
-  t = linux_target ();
+  struct target_ops *t = linux_target ();
 
+  /* Initialize the debug register function vectors.  */
   i386_use_watchpoints (t);
-
   i386_dr_low.set_control = x86_linux_dr_set_control;
   i386_dr_low.set_addr = x86_linux_dr_set_addr;
   i386_dr_low.get_addr = x86_linux_dr_get_addr;
   i386_dr_low.get_status = x86_linux_dr_get_status;
   i386_dr_low.get_control = x86_linux_dr_get_control;
-  i386_set_debug_register_length (8);
+  i386_set_debug_register_length (sizeof (void *));
 
   /* Override the GNU/Linux inferior startup hook.  */
   super_post_startup_inferior = t->to_post_startup_inferior;
   t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
 
-  /* Add our register access methods.  */
-  t->to_fetch_registers = amd64_linux_fetch_inferior_registers;
-  t->to_store_registers = amd64_linux_store_inferior_registers;
-
+  /* Add the description reader.  */
   t->to_read_description = x86_linux_read_description;
 
   /* Add btrace methods.  */
@@ -1314,11 +1300,47 @@ _initialize_amd64_linux_nat (void)
   t->to_teardown_btrace = x86_linux_teardown_btrace;
   t->to_read_btrace = x86_linux_read_btrace;
 
-  /* Register the target.  */
+  return t;
+}
+
+/* Register an x86 GNU/Linux target.  */
+
+static void
+x86_linux_register_target (struct target_ops *t)
+{
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, x86_linux_new_thread);
   linux_nat_set_new_fork (t, x86_linux_new_fork);
   linux_nat_set_forget_process (t, i386_forget_process);
-  linux_nat_set_siginfo_fixup (t, amd64_linux_siginfo_fixup);
   linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
 }
+
+/* Provide a prototype to silence -Wmissing-prototypes.  */
+void _initialize_amd64_linux_nat (void);
+
+void
+_initialize_amd64_linux_nat (void)
+{
+  struct target_ops *t;
+
+  amd64_native_gregset32_reg_offset = amd64_linux_gregset32_reg_offset;
+  amd64_native_gregset32_num_regs = I386_LINUX_NUM_REGS;
+  amd64_native_gregset64_reg_offset = amd64_linux_gregset_reg_offset;
+  amd64_native_gregset64_num_regs = AMD64_LINUX_NUM_REGS;
+
+  gdb_assert (ARRAY_SIZE (amd64_linux_gregset32_reg_offset)
+	      == amd64_native_gregset32_num_regs);
+
+  /* Create a generic x86 GNU/Linux target.  */
+  t = x86_linux_create_target ();
+
+  /* Add our register access methods.  */
+  t->to_fetch_registers = amd64_linux_fetch_inferior_registers;
+  t->to_store_registers = amd64_linux_store_inferior_registers;
+
+  /* Register the target.  */
+  x86_linux_register_target (t);
+
+  /* Add our siginfo layout converter.  */
+  linux_nat_set_siginfo_fixup (t, amd64_linux_siginfo_fixup);
+}
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index d647c3d..5ef69ff 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -1217,37 +1217,28 @@ x86_linux_read_btrace (struct target_ops *self,
   return linux_read_btrace (data, btinfo, type);
 }
 
-/* -Wmissing-prototypes */
-extern initialize_file_ftype _initialize_i386_linux_nat;
+/* Create an x86 GNU/Linux target.  */
 
-void
-_initialize_i386_linux_nat (void)
+static struct target_ops *
+x86_linux_create_target (void)
 {
-  struct target_ops *t;
-
   /* Fill in the generic GNU/Linux methods.  */
-  t = linux_target ();
+  struct target_ops *t = linux_target ();
 
+  /* Initialize the debug register function vectors.  */
   i386_use_watchpoints (t);
-
   i386_dr_low.set_control = x86_linux_dr_set_control;
   i386_dr_low.set_addr = x86_linux_dr_set_addr;
   i386_dr_low.get_addr = x86_linux_dr_get_addr;
   i386_dr_low.get_status = x86_linux_dr_get_status;
   i386_dr_low.get_control = x86_linux_dr_get_control;
-  i386_set_debug_register_length (4);
-
-  /* Override the default ptrace resume method.  */
-  t->to_resume = i386_linux_resume;
+  i386_set_debug_register_length (sizeof (void *));
 
   /* Override the GNU/Linux inferior startup hook.  */
   super_post_startup_inferior = t->to_post_startup_inferior;
   t->to_post_startup_inferior = x86_linux_child_post_startup_inferior;
 
-  /* Add our register access methods.  */
-  t->to_fetch_registers = i386_linux_fetch_inferior_registers;
-  t->to_store_registers = i386_linux_store_inferior_registers;
-
+  /* Add the description reader.  */
   t->to_read_description = x86_linux_read_description;
 
   /* Add btrace methods.  */
@@ -1257,10 +1248,37 @@ _initialize_i386_linux_nat (void)
   t->to_teardown_btrace = x86_linux_teardown_btrace;
   t->to_read_btrace = x86_linux_read_btrace;
 
-  /* Register the target.  */
+  return t;
+}
+
+/* Register an x86 GNU/Linux target.  */
+
+static void
+x86_linux_register_target (struct target_ops *t)
+{
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, x86_linux_new_thread);
   linux_nat_set_new_fork (t, x86_linux_new_fork);
   linux_nat_set_forget_process (t, i386_forget_process);
   linux_nat_set_prepare_to_resume (t, x86_linux_prepare_to_resume);
 }
+
+/* -Wmissing-prototypes */
+extern initialize_file_ftype _initialize_i386_linux_nat;
+
+void
+_initialize_i386_linux_nat (void)
+{
+  /* Create a generic x86 GNU/Linux target.  */
+  struct target_ops *t = x86_linux_create_target ();
+
+  /* Override the default ptrace resume method.  */
+  t->to_resume = i386_linux_resume;
+
+  /* Add our register access methods.  */
+  t->to_fetch_registers = i386_linux_fetch_inferior_registers;
+  t->to_store_registers = i386_linux_store_inferior_registers;
+
+  /* Register the target.  */
+  x86_linux_register_target (t);
+}
-- 
1.7.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/7 v2] Rename identical functions
  2014-06-27 14:12 ` [PATCH 1/7 v2] Rename identical functions Gary Benson
@ 2014-07-09 13:04   ` Pedro Alves
  0 siblings, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:04 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis, Pedro Alves

On 06/27/2014 03:12 PM, Gary Benson wrote:
> amd64-linux-nat.c and i386-linux-nat.c contain a number of functions
> which are identical but for prefix on their names.  This commit
> renames all such functions to have the prefix x86_ instead of the
> prefixes amd64_ or i386_ and updates all uses of those functions.
> 
> This patch is unchanged from the original version in this series.

Looks good to me.  It's worth mentioning that the x86_ functions
will be pulled out to a separate and shared x86-* file in a later
patch in the series.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description
  2014-06-27 14:12 ` [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description Gary Benson
@ 2014-07-09 13:07   ` Pedro Alves
  0 siblings, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:07 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis, Pedro Alves

On 06/27/2014 03:12 PM, Gary Benson wrote:
> This commit merges i386_ and amd64_linux_read_description, renaming
> both to x86_linux_read_description.
> 
> This patch differs from the original version in this series
> in that x86_linux_read_description is much cleaner, having
> been rewritten to avoid "#ifdef spaghetti".
> 
> gdb/
> 2014-06-27  Gary Benson  <gbenson@redhat.com>
> 
> 	* amd64-linux-nat.c (amd64_linux_read_description): Renamed to
> 	x86_linux_read_description.  All uses updated.  amd64-specific
> 	code conditionalized.  Conditionalized i386-specific code added.
> 	Redundant cast removed.
> 	* i386-linux-nat.c (i386_linux_read_description): Renamed to
> 	x86_linux_read_description.  All uses updated.  i386-specific
> 	code conditionalized.  Conditionalized amd64-specific code added.
> 	One sizeof replaced with the actual type it is describing.

I compared the old vs new files side by side to try to check that
the merged code behaved the same the as before.  This version does
look much cleaner.

Looks good to me, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/7 v2] Merge ps_get_thread_area
  2014-06-27 14:12 ` [PATCH 3/7 v2] Merge ps_get_thread_area Gary Benson
  2014-07-09 13:08   ` Pedro Alves
@ 2014-07-09 13:08   ` Pedro Alves
  1 sibling, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:08 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

On 06/27/2014 03:12 PM, Gary Benson wrote:
> This commit adds a new helper, x86_linux_get_thread_area, to
> hold the common parts of the ps_get_thread_area functions in
> i386-linux-nat.c and amd64-linux-nat.c.
> 
> This patch is unchanged from the original version in this series.
> 
> gdb/
> 2014-06-27  Gary Benson  <gbenson@redhat.com>
> 
> 	* amd64-linux-nat.c (x86_linux_get_thread_area): New function.
> 	(ps_get_thread_area): Delegate to the above in 32-bit mode.
> 	* i386-linux-nat.c (x86_linux_get_thread_area): New function.
> 	(ps_get_thread_area): Delegate to the above.

Looks good to me, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/7 v2] Merge ps_get_thread_area
  2014-06-27 14:12 ` [PATCH 3/7 v2] Merge ps_get_thread_area Gary Benson
@ 2014-07-09 13:08   ` Pedro Alves
  2014-07-09 13:08   ` Pedro Alves
  1 sibling, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:08 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

On 06/27/2014 03:12 PM, Gary Benson wrote:
> This commit adds a new helper, x86_linux_get_thread_area, to
> hold the common parts of the ps_get_thread_area functions in
> i386-linux-nat.c and amd64-linux-nat.c.
> 
> This patch is unchanged from the original version in this series.
> 
> gdb/
> 2014-06-27  Gary Benson  <gbenson@redhat.com>
> 
> 	* amd64-linux-nat.c (x86_linux_get_thread_area): New function.
> 	(ps_get_thread_area): Delegate to the above in 32-bit mode.
> 	* i386-linux-nat.c (x86_linux_get_thread_area): New function.
> 	(ps_get_thread_area): Delegate to the above.

Looks good to me, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/7 v2] Pull out common parts of _initialize_{i386,amd64}_linux_nat
  2014-06-27 14:53 ` [PATCH 4/7 v2] Pull out common parts of _initialize_{i386,amd64}_linux_nat Gary Benson
@ 2014-07-09 13:09   ` Pedro Alves
  0 siblings, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:09 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

I think I'd prefer s/x86_linux_register_target/x86_linux_add_target,
as we use "add" everywhere else related, like in add_target/linux_nat_add_target.

Otherwise looks good to me.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/7 v2] Comment and whitespace changes
  2014-06-27 14:40 ` [PATCH 5/7 v2] Comment and whitespace changes Gary Benson
@ 2014-07-09 13:11   ` Pedro Alves
  0 siblings, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:11 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

On 06/27/2014 03:12 PM, Gary Benson wrote:
> This commit merges the comments and whitespace in the common
> parts of i386-linux-nat.c and amd64-linux-nat.c.
> 
> This patch is unchanged from the original version in this series.
> 
> gdb/
> 2014-06-27  Gary Benson  <gbenson@redhat.com>
> 
> 	* amd64-linux-nat.c: Comment and whitespace changes.
> 	* i386-linux-nat.c: Comment and whitespace changes.

Looks good to me, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 6/7 v2] Move duplicated code into new files
  2014-06-27 14:28 ` [PATCH 6/7 v2] Move duplicated code into new files Gary Benson
@ 2014-07-09 13:12   ` Pedro Alves
  0 siblings, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:12 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

On 06/27/2014 03:12 PM, Gary Benson wrote:
> This commit moves the shared code in {i386,amd64}-linux-nat.c

Nit: s/shared/identical/.  It's only shared afterwards.

> into the new files x86-linux-nat.[ch].  Additionally, a new
> file i386-linux-nat.h was required to expose a value required
> by the 32-bit code in x86-linux-nat.c.
> 
> This patch differs from the original version in this series
> in that x86_linux_read_description has been rewritten in an
> earlier patch.

> 	(x86_linux_dr_get_addr): Likewise.
> 	(x86_linux_dr_get_control) Likewise.:

Typo, ':' position.

> 	(x86_linux_dr_get_status): Likewise.

Otherwise looks good to me.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 7/7 v2] Tidy #include lists
  2014-06-27 14:52 ` [PATCH 7/7 v2] Tidy #include lists Gary Benson
@ 2014-07-09 13:13   ` Pedro Alves
  2014-08-06  9:36   ` Yao Qi
  2014-09-09 15:09   ` [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists") Joel Brobecker
  2 siblings, 0 replies; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:13 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

On 06/27/2014 03:12 PM, Gary Benson wrote:
> This commit tidies up the #include lists in {i386,amd64}-linux-nat.c,
> removing headers that are no longer required and reordering some lines
> so that both files roughly match.  Additionally, an unused definition
> was removed from the middle of the #include list in i386-linux-nat.c.
> 
> This patch is unchanged from the original version in this series.
> 
> gdb/
> 2014-06-27  Gary Benson  <gbenson@redhat.com>
> 
> 	* amd64-linux-nat.c (gdbcore.h): Removed include.
...
> 	(ORIG_EAX): Removed definition.
...

Nit, it's more standard to use the imperative, "Remove".

Looks good to me.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c
  2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
                   ` (6 preceding siblings ...)
  2014-06-27 14:53 ` [PATCH 4/7 v2] Pull out common parts of _initialize_{i386,amd64}_linux_nat Gary Benson
@ 2014-07-09 13:17 ` Pedro Alves
  2014-07-11 12:34   ` Gary Benson
  7 siblings, 1 reply; 24+ messages in thread
From: Pedro Alves @ 2014-07-09 13:17 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis

On 06/27/2014 03:12 PM, Gary Benson wrote:
> Hi all,
> 
> This series is an updated version of the series I posted this morning.
> Mark Kettenis rightly pointed out that x86_linux_read_description was
> a mess of #ifdef spaghetti so I have rewritten it.
> 
> Patch 2 has changed because that's where the x86_linux_read_description
> merge is.  Patch 6 has changed because that's where the code is pulled
> from {i386,amd64}-linux-nat.c into x86-linux-nat.c.  The remaining
> patches are the same.
> 
> I've inlined the original description of the series below.
> 
> Is this ok to commit?

I read this, and apart from a couple nits, it all looked good to me.

I think we can take Mark's silence as meaning his objections have
been addressed, but in case he just didn't have time to reply, let's
wait till Friday, and then push this in.

Thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c
  2014-07-09 13:17 ` [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Pedro Alves
@ 2014-07-11 12:34   ` Gary Benson
  0 siblings, 0 replies; 24+ messages in thread
From: Gary Benson @ 2014-07-11 12:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Mark Kettenis

Pedro Alves wrote:
> On 06/27/2014 03:12 PM, Gary Benson wrote:
> > This series is an updated version of the series I posted this
> > morning.  Mark Kettenis rightly pointed out that
> > x86_linux_read_description was a mess of #ifdef spaghetti so
> > I have rewritten it.
> > 
> > Patch 2 has changed because that's where the
> > x86_linux_read_description merge is.  Patch 6 has changed because
> > that's where the code is pulled from {i386,amd64}-linux-nat.c into
> > x86-linux-nat.c.  The remaining patches are the same.
> > 
> > I've inlined the original description of the series below.
> > 
> > Is this ok to commit?
> 
> I read this, and apart from a couple nits, it all looked good to me.
> 
> I think we can take Mark's silence as meaning his objections have
> been addressed, but in case he just didn't have time to reply, let's
> wait till Friday, and then push this in.

Pushed with the changes you suggested.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 7/7 v2] Tidy #include lists
  2014-06-27 14:52 ` [PATCH 7/7 v2] Tidy #include lists Gary Benson
  2014-07-09 13:13   ` Pedro Alves
@ 2014-08-06  9:36   ` Yao Qi
  2014-08-06 10:19     ` Gary Benson
  2014-09-09 15:09   ` [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists") Joel Brobecker
  2 siblings, 1 reply; 24+ messages in thread
From: Yao Qi @ 2014-08-06  9:36 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Mark Kettenis, Pedro Alves

On 06/27/2014 10:12 PM, Gary Benson wrote:
> +#include "x86-linux-nat.h"

x86-linux-nat.h is included ....

>  #include "amd64-tdep.h"
> +#include "amd64-linux-tdep.h"
>  #include "i386-linux-tdep.h"
> -#include "amd64-nat.h"
> -#include "i386-nat.h"
>  #include "i386-xstate.h"
>  
>  #include "x86-linux-nat.h"

... here.  We can remove one of the two includes.

>  
> +#include "x86-linux-nat.h"
> +#include "i386-linux-nat.h"
>  #include "i387-tdep.h"
>  #include "i386-tdep.h"
>  #include "i386-linux-tdep.h"
> -
> -/* Defines ps_err_e, struct ps_prochandle.  */
> -#include "gdb_proc_service.h"
> -
>  #include "i386-xstate.h"
>  
>  #include "x86-linux-nat.h"

Likewise.  How about the patch below?

-- 
Yao (齐尧)

Subject: [PATCH] Remove duplicated include file

File x86-linux-nat.h is included twice in amd64-linux-nat.c and
i386-linux-nat.c.  This patch is to remove one.

gdb:

2014-08-06  Yao Qi  <yao@codesourcery.com>

	* amd64-linux-nat.c: Remove duplicated include
	"x86-linux-nat.h".
	* i386-linux-nat.c: Likewise.
---
 gdb/amd64-linux-nat.c | 1 -
 gdb/i386-linux-nat.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 0885a0a..def12ee 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -30,7 +30,6 @@

 #include "amd64-nat.h"
 #include "linux-nat.h"
-#include "x86-linux-nat.h"
 #include "amd64-tdep.h"
 #include "amd64-linux-tdep.h"
 #include "i386-linux-tdep.h"
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index bb0f2c8..8227d4a 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -26,7 +26,6 @@
 #include "gregset.h"
 #include "gdb_proc_service.h"

-#include "x86-linux-nat.h"
 #include "i386-linux-nat.h"
 #include "i387-tdep.h"
 #include "i386-tdep.h"
-- 
1.9.0

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 7/7 v2] Tidy #include lists
  2014-08-06  9:36   ` Yao Qi
@ 2014-08-06 10:19     ` Gary Benson
  2014-08-06 11:16       ` Yao Qi
  0 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-08-06 10:19 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Mark Kettenis, Pedro Alves

Yao Qi wrote:
> File x86-linux-nat.h is included twice in amd64-linux-nat.c and
> i386-linux-nat.c.  This patch is to remove one.

Thanks Yao.  Please commit this as obvious.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 7/7 v2] Tidy #include lists
  2014-08-06 10:19     ` Gary Benson
@ 2014-08-06 11:16       ` Yao Qi
  0 siblings, 0 replies; 24+ messages in thread
From: Yao Qi @ 2014-08-06 11:16 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches, Mark Kettenis, Pedro Alves

On 08/06/2014 06:19 PM, Gary Benson wrote:
> Thanks Yao.  Please commit this as obvious.

Sure, patch is pushed in.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists")
  2014-06-27 14:52 ` [PATCH 7/7 v2] Tidy #include lists Gary Benson
  2014-07-09 13:13   ` Pedro Alves
  2014-08-06  9:36   ` Yao Qi
@ 2014-09-09 15:09   ` Joel Brobecker
  2014-09-10 10:59     ` Gary Benson
  2 siblings, 1 reply; 24+ messages in thread
From: Joel Brobecker @ 2014-09-09 15:09 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches, Mark Kettenis, Pedro Alves

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

> This commit tidies up the #include lists in {i386,amd64}-linux-nat.c,
> removing headers that are no longer required and reordering some lines
> so that both files roughly match.  Additionally, an unused definition
> was removed from the middle of the #include list in i386-linux-nat.c.
> 
> This patch is unchanged from the original version in this series.
> 
> gdb/
> 2014-06-27  Gary Benson  <gbenson@redhat.com>
> 
> 	* amd64-linux-nat.c (gdbcore.h): Removed include.
> 	(regset.h): Likewise.
> 	(nat/linux-btrace.h): Likewise.
> 	(btrace.h): Likewise.
> 	(gdb_assert.h): Likewise.
> 	(string.h): Likewise.
> 	(sys/uio.h): Likewise.
> 	(sys/debugreg.h): Likewise.
> 	(sys/syscall.h): Likewise.
> 	(sys/procfs.h): Likewise.
> 	(sys/user.h): Likewise.
> 	(asm/ptrace.h): Likewise.
> 	(i386-nat.h): Likewise.
> 	* i386-linux-nat.c (i386-nat.h): Likewise.
> 	(regset.h): Likewise.
> 	(target.h): Likewise.
> 	(linux-nat.h): Likewise.
> 	(nat/linux-btrace.h): Likewise.
> 	(btrace.h): Likewise.
> 	(gdb_assert.h): Likewise.
> 	(string.h): Likewise.
> 	(sys/uio.h): Likewise.
> 	(sys/user.h): Likewise.
> 	(sys/procfs.h): Likewise.
> 	(sys/reg.h): Likewise.
> 	(sys/debugreg.h): Likewise.
> 	(ORIG_EAX): Removed definition.

Unfortunately, this patch broke GDB builds on some GNU/Linux
distros. I fixed the issue I got by applying the attached patch.

-- 
Joel

[-- Attachment #2: 0001-Fix-missing-struct-iovec-definition-on-some-x86-linu.patch --]
[-- Type: text/x-diff, Size: 2422 bytes --]

From 72fde3dfe9a2367abc593684b9b4d2343d367d85 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 9 Sep 2014 16:49:13 +0200
Subject: [PATCH] Fix missing "struct iovec" definition on some x86-linux.

The following patch...

    commit 3116063bd617de56fbc3bad046a692b1fb363a9d
    Date:   Fri Jun 27 09:52:29 2014 +0100
    Subject: Tidy #include lists

... introduced a build failure on certain x86 GNU/Linux distributions
(reproduced on SuSE 10 and RHES4) due to "struct iovec" not being
defined. This struct is defined in <sys/uio.h>, which used to be
explicitly included, but no longer is after the commit above was
applied.

    [...]/i386-linux-nat.c: In function 'fetch_xstateregs':
    [...]/i386-linux-nat.c:325:16: error: storage size of 'iov' isn't known
    [...]/i386-linux-nat.c: In function 'store_xstateregs':
    [...]/i386-linux-nat.c:348:16: error: storage size of 'iov' isn't known
    make[2]: *** [i386-linux-nat.o] Error 1

It seems to be working on newer GNU/Linux distros thanks to indirect
inclusion of <sys/uio.h>, but it does not work on some other versions
of the same distros. This is why indirect includes of public APIs
should be avoided if at all possible.

This patch fixes the issue by adding the explicit include back.

gdb/ChangeLog:

        * i386-linux-nat.c, x86-linux-nat.c: Add <sys/uio.h> #include.
---
 gdb/ChangeLog        | 4 ++++
 gdb/i386-linux-nat.c | 1 +
 gdb/x86-linux-nat.c  | 1 +
 3 files changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9d3f392..393e4a0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2014-09-09  Joel Brobecker  <brobecker@adacore.com>
+
+	* i386-linux-nat.c, x86-linux-nat.c: Add <sys/uio.h> #include.
+
 2014-09-08  Doug Evans  <xdje42@gmail.com>
 
 	PR 17247
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 32a82e9..a08b9b8 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -23,6 +23,7 @@
 #include "regcache.h"
 #include "elf/common.h"
 #include <sys/ptrace.h>
+#include <sys/uio.h>
 #include "gregset.h"
 #include "gdb_proc_service.h"
 
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 67300d8..b2141eb 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -24,6 +24,7 @@
 #include <sys/ptrace.h>
 #include <sys/user.h>
 #include <sys/procfs.h>
+#include <sys/uio.h>
 
 #include "x86-nat.h"
 #include "linux-nat.h"
-- 
1.9.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists")
  2014-09-09 15:09   ` [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists") Joel Brobecker
@ 2014-09-10 10:59     ` Gary Benson
  2014-09-10 13:14       ` Joel Brobecker
  0 siblings, 1 reply; 24+ messages in thread
From: Gary Benson @ 2014-09-10 10:59 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches, Mark Kettenis, Pedro Alves

Joel Brobecker wrote:
> > This commit tidies up the #include lists in {i386,amd64}-linux-nat.c,
> > removing headers that are no longer required and reordering some
> > lines so that both files roughly match.  Additionally, an unused
> > definition was removed from the middle of the #include list in
> > i386-linux-nat.c.
> 
> Unfortunately, this patch broke GDB builds on some GNU/Linux
> distros. I fixed the issue I got by applying the attached patch.

Thanks for fixing this Joel.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists")
  2014-09-10 10:59     ` Gary Benson
@ 2014-09-10 13:14       ` Joel Brobecker
  0 siblings, 0 replies; 24+ messages in thread
From: Joel Brobecker @ 2014-09-10 13:14 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

> > > This commit tidies up the #include lists in {i386,amd64}-linux-nat.c,
> > > removing headers that are no longer required and reordering some
> > > lines so that both files roughly match.  Additionally, an unused
> > > definition was removed from the middle of the #include list in
> > > i386-linux-nat.c.
> > 
> > Unfortunately, this patch broke GDB builds on some GNU/Linux
> > distros. I fixed the issue I got by applying the attached patch.
> 
> Thanks for fixing this Joel.

Of course! As it happens, we need to do the same for amd64-linux-nat.c
as well. It makes sense, not sure why I didn't think of that before.
Attached is the patch I just pushed.

gdb/ChangeLog:

        * amd64-linux-nat.c: Add <sys/uio.h> #include.

-- 
Joel

[-- Attachment #2: 0001-Add-sys-uio.h-include-back-in-amd64-linux-nat.c.patch --]
[-- Type: text/x-diff, Size: 1099 bytes --]

From 35782f1465dd014737fcf6c82bdf733598a5c9b8 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 10 Sep 2014 09:06:50 -0400
Subject: [PATCH] Add <sys/uio.h> #include back in amd64-linux-nat.c.

This include is needed to access the definition of "struct iovec".

gdb/ChangeLog:

        * amd64-linux-nat.c: Add <sys/uio.h> #include.
---
 gdb/ChangeLog         | 4 ++++
 gdb/amd64-linux-nat.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 93a3931..2d20b1c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2014-09-10  Joel Brobecker  <brobecker@adacore.com>
+
+	* amd64-linux-nat.c: Add <sys/uio.h> #include.
+
 2014-09-09  Doug Evans  <xdje42@gmail.com>
 
 	PR guile/17367
diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index a00fbd6..09a4dfd 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -22,6 +22,7 @@
 #include "inferior.h"
 #include "regcache.h"
 #include "elf/common.h"
+#include <sys/uio.h>
 #include <sys/ptrace.h>
 #include <asm/prctl.h>
 #include <sys/reg.h>
-- 
1.9.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2014-09-10 13:14 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-27 14:12 [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Gary Benson
2014-06-27 14:12 ` [PATCH 2/7 v2] Merge {i386,amd64}_linux_read_description Gary Benson
2014-07-09 13:07   ` Pedro Alves
2014-06-27 14:12 ` [PATCH 1/7 v2] Rename identical functions Gary Benson
2014-07-09 13:04   ` Pedro Alves
2014-06-27 14:12 ` [PATCH 3/7 v2] Merge ps_get_thread_area Gary Benson
2014-07-09 13:08   ` Pedro Alves
2014-07-09 13:08   ` Pedro Alves
2014-06-27 14:28 ` [PATCH 6/7 v2] Move duplicated code into new files Gary Benson
2014-07-09 13:12   ` Pedro Alves
2014-06-27 14:40 ` [PATCH 5/7 v2] Comment and whitespace changes Gary Benson
2014-07-09 13:11   ` Pedro Alves
2014-06-27 14:52 ` [PATCH 7/7 v2] Tidy #include lists Gary Benson
2014-07-09 13:13   ` Pedro Alves
2014-08-06  9:36   ` Yao Qi
2014-08-06 10:19     ` Gary Benson
2014-08-06 11:16       ` Yao Qi
2014-09-09 15:09   ` [pushed] Fix missing "struct iovec" definition on some x86-linux. (was: "[PATCH 7/7 v2] Tidy #include lists") Joel Brobecker
2014-09-10 10:59     ` Gary Benson
2014-09-10 13:14       ` Joel Brobecker
2014-06-27 14:53 ` [PATCH 4/7 v2] Pull out common parts of _initialize_{i386,amd64}_linux_nat Gary Benson
2014-07-09 13:09   ` Pedro Alves
2014-07-09 13:17 ` [PATCH 0/7 v2] Refactor shared code in {i386,amd64}-linux-nat.c Pedro Alves
2014-07-11 12:34   ` Gary Benson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).