Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: gdb-patches@sourceware.org
Subject: Re: x32 ABI Support (was Re: Three weeks to branching (gdb 7.5 release))
Date: Thu, 31 May 2012 18:18:00 -0000	[thread overview]
Message-ID: <CAMe9rOp1Z7kQb3PzYMnCU1QurNgS+zHxf=RryyUTbk6R3SsdMQ@mail.gmail.com> (raw)
In-Reply-To: <CAMe9rOoQm1eh4u+t5F-gGzyB9bqwLo4h7S=VNP4BPZXSu9RZCA@mail.gmail.com>

On Mon, May 28, 2012 at 2:18 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, May 28, 2012 at 1:26 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>> Date: Sun, 20 May 2012 15:48:54 -0700
>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>>
>>> Does this one look OK.  I extracted x32_init_abi from amd64_x32_init_abi
>>> since amd64_x32_linux_init_abi can't call amd64_init_abi after
>>> calling amd64_linux_init_abi.
>>
>> I guess multiple-inheritance is a bad idea, even when implemented in C ;)
>>
>> I really do think that amd64_x32_linux_init_abi() should call
>> amd64_x32_init_abi().  That way it is immediately obvious that the OS-specific ABI inherits everything from the generic ABI.
>
> X32 kernel interface are highly OS specific.  Different OSes can implement
> very different kernel interfaces.  The only generic x32 bits are
>
>  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
>  tdep->num_dword_regs = 17;
>
>  set_tdesc_pseudo_register_type (gdbarch, amd64_x32_pseudo_register_type);
>
>  set_gdbarch_long_bit (gdbarch, 32);
>  set_gdbarch_ptr_bit (gdbarch, 32);
>
> They are the same for all x32 OSes since they are determined by
> hardware, not OS.
>
>> In order too avoid too much code duplication, the common bits should
>> be split out from amd64_linux_init_abi() into a seperate function that
>> gets called from both amd64_linux_init_abi() and
>> amd64_x32_linux_init_abi().  As I wrote earlier, it isn't entirely
>> obvious that everything in amd64_linix_init_abi() applies to the x32
>> ABI.  So we should be conservative in moving stuff into the common
>> function.  In fact it might be a good idea to start out with something
>> like the attached diff, and gradually move things over.
>
> Linux x32 kernel interface shares > 90% of Linux amd64 kernel
> interface (309 system calls out of 337 are the same).  See
> 64-bit system call table in Linux kernel 3.4:
>
> http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=arch/x86/syscalls/syscall_64.tbl;h=dd29a9ea27c560a9d2fcb6e1c2983f8b8e9be407;hb=HEAD
>
> I believe we should start with sharing everything between Linux/x32
> and Linux/amd64.  We can update x32 part as we go.
>
>> Cheers,
>>
>> Mark
>>
>> Index: amd64-linux-tdep.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/amd64-linux-tdep.c,v
>> retrieving revision 1.50
>> diff -u -p -r1.50 amd64-linux-tdep.c
>> --- amd64-linux-tdep.c  12 May 2012 08:54:03 -0000      1.50
>> +++ amd64-linux-tdep.c  28 May 2012 20:24:28 -0000
>> @@ -1298,8 +1298,6 @@ amd64_linux_init_abi (struct gdbarch_inf
>>
>>   gdb_assert (tdesc_data);
>>
>> -  linux_init_abi (info, gdbarch);
>> -
>>   tdep->gregset_reg_offset = amd64_linux_gregset_reg_offset;
>>   tdep->gregset_num_regs = ARRAY_SIZE (amd64_linux_gregset_reg_offset);
>>   tdep->sizeof_gregset = 27 * 8;
>> @@ -1323,6 +1321,8 @@ amd64_linux_init_abi (struct gdbarch_inf
>>   if (!valid_p)
>>     return;
>>
>> +  linux_init_abi (info, gdbarch);
>> +
>>   tdep->sigtramp_p = amd64_linux_sigtramp_p;
>>   tdep->sigcontext_addr = amd64_linux_sigcontext_addr;
>>   tdep->sc_reg_offset = amd64_linux_sc_reg_offset;
>> @@ -1543,6 +1543,44 @@ amd64_linux_init_abi (struct gdbarch_inf
>>
>>   tdep->i386_syscall_record = amd64_linux_syscall_record;
>>  }
>> +
>> +static void
>> +amd64_x32_linux_init_abi(struct gdbarch_info info, struct gdbarch *gdbarch)
>> +{
>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> +  const struct target_desc *tdesc = info.target_desc;
>> +
>> +  gdb_assert (tdesc_data);
>> +
>> +  tdep->gregset_reg_offset = amd64_linux_gregset_reg_offset;
>> +  tdep->gregset_num_regs = ARRAY_SIZE (amd64_linux_gregset_reg_offset);
>> +  tdep->sizeof_gregset = 27 * 8;
>> +
>> +  amd64_x32_init_abi (info, gdbarch);
>> +
>> +  /* Reserve a number for orig_rax.  */
>> +  set_gdbarch_num_regs (gdbarch, AMD64_LINUX_NUM_REGS);
>> +
>> +  if (! tdesc_has_registers (tdesc))
>> +    tdesc = tdesc_x32_linux;
>> +  tdep->tdesc = tdesc;
>> +
>> +  feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
>> +  if (feature == NULL)
>> +    return;
>> +
>> +  valid_p = tdesc_numbered_register (feature, tdesc_data,
>> +                                    AMD64_LINUX_ORIG_RAX_REGNUM,
>> +                                    "orig_rax");
>> +  if (!valid_p)
>> +    return;
>> +
>> +  linux_init_abi (info, gdbarch);
>> +
>> +  /* GNU/Linux uses SVR4-style shared libraries.  */
>> +  set_solib_svr4_fetch_link_map_offsets
>> +    (gdbarch, svr4_ilp32_fetch_link_map_offsets);
>> +}
>
> These won't work for x32.  We have to copy almost everything in
> amd64_linux_init_abi.
>
>> +void
>> +amd64_x32_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> +{
>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> +  const struct target_desc *tdesc = info.target_desc;
>> +
>> +  amd64_init_abi (info, gdbarch);
>> +
>> +  if (! tdesc_has_registers (tdesc))
>> +    tdesc = tdesc_x32;
>> +  tdep->tdesc = tdesc;
>> +
>> +  tdep->num_dword_regs = 17;
>> +  set_tdesc_pseudo_register_type (gdbarch, amd64_x32_pseudo_register_type);
>> +
>> +  set_gdbarch_long_bit (gdbarch, 32);
>> +  set_gdbarch_ptr_bit (gdbarch, 32);
>> +}
>>
>
> I think this is the wrong approach for x32 support. We should
> break amd64_x32_init_abi into 2 parts:
>
> 1. OS specific part.  It should be private to amd64-tdep.c.
> 2. Generic part.  It should be exported from amd64-tdep.c and can
> be used by all x32 OSes.
>
> Thanks.
>
>

Here is the updated patch.  I added amd64_x32_init for generic x32
setting.  It can be used by all x32 init_abi functions.  Since
tdep->tdesc can't be changed after being used, I renamed
amd64_init_abi/amd64_init_abi to amd64_init/amd64_linux_init
to take an  argument to set tdep->tdesc properly.  OK for trunk?

Thanks.


-- 
H.J.
---
2012-05-31  Mark Kettenis  <kettenis@gnu.org>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* amd64-linux-tdep.c (amd64_linux_init_abi): Renamed to ...
	(amd64_linux_init): This.  Add an argument, amd64_linux_tdesc.
	(amd64_linux_init_abi): Call amd64_linux_init.
	(amd64_x32_linux_init_abi): New function.
	(_initialize_amd64_linux_tdep): Register bfd_mach_x64_32 with
	amd64_x32_linux_init_abi.

	* amd64-tdep.c (amd64_dword_names): Add "eip".
	(amd64_x32_pseudo_register_type): New function.
	(amd64_x32_init): Likewise.
	(amd64_x32_init_abi): Likewise.
	(amd64_init_abi): Renamed to ...
	(amd64_init): This. Add an argument, amd64_tdesc.
	(amd64_init_abi): Call amd64_init.

	* amd64-tdep.h (amd64_x32_init_abi): New prototype.
	(amd64_x32_init): Likewise.

	* i386-tdep.c (i386_pseudo_register_type): Make it global.

	* i386-tdep.h (i386_pseudo_register_type): New prototype.

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 42dc89a..23bf318 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1288,7 +1288,8 @@ amd64_linux_core_read_description (struct
gdbarch *gdbarch,
 }

 static void
-amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+amd64_linux_init (struct gdbarch_info info, struct gdbarch *gdbarch,
+		  const struct target_desc *amd64_linux_tdesc)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   const struct target_desc *tdesc = info.target_desc;
@@ -1310,9 +1311,10 @@ amd64_linux_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)
   set_gdbarch_num_regs (gdbarch, AMD64_LINUX_NUM_REGS);

   if (! tdesc_has_registers (tdesc))
-    tdesc = tdesc_amd64_linux;
+    tdesc = amd64_linux_tdesc;
   tdep->tdesc = tdesc;

+  /* tdep->tdesc can't be changed after being used here.  */
   feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
   if (feature == NULL)
     return;
@@ -1543,6 +1545,24 @@ amd64_linux_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)

   tdep->i386_syscall_record = amd64_linux_syscall_record;
 }
