Patch 2 of 2 for aarch64 ILP32 support in gdb

Message ID 1485555939.2509.5.camel@caviumnetworks.com
State New, archived
Headers

Commit Message

Steve Ellcey Jan. 27, 2017, 10:25 p.m. UTC
  On Thu, 2017-01-26 at 14:14 +0000, Yao Qi wrote:

> Could you remove these ilp32 xml files, use the existing aarch64 target
> description, and try the patch below?

Yao,

I removed the ilp32 xml files and applied the patch you included and
made the other changes.  gdb seems to be working as well as it did
with the original patch but I ran into a problem that I missed before.

If I apply the gdb/gdbserver patch attached to this email and the
earlier bfd patch:

	 https://sourceware.org/ml/gdb-patches/2017-01/msg00508.html

and I try to run gdb on an ILP32 program I get:

(gdb) r
Starting program: /home/ubuntu/sellcey/gdb-ilp32/x32 
PC register is not available


I didn't see this before because I didn't have this bfd patch:

	https://sourceware.org/ml/binutils/2016-12/msg00167.html

That patch seems reasonable and it fixed some test failures I had
in the glibc testsuite but it is breaking gdb.  If I undo this
patch by making this change to bfd/cpu-aarch64.c:

 static const bfd_arch_info_type bfd_aarch64_arch_ilp32 =
-  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", 32, FALSE, NULL);
+  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", 64, FALSE, NULL);

Then gdb starts working again and I can debug 32 bit programs.

I am not sure how this change to the ILP32 aarch64 bfd description
is affecting gdb and why this change would break things.  Do you
know why this change messes up gdb?

Steve Ellcey
sellcey@cavium.com

2017-01-27  Andrew Pinski  <apinski@cavium.com>
	    Steve Ellcey  <sellcey@cavium.com>
	    Yao Qi  <yao.qi@linaro.org>

	* aarch64-linux-tdep.c (aarch64_linux_init_abi): Set link
	map offsets to 32 or 64 bits.
	* aarch64-tdep.c (aarch64_ilp32_register_type): New function.
	(aarch64_gdbarch_init): Setup ILP32 support.
	Make sure the gdbarches have compatible ilp32 flags.
	Set long and ptr sizes correctly for ilp32.
	* aarch64-tdep.h (gdbarch_tdep) <ilp32>: New field.

2017-01-27  Andrew Pinski  <apinski@cavium.com>
	    Steve Ellcey  <sellcey@cavium.com>

	* linux-aarch64-low.c (aarch64_linux_read_description): Use
	machine instead of is_elf64 to determine architecture.  Give an
	error when using 32 bit gdbserver on 64 bit program.
  

Comments

Yao Qi Jan. 29, 2017, 10:40 p.m. UTC | #1
On 17-01-27 14:25:39, Steve Ellcey wrote:
> That patch seems reasonable and it fixed some test failures I had
> in the glibc testsuite but it is breaking gdb.????If I undo this
> patch by making this change to bfd/cpu-aarch64.c:
> 
> ??static const bfd_arch_info_type bfd_aarch64_arch_ilp32 =
> -????N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", 32, FALSE, NULL);
> +????N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", 64, FALSE, NULL);
>

This changes the word size and address size, which is used to determine
the inferior is arm or aarch64 by checking "bits_per_word == 32" in
multi-arch debugging.  You can find some instances of such check in
aarch64-linux-nat.c.  If the bits_per_word is 32 in ILP32, GDB thinks
the inferior is an ARM one, rather than an AArch64 ILP32 one. which is
wrong.  We need to tweak the condition above to correctly identify the
ARM inferior.

> Then gdb starts working again and I can debug 32 bit programs.
> 
> I am not sure how this change to the ILP32 aarch64 bfd description
> is affecting gdb and why this change would break things.????Do you
> know why this change messes up gdb?
> 

D0 you compare the ILP32 gdb test result with normal aarch64 one?

> +
>  /* Return the pseudo register name corresponding to register regnum.  */
>  
>  static const char *
> @@ -2851,6 +2867,10 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    const struct tdesc_feature *feature;
>    int num_regs = 0;
>    int num_pseudo_regs = 0;
> +  bool ilp32 = FALSE;
> +

s/FALSE/false/

> +  if (info.bfd_arch_info->mach == bfd_mach_aarch64_ilp32)
> +    ilp32 = TRUE;

s/TRUE/true/
  

Patch

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index b94ccb2..6d0c3bd 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -1000,8 +1000,12 @@  aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   linux_init_abi (info, gdbarch);
 
-  set_solib_svr4_fetch_link_map_offsets (gdbarch,
-					 svr4_lp64_fetch_link_map_offsets);
+  if (tdep->ilp32)
+    set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					   svr4_ilp32_fetch_link_map_offsets);
+  else
+    set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					   svr4_lp64_fetch_link_map_offsets);
 
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 801c03d..bc6b0be 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2091,6 +2091,22 @@  aarch64_gen_return_address (struct gdbarch *gdbarch,
 }
 
 
+/* Implement the "register_type" gdbarch method.
+   Adjust the register type of $PC and $SP on ILP32.  */
+
+static struct type *
+aarch64_ilp32_register_type (struct gdbarch *gdbarch, int regnum)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  gdb_assert (tdep->ilp32);
+
+  if (regnum == AARCH64_SP_REGNUM || regnum == AARCH64_PC_REGNUM)
+    return builtin_type (gdbarch)->builtin_uint64;
+  else
+    return tdesc_register_type (gdbarch, regnum);
+}
+
 /* Return the pseudo register name corresponding to register regnum.  */
 
 static const char *
@@ -2851,6 +2867,10 @@  aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   const struct tdesc_feature *feature;
   int num_regs = 0;
   int num_pseudo_regs = 0;
+  bool ilp32 = FALSE;
+
+  if (info.bfd_arch_info->mach == bfd_mach_aarch64_ilp32)
+    ilp32 = TRUE;
 
   /* Ensure we always have a target descriptor.  */
   if (!tdesc_has_registers (tdesc))
@@ -2908,6 +2928,9 @@  aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
        best_arch != NULL;
        best_arch = gdbarch_list_lookup_by_info (best_arch->next, &info))
     {
+      /* ILP32 and LP64 are incompatible. */
+      if (gdbarch_tdep (arches->gdbarch)->ilp32 != ilp32)
+	continue;
       /* Found a match.  */
       break;
     }
@@ -2926,6 +2949,7 @@  aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep->lowest_pc = 0x20;
   tdep->jb_pc = -1;		/* Longjump support not enabled by default.  */
   tdep->jb_elt_size = 8;
+  tdep->ilp32 = ilp32;
 
   set_gdbarch_push_dummy_call (gdbarch, aarch64_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, aarch64_frame_align);
@@ -2968,9 +2992,9 @@  aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_float_bit (gdbarch, 32);
   set_gdbarch_double_bit (gdbarch, 64);
   set_gdbarch_long_double_bit (gdbarch, 128);
-  set_gdbarch_long_bit (gdbarch, 64);
+  set_gdbarch_long_bit (gdbarch, ilp32 ? 32 : 64);
   set_gdbarch_long_long_bit (gdbarch, 64);
-  set_gdbarch_ptr_bit (gdbarch, 64);
+  set_gdbarch_ptr_bit (gdbarch, ilp32 ? 32 : 64);
   set_gdbarch_char_signed (gdbarch, 0);
   set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
   set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
@@ -3012,6 +3036,13 @@  aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   tdesc_use_registers (gdbarch, tdesc, tdesc_data);
 
+  if (ilp32)
+    {
+      /* Override tdesc_register_type to adjust the types of $PC and
+	 $SP in ILP32.  */
+      set_gdbarch_register_type (gdbarch, aarch64_ilp32_register_type);
+    }
+
   /* Add standard register aliases.  */
   for (i = 0; i < ARRAY_SIZE (aarch64_register_aliases); i++)
     user_reg_add (gdbarch, aarch64_register_aliases[i].name,
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index 85c6a97..87d36b6 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -97,6 +97,9 @@  struct gdbarch_tdep
 
   /* syscall record.  */
   int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number);
+  /* If this is ILP32 or LP64.  */
+  bool ilp32;
+
 };
 
 extern struct target_desc *tdesc_aarch64;
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 334310b..e31d5c5 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -484,7 +484,12 @@  aarch64_linux_read_description (void)
 
   is_elf64 = linux_pid_exe_is_elf_64_file (tid, &machine);
 
-  if (is_elf64)
+  /* There are problems with ptrace when gdbserver is 32 bits and the
+     program being debugged is 64 bits.  */
+  if (sizeof (void *) == 4 && is_elf64)
+    error (_("Can't debug 64-bit process with 32-bit GDBserver"));
+
+  if (machine == EM_AARCH64)
     return tdesc_aarch64;
   else
     return tdesc_arm_with_neon;