Re: [PATCH] Handle loading improper core files gracefully in the mips backend.

[Luis Machado <lgustavo at codesourcery dot com>]

On 01/12/2016 12:10 PM, Pedro Alves wrote:
> On 01/12/2016 01:25 PM, Luis Machado wrote:
> > On 01/12/2016 10:46 AM, Pedro Alves wrote:
> > > On 01/11/2016 03:47 PM, Luis Machado wrote:
> > > > diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> > > > index ca17864..cdfd80e 100644
> > > > --- a/gdb/mips-tdep.c
> > > > +++ b/gdb/mips-tdep.c
> > > > @@ -8208,6 +8208,12 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > > >      int dspacc;
> > > >      int dspctl;
> > > >
> > > > +  /* Sanity check the e_machine field.  */
> > > > +  if (info.abfd
> > > > +      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
> > > > +      && elf_elfheader (info.abfd)->e_machine != EM_MIPS)
> > > > +    return NULL;
> > >
> > > This callback is registered for bfd_arch_mips:
> > >
> > >     gdbarch_register (bfd_arch_mips, mips_gdbarch_init, mips_dump_tdep);
> > >
> > > Does bfd think this a bfd_arch_mips binary?  How so?
> >
> > In the second time we call gdbarch_info_fill, when opening the core file
> > alone, we have this:
> >
> > p *info
> > $8 = {bfd_arch_info = 0x0, byte_order = BFD_ENDIAN_UNKNOWN,
> > byte_order_for_code = BFD_ENDIAN_UNKNOWN, abfd = 0xe1ce80, tdep_info =
> > 0x0, osabi = GDB_OSABI_UNINITIALIZED, target_desc = 0x0}
> >
> > p *info->abfd->arch_info
> > $10 = {bits_per_word = 32, bits_per_address = 32, bits_per_byte = 8,
> > arch = bfd_arch_unknown, mach = 0, arch_name = 0x9b799f "unknown",
> > printable_name = 0x9b799f "unknown", section_align_power = 2,
> > the_default = 1, compatible = 0x78a592 <bfd_default_compatible>,
> >     scan = 0x78a60a <bfd_default_scan>, fill = 0x78acc6
> > <bfd_arch_default_fill>, next = 0x0}
> >
> > p *default_bfd_arch
> > $12 = {bits_per_word = 32, bits_per_address = 32, bits_per_byte = 8,
> > arch = bfd_arch_mips, mach = 0, arch_name = 0x9d98e0 "mips",
> > printable_name = 0x9d98e0 "mips", section_align_power = 3, the_default =
> > 1, compatible = 0x832b40 <mips_compatible>,
> >     scan = 0x78a60a <bfd_default_scan>, fill = 0x78acc6
> > <bfd_arch_default_fill>, next = 0x9d9b00 <arch_info_struct>}
> >
> > The data above leads gdbarch_info_fill to assign default_bfd_arch to
> > info->bfd_arch_info here:
> >
> >     /* From the default.  */
> >     if (info->bfd_arch_info == NULL)
> >       info->bfd_arch_info = default_bfd_arch;
> >
> > So the core file essentially turns into a mips-compatible core file.
>
> Hmmm.  I see.  I think we can't really change this, given that there
> are formats that don't have an architecture.  Like, e.g., srec:
>
>   (gdb) file testsuite/gdb.base/intstr2.srec
>   Reading symbols from testsuite/gdb.base/intstr2.srec...(no debugging symbols found)...done.
>
> I take it that a --enable-targets=all wouldn't fail like this?

Yes, because, at least in my case, we default to the proper i386 
architecture.

> Also, sounds like you should be able to trigger these incompatibilities
> and assertion by loading a 32-bit MIPS binary and playing with
> "set mips abi n64/o64", etc?

Yes, most likely, but see below.

> All in all, maybe your original patch that flagged incompatible
> abi/isa combination is the way to go?
>
> I also wonder whether the bfd arch detection couldn't be always
> compiled in, at least for elf.  Why does bfd fail to detect that this
> is an bfd_arch_i386 file in the first place?

It seems bfd also falls back to the default, which is mips in this case.

p bfd_default_vector[0]
$3 = (const bfd_target *) 0x9beac0 <mips_elf32_trad_be_vec>

I gave it a try with a legitimate x86 core file being loaded in a 
mips-targeted gdb and i see the same problem with the internal error. 
Initially, when loading the core, the bfd arch is unknown, and then we 
pick the default arch in bfd_find_target here:

  /* This is safe; the vector cannot be null.  */
  if (targname == NULL || strcmp (targname, "default") == 0)
    {
      if (bfd_default_vector[0] != NULL)
        target = bfd_default_vector[0];
      else
        target = bfd_target_vector[0];
      if (abfd)
        {
          abfd->xvec = target;
          abfd->target_defaulted = TRUE;
        }
      return target;
    }

Sounds like we have a couple issues. The mips backend not handling weird 
abi/isa combinations and GDB not preventing clearly incompatible core 
files from proceeding further into processing in the target's backend?