Bug 16807 - Bad behavior with resources of Windows applications with recent binutils upgrade on Cygwin64
Summary: Bad behavior with resources of Windows applications with recent binutils upgr...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.25
: P2 normal
Target Milestone: ---
Assignee: Nick Clifton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-03 20:32 UTC by Angelo Graziosi
Modified: 2014-06-03 16:21 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
The tar-ball containing the source of test case (2.04 KB, application/x-bzip)
2014-04-03 20:32 UTC, Angelo Graziosi
Details
Proposed patch (535 bytes, patch)
2014-04-11 15:48 UTC, Nick Clifton
Details | Diff
test.c (35 bytes, text/plain)
2014-05-29 22:40 UTC, Ken Brown
Details
emacs.rc (456 bytes, text/plain)
2014-05-29 22:44 UTC, Ken Brown
Details
default-manifest.rc (541 bytes, text/plain)
2014-05-29 22:44 UTC, Ken Brown
Details
emacs-x64.manifest (599 bytes, application/xml)
2014-05-30 11:58 UTC, Ken Brown
Details
Additional patch - page align the .rsrc section (935 bytes, patch)
2014-06-02 14:55 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Angelo Graziosi 2014-04-03 20:32:41 UTC
Created attachment 7528 [details]
The tar-ball containing the source of test case

It seems that the attached test case does not work rightly with current binutils [1] on Cygwin64 (but probably is a general issue regarding all Windows applications).

The Help menu in the test case contains two About items, and About2 is created from About1 with Copy/Paste. Now, while clicking on About1 works as expected, the click on About2 does not produce anything and fails. If I comment out the FILE_EXIT stuff (from .c, .h and .rc files) the issue is for About1 and not for About2 (it "swapped"!).

If, as binutils, I use the old binutils-2.23.52-5.tar.bz2 package, all works as expected.

I can reproduce the issue also with the current mingw-w64 binutils [2] (and the relative mingw-w64 compilers, i686-w64-mingw32, x86_64-w64-mingw32).

The issue does not exist using mingw-binutils-2.23.1-1 (i686-pc-mingw32).


The attached tar-ball can be used in this way:

$ tar -xvf test_case_b.tar.bz2
$ cd test_case_b
$ ./dlg_one.out

To create the test case I have adapted an application found on the web.

Consider that I discovered the issue with a more complicated Fortran application.

Ciao,
Angelo.

P.S. See also the thread on Cygwin list:
http://cygwin.com/ml/cygwin/2014-04/msg00016.html

----
[1] binutils-2.24.51-2
[2] mingw64-i686-binutils-2.24.0.1.acd6540-1, mingw64-x86_64-binutils-2.24.0.1.acd6540-1
Comment 1 Nick Clifton 2014-04-04 17:00:20 UTC
Hi Corinna,

   I have started looking into this bug, but I am a bit stumped. :-(

   I guess that the problem is connected with the default manifest being 
added in to the executable's resources.  But looking at the decompiled 
resources in the dlg_one.out executable everything looks fine.  Then I 
started to wonder.  Maybe it is the fact that there are now *3* top 
level entries in the resource Type table instead of 2.  Maybe the 
dlg_one.c application only expects there to be 2 entries and somehow it 
is getting confused.  Unfortunately I am not enough of a Window resource 
expert to tell.  But you are...  So - is this a possibility ?

Cheers
   Nick
Comment 2 Angelo Graziosi 2014-04-07 21:15:57 UTC
(In reply to Nick Clifton from comment #1)
> Hi Corinna,
> 
>    I have started looking into this bug, but I am a bit stumped. :-(
> 
>    I guess that the problem is connected with the default manifest being 
> added in to the executable's resources.  But looking at the decompiled 
> resources in the dlg_one.out executable everything looks fine.  Then I 
> started to wonder.  Maybe it is the fact that there are now *3* top 
> level entries in the resource Type table instead of 2.  Maybe the 
> dlg_one.c application only expects there to be 2 entries and somehow it 
> is getting confused.  Unfortunately I am not enough of a Window resource 
> expert to tell.  But you are...  So - is this a possibility ?

Just for completeness, the workaround described here

   http://cygwin.com/ml/cygwin/2014-04/msg00134.html

works for me.

It seems to confirm that the origin of these issues is this defaul manifest in new binutils releases.


Ciao,
  Angelo.
Comment 3 Nick Clifton 2014-04-11 15:48:57 UTC
Created attachment 7550 [details]
Proposed patch
Comment 4 Nick Clifton 2014-04-11 15:50:36 UTC
Hi Angelo,  Hi Corinna,

  Please could you try out the uploaded patch.

  I think that it should work for this particular test case, but I also think that it will fail when you have multiple resources with identical entries.  If you can find a test case for that and try it out too that would be very helpful.

Cheers
  Nick
Comment 5 Angelo Graziosi 2014-04-11 20:22:55 UTC
(In reply to Nick Clifton from comment #4)
> Hi Angelo,  Hi Corinna,
> 
>   Please could you try out the uploaded patch.
> 
>   I think that it should work for this particular test case, but I also
> think that it will fail when you have multiple resources with identical
> entries.  If you can find a test case for that and try it out too that would
> be very helpful.

Hmm... I have done the following and for me it doesn't work... maybe I didn't all the right things.

I downloaded the source of current binutils (binutils-2.24.51-2) in Cygwin, then I have applied the patch,

  $ patch tmp/binutils-2.24.51-2/bfd/peXXigen.c binutils-peXXigen.c.patch


and have built with

  $ ./configure --prefix=/usr/local/binutils-test
  $ make
  $ make install


After this, I have started another mintty window and changed the PATH,

  $ export PATH="/usr/local/binutils-test/bin:$PATH"


and compiled my test case,

  $ windres dlg_one.rc -O coff -o dlg_one.res
  $ gcc -Wall -mwindows dlg_one.c dlg_one.res -o dlg_one.out


but it doesn't work as expected, always the same issues.


But, as I said, probably I didn't the right things. At this point, someone more expert in binutilis should test the patch.


Thanks,
 Angelo.
Comment 6 Nick Clifton 2014-04-23 16:52:14 UTC
Hi Angelo,

  Hmm, the patch does work in my tests.  My guess is either the patched linker was not installed, or that it was installed but gcc was using the old linker.

>  $ ./configure --prefix=/usr/local/binutils-test
>  $ make
>  $ make install

Could you try adding:

   $ make all-ld
   $ make install-ld
   $ ls /usr/local/binutils-test/bin


>  $ gcc -Wall -mwindows dlg_one.c dlg_one.res -o dlg_one.out

Could you adding "-v -Wl,-debug" to the gcc command line, and see if you can find which ld executable is being run ?

Cheers
  Nick
Comment 7 Sourceware Commits 2014-04-24 10:18:08 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  3714081cb37fc60f3262b4c64e81539eb4f3592f (commit)
      from  2a87f7b84f1a3d101a19d7008802172ff50596e5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3714081cb37fc60f3262b4c64e81539eb4f3592f

commit 3714081cb37fc60f3262b4c64e81539eb4f3592f
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Apr 24 11:15:43 2014 +0100

    Fix PE/COFF resource merging problems.  There were two issues:
    
      1. Strings (and then resource data) must follow immediately after
         the end of the tables.
      2. Units of resource data must be 8-byte aligned.
    
    	PR ld/16807
    	* peXXigen.c (struct rsrc_regions): New structure.
    	(rsrc_print_resource_directory): Use new structure.  Include
    	offset of directory in listing.
    	(rsrc_print_resource_entry): Likewise.
    	(rsrc_print_section): Likewise.
    	(rsrc_count_entries): Do not increment sizeof_strings or
    	sizeof_leaves.
    	(rsrc_count_directory): Do not increment sizeof_tables.
    	(rsrc_compute_region_sizes): New function.
    	(rsrc_write_leaf): Maintain 8-byte alignment for resource data.
    	(rsrc_process_section): Compute size of regions after merging
    	entries.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog  |   16 +++++
 bfd/peXXigen.c |  205 +++++++++++++++++++++++++++++++++++---------------------
 2 files changed, 144 insertions(+), 77 deletions(-)
Comment 8 Nick Clifton 2014-04-24 10:20:11 UTC
Hi Angelo,

  Sorry about my previous post.  You were right - the uploaded patch did not work.  But I have now tracked down the problem - an undocumented requirement that resource data be 8-byte aligned.  I have checked in a patch to fix this and you should now find that the linker merges resources correctly.

Cheers
  Nick
Comment 9 Angelo Graziosi 2014-04-24 11:42:18 UTC
(In reply to Nick Clifton from comment #8)

Ciao Nick,

> Hi Angelo,
> 
>   Sorry about my previous post.  You were right - the uploaded patch did not
> work.  But I have now tracked down the problem - an undocumented requirement
> that resource data be 8-byte aligned.  I have checked in a patch to fix this
> and you should now find that the linker merges resources correctly.
> 

yesterday I haven't time to follow your request. I am sure you have done a very good work.. :)

As I flagged on Cygwin list, http://cygwin.com/ml/cygwin/2014-04/msg00405.html, the issue is already fixed on mingw64 side..


Thanks,
 Angelo.
Comment 10 Angelo Graziosi 2014-04-25 17:02:14 UTC
(In reply to Angelo Graziosi from comment #9)

Just for completeness,

> (In reply to Nick Clifton from comment #8)
> > 
> >   Sorry about my previous post.  You were right - the uploaded patch did not
> > work.  But I have now tracked down the problem - an undocumented requirement
> > that resource data be 8-byte aligned.  I have checked in a patch to fix this
> > and you should now find that the linker merges resources correctly.
> > 
> 
> yesterday I haven't time to follow your request. I am sure you have done a
> very good work.. :)
> 
> As I flagged on Cygwin list,
> http://cygwin.com/ml/cygwin/2014-04/msg00405.html, the issue is already
> fixed on mingw64 side..

I tried to build today trunk of binutils but it fails, so I tried with the sources which come with the mingw64 package (2.24.0.3.85cf705-1). I did:

./configure --prefix=$HOME/binutils
make
make install

Then I added the $HOME/binutils/bin to PATH...

export PATH="$HOME/binutils:$PATH"


..and now my applications work as expected also on Cygwin side.. :-)

I would say your patches are right, definitively.. :)


Thanks,
Angelo.
Comment 11 Ken Brown 2014-05-29 22:40:39 UTC
Created attachment 7620 [details]
test.c
Comment 12 Ken Brown 2014-05-29 22:43:00 UTC
There are still problems with merged resources.  Namely, the resulting executable after being stripped won't run, at least in some circumstances.  Here's a simple test case on 64-bit Cygwin with the just-released binutils- 2.24.51-3.  It uses the attachments test.c, emacs.rc, and default-manifest.rc.

1. Compile the three source files:

  $ gcc -c test.c
  $ windres -o emacs.o emacs.rc
  $ windres -o default-manifest.o default-manifest.rc

2. Link them:

  $ gcc -o test test.o emacs.o default-manifest.o

3. Strip the resulting executable:

  $ strip test.exe

4. Try to run it:

  $ ./test.exe
  -bash: ./test.exe: cannot execute binary file

5. Observe that the resource section of the stripped executable is messed up:

  $ objdump -j .rsrc -s test.exe

  test.exe:     file format pei-x86-64

  Contents of section .rsrc:
  objdump: Reading section failed
Comment 13 Ken Brown 2014-05-29 22:44:13 UTC
Created attachment 7621 [details]
emacs.rc
Comment 14 Ken Brown 2014-05-29 22:44:43 UTC
Created attachment 7622 [details]
default-manifest.rc
Comment 15 Angelo Graziosi 2014-05-30 09:47:20 UTC
Ciao Ken,

(In reply to Ken Brown from comment #12)
> There are still problems with merged resources.  Namely, the resulting
> executable after being stripped won't run, at least in some circumstances. 
> Here's a simple test case on 64-bit Cygwin with the just-released binutils-
> 2.24.51-3.  It uses the attachments test.c, emacs.rc, and
> default-manifest.rc.
> 
> 1. Compile the three source files:
> 
>   $ gcc -c test.c
>   $ windres -o emacs.o emacs.rc

when I try this I get :(

windres: impossibile aprire file "emacs-x64.manifest": No such file or directory


i.e. "cannot open emacs-x64.manifest> No such file..."


indeed

$ head emacs.rc
1 24 "emacs-x64.manifest"

#ifndef VS_VERSION_INFO
#define VS_VERSION_INFO 1
#endif

VS_VERSION_INFO VERSIONINFO
 FILEVERSION 24,3,50,0
 PRODUCTVERSION 24,3,50,0
 FILEFLAGSMASK 0x3FL


Ciao,
Angelo.


>   $ windres -o default-manifest.o default-manifest.rc
> 
> 2. Link them:
> 
>   $ gcc -o test test.o emacs.o default-manifest.o
> 
> 3. Strip the resulting executable:
> 
>   $ strip test.exe
> 
> 4. Try to run it:
> 
>   $ ./test.exe
>   -bash: ./test.exe: cannot execute binary file
> 
> 5. Observe that the resource section of the stripped executable is messed up:
> 
>   $ objdump -j .rsrc -s test.exe
> 
>   test.exe:     file format pei-x86-64
> 
>   Contents of section .rsrc:
>   objdump: Reading section failed
Comment 16 Ken Brown 2014-05-30 11:58:40 UTC
Created attachment 7623 [details]
emacs-x64.manifest

Sorry, I forgot to attach this.
Comment 17 Christopher G. Faylor 2014-05-30 17:26:54 UTC
(In reply to Ken Brown from comment #12)
> There are still problems with merged resources.  Namely, the resulting
> executable after being stripped won't run, at least in some circumstances. 
> Here's a simple test case on 64-bit Cygwin with the just-released binutils-
> 2.24.51-3.  It uses the attachments test.c, emacs.rc, and
> default-manifest.rc.
> 
> 1. Compile the three source files:
> 
>   $ gcc -c test.c
>   $ windres -o emacs.o emacs.rc
>   $ windres -o default-manifest.o default-manifest.rc
> 
> 2. Link them:
> 
>   $ gcc -o test test.o emacs.o default-manifest.o
> 
> 3. Strip the resulting executable:
> 
>   $ strip test.exe
> 
> 4. Try to run it:
> 
>   $ ./test.exe
>   -bash: ./test.exe: cannot execute binary file

I see the above behavior even without stripping.
Comment 18 Angelo Graziosi 2014-05-31 22:18:07 UTC
(In reply to Ken Brown from comment #16)
> Created attachment 7623 [details]
> emacs-x64.manifest
> 
> Sorry, I forgot to attach this.

Thanks... ;-) Now I can try your test case. I can confirm the issues at step 4. ONLY if I strip test.exe.

Ciao,
 Angelo.
Comment 19 Nick Clifton 2014-06-02 14:55:23 UTC
Created attachment 7625 [details]
Additional patch - page align the .rsrc section
Comment 20 Nick Clifton 2014-06-02 14:56:43 UTC
Hi Ken, Angelo, Chris,

  This problem appears to be due to requirement to page-align the .rsrc section.  Please could you try out the newly uploaded patch and let me know if it resolves the problem for you.

Cheers
  Nick
Comment 21 Ken Brown 2014-06-02 22:15:16 UTC
(In reply to Nick Clifton from comment #20)
> Hi Ken, Angelo, Chris,
> 
>   This problem appears to be due to requirement to page-align the .rsrc
> section.  Please could you try out the newly uploaded patch and let me know
> if it resolves the problem for you.

That seems to have fixed it.  Thanks.

Ken
Comment 22 Sourceware Commits 2014-06-03 16:21:07 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  c32abae8456a2cb959862626b5ff9ebdd1543514 (commit)
      from  1e2ccb612d2b61014bb7e9fef3eb58e4947b9d2b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c32abae8456a2cb959862626b5ff9ebdd1543514

commit c32abae8456a2cb959862626b5ff9ebdd1543514
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Jun 3 17:18:27 2014 +0100

    This is to fix a further problem with merging resource sections.  It turns
    out that the section is supposed to be page-aligned, but the newly merged
    section was not being padded out to a page boundary.  This meant that when
    the executable was stripped a badly sized .rsrc section was written out.
    
    	PR ld/16807
    	* peXXigen.c (rsrc_process_section): Page align the new contents
    	befgore writing out.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog  |    6 ++++++
 bfd/peXXigen.c |   32 ++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)
Comment 23 Nick Clifton 2014-06-03 16:21:40 UTC
New patch applied.