Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@imgtec.com>
To: Luis Machado <lgustavo@codesourcery.com>
Cc: Pedro Alves <palves@redhat.com>, <gdb-patches@sourceware.org>,
	<jan.kratochvil@redhat.com>
Subject: Re: [PATCH] Handle loading improper core files gracefully in the mips backend.
Date: Tue, 12 Jan 2016 18:30:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1601121710020.5958@tp.orcam.me.uk> (raw)
In-Reply-To: <56951F29.7070000@codesourcery.com>

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


  parent reply	other threads:[~2016-01-12 18:30 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-08 18:32 Luis Machado
2016-01-09  3:02 ` Maciej W. Rozycki
2016-01-11 15:47   ` Luis Machado
2016-01-12 12:46     ` Pedro Alves
2016-01-12 13:25       ` Luis Machado
2016-01-12 14:10         ` Pedro Alves
2016-01-12 15:43           ` Luis Machado
2016-01-12 16:00             ` Pedro Alves
2016-01-12 18:30             ` Maciej W. Rozycki [this message]
2016-01-12 19:08               ` Pedro Alves
2016-02-02 12:58               ` Luis Machado
2016-02-02 14:19                 ` Pedro Alves
2016-02-02 14:22                   ` Pedro Alves
2016-02-04 21:01                     ` Maciej W. Rozycki
2016-02-05 11:29                       ` Luis Machado
2016-02-05 14:10                         ` Maciej W. Rozycki
2017-01-09 19:57               ` Luis Machado
2017-01-19 16:56                 ` Pedro Alves
2017-01-19 17:05                   ` Luis Machado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1601121710020.5958@tp.orcam.me.uk \
    --to=macro@imgtec.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=lgustavo@codesourcery.com \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox