public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Yao Qi <qiyaoltc@gmail.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 07/26] Lazily and dynamically create i386-linux target descriptions
Date: Mon, 10 Jul 2017 13:56:00 -0000	[thread overview]
Message-ID: <1499694940-23564-8-git-send-email-yao.qi@linaro.org> (raw)
In-Reply-To: <1499694940-23564-1-git-send-email-yao.qi@linaro.org>

Instead of using pre-generated target descriptions, this patch
changes GDB to lazily and dynamically create target descriptions
according to the target hardware capability (xcr0 in i386).
This support any combination of target features.

Some reg in target description has "regnum" attribute, so its register
number is got from the attribute value instead from sequential allocation.

  <reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>

when target description is created, it should match the regnum, so this
patch adds a new field m_next_regnum to track it, if attribute number is
greater than the m_next_regnum, print the code to set register number
explicitly.

v3:
 - Add sanity check, use error rather than gdb_assert, because it checks
   GDB input (The invalid input shouldn't trigger gdb internal error).
 - Add comments,

gdb:

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

	* i386-linux-tdep.c: Don't include features/i386/i386-*linux.c.
	Include features/i386/32bit-*.c.
	(i386_linux_read_description): Generate target description if it
	doesn't exist.
	(_initialize_i386_linux_tdep): Don't call _initialize_tdesc_i386
	functions.
	* features/i386/32bit-linux.c: Re-generated.
	* features/i386/32bit-sse.c: Likewise.
	* target-descriptions.c (print_c_feature::visit): Print code to
	set register number if needed.
	(print_c_feature) <m_next_regnum>: New field.
---
 gdb/features/i386/32bit-linux.c |  1 +
 gdb/features/i386/32bit-sse.c   |  1 +
 gdb/i386-linux-tdep.c           | 83 ++++++++++++++++++++++++-----------------
 gdb/target-descriptions.c       | 47 +++++++++++++++++++++++
 4 files changed, 98 insertions(+), 34 deletions(-)

diff --git a/gdb/features/i386/32bit-linux.c b/gdb/features/i386/32bit-linux.c
index 686a2f1..1b50882 100644
--- a/gdb/features/i386/32bit-linux.c
+++ b/gdb/features/i386/32bit-linux.c
@@ -9,6 +9,7 @@ create_feature_i386_32bit_linux (struct target_desc *result, long regnum)
   struct tdesc_feature *feature;
 
   feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
+  regnum = 41;
   tdesc_create_reg (feature, "orig_eax", regnum++, 1, NULL, 32, "int");
   return regnum;
 }
diff --git a/gdb/features/i386/32bit-sse.c b/gdb/features/i386/32bit-sse.c
index 032623e..c0684fb 100644
--- a/gdb/features/i386/32bit-sse.c
+++ b/gdb/features/i386/32bit-sse.c
@@ -61,6 +61,7 @@ create_feature_i386_32bit_sse (struct target_desc *result, long regnum)
   tdesc_add_flag (type, 12, "PM");
   tdesc_add_flag (type, 15, "FZ");
 
+  regnum = 32;
   tdesc_create_reg (feature, "xmm0", regnum++, 1, NULL, 128, "vec128");
   tdesc_create_reg (feature, "xmm1", regnum++, 1, NULL, 128, "vec128");
   tdesc_create_reg (feature, "xmm2", regnum++, 1, NULL, 128, "vec128");
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index e986d6f..4c0f597 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -45,13 +45,14 @@
 
 #include "record-full.h"
 #include "linux-record.h"
-#include "features/i386/i386-linux.c"
-#include "features/i386/i386-mmx-linux.c"
-#include "features/i386/i386-mpx-linux.c"
-#include "features/i386/i386-avx-mpx-linux.c"
-#include "features/i386/i386-avx-linux.c"
-#include "features/i386/i386-avx-avx512-linux.c"
-#include "features/i386/i386-avx-mpx-avx512-pku-linux.c"
+
+#include "features/i386/32bit-core.c"
+#include "features/i386/32bit-sse.c"
+#include "features/i386/32bit-linux.c"
+#include "features/i386/32bit-avx.c"
+#include "features/i386/32bit-mpx.c"
+#include "features/i386/32bit-avx512.c"
+#include "features/i386/32bit-pkeys.c"
 
 /* Return non-zero, when the register is in the corresponding register
    group.  Put the LINUX_ORIG_EAX register in the system group.  */
