Re: PATCH: Check bfd_mach_x64_32 to set tdesc for x32 binary

[Mark Kettenis <mark.kettenis@xs4all.nl>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5411 bytes --]

> Date: Fri, 11 May 2012 16:13:04 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
> 
> On Fri, May 11, 2012 at 1:20 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Hi,
> >>
> >> This patch checks bfd_mach_x64_32 to set tdesc for x32 binary.  Tested
> >> on Linux/x86-64.  OK to install?
> >>
> >> Thanks.
> >>
> >>
> >> H.J.
> >> ---
> >>       * amd64-linux-tdep.c: Include features/i386/x32-linux.c
> >>       and features/i386/x32-avx-linux.c.
> >>       (amd64_linux_init_abi): Check bfd_mach_x64_32 for x32 process.
> >>       (_initialize_amd64_linux_tdep): Register bfd_mach_x64_32.  Call
> >>       initialize_tdesc_x32_linux and initialize_tdesc_x32_avx_linux.
> >>
> >>       * amd64-linux-tdep.h (tdesc_x32_linux): New.
> >>       (tdesc_x32_avx_linux): Likewise.
> >>
> >> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> >> index acc7303..601ce20 100644
> >> --- a/gdb/amd64-linux-tdep.c
> >> +++ b/gdb/amd64-linux-tdep.c
> >> @@ -1545,8 +1578,12 @@ _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_linux_init_abi);
> >
> > Instead of doing this, you should introduce an
> > amd64_x32_linux_init_abi() function, make that call amd64_linix_init_abi() and ovverride the bits that are different for X32.
> 
> I don't think I should override tdep->tdesc.  Here is a different approach.
> I updated amd64_linux_init_abi to take amd64_linux_tdesc and fetch_func.
> I also added amd64_lp64_linux_init_abi and amd64_ilp32_linux_init_abi
> to pass the right ones to amd64_linux_init_abi.  Tested on Linux/x86-64.
> OK to install?

No.  Sorry, but you missed my point.  I want you to introduce a
seperate function such that it will be easy to override stuff
specifically for x32 without adding lots of if statements that turn
the code into spaghetti.

> -- 
> H.J.
> --
> 	* amd64-linux-tdep.c (amd64_linux_init_abi): Add 2 arguments,
> 	amd64_linux_tdesc and fetch_func.  Replace tdesc_amd64_linux
> 	with amd64_linux_tdesc and svr4_lp64_fetch_link_map_offsets
> 	with fetch_func.
> 	(amd64_lp64_linux_init_abi): New function.
> 	(amd64_ilp32_linux_init_abi): Likewise.
> 	(_initialize_amd64_linux_tdep): Replace amd64_linux_init_abi
> 	with amd64_lp64_linux_init_abi.  Register bfd_mach_x64_32 with
> 	amd64_ilp32_linux_init_abi.  Call initialize_tdesc_x32_linux
> 	and initialize_tdesc_x32_avx_linux.
> 
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index cfe6e4b..e68d70f 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -1288,7 +1288,9 @@ amd64_linux_core_read_description (struct
> gdbarch *gdbarch,
>  }
> 
>  static void
> -amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> +		      const struct target_desc *amd64_linux_tdesc,
> +		      struct link_map_offsets *(*fetch_func) (void))
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>    const struct target_desc *tdesc = info.target_desc;
> @@ -1310,7 +1312,7 @@ 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;
> 
>    feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux");
> @@ -1331,8 +1333,7 @@ amd64_linux_init_abi (struct gdbarch_info info,
> struct gdbarch *gdbarch)
>    tdep->xsave_xcr0_offset = I386_LINUX_XSAVE_XCR0_OFFSET;
> 
>    /* GNU/Linux uses SVR4-style shared libraries.  */
> -  set_solib_svr4_fetch_link_map_offsets
> -    (gdbarch, svr4_lp64_fetch_link_map_offsets);
> +  set_solib_svr4_fetch_link_map_offsets (gdbarch, fetch_func);
> 
>    /* Add the %orig_rax register used for syscall restarting.  */
>    set_gdbarch_write_pc (gdbarch, amd64_linux_write_pc);
> @@ -1543,6 +1544,22 @@ amd64_linux_init_abi (struct gdbarch_info info,
> struct gdbarch *gdbarch)
> 
>    tdep->i386_syscall_record = amd64_linux_syscall_record;
>  }
> +
> +static void
> +amd64_lp64_linux_init_abi (struct gdbarch_info info,
> +			   struct gdbarch *gdbarch)
> +{
> +  amd64_linux_init_abi (info, gdbarch, tdesc_amd64_linux,
> +			svr4_lp64_fetch_link_map_offsets);
> +}
> +
> +static void
> +amd64_ilp32_linux_init_abi (struct gdbarch_info info,
> +			   struct gdbarch *gdbarch)
> +{
> +  amd64_linux_init_abi (info, gdbarch, tdesc_x32_linux,
> +			svr4_ilp32_fetch_link_map_offsets);
> +}
> 
> 
> 
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
> @@ -1552,9 +1569,13 @@ void
>  _initialize_amd64_linux_tdep (void)
>  {
>    gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x86_64,
> -			  GDB_OSABI_LINUX, amd64_linux_init_abi);
> +			  GDB_OSABI_LINUX, amd64_lp64_linux_init_abi);
> +  gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32,
> +			  GDB_OSABI_LINUX, amd64_ilp32_linux_init_abi);
> 
>    /* Initialize the Linux target description.  */
>    initialize_tdesc_amd64_linux ();
>    initialize_tdesc_amd64_avx_linux ();
> +  initialize_tdesc_x32_linux ();
> +  initialize_tdesc_x32_avx_linux ();
>  }
> 
>