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

[Pedro Alves <palves@redhat.com>]

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?

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?

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?

> This also happens with a powerpc-targeted gdb and likely any other 
> architecture.
> 
> For powerpc we get lucky and end up "passing" this test because it has 
> no fatal failing conditions. It ends up displaying frame -1 for me, like so:
> 
> PC not available^M
> #-1 <unavailable> in ?? ()

Which is obviously bogus.

Thanks,
Pedro Alves