Re: Three weeks to branching (gdb 7.5 release)

H.J. Lu
2012-05-20T22:49:14Z

On Sun, May 20, 2012 at 2:38 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Sun, 20 May 2012 14:24:46 -0700
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>
>> On Sun, May 20, 2012 at 1:43 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> Date: Sun, 20 May 2012 08:40:26 -0700
>> >> From: "H.J. Lu" <hjl.tools@gmail.com>
>> >>
>> >> On Fri, May 11, 2012 at 11:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >> > On Fri, May 11, 2012 at 11:17 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> >> >> Hello,
>> >> >>
>> >> >> Just a quick heads up: The current tentative date for branching
>> >> >> GDB (7.5 release) is Mon Jun 4th, which is a little over three weeks
>> >> >> away.
>> >> >>
>> >> >> I've created a wiki page for known issues that need to be fixed
>> >> >> before then:
>> >> >>
>> >> >> á áhttp://sourceware.org/gdb/wiki/GDB_7.5_Release
>> >> >>
>> >> >> When you add an issue, please make sure you add a name so we know
>> >> >> who is coordinating the effort. áIf you don't know who can work
>> >> >> on it, please just post the issue here, and we'll try to find some
>> >> >> help.
>> >> >>
>> >> >> I only know of one issue, which is a noticeable performance degradation
>> >> >> that was reported a while ago:
>> >> >>
>> >> >
>> >> > I'd like to merge x32 into GDB 7.5. áMy x32 change is on hjl/x32/master
>> >> > branch at
>> >> >
>> >> > http://sourceware.org/git/?p=gdb.git;a=summary
>> >> >
>> >> > The current diff only has 864 lines. áOne patch:
>> >> >
>> >> > http://sourceware.org/ml/gdb-patches/2012-05/msg00097.html
>> >> >
>> >> > isn't reviewed yet. áI will open a meta bug for x32 integration.
>> >> >
>> >>
>> >> I opened:
>> >>
>> >> http://sourceware.org/bugzilla/show_bug.cgi?id=14099
>> >>
>> >> Thanks for help from everyone. áThe full GDBserver x32 support
>> >> as well as partial GDB x32 support have been checked in. áThe
>> >> remaining patches are:
>> >>
>> >> http://sourceware.org/ml/gdb-patches/2012-04/msg00195.html
>> >> http://sourceware.org/ml/gdb-patches/2012-04/msg00191.html
>> >> http://sourceware.org/ml/gdb-patches/2012-05/msg00744.html
>> >> http://sourceware.org/ml/gdb-patches/2012-05/msg00531.html
>> >> http://sourceware.org/ml/gdb-patches/2012-05/msg00533.html
>> >> http://sourceware.org/ml/gdb-patches/2012-05/msg00489.html
>> >> http://sourceware.org/ml/gdb-patches/2012-05/msg00438.html
>> >> http://sourceware.org/ml/gdb-patches/2012-05/msg00097.html
>> >>
>> >> I would appreciate help to get them reviewed and approved.
>> >
>> > As I wrote before, I don't think adding lots if if-statements is the
>> > proper way to add a new ABI to GDB. áThe proper way is to do it like
>> > the diff below. áIn that diff, I'm not entirely confident that calling
>> > amd64_linux_init_abi() from amd64_x32_linux_init_abi() makes all that
>> > much sense. áFor example the amd64_linux_record_tdep stuff probably
>> > isn't right for the x32 ABI. áBut at least this will give us a
>> > starting point where we won't end up adding
>> >
>> > áif (gdbarch_ptr_bit (gdbarch) == 32)
>> > á á{
>> > á á á...
>> > á á}
>>
>> Please take a look at
>>
>> http://sourceware.org/ml/gdb-patches/2012-04/msg00195.html
>>
>> It doesn't add any  (gdbarch_ptr_bit (gdbarch) == 32).  It just changes
>> it to bits_per_word.  I add one  "gdbarch_ptr_bit (gdbarch) == 32" in
>> amd64_linux_sigtramp_start and I will remove them from
>>
>> http://sourceware.org/ml/gdb-patches/2012-05/msg00744.html
>
> Well, I'm also thining about the future here.  Stuff you haven't
> addressed yet in your diff series and stuff that will be added later.
>
>> > 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 á20 May 2012 20:31:53 -0000
>> > @@ -1543,6 +1543,24 @@ 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;
>> > +
>> > + áamd64_linux_init_abi (info, gdbarch);
>> > + áamd64_x32_init_abi (info, gdbarch);
>> > +
>> > + áif (! tdesc_has_registers (tdesc))
>> > + á átdesc = tdesc_amd64_linux;
>>
>> I assume you meant tdesc_x32_linux here.  The problem is
>> when we reach here, if (! tdesc_has_registers (tdesc)) will always
>> be false since tdep->tdesc has been set by amd64_linux_init_abi.
>>
>> > + átdep->tdesc = tdesc;
>> > +
>> > + á/* 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 +1571,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 ();
>> > Index: amd64-tdep.c
>> > ===================================================================
>> > RCS file: /cvs/src/src/gdb/amd64-tdep.c,v
>> > retrieving revision 1.104
>> > diff -u -p -r1.104 amd64-tdep.c
>> > --- amd64-tdep.c á á á á14 May 2012 18:56:40 -0000 á á á1.104
>> > +++ amd64-tdep.c á á á á20 May 2012 20:31:54 -0000
>> > @@ -258,7 +258,8 @@ 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 name of register REGNUM. á*/
>> > @@ -2729,6 +2730,43 @@ amd64_init_abi (struct gdbarch_info info
>> > á set_gdbarch_stap_parse_special_token (gdbarch,
>> > á á á á á á á á á á á á á á á á á á á ái386_stap_parse_special_token);
>> > á}
>> > +
>> > +
>> > +static struct type *
>> > +amd64_x32_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
>> > +{
>> > + ástruct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> > +
>> > + áswitch (regnum - tdep->eax_regnum)
>> > + á á{
>> > + á á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);
>> > +}
>> > +
>> > +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;
>>
>> Again, " if (! tdesc_has_registers (tdesc))" will always false
>> since tdep->tdesc has been set in amd64_init_abi.
>
> I don't think that's true.  Here tdesc comes from info.target_desc,
> not tdep->tdesc, so the check should still do the right thing.  In
> fact it must do the right thing, since amd64_linux_init_abi() does the
> same thing already.

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.