@@ -683,27 +684,50 @@ i386_linux_core_read_xcr0 (bfd *abfd)
 const struct target_desc *
 i386_linux_read_description (uint64_t xcr0)
 {
-  switch ((xcr0 & X86_XSTATE_ALL_MASK))
+  if (xcr0 == 0)
+    return NULL;
+
+  static struct target_desc *i386_linux_tdescs \
+    [2/*X87*/][2/*SSE*/][2/*AVX*/][2/*MPX*/][2/*AVX512*/][2/*PKRU*/] = {};
+  struct target_desc **tdesc;
+
+  tdesc = &i386_linux_tdescs[(xcr0 & X86_XSTATE_X87) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_SSE) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_AVX) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_MPX) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_AVX512) ? 1 : 0]
+    [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0];
+
+  if (*tdesc == NULL)
     {
-    case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK:
-      return tdesc_i386_avx_mpx_avx512_pku_linux;
-    case X86_XSTATE_AVX_AVX512_MASK:
-      return tdesc_i386_avx_avx512_linux;
-    case X86_XSTATE_MPX_MASK:
-      return tdesc_i386_mpx_linux;
-    case X86_XSTATE_AVX_MPX_MASK:
-      return tdesc_i386_avx_mpx_linux;
-    case X86_XSTATE_AVX_MASK:
-      return tdesc_i386_avx_linux;
-    case X86_XSTATE_SSE_MASK:
-      return tdesc_i386_linux;
-    case X86_XSTATE_X87_MASK:
-      return tdesc_i386_mmx_linux;
-    default:
-      break;
+      *tdesc = allocate_target_description ();
+      set_tdesc_architecture (*tdesc, bfd_scan_arch ("i386"));
+      set_tdesc_osabi (*tdesc, osabi_from_tdesc_string ("GNU/Linux"));
+
+      long regnum = 0;
+
+      if (xcr0 & X86_XSTATE_X87)
+	regnum = create_feature_i386_32bit_core (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_SSE)
+	regnum = create_feature_i386_32bit_sse (*tdesc, regnum);
+
+      regnum = create_feature_i386_32bit_linux (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_AVX)
+	regnum = create_feature_i386_32bit_avx (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_MPX)
+	regnum = create_feature_i386_32bit_mpx (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_AVX512)
+	regnum = create_feature_i386_32bit_avx512 (*tdesc, regnum);
+
+      if (xcr0 & X86_XSTATE_PKRU)
+	regnum = create_feature_i386_32bit_pkeys (*tdesc, regnum);
     }
 
-  return NULL;
+  return *tdesc;
 }
 
 /* Get Linux/x86 target description from core dump.  */
@@ -1092,13 +1116,4 @@ _initialize_i386_linux_tdep (void)
 {
   gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_LINUX,
 			  i386_linux_init_abi);
-
-  /* Initialize the Linux target description.  */
-  initialize_tdesc_i386_linux ();
-  initialize_tdesc_i386_mmx_linux ();
-  initialize_tdesc_i386_avx_linux ();
-  initialize_tdesc_i386_mpx_linux ();
-  initialize_tdesc_i386_avx_mpx_linux ();
-  initialize_tdesc_i386_avx_avx512_linux ();
-  initialize_tdesc_i386_avx_mpx_avx512_pku_linux ();
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 518f2dc..d1755f4 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -2101,6 +2101,48 @@ public:
 
   void visit (const tdesc_reg *reg) override
   {
+    /* Most "reg" in XML target descriptions don't have "regnum"
+       attribute, so the register number is allocated sequentially.
+       In case that reg has "regnum" attribute, register number
+       should be set by that explicitly.  */
+
+    if (reg->target_regnum < m_next_regnum)
+      {
+	/* The integrity check, it can catch some errors on register
+	   number collision, like this,
+
+	  <reg name="x0" bitsize="32"/>
+	  <reg name="x1" bitsize="32"/>
+	  <reg name="x2" bitsize="32"/>
+	  <reg name="x3" bitsize="32"/>
+	  <reg name="ps" bitsize="32" regnum="3"/>
+
+	  but it also has false negatives.  The target description
+	  below is correct,
+
+	  <reg name="x1" bitsize="32" regnum="1"/>
+	  <reg name="x3" bitsize="32" regnum="3"/>
+	  <reg name="x2" bitsize="32" regnum="2"/>
+	  <reg name="x4" bitsize="32" regnum="4"/>
+
+	  but it is not a good practice, so still error on this,
+	  and also print the message so that it can be saved in the
+	  generated c file.  */
+
+	printf_unfiltered ("ERROR: \"regnum\" attribute %ld ",
+			   reg->target_regnum);
+	printf_unfiltered ("is not the largest number (%d).\n",
+			   m_next_regnum);
+	error (_("\"regnum\" attribute %ld is not the largest number (%d)."),
+	       reg->target_regnum, m_next_regnum);
+      }
+
+    if (reg->target_regnum > m_next_regnum)
+      {
+	printf_unfiltered ("  regnum = %ld;\n", reg->target_regnum);
+	m_next_regnum = reg->target_regnum;
+      }
+
     printf_unfiltered ("  tdesc_create_reg (feature, \"%s\", regnum++, %d, ",
 		       reg->name, reg->save_restore);
     if (reg->group)
@@ -2108,8 +2150,13 @@ public:
     else
       printf_unfiltered ("NULL, ");
     printf_unfiltered ("%d, \"%s\");\n", reg->bitsize, reg->type);
+
+    m_next_regnum++;
   }
 
