From: Alan Modra <amodra@gmail.com>
To: John Baldwin <jhb@freebsd.org>
Cc: Tom de Vries <tdevries@suse.de>,
gdb-patches@sourceware.org, binutils@sourceware.org
Subject: Re: [PATCH][gdb] Fix fbsd core matching
Date: Tue, 14 Jun 2022 09:35:46 +0930 [thread overview]
Message-ID: <YqfQ2qTmG3KWcwor@squeak.grove.modra.org> (raw)
In-Reply-To: <790e4948-9a70-a7c7-f562-a2bf16a54506@FreeBSD.org>
On Thu, Jun 09, 2022 at 08:59:37AM -0700, John Baldwin wrote:
> On 6/9/22 1:58 AM, Tom de Vries via Gdb-patches wrote:
> > Hi,
> >
> > With an --enable-targets=all build and target board unix/-m32 I run into a
> > FAIL in test-case gdb.base/corefile.exp:
> > ...
> > (gdb) file outputs/gdb.base/corefile/corefile^M
> > Reading symbols from outputs/gdb.base/corefile/corefile...^M
> > (gdb) core-file outputs/gdb.base/corefile/corefile.core^M
> > warning: core file may not match specified executable file.^M
> > [New LWP 12011]^M
> > Core was generated by `outputs/gdb.base/corefile/co'.^M
> > Program terminated with signal SIGABRT, Aborted.^M
> > (gdb) FAIL: gdb.base/corefile.exp: core-file warning-free
> > ...
> >
> > The warning is there because of this mismatch between core and exec:
> > ...
> > (gdb) p core_bfd->xvec
> > $3 = (const struct bfd_target *) 0x20112a0 <i386_elf32_fbsd_vec>
> > (gdb) p exec_bfd->xvec
> > $4 = (const struct bfd_target *) 0x2010b00 <i386_elf32_vec>
> > ...
> >
> > In the exec case, the detected architecture is i386_elf32_vec because this bit
> > of code in elfcode.h:elf_object_p():
> > ...
> > if (ebd->elf_machine_code != EM_NONE
> > && i_ehdrp->e_ident[EI_OSABI] != ebd->elf_osabi
> > && ebd->elf_osabi != ELFOSABI_NONE)
> > goto got_wrong_format_error;
> > ...
> > prevents i386_elf32_fbsd from matching.
> >
> > Fix the core matching by copying that code to elfcore.h:elf_core_file_p().
> >
> > Tested on x86_64-linux.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29227
> >
> > Any comments?
Looks good.
> Looking at elfcore.h, it seems to have not gotten changes made to elfcode.h over
> time and is a bit rotted. I suspect that all of changes made in commit 0aabe54e6222
> that added these lines in elfcode.h (along with several other changes) need to
> be applied to this function in elfcore.h, not just adding these lines.
Yes, the commit 0aabe54e6222 changes likely should go in too. I'm a
little wary of adding all the sanity checks to elf_core_file_p since
that might result in some core files not being recognised at all. For
example, despite the FIXME I'd guess leaving out the EI_VERSION check
was deliberate. The following seems reasonable to me. Please test.
diff --git a/bfd/elfcore.h b/bfd/elfcore.h
index 809f6711aed..59b73bd57de 100644
--- a/bfd/elfcore.h
+++ b/bfd/elfcore.h
@@ -149,37 +149,14 @@ elf_core_file_p (bfd *abfd)
&& (ebd->elf_machine_alt1 == 0
|| i_ehdrp->e_machine != ebd->elf_machine_alt1)
&& (ebd->elf_machine_alt2 == 0
- || i_ehdrp->e_machine != ebd->elf_machine_alt2))
- {
- const bfd_target * const *target_ptr;
-
- if (ebd->elf_machine_code != EM_NONE)
- goto wrong;
-
- /* This is the generic ELF target. Let it match any ELF target
- for which we do not have a specific backend. */
+ || i_ehdrp->e_machine != ebd->elf_machine_alt2)
+ && ebd->elf_machine_code != EM_NONE)
+ goto wrong;
- for (target_ptr = bfd_target_vector; *target_ptr != NULL; target_ptr++)
- {
- const struct elf_backend_data *back;
-
- if ((*target_ptr)->flavour != bfd_target_elf_flavour)
- continue;
- back = xvec_get_elf_backend_data (*target_ptr);
- if (back->s->arch_size != ARCH_SIZE)
- continue;
- if (back->elf_machine_code == i_ehdrp->e_machine
- || (back->elf_machine_alt1 != 0
- && i_ehdrp->e_machine == back->elf_machine_alt1)
- || (back->elf_machine_alt2 != 0
- && i_ehdrp->e_machine == back->elf_machine_alt2))
- {
- /* target_ptr is an ELF backend which matches this
- object file, so reject the generic ELF target. */
- goto wrong;
- }
- }
- }
+ if (ebd->elf_machine_code != EM_NONE
+ && i_ehdrp->e_ident[EI_OSABI] != ebd->elf_osabi
+ && ebd->elf_osabi != ELFOSABI_NONE)
+ goto wrong;
/* If there is no program header, or the type is not a core file, then
we are hosed. */
@@ -199,6 +176,9 @@ elf_core_file_p (bfd *abfd)
Elf_Internal_Shdr i_shdr;
file_ptr where = (file_ptr) i_ehdrp->e_shoff;
+ if (i_ehdrp->e_shoff < sizeof (x_ehdr))
+ goto wrong;
+
/* Seek to the section header table in the file. */
if (bfd_seek (abfd, where, SEEK_SET) != 0)
goto fail;
> I've cc'd Alan who made that commit above to elfcore.h. There also seem to be
> some other inconsistencies between the functions in elfcore.h and and elfcode.h
> and perhaps the common functionality in these functions could be refactored
> into a separate function to avoid the code duplication?
>
> Note that the issue isn't really FreeBSD specific, I just suspect that the FreeBSD
> i386 vec happens to be sorted first in the list for some reason.
> > Thanks,
> > - Tom
> >
> > [gdb] Fix fbsd core matching
> >
> > ---
> > bfd/elfcore.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/bfd/elfcore.h b/bfd/elfcore.h
> > index 809f6711aed..4ce81e2e383 100644
> > --- a/bfd/elfcore.h
> > +++ b/bfd/elfcore.h
> > @@ -272,6 +272,11 @@ elf_core_file_p (bfd *abfd)
> > && ebd->elf_machine_code != EM_NONE)
> > goto fail;
> > + if (ebd->elf_machine_code != EM_NONE
> > + && i_ehdrp->e_ident[EI_OSABI] != ebd->elf_osabi
> > + && ebd->elf_osabi != ELFOSABI_NONE)
> > + goto fail;
> > +
> > /* Let the backend double check the format and override global
> > information. We do this before processing the program headers
> > to allow the correct machine (as opposed to just the default
>
>
> --
> John Baldwin
--
Alan Modra
Australia Development Lab, IBM
next prev parent reply other threads:[~2022-06-14 0:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-09 8:58 Tom de Vries
2022-06-09 15:59 ` John Baldwin
2022-06-14 0:05 ` Alan Modra [this message]
2022-06-14 9:03 ` Tom de Vries
2022-06-16 0:52 ` Alan Modra
2022-06-16 8:04 ` Tom de Vries
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=YqfQ2qTmG3KWcwor@squeak.grove.modra.org \
--to=amodra@gmail.com \
--cc=binutils@sourceware.org \
--cc=gdb-patches@sourceware.org \
--cc=jhb@freebsd.org \
--cc=tdevries@suse.de \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).