diff mbox

[RFC] Fix amd64-elf 32/64 arch selection

Message ID 001a1147e9e863c0260540bfc1d8@google.com
State New
Headers show

Commit Message

Doug Evans Nov. 8, 2016, 1:06 a.m. UTC
Hi.

While trying to use an amd64-elf gdb with qemu I was tripping
over various problems. One problem I tripped over is that
gdb errantly thought it was debugging 32-bit instead of 64-bit x86.
Plus I was seeing this problem with amd64-elf but not amd64-linux.
I traced this to the fact that amd64-* uses i386_gdbarch_init
to initialize gdb for 64-bit x86: gdb starts out assuming the arch is  
32-bit,
and relies on this call in i386_gdbarch_init to switch the arch to 64-bit:

   info.tdep_info = tdesc_data;
   gdbarch_init_osabi (info, gdbarch);

On amd64-elf there is no usable registered osabi for 64-bit x86
so the switchover never happens.

Things are further confusing because "set osabi none" does *not*
do the intuitive thing, to me anyway. When I read "none" I think
"bare metal", but that's not what "none" does.
"set osabi none" sets the osabi to "unknown" (with
user_osabi_state=osabi_user) so gdbarch_lookup_osabi will
immediately return GDB_OSABI_UNKNOWN:

   /* If we aren't in "auto" mode, return the specified OS ABI.  */
   if (user_osabi_state == osabi_user)
     return user_selected_osabi;

and there is code that looks at an osabi of "unknown" and thinks
"ok, it's unknown, so let's try other means to find one".

gdbarch_info_fill:

   /* "(gdb) set osabi ...".  Handled by gdbarch_lookup_osabi.  */
   /* From the manual override, or from file.  */
   if (info->osabi == GDB_OSABI_UNINITIALIZED)
     info->osabi = gdbarch_lookup_osabi (info->abfd);
   /* From the target.  */
   if (info->osabi == GDB_OSABI_UNKNOWN && info->target_desc != NULL)
     info->osabi = tdesc_osabi (info->target_desc);
   /* From the configured default.  */
#ifdef GDB_OSABI_DEFAULT
   if (info->osabi == GDB_OSABI_UNKNOWN)
     info->osabi = GDB_OSABI_DEFAULT;
#endif

But that is not what any sane reading of "none" in the English
language would mean.
Alas I don't think we can change this without potentially
breaking some existing usage of "none".

Thus this patch adds "bare-metal" to really mean "none".
[Technically even a "bare metal" osabi needs to, I think, specify
some ABI-like things so there can't be a real "none", but there should
still be, IMO, some base level osabi that makes a minimal number of
os/abi assumptions.]

However, I'm not entirely happy with this patch.
On the one hand, it provides something that really means "none" which is  
good!
On the other, it doesn't feel right to have to use the osabi mechanism
to resolve i386 -> amd64. Plus this sets GDB_OSABI_DEFAULT to
GDB_OSABI_BARE_METAL for all ELF arches (though one could easily only
do so for amd64-elf). I think it's the right default to have, but I fear
it's too late to fix.

I played around with this patch instead:
diff mbox

Patch

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 22fb54c..623fb82 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -47,6 +47,7 @@ 
  #include "i386-tdep.h"
  #include "i387-tdep.h"
  #include "x86-xstate.h"
+#include "amd64-tdep.h"

  #include "record.h"
  #include "record-full.h"
@@ -8568,6 +8578,18 @@  i386_gdbarch_init (struct gdbarch_info info, struct  
gdbarch_list *arches)
    info.tdep_info = tdesc_data;
    gdbarch_init_osabi (info, gdbarch);

+  /* If we're a 64-bit arch but no osabi to fill in 64-bit values,
+     do so now.  */
+  if (info.osabi == GDB_OSABI_UNKNOWN
+      && info.bfd_arch_info->bits_per_address == 64
+      && gdbarch_ptr_bit (gdbarch) == 32)
+    {
+      if (strcmp (info.bfd_arch_info->printable_name, "i386:x64-32") == 0)
+	amd64_x32_init_abi (info, gdbarch);
+      else
+	amd64_init_abi (info, gdbarch);
+    }
+
    if (!i386_validate_tdesc_p (tdep, tdesc_data))
      {
        tdesc_data_cleanup (tdesc_data);

but that was awhile ago (and I may have grabbed the wrong one, but I hope
you get the idea). I had some problems with it so I abandoned it.
If people think this is a better way to go than the patch below
I'm happy to give it another go.

Anyways this is RFC.

2016-11-07  Doug Evans  <dje@google.com>

	* amd64-tdep.c (_initialize_amd64_tdep): Register "bare-metal"
	osabi handlers.
	* configure.tgt (*-*-elf): Make default osabi "bare-metal".
	* defs.h (gdb_osabi): New enum value GDB_OSABI_BARE_METAL.
	* i386-tdep.c (i386_init_abi): New function.
	(): Register "bare-metal" osabi.
	* osabi.c (gdb_osabi_names): Add "bare-metal".

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index a3a1fde..ec792c2 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3209,6 +3209,11 @@  void _initialize_amd64_tdep (void);
  void
  _initialize_amd64_tdep (void)
  {
+  gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x86_64,
+			  GDB_OSABI_BARE_METAL, amd64_init_abi);
+  gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32,
+			  GDB_OSABI_BARE_METAL, amd64_x32_init_abi);
+
    initialize_tdesc_amd64 ();
    initialize_tdesc_amd64_avx ();
    initialize_tdesc_amd64_mpx ();
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 79473c9..dfd781f 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -719,6 +719,7 @@  esac
  # map target onto default OS ABI

  case "${targ}" in
+*-*-elf*)	gdb_osabi=GDB_OSABI_BARE_METAL ;;
  *-*-freebsd* | *-*-kfreebsd*-gnu)
  		gdb_osabi=GDB_OSABI_FREEBSD_ELF ;;
  *-*-linux* | *-*-uclinux*)
diff --git a/gdb/defs.h b/gdb/defs.h
index 6d0d2dd..b235c23 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -607,6 +607,7 @@  enum gdb_osabi
    GDB_OSABI_LYNXOS178,
    GDB_OSABI_NEWLIB,
    GDB_OSABI_SDE,
+  GDB_OSABI_BARE_METAL,

    GDB_OSABI_INVALID		/* keep this last */
  };
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 22fb54c..fc1d414 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -4412,6 +4412,15 @@  i386_gnu_triplet_regexp (struct gdbarch *gdbarch)

  

+/* A nop init_abi routine needed for i386-elf configs.
+   There's nothing to do, but gdbarch_init_osabi still needs to find an  
entry
+   for bfd_mach_i386_i386 + GDB_OSABI_BARE_METAL.  */
+
+static void
+i386_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+}
+
  /* Generic ELF.  */

  void
@@ -9025,6 +9034,9 @@  Show Intel Memory Protection Extensions specific  
variables."),
    gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_GO32,
  			  i386_go32_init_abi);

+  gdbarch_register_osabi (bfd_arch_i386, bfd_mach_i386_i386,
+			  GDB_OSABI_BARE_METAL, i386_init_abi);
+
    /* Initialize the i386-specific register groups.  */
    i386_init_reggroups ();

diff --git a/gdb/osabi.c b/gdb/osabi.c
index 8b44a85..cc5d569 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -83,6 +83,7 @@  static const struct osabi_names gdb_osabi_names[] =
    { "LynxOS178", NULL },
    { "Newlib", NULL },
    { "SDE", NULL },
+  { "bare-metal", NULL },

    { "<invalid>", NULL }
  };