Bug 69880 - Linking Windows resource + implicit 'default-manifest.o' creates bad .exe
Summary: Linking Windows resource + implicit 'default-manifest.o' creates bad .exe
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 5.3.0
: P3 normal
Target Milestone: ---
Assignee: Nick Clifton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-20 02:47 UTC by vszakats
Modified: 2017-02-23 16:36 UTC (History)
5 users (show)

See Also:
Host:
Target: mingw-w64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-07-13 00:00:00


Attachments
Test case #1 (8.90 KB, application/zip)
2016-07-12 17:43 UTC, vszakats
Details
Proposed patch (814 bytes, patch)
2016-07-13 12:20 UTC, Nick Clifton
Details | Diff
Proposed patch (936 bytes, patch)
2016-07-13 12:20 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description vszakats 2016-02-20 02:47:22 UTC
Hi,

Sorry for the ambiguous title, I'm not completely sure where/what is the culprit yet. Please bear with me.

While migrating to MSYS2 and its own mingw-w64 (5.3.0) toolchain (from a non-MSYS2 build of the same toolchain), I stumbled into a problem. When linking an application that has its own Windows resource, including a Windows Manifest, I noticed that the final .exe is not compressible with UPX (3.91) anymore, with the error:
   upx: my.exe: CantPackException: superfluous data between sections

Further investigation revealed that MSYS2 comes with a _default Windows manifest_, as part of the toolchain. They are packaged as `*windows-default-manifest`.

The GCC patch below made this feature possible, by automatically linking a default resource object, whenever it's found on disk:
   https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01378.html

The problem happens when the application is also supplying its own resource object (like in my case), and above default resource object is found as well. In this case _both_ objects get linked to the executable. It means that the .exe now has _two_ manifests. This by itself is already an ambiguity and some unnecessary bloat. But, the way these two objects are linked also causes the second instance to be apparently "orphaned" between two valid sections (.rsrc and .reloc in my particular case) of the executable.

This orphaned data seems to be confusing UPX, hence its fatal error.

The orphaned data can be discarded by using `strip -g test.exe` (or `strip --strip-unneeded test.exe`). In my case it deleted the extra data, and UPX worked again. I'm not sure this works in all possible cases though, and unfortunately it strips other information too, f.e. digital signatures. Plus this extra step requires a revamp of existing build scripts and tooling.

To my best knowledge mingw never handled linking multiple Windows resources well, but so far this could be easily mitigated by either merging .rc files in advance and/or avoiding to pass multiple resource objects explicitly. With the newly added implicit object feature, such care can no longer be taken.

IMO it'd be nice to have option to disable picking up the implicit `default-manifest.o` object (deleting it cannot always be done and/or cannot be done cleanly), or even better automatically drop it if there is any user supplied resource object. Cleverly merging them and dropping any duplicate entries, while giving the user-supplied object a priority would also be helpful, if feasible. Anyhow, the goal is to allow to have a _single_ manifest in the final executable, controlled by the user.

Here's my original bug report with some more aspects/information, plus steps to recreate a minimal example:
   https://github.com/Alexpux/MSYS2-packages/issues/454

-Viktor
Comment 1 Boris Kolpackov 2016-07-09 16:14:49 UTC
Could this be the same merging issue as discussed here:

https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01386.html

BTW, looking at the patch, I initially thought that one workaround would be to specify a directory (with -L) that contains a custom default-manifest.o so that it is found before the system one. This, however, does not work since GCC searches for it itself and only in its own library search paths (i.e., collect2.exe is already passed the absolute path).

Which suggests one possible way to support disabling this functionality: change default-manifest.o to libdefault-manifest.a and use the standard -ldefault-manifest logic to link it. This way the user will be able to provide their own (potentially empty) version.
Comment 2 Nick Clifton 2016-07-11 10:11:47 UTC
Hi Boris,

  Which version of the linker are you using ?  I ask because there have been
  several bugs reported with merging windows resources, but I think that these
  have all been fixed now.

> But, the way these two objects are linked also causes the second instance 
> to be apparently "orphaned" between two valid sections (.rsrc and .reloc 
> in my particular case) of the executable.

  If you are using the latest version of the linker and this is still happening
  then it would definitely seem to be a linker bug.  If so, it would be useful
  if you could report this on the binutils bugzilla system, and provide a test
  case that demonstrates the problem.


> Which suggests one possible way to support disabling this functionality:
> change default-manifest.o to libdefault-manifest.a and use the standard
> -ldefault-manifest logic to link it. This way the user will be able to
> provide their own (potentially empty) version.

  Personally I have no objections to this idea.  It would be nice however if
  one of the Cygwin maintainers could check it out and make sure that it does
  not stop default manifects from working in the way that they want.

