[2/7,v2] Merge {i386,amd64}_linux_read_description

Message ID 1403878351-22974-3-git-send-email-gbenson@redhat.com
State Committed
Headers

Commit Message

Gary Benson June 27, 2014, 2:12 p.m. UTC
  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(-)
  

Comments

Pedro Alves July 9, 2014, 1:07 p.m. UTC | #1
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.
  

Patch

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;