Thanks.

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

* amd64-linux-tdep.c (amd64_x32_linux_init_abi): New functiom.
(_initialize_amd64_linux_tdep): Register bfd_mach_x64_32 with
amd64_x32_linux_init_abi.

* amd64-tdep.c (amd64_x32_pseudo_register_type): New function.
(amd64_x32_pseudo_register_name): Likewise.
(x32_init_abi): Likewise.
(amd64_x32_init_abi): Likewise.

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

* i386-tdep.c (i386_pseudo_register_type): Make it global.
(i386_gdbarch_init): Initialize sp_regnum_from_eax and
pc_regnum_from_eax to -1. Update SP regnum from
sp_regnum_from_eax and PC regnum from pc_regnum_from_eax if
needed.

* i386-tdep.h (gdbarch_tdep): Add sp_regnum_from_eax and
pc_regnum_from_eax.
(i386_pseudo_register_type): New prototype.

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 22a3464..470680d 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1543,6 +1558,26 @@ amd64_linux_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)

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;
+
+ amd64_linux_init_abi (info, gdbarch);
+
+ if (! tdesc_has_registers (tdesc))
+ tdesc = tdesc_amd64_linux;
+ tdep->tdesc = tdesc;
+
+ x32_init_abi (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 +1588,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..efa9f5e 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -261,6 +261,28 @@ static const char *amd64_dword_names[] =
"r8d", "r9d", "r10d", "r11d", "r12d", "r13d", "r14d", "r15d"
};

+/* 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 *
@@ -279,6 +301,17 @@ amd64_pseudo_register_name (struct gdbarch
*gdbarch, int regnum)
return i386_pseudo_register_name (gdbarch, regnum);
}

+/* Return the name of register REGNUM. Only used for x32. */
+
+static const char *
+amd64_x32_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
+{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ if ((regnum - tdep->eax_regnum) == AMD64_RIP_REGNUM)
+ return "eip";
+ return amd64_pseudo_register_name (gdbarch, regnum);
+}
+
static struct value *
amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
struct regcache *regcache,
@@ -2730,6 +2803,37 @@ amd64_init_abi (struct gdbarch_info info,
struct gdbarch *gdbarch)
i386_stap_parse_special_token);
}

+void
+x32_init_abi (struct gdbarch *gdbarch)
+{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+ tdep->num_dword_regs = 17;
+ tdep->sp_regnum_from_eax = AMD64_RSP_REGNUM;
+ tdep->pc_regnum_from_eax = AMD64_RIP_REGNUM;
+
+ set_tdesc_pseudo_register_type (gdbarch, amd64_x32_pseudo_register_type);
+ set_tdesc_pseudo_register_name (gdbarch, amd64_x32_pseudo_register_name);
+
+ set_gdbarch_long_bit (gdbarch, 32);
+ set_gdbarch_ptr_bit (gdbarch, 32);
+}
+
+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;
+
+ x32_init_abi (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..401b379 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 x32_init_abi (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..9cc2e30 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))
@@ -7787,6 +7787,9 @@ i386_gdbarch_init (struct gdbarch_info info,
struct gdbarch_list *arches)
tdep->num_mmx_regs = 8;
tdep->num_ymm_regs = 0;

+ tdep->sp_regnum_from_eax = -1;
+ tdep->pc_regnum_from_eax = -1;
+
tdesc_data = tdesc_data_alloc ();

set_gdbarch_relocate_instruction (gdbarch, i386_relocate_instruction);
@@ -7831,6 +7834,14 @@ i386_gdbarch_init (struct gdbarch_info info,
struct gdbarch_list *arches)
/* Support dword pseudo-register if it hasn't been disabled. */
tdep->eax_regnum = ymm0_regnum;
ymm0_regnum += tdep->num_dword_regs;
+ if (tdep->sp_regnum_from_eax != -1)
+ set_gdbarch_sp_regnum (gdbarch,
+ (tdep->eax_regnum
+ + tdep->sp_regnum_from_eax));
+ if (tdep->pc_regnum_from_eax != -1)
+ set_gdbarch_pc_regnum (gdbarch,
+ (tdep->eax_regnum
+ + tdep->pc_regnum_from_eax));
}
else
tdep->eax_regnum = -1;
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index f297ae7..e1f7c44 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -149,6 +149,14 @@ struct gdbarch_tdep
of pseudo dword register support. */
int eax_regnum;

+ /* Register number for SP, relative to %eax. Set this to -1 to
+ indicate the absence of pseudo SP register support. */
+ int sp_regnum_from_eax;
+
+ /* Register number for PC, relative to %eax. Set this to -1 to
+ indicate the absence of pseudo PC register support. */
+ int pc_regnum_from_eax;
+
/* Number of core registers. */
int num_core_regs;

@@ -307,6 +315,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);