Cheers
  Nick
Comment 3 Boris Kolpackov 2016-07-11 15:35:52 UTC
Ok, I've added support for embedding custom manifests in build2. The resulting .exe's work as expected: custom manifest is used instead of (or in addition to) the default one. However, with grep I can still see both manifests being present in the executable (which seems to be the problem the original reporter is experiencing).

ld --version
GNU ld (GNU Binutils) 2.25.1

I see this is not the latest generally, but it is the latest available from the MSYS2 repositories. The last commit here says 2.26 is still no usable:

https://github.com/Alexpux/MSYS2-packages/tree/master/binutils

Does it still make sense to file a binutils bug report?
Comment 4 Nick Clifton 2016-07-11 16:11:03 UTC
(In reply to Boris Kolpackov from comment #3)

> I see this is not the latest generally, but it is the latest available from
> the MSYS2 repositories. The last commit here says 2.26 is still no usable:

It is a shame that the commit does not say why 2.26 is unusable.  A reference
to a bug report would be very helpful.

> https://github.com/Alexpux/MSYS2-packages/tree/master/binutils
> 
> Does it still make sense to file a binutils bug report?

Yes, if the resource merge code is putting "orphaned" code in between sections
then this is definitely a bug and I would like to fix it.  A test case would be
a real help as well.
Comment 5 vszakats 2016-07-11 16:22:41 UTC
You may find a self-contained example under the GitHub link included in the original report. Direct link:

https://github.com/Alexpux/MSYS2-packages/issues/454#issuecomment-186433276

(This reproduced the problem at the time of the post, using the then recent MSYS2.)
Comment 6 Nick Clifton 2016-07-12 16:40:00 UTC
(In reply to vsz.bugzilla from comment #5)
> You may find a self-contained example under the GitHub link included in the
> original report. Direct link:

Unfortunately I do not have a MYSY2 installation, or even a Windows machine,
so I cannot reproduce the example.  Would it be possible for you to upload a zip
file archive containing the win.o, test.o, default-manifest.o and test.exe binaries ?  If you can capture the linker command line that produces the test.exe file as well, that would help too.

Cheers
  Nick
Comment 7 vszakats 2016-07-12 17:43:04 UTC
Created attachment 38883 [details]
Test case #1

Attached a test case that includes all referred sources, generated .o binaries, and .exe, plus the default manifest .o from the MSYS2 installation.
Comment 8 vszakats 2016-07-12 17:46:54 UTC
And the link command output:
```
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/lto-wrapper.exe
Target: i686-w64-mingw32
Configured with: ../gcc-5.4.0/configure --prefix=/mingw32 --with-local-prefix=/mingw32/local --build=i686-w64-mingw32 --host=i686-w64-mingw32 --target=i686-w64-mingw32 --with-native-system-header-dir=/mingw32/i686-w64-mingw32/include --libexecdir=/mingw32/lib --with-gxx-include-dir=/mingw32/include/c++/5.4.0 --enable-bootstrap --with-arch=i686 --with-tune=generic --enable-languages=c,lto,c++,objc,obj-c++,fortran,ada --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-time=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --enable-version-specific-runtime-libs --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw32 --with-mpfr=/mingw32 --with-mpc=/mingw32 --with-isl=/mingw32 --with-pkgversion='Rev1, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld --disable-sjlj-exceptions --with-dwarf2
Thread model: posix
gcc version 5.4.0 (Rev1, Built by MSYS2 project)
COMPILER_PATH=C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/;C:/msys64/mingw32/bin/../lib/gcc/;C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/bin/
LIBRARY_PATH=C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/;C:/msys64/mingw32/bin/../lib/gcc/;C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/lib/../lib/;C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../lib/;C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/lib/;C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../
COLLECT_GCC_OPTIONS='-v' '-m32' '-o' 'test.exe' '-s' '-mtune=generic' '-march=i686'
 C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/collect2.exe -plugin C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/liblto_plugin-0.dll -plugin-opt=C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/lto-wrapper.exe -plugin-opt=-fresolution=C:\Users\username\AppData\Local\Temp\ccx3Npce.res -plugin-opt=-pass-through=-lmingw32 -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_eh -plugin-opt=-pass-through=-lmoldname -plugin-opt=-pass-through=-lmingwex -plugin-opt=-pass-through=-lmsvcrt -plugin-opt=-pass-through=-lpthread -plugin-opt=-pass-through=-ladvapi32 -plugin-opt=-pass-through=-lshell32 -plugin-opt=-pass-through=-luser32 -plugin-opt=-pass-through=-lkernel32 -plugin-opt=-pass-through=-lmingw32 -plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_eh -plugin-opt=-pass-through=-lmoldname -plugin-opt=-pass-through=-lmingwex -plugin-opt=-pass-through=-lmsvcrt -m i386pe -Bdynamic -o test.exe -s C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/lib/../lib/crt2.o C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/crtbegin.o -LC:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0 -LC:/msys64/mingw32/bin/../lib/gcc -LC:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/lib/../lib -LC:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../lib -LC:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/lib -LC:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../.. test.o win.o -lmingw32 -lgcc -lgcc_eh -lmoldname -lmingwex -lmsvcrt -lpthread -ladvapi32 -lshell32 -luser32 -lkernel32 -lmingw32 -lgcc -lgcc_eh -lmoldname -lmingwex -lmsvcrt C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/../../../../i686-w64-mingw32/lib/../lib/default-manifest.o C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/5.4.0/crtend.o
```
Comment 9 Nick Clifton 2016-07-13 12:20:30 UTC
Created attachment 38884 [details]
Proposed patch

Hi,

  Thanks for the test case.  With that I was able to reproduce the problem
  and I think that I have a patch that can fix it.  Please could you test it
  for yourself and let me know if it works.  (I have uploaded two versions,
  one for the current FSF binutils mainline development sources, and one for
  the 2.25.1 release sources).

Cheers
  Nick
Comment 10 Nick Clifton 2016-07-13 12:20:51 UTC
Created attachment 38885 [details]
Proposed patch
Comment 11 vszakats 2016-07-13 21:08:26 UTC
Thank you, Nick.

I'd be glad to make tests with a binary pre-built using your patches, but building binutils from source myself, appears to be a too long shot at this point.

Is there someone who could help out with test binutils binaries?
Comment 12 mati865 2017-02-21 18:41:51 UTC
Patch for binutils 2.27 is ok, it got included in MSYS2 mingw binutils https://github.com/Alexpux/MINGW-packages/commit/0e79e62b72830f3161d7d8f80381a47c25bc95e1

$ upx test.exe
                       Ultimate Packer for eXecutables
                          Copyright (C) 1996 - 2013
UPX 3.91        Markus Oberhumer, Laszlo Molnar & John Reiser   Sep 30th 2013

        File size         Ratio      Format      Name
   --------------------   ------   -----------   -----------
     19968 ->      9216   46.15%    win64/pe     test.exe

Packed 1 file.
Comment 13 vszakats 2017-02-21 19:11:53 UTC
Repeating the minimal tests with MSYS2 `mingw-w64-{i686,x86_64}-binutils-2.27-4-any` (the build that includes the proposed patch), it seems the problem is now fixed for both 32-bit and 64-bit targets. UPX 3.91 is silent now and the previously visible default manifest bits are no longer there inside the `.exe` files when a custom Windows resource is linked.

Thanks to all for all the efforts and to Nick for the patch!
Comment 14 vszakats 2017-02-21 20:02:34 UTC
Setting status to resolved/fixed.
Comment 15 Eduard Braun 2017-02-21 21:42:17 UTC
> Setting status to resolved/fixed.

I don't think this is merged yet!
There's only a patch MINGW-packages, if this works out it should be fixed upstream!
Comment 16 vszakats 2017-02-21 21:53:07 UTC
Sorry, status reset. Please close when appropriate.
Comment 17 Nick Clifton 2017-02-22 10:07:51 UTC
Hi Guys,

(In reply to Eduard Braun from comment #15)
> > Setting status to resolved/fixed.
> 
> I don't think this is merged yet!
> There's only a patch MINGW-packages, if this works out it should be fixed
> upstream!

I was waiting for confirmation that the patch worked before applying it.

In the meantime however a linker bug report was filed which I think was
effectively covering the same problem:

  https://sourceware.org/bugzilla/show_bug.cgi?id=20193

That bug has been fixed by a patch, and I think that that patch might also 
fix the problem reported here.  Please could somebody check and let me know ?

Cheers
  Nick
Comment 18 mati865 2017-02-22 22:24:24 UTC
Nick I applied https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=ec8f76882145c71bef81a9cadf0bf51ff9fa5b35 on top of 2.27 binutils and cannot reproduce issue anymore.
Comment 19 Nick Clifton 2017-02-23 16:36:52 UTC
Excellent - in which case I will close this PR.