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


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

Re: [PATCH 04/25] Centralize i386 linux target descriptions


On 2017-06-12 10:41, Yao Qi wrote:
This patch moves all the tdesc_i386*_linux target descriptions to a
function i386_linux_read_description, which returns the right target
description according to xcr0.  This also remove the duplication in
getting target descriptions in corefile and native target.

gdb:

2017-04-27  Yao Qi  <yao.qi@linaro.org>

	* i386-linux-tdep.c (i386_linux_read_description): New function.
	(i386_linux_core_read_description): Call
	i386_linux_read_description.
	* i386-linux-tdep.h (i386_linux_read_description): Declare.
	(tdesc_i386_linux, tdesc_i386_mmx_linux): Remove declarations.
	(tdesc_i386_avx_linux, tdesc_i386_mpx_linux): Likewise
	(tdesc_i386_avx_mpx_linux, tdesc_i386_avx_avx512_linux): Likewise.
	(tdesc_i386_avx_mpx_avx512_pku_linux): Likewise.
	* x86-linux-nat.c (x86_linux_read_description): Call
	i386_linux_read_description.
---
 gdb/i386-linux-tdep.c | 34 ++++++++++++++++++++++------------
 gdb/i386-linux-tdep.h | 10 ++--------
 gdb/x86-linux-nat.c   | 24 ++++++++----------------
 3 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 1909d61..1bc1a6f 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -678,16 +678,9 @@ i386_linux_core_read_xcr0 (bfd *abfd)
   return xcr0;
 }

-/* Get Linux/x86 target description from core dump.  */
-
-static const struct target_desc *
-i386_linux_core_read_description (struct gdbarch *gdbarch,
-				  struct target_ops *target,
-				  bfd *abfd)
+const struct target_desc *
+i386_linux_read_description (uint64_t xcr0)

This would need a comment

  /* See i386-linux-tdep.h.  */

 {
-  /* Linux/i386.  */
-  uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
-
   switch ((xcr0 & X86_XSTATE_ALL_MASK))
     {
     case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
@@ -708,10 +701,27 @@ i386_linux_core_read_description (struct gdbarch *gdbarch,
       break;
     }

+  return NULL;
+}

I was going to say: I think it would make more sense if i386_linux_read_description always returned something non-NULL, but then I realized you need to handle the special case in i386_linux_core_read_description...

+
+/* Get Linux/x86 target description from core dump.  */
+
+static const struct target_desc *
+i386_linux_core_read_description (struct gdbarch *gdbarch,
+				  struct target_ops *target,
+				  bfd *abfd)
+{
+  /* Linux/i386.  */
+  uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd);
+ const struct target_desc * tdesc = i386_linux_read_description (xcr0);

Extra space after *.

The logic seems OK to me. It's not related to your patch, but I find a bit confusing that i386-linux has SSE, and in i386-mmx-linux, the mmx is there to denote the lack of sse. I looks like Mark agrees with me :)

  https://sourceware.org/ml/gdb-patches/2010-04/msg00300.html

Simon


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