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

Pedro Alves
2016-01-12T14:10:34Z

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