+
+static void
+amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  amd64_linux_init (info, gdbarch, tdesc_amd64_linux);
+}
+
+static void
+amd64_x32_linux_init_abi (struct gdbarch_info info,
+			  struct gdbarch *gdbarch)
+{
+  amd64_linux_init (info, gdbarch, tdesc_x32_linux);
+  amd64_x32_init (gdbarch);
+
+   /* GNU/Linux uses SVR4-style shared libraries.  */
+  set_solib_svr4_fetch_link_map_offsets
+    (gdbarch, svr4_ilp32_fetch_link_map_offsets);
+}



 /* Provide a prototype to silence -Wmissing-prototypes.  */
@@ -1553,6 +1573,8 @@ _initialize_amd64_linux_tdep (void)
 {
   gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x86_64,
 			  GDB_OSABI_LINUX, amd64_linux_init_abi);
+  gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32,
+			  GDB_OSABI_LINUX, amd64_x32_linux_init_abi);

   /* Initialize the Linux target description.  */
   initialize_tdesc_amd64_linux ();
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index df91a51..47f8019 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -258,9 +258,32 @@ static const char *amd64_word_names[] =
 static const char *amd64_dword_names[] =
 {
   "eax", "ebx", "ecx", "edx", "esi", "edi", "ebp", "esp",
-  "r8d", "r9d", "r10d", "r11d", "r12d", "r13d", "r14d", "r15d"
+  "r8d", "r9d", "r10d", "r11d", "r12d", "r13d", "r14d", "r15d",
+  "eip"
 };

