PR 25562: New binutils testsuite failures

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

PR 25562: New binutils testsuite failures

Sourceware - binutils list mailing list
Hi Jozef,

  I am seeing some new binutils testsuite failures for various different
  targets:

Checking Binutils in: arm-wince-pe ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: i686-pc-cygwin ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: i686-pc-mingw32 ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: mcore-pe ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: mips-elf ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: mips-sgi-irix6 ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: mmix-mmixware ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: tx39-elf ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: x86_64-pc-cygwin ... BIN REGRESSION: objcopy executable (pr25662)  
Checking Binutils in: x86_64-pc-mingw64 ... BIN REGRESSION: objcopy executable (pr25662)  

  Would you mind taking a look please ?

Cheers
  Nick
 

Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Jozef Lawrynowicz-2
On Mon, 30 Mar 2020 10:48:32 +0100
Nick Clifton <[hidden email]> wrote:

> Hi Jozef,
>
>   I am seeing some new binutils testsuite failures for various different
>   targets:
>
> Checking Binutils in: arm-wince-pe ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: i686-pc-cygwin ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: i686-pc-mingw32 ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: mcore-pe ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: mips-elf ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: mips-sgi-irix6 ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: mmix-mmixware ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: tx39-elf ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: x86_64-pc-cygwin ... BIN REGRESSION: objcopy executable (pr25662)  
> Checking Binutils in: x86_64-pc-mingw64 ... BIN REGRESSION: objcopy executable (pr25662)  
>
>   Would you mind taking a look please ?

Hi Nick,

The test is working as expected, for those targets an objcopy of an executable
is not creating an exact copy of the original file.

