[PATCH][gdb] Fix fbsd core matching

[PATCH][gdb] Fix fbsd core matching

[Tom de Vries]

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?

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

Re: [PATCH][gdb] Fix fbsd core matching

[John Baldwin]

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?

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.

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

Re: [PATCH][gdb] Fix fbsd core matching

[Alan Modra]

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

Re: [PATCH][gdb] Fix fbsd core matching

[Tom de Vries]

On 6/14/22 02:05, Alan Modra wrote:
> 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.
> 

I've run the gdb testsuite on x86_64-linux with native and target board 
unix-/m32, no regressions and originally reported FAIL fixed.

Thanks,
- Tom

> 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
> 

Re: [PATCH][gdb] Fix fbsd core matching

[Alan Modra]

On Tue, Jun 14, 2022 at 11:03:22AM +0200, Tom de Vries wrote:
> I've run the gdb testsuite on x86_64-linux with native and target board
> unix-/m32, no regressions and originally reported FAIL fixed.

"Revert "Fix fbsd core matching"" now reverted.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH][gdb] Fix fbsd core matching

[Tom de Vries]

On 6/16/22 02:52, Alan Modra wrote:
> On Tue, Jun 14, 2022 at 11:03:22AM +0200, Tom de Vries wrote:
>> I've run the gdb testsuite on x86_64-linux with native and target board
>> unix-/m32, no regressions and originally reported FAIL fixed.
> 
> "Revert "Fix fbsd core matching"" now reverted.
> 

Hi Alan,

sorry for the noise due to the accidental commit, thanks for fixing this.

Thanks,
- Tom