+/* Return the GDB type object for the "standard" data type of data in
+   register REGNUM.  Only used for x32.  */
+
+static struct type *
+amd64_x32_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
+{
+  /* Use pointer types for ebp, esp and eip registers in x32.  */
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  switch (regnum - tdep->eax_regnum)
+    {
+    default:
+      break;
+    case AMD64_RBP_REGNUM:	/* ebp  */
+    case AMD64_RSP_REGNUM:	/* esp	*/
+      return builtin_type (gdbarch)->builtin_data_ptr;
+    case AMD64_RIP_REGNUM:	/* eip */
+      return builtin_type (gdbarch)->builtin_func_ptr;
+    }
+
+  return i386_pseudo_register_type (gdbarch, regnum);
+}
+
 /* Return the name of register REGNUM.  */

 static const char *
@@ -2606,8 +2629,9 @@ static const int amd64_record_regmap[] =
   AMD64_DS_REGNUM, AMD64_ES_REGNUM, AMD64_FS_REGNUM, AMD64_GS_REGNUM
 };

-void
-amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+static void
+amd64_init (struct gdbarch_info info, struct gdbarch *gdbarch,
+	    const struct target_desc *amd64_tdesc)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   const struct target_desc *tdesc = info.target_desc;
@@ -2617,12 +2641,13 @@ amd64_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)
   tdep->sizeof_fpregset = I387_SIZEOF_FXSAVE;

   if (! tdesc_has_registers (tdesc))
-    tdesc = tdesc_amd64;
+    tdesc = amd64_tdesc;
   tdep->tdesc = tdesc;

   tdep->num_core_regs = AMD64_NUM_GREGS + I387_NUM_REGS;
   tdep->register_names = amd64_register_names;

+  /* tdep->tdesc can't be changed after being used here.  */
   if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.avx") != NULL)
     {
       tdep->ymmh_register_names = amd64_ymmh_names;
@@ -2730,6 +2755,32 @@ amd64_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)
 					i386_stap_parse_special_token);
 }

+void
+amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  amd64_init (info, gdbarch, tdesc_amd64);
+}
+
+void
+amd64_x32_init (struct gdbarch *gdbarch)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  tdep->num_dword_regs = 17;
+
+  set_tdesc_pseudo_register_type (gdbarch, amd64_x32_pseudo_register_type);
+
+  set_gdbarch_long_bit (gdbarch, 32);
+  set_gdbarch_ptr_bit (gdbarch, 32);
+}
+
+void
+amd64_x32_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  amd64_init (info, gdbarch, tdesc_x32);
+  amd64_x32_init (gdbarch);
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 void _initialize_amd64_tdep (void);

diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 1ed109c..ce47ae7 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -80,6 +80,9 @@ extern void amd64_displaced_step_fixup (struct
gdbarch *gdbarch,
 					struct regcache *regs);

 extern void amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
+extern void amd64_x32_init_abi (struct gdbarch_info info,
+				struct gdbarch *gdbarch);
+extern void amd64_x32_init (struct gdbarch *gdbarch);

 /* Fill register REGNUM in REGCACHE with the appropriate
    floating-point or SSE register value from *FXSAVE.  If REGNUM is
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 5b04505..6333556 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -2780,7 +2780,7 @@ i386_mmx_type (struct gdbarch *gdbarch)
 /* Return the GDB type object for the "standard" data type of data in
    register REGNUM.  */

-static struct type *
+struct type *
 i386_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
 {
   if (i386_mmx_regnum_p (gdbarch, regnum))
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index f297ae7..a12b1f5 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -307,6 +307,7 @@ extern int i386_dword_regnum_p (struct gdbarch
*gdbarch, int regnum);
 extern int i386_xmm_regnum_p (struct gdbarch *gdbarch, int regnum);
 extern int i386_ymm_regnum_p (struct gdbarch *gdbarch, int regnum);

+extern struct type *i386_pseudo_register_type (struct gdbarch *, int);
 extern const char *i386_pseudo_register_name (struct gdbarch *gdbarch,
 					      int regnum);


  reply	other threads:[~2012-05-31 18:18 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-11 18:17 Three weeks to branching (gdb 7.5 release) Joel Brobecker
2012-05-11 18:43 ` H.J. Lu
2012-05-14 14:45   ` Joel Brobecker
2012-05-20 15:40   ` H.J. Lu
2012-05-20 20:44     ` Mark Kettenis
2012-05-20 21:25       ` H.J. Lu
2012-05-20 21:39         ` Mark Kettenis
2012-05-20 22:49           ` H.J. Lu
2012-05-25 16:52             ` H.J. Lu
2012-05-28 20:26             ` x32 ABI Support (was Re: Three weeks to branching (gdb 7.5 release)) Mark Kettenis
2012-05-28 21:18               ` H.J. Lu
2012-05-31 18:18                 ` H.J. Lu [this message]
2012-06-05 12:58                   ` H.J. Lu
2012-06-05 13:16                     ` Mark Kettenis
2012-06-09 14:30                       ` H.J. Lu
2012-06-10 21:52                   ` Mark Kettenis
2012-06-11 13:42                     ` H.J. Lu
2012-06-12 21:23                       ` Mark Kettenis
2012-06-13 20:29                         ` x32 ABI Support (committed) Mark Kettenis
2012-06-13 20:41                           ` Mark Kettenis
2012-05-12 15:26 ` Three weeks to branching (gdb 7.5 release) Doug Evans
2012-05-20 14:59   ` Doug Evans
2012-06-11 15:35     ` Branching time + 1 week (was: "Re: Three weeks to branching (gdb 7.5 release)") Joel Brobecker
2012-06-11 15:52       ` Doug Evans
2012-06-11 16:00         ` Joel Brobecker
2012-06-21 20:20         ` Joel Brobecker
2012-06-21 20:25           ` Doug Evans
2012-06-22 14:47           ` Jan Kratochvil
2012-06-22 16:32             ` Joel Brobecker
2012-06-22 16:41               ` Jan Kratochvil
2012-06-22 17:38                 ` Branching time + 1 week Tom Tromey
2012-06-22 20:31                 ` Branching time + 1 week (was: "Re: Three weeks to branching (gdb 7.5 release)") Joel Brobecker
2012-06-24  9:14                   ` [commit] gnulib update [Re: Branching time + 1 week] Jan Kratochvil
2012-06-22 16:42               ` Branching time + 1 week (was: "Re: Three weeks to branching (gdb 7.5 release)") H.J. Lu
2012-06-11 15:59       ` H.J. Lu
2012-06-11 16:11         ` Joel Brobecker
2012-06-11 16:20           ` H.J. Lu
2012-05-29 17:17 ` Three weeks to branching (gdb 7.5 release) Jan Kratochvil
2012-05-30 22:04   ` Joel Brobecker
2012-06-07 19:36     ` Maciej W. Rozycki
2012-06-11 14:58       ` Joel Brobecker
2012-06-12 15:39         ` Maciej W. Rozycki

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='CAMe9rOp1Z7kQb3PzYMnCU1QurNgS+zHxf=RryyUTbk6R3SsdMQ@mail.gmail.com' \
    --to=hjl.tools@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=mark.kettenis@xs4all.nl \
    /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