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

[Luis Machado <lgustavo@codesourcery.com>]

On 01/12/2016 12:30 PM, Maciej W. Rozycki wrote:
> On Tue, 12 Jan 2016, Luis Machado wrote:
>
>>>> 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.
>
>  Or we could be talking to a live target with no executable selected at
> all.  This is also why there are settings like `set mips abi ...'
> available -- to let the user select the executable model for a target
> there's no other source of information about.
>
>>> 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?
>
>  The mapping between `e_machine' and `bfd_architecture' is only provided
> by individual BFD ELF target backends, via the ELF_MACHINE_CODE and
> ELF_ARCH macros.
>
>> 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>
>
>  Regardless, I'd expect a suitable generic ELF BFD target to be selected,
> which is what AFAICT `bfd_check_format' does.  It is called by our
> `core_open' function and has a `core_file_p' handler, which makes suitable
> checks including `e_machine' in particular, except for generic ELF BFD
> targets, which are special-cased (and always come last).  So in the
> absence of specific ELF target support in BFD I'd expect a compatible
> generic ELF target to be chosen rather than the default BFD target, which
> might be incompatible.
>
>> 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?
>
>  I have given it some thought and came to a conclusion that we should at
> least try being consistent.  Which means I think we should not try to
> handle files within the MIPS backend which would not be passed in the
> first place in an `--enable-targets=all' configuration.  Rather than
> checking `e_machine' explicitly I'd be leaning towards using BFD to detect
> such a situation though, perhaps by using a condition like
>
>   if (info.abfd != NULL
>       && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour
>       && bfd_get_arch (info.abfd) != bfd_arch_mips)
>     return NULL;
>
> (maybe with an additional error message) though ultimately I think it
> would make sense to define different BFD architecture codes for file
> formats which by definition carry no architecture information and for ones
> that do but are not supported.  Then for the formers we could continue
> selecting the target using the current algorithm and for the latters we'd
> just reject them as incompatible with the given backend -- all somewhere
> in generic code so that individual target backends do not have to repeat
> it all.
>
>  As to ABI, ISA, etc. settings -- these are internal to the MIPS backend,
> so its the backend's job to sanitise them.
>
>   Maciej
>

After quite a while, i'm revisiting this one.

Reading the thread once again, is my understanding correct that the 
first patch is more suitable now? Possibly with some cleanups?

https://sourceware.org/ml/gdb-patches/2016-01/msg00134.html

Regards,
Luis