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

[Luis Machado <lgustavo@codesourcery.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?