Alan Modra has already done a triage of the failures (see this thread
https://sourceware.org/pipermail/binutils/2020-March/110422.html), and has setup
XFAILs for some targets. For the remaining failures I think the maintainers of
those targets need to investigate to see if XFAILs are necessary or if there is
a real bug in the toolchain.

For the PE targets the datestamp in the executable file is not being copied by
objcopy.

For the MIPS targets, objcopy has added the names of output sections to the
string table, where in the linked executable they were not in the string table.

I can file bugzillas for these if you would like.

Thanks,
Jozef
>
> Cheers
>   Nick
>  
>

Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Sourceware - binutils list mailing list
Hi Jozef,

> The test is working as expected, for those targets an objcopy of an executable
> is not creating an exact copy of the original file.

Ah, OK.

> For the PE targets the datestamp in the executable file is not being copied by
> objcopy.

The old determinism problem.  *sigh*  We should probably just xfail these
targets since we know that the binaries can never be the same.

> For the MIPS targets, objcopy has added the names of output sections to the
> string table, where in the linked executable they were not in the string table.

Where were the names stored then, if not in the string table ?  Or did the
objcopy actually add new section symbols ?

This does sound like something that the MIPS maintainers ought to investigate
and decide whether it is a real bug or not.

> I can file bugzillas for these if you would like.

No, that should not be necessary.  I will check in an update for the PE targets
once I have tested it locally.

Cheers
  Nick


Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Jozef Lawrynowicz-2
On Mon, 30 Mar 2020 13:26:53 +0100
Nick Clifton <[hidden email]> wrote:

> Hi Jozef,
>
> > The test is working as expected, for those targets an objcopy of an executable
> > is not creating an exact copy of the original file.  
>
> Ah, OK.
>
> > For the PE targets the datestamp in the executable file is not being copied by
> > objcopy.  
>
> The old determinism problem.  *sigh*  We should probably just xfail these
> targets since we know that the binaries can never be the same.

I should clarify, the datestamp is actually just being reset completely to
the epoch, rather than being set to the time of the objcopy.

>
> > For the MIPS targets, objcopy has added the names of output sections to the
> > string table, where in the linked executable they were not in the string table.  
>
> Where were the names stored then, if not in the string table ?  Or did the
> objcopy actually add new section symbols ?

They're in .shstrab originally, but then the objcopy adds them to .strtab as
well. Here's the readelf output (bintest is the original file):

$ ../readelf -p .strtab -p .shstrtab bintest

String dump of section '.strtab':
  [     1]  tmpdir/bintest.o
  [    12]  a
  [    14]  c
  [    16]  _start


String dump of section '.shstrtab':
  [     1]  .symtab
  [     9]  .strtab
  [    11]  .shstrtab
  [    1b]  .data
  [    21]  .text
  [    27]  .MIPS.abiflags
  [    36]  .bss
  [    3b]  .reginfo
  [    44]  .gnu.attributes

$ ../readelf -p .strtab -p .shstrtab copy

String dump of section '.strtab':
  [     1]  .data
  [     7]  .text
  [     d]  .MIPS.abiflags
  [    1c]  .bss
  [    21]  .reginfo
  [    2a]  .gnu.attributes
  [    3a]  tmpdir/bintest.o
  [    4b]  c
  [    4d]  _start


String dump of section '.shstrtab':
  [     1]  .symtab
  [     9]  .strtab
  [    11]  .shstrtab
  [    1b]  .data
  [    21]  .text
  [    27]  .MIPS.abiflags
  [    36]  .bss
  [    3b]  .reginfo
  [    44]  .gnu.attributes

This means that in the copied executable you can see the names of the SECTION
type symbols in .symtab:

$ ../readelf -s bintest

Symbol table '.symtab' contains 11 entries:
   Num:    Value  Size Type    Bind   Vis      Ndx Name
     0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 00000000     0 SECTION LOCAL  DEFAULT    1
     2: 00001204     0 SECTION LOCAL  DEFAULT    2
     3: 00001208     0 SECTION LOCAL  DEFAULT    3
     4: 00000201     0 SECTION LOCAL  DEFAULT    4
     5: 00000000     0 SECTION LOCAL  DEFAULT    5
     6: 00000000     0 SECTION LOCAL  DEFAULT    6
............. snip ........

$ ../readelf -s copy

Symbol table '.symtab' contains 11 entries:
   Num:    Value  Size Type    Bind   Vis      Ndx Name
     0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 00000000     0 SECTION LOCAL  DEFAULT    1 .data
     2: 00001204     0 SECTION LOCAL  DEFAULT    2 .text
     3: 00001208     0 SECTION LOCAL  DEFAULT    3 .MIPS.abiflags
     4: 00000201     0 SECTION LOCAL  DEFAULT    4 .bss
     5: 00000000     0 SECTION LOCAL  DEFAULT    5 .reginfo
     6: 00000000     0 SECTION LOCAL  DEFAULT    6 .gnu.attributes
............... snip ............


>
> This does sound like something that the MIPS maintainers ought to investigate
> and decide whether it is a real bug or not.
>
> > I can file bugzillas for these if you would like.  
>
> No, that should not be necessary.  I will check in an update for the PE targets
> once I have tested it locally.

Ok sounds good, thanks.
Jozef

>
> Cheers
>   Nick
>
>

Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Sourceware - binutils list mailing list
In reply to this post by Sourceware - binutils list mailing list
On Mon, Mar 30, 2020 at 01:26:53PM +0100, Nick Clifton via Binutils wrote:

> Hi Jozef,
>
> > The test is working as expected, for those targets an objcopy of an executable
> > is not creating an exact copy of the original file.
>
> Ah, OK.
>
> > For the PE targets the datestamp in the executable file is not being copied by
> > objcopy.
>
> The old determinism problem.  *sigh*  We should probably just xfail these
> targets since we know that the binaries can never be the same.

Except that objcopy -p ought to preserve the time, shouldn't it?

> > For the MIPS targets, objcopy has added the names of output sections to the
> > string table, where in the linked executable they were not in the string table.
>
> Where were the names stored then, if not in the string table ?  Or did the
> objcopy actually add new section symbols ?

ELF section symbols normally have 0 in st_name, with an implicit name
taken from their section.  MIPS for backwards compatibility with other
linkers gives them a string table entry.  objcopy decided the copy
needed that backward compat, while ld decided the original didn't.

>
> This does sound like something that the MIPS maintainers ought to investigate
> and decide whether it is a real bug or not.
>
> > I can file bugzillas for these if you would like.
>
> No, that should not be necessary.  I will check in an update for the PE targets
> once I have tested it locally.
>
> Cheers
>   Nick
>

--
Alan Modra
Australia Development Lab, IBM
Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Sourceware - binutils list mailing list
Hi Guys,

> Alan Modra wrote:
> Except that objcopy -p ought to preserve the time, shouldn't it?

It certainly should.  I am testing a local patch now that should fix
this...

Cheers
  Nick

Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Sourceware - binutils list mailing list
In reply to this post by Sourceware - binutils list mailing list
Hi Guys,

  Right, I am checking in the attached patch which makes objcopy's -p
  work with PE format files.  I had to change the insert_timestamp field
  in the pe_data structure to an integer containing the time to put in
  the header.  The value can be 0 for a deterministic output, and it can
  also be -1, which is interpreted as 'insert the current time'.

  I have rerun the pr25662 tests with this change and all of the PE
  format targets now pass.

Cheers
  Nick

bfd/ChangeLog
2020-03-30  Nick Clifton  <[hidden email]>

        PR binutils/pr25662
        * libcoff-in.h (struct pe_tdata): Rename the insert_timestamp
        field to timestamp and make it an integer.
        * libcoff.h: Regenerate.
        * peXXigen.c (_bfd_XXi_only_swap_filehdr_out): Test the timestamp
        field in the pe_data structure rather than the insert_timestamp
        field.

binutils/ChangeLog
2020-03-30  Nick Clifton  <[hidden email]>

        PR binutils/25662
        * objcopy.c (copy_object): When copying PE format files set the
        timestamp field in the pe_data structure if the preserve_dates
        flag is set.
        * testsuite/binutils-all/objcopy.exp (objcopy_test) Use
        --preserve-dates in place of the -p option, in order to make its
        effect more obvious.

ld/ChangeLog
2020-03-30  Nick Clifton  <[hidden email]>

        PR binutils/25662
        * emultempl/pe.em (after_open): Replace initialisation of the
        insert_timestamp field in the pe_data structure with an
        initialisation of the timestamp field.
        * emultemp/pep.em: Likewise.
        * pe-dll.c (fill_edata): Use the timestamp field in the pe_data
        structure instead of the insert_timestamp field.

pr25662.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PR 25562: New binutils testsuite failures

Maciej W. Rozycki
In reply to this post by Sourceware - binutils list mailing list
On Mon, 30 Mar 2020, Alan Modra via Binutils wrote:

> > Where were the names stored then, if not in the string table ?  Or did the
> > objcopy actually add new section symbols ?
>
> ELF section symbols normally have 0 in st_name, with an implicit name
> taken from their section.  MIPS for backwards compatibility with other
> linkers gives them a string table entry.  objcopy decided the copy
> needed that backward compat, while ld decided the original didn't.

 This came from commit 174fd7f95561 ("New bfd elf hook: force naming of
local section symbols"),
<https://sourceware.org/ml/binutils/2004-02/msg00072.html>; I guess the
hook only needs to return TRUE iff (elf_elfheader (abfd)->e_type ==
ET_REL).  Like below.

 However this is actually not enough to fix the `pr25662' test case,
because apparently we have another MIPS IRIX-compatibility bug, which
makes `sh_info' set differently for `.symtab' by `objcopy' -- it's 10 in
input and 7 in output; of course the underlying cause is IRIX's different
interpretation of symbol table splitting.  Which happens regardless of
this change and is possibly more serious than just an inconsistency with
section symbol names.

 So I don't feel like committing this as it stands: I think we need the
other bug tracked down and then all this mess covered with proper
testsuite cases before I feel comfortable with making such changes.

  Maciej
---
 bfd/elfxx-mips.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

binutils-mips-bfd-local-section-symbols-et-rel.diff
Index: binutils/bfd/elfxx-mips.c
===================================================================
--- binutils.orig/bfd/elfxx-mips.c
+++ binutils/bfd/elfxx-mips.c
@@ -7284,7 +7284,7 @@ _bfd_mips_elf_eh_frame_address_size (bfd
 bfd_boolean
 _bfd_mips_elf_name_local_section_symbols (bfd *abfd)
 {
-  return SGI_COMPAT (abfd);
+  return elf_elfheader (abfd)->e_type == ET_REL && SGI_COMPAT (abfd);
 }
 
 /* Work over a section just before writing it out.  This routine is