+private:
+  /* The register number to use for the next register we see.  */
+  int m_next_regnum = 0;
 };
 
 static void
-- 
1.9.1

  parent reply	other threads:[~2017-07-10 13:56 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-10 13:56 [PATCH 00/26 v3] Make GDB builtin target descriptions more flexible Yao Qi
2017-07-10 13:55 ` [PATCH 03/26] Add optional argument to command "maint prints c-tdesc" Yao Qi
2017-07-10 17:21   ` Eli Zaretskii
2017-07-10 13:55 ` [PATCH 01/26] Improve doc about "maint print c-tdesc" Yao Qi
2017-07-18 11:40   ` Yao Qi
2017-07-10 13:55 ` [PATCH 04/26] Centralize i386 linux target descriptions Yao Qi
2017-07-10 13:56 ` Yao Qi [this message]
2017-07-10 13:56 ` [PATCH 21/26] Centralize amd64-linux " Yao Qi
2017-07-10 13:56 ` [PATCH 08/26] Add "maint check xml-descriptions" to test builtin xml " Yao Qi
2017-07-10 13:56 ` [PATCH 06/26] Generate c for feature instead of tdesc Yao Qi
2017-07-10 13:56 ` [PATCH 25/26] [GDBserver] shorten srv_amd64_linux_xmlfiles Yao Qi
2017-07-10 13:56 ` [PATCH 13/26] GDBserver self test Yao Qi
2017-07-10 13:56 ` [PATCH 22/26] Lazily and dynamically create amd64-linux target descriptions Yao Qi
2017-07-10 13:56 ` [PATCH 02/26] Class-fy target_desc Yao Qi
2017-07-10 13:56 ` [PATCH 19/26] [GDBserver] Shorten srv_i386_linux_xmlfiles Yao Qi
2017-07-10 13:56 ` [PATCH 24/26] [GDBserver] Use pre-generated amd64-linux tdesc as test Yao Qi
2017-07-10 13:56 ` [PATCH 10/26] Use VEC for target_desc.reg_defs Yao Qi
2017-07-10 13:56 ` [PATCH 12/26] [GDBserver] Centralize tdesc for i386-linux Yao Qi
2017-07-10 13:56 ` [PATCH 20/26] Update comments in amd64_linux_core_read_description Yao Qi
2017-07-10 13:56 ` [PATCH 09/26] Adjust code generated by regformats/regdat.sh Yao Qi
2017-07-10 13:56 ` [PATCH 11/26] Return X86_TDESC_MMX in x86_get_ipa_tdesc_idx Yao Qi
2017-07-10 13:56 ` [PATCH 23/26] Convert amd64-linux target descriptions Yao Qi
2017-07-10 13:56 ` [PATCH 14/26] [GDBserver] unit test to i386_tdesc Yao Qi
2017-07-10 13:56 ` [PATCH 05/26] Use visitor pattern for "maint print c-tdesc" Yao Qi
2017-07-10 13:56 ` [PATCH 18/26] [GDBserver] Use pre-generated tdesc as test Yao Qi
2017-07-10 13:56 ` [PATCH 15/26] Dynamically composite xml in reply to GDB Yao Qi
2017-07-10 13:56 ` [PATCH 16/26] Share i386-linux target description between GDB and GDBserver Yao Qi
2017-07-10 13:56 ` [PATCH 17/26] Remove features/i386/i386-*linux.c Yao Qi
2017-07-10 13:57 ` [PATCH 26/26] Remove features/i386/amd64-*linux.c and features/i386/x32-*linux.c Yao Qi
2017-07-26 13:59 ` [PATCH 00/26 v3] Make GDB builtin target descriptions more flexible Yao Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1499694940-23564-8-git-send-email-yao.qi@linaro.org \
    --to=qiyaoltc@gmail.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).