Bug 12614 - mingw ld omits needed jump stubs in link involving --start/end-group
Summary: mingw ld omits needed jump stubs in link involving --start/end-group
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-28 21:24 UTC by Daniel C. Klauer
Modified: 2011-04-28 15:34 UTC (History)
1 user (show)

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


Attachments
Example with makefile (574 bytes, application/x-gzip)
2011-03-28 21:24 UTC, Daniel C. Klauer
Details
Proposed patch (340 bytes, patch)
2011-04-27 19:14 UTC, Daniel C. Klauer
Details | Diff
Updated patch (343 bytes, patch)
2011-04-28 13:19 UTC, Daniel C. Klauer
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel C. Klauer 2011-03-28 21:24:26 UTC
Created attachment 5337 [details]
Example with makefile

Related to a patch that was applied to ld between binutils 2.17 and 2.18:
    "Eliminate redundant jump stubs on windows/cygwin/MinGW"
    <http://sourceware.org/ml/binutils/2007-01/msg00162.html>

See also:
<http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/pe-dll.c?cvsroot=src#rev1.95>
<http://sourceware.org/git/?p=binutils.git;a=commit;h=6fd5362b96f51a2763e1a55e6cc78e604f369957>


It appears that ld accidentally excludes jump stubs from an import library,
when linking like this:
    main.o -( libfoo.dll.a test.o -)
where main.o references a symbol from the import library, and test.o uses
a second symbol from the import library. In that case the jump stub for the
second symbol (referenced only by test.o) is not included into the executable.

If only test.o but not main.o reference a symbol in the import library, the
problem does not occur. (Using __declspec(dllimport) on the symbol referenced
in test.o prevents running into this issue completely.)

Here is an example; it requires a toolchain targetting mingw32:

    $ cat main.c
    void test();
    void bar();
    void mainCRTStartup() {
        test();
        bar();
    }
    $ cat test.c
    void foo();
    void test() {
        foo();
    }
    $ cat foo.def
    LIBRARY foo.dll
    EXPORTS
    foo
    bar
    $ i586-mingw32msvc-gcc -Wall -c main.c -o main.o
    $ i586-mingw32msvc-gcc -Wall -c test.c -o test.o
    $ i586-mingw32msvc-dlltool --input-def foo.def --output-lib libfoo.dll.a
    $ i586-mingw32msvc-ld -o good.exe main.o test.o libfoo.dll.a
    $ i586-mingw32msvc-ld -o bad.exe main.o "-(" libfoo.dll.a test.o "-)"
    $ i586-mingw32msvc-objdump -D good.exe > good-disasm.txt
    $ i586-mingw32msvc-objdump -D bad.exe > bad-disasm.txt

Excerpt from good-disasm.txt:

    00401000 <_mainCRTStartup>:
      401000:	55                   	push   %ebp
      401001:	89 e5                	mov    %esp,%ebp
      401003:	83 ec 08             	sub    $0x8,%esp
      401006:	e8 09 00 00 00       	call   401014 <_test>
      40100b:	e8 14 00 00 00       	call   401024 <_bar>
      401010:	c9                   	leave  
      401011:	c3                   	ret    
      401012:	90                   	nop
      401013:	90                   	nop

    00401014 <_test>:
      401014:	55                   	push   %ebp
      401015:	89 e5                	mov    %esp,%ebp
      401017:	83 ec 08             	sub    $0x8,%esp
      40101a:	e8 0d 00 00 00       	call   40102c <_foo>
      40101f:	c9                   	leave  
      401020:	c3                   	ret    
      401021:	90                   	nop
      401022:	90                   	nop
      401023:	90                   	nop

    00401024 <_bar>:
      401024:	ff 25 34 20 40 00    	jmp    *0x402034
      40102a:	90                   	nop
      40102b:	90                   	nop

    0040102c <_foo>:
      40102c:	ff 25 38 20 40 00    	jmp    *0x402038
      401032:	90                   	nop
      401033:	90                   	nop

Excerpt from bad-disasm.txt:

    00401000 <_mainCRTStartup>:
      401000:	55                   	push   %ebp
      401001:	89 e5                	mov    %esp,%ebp
      401003:	83 ec 08             	sub    $0x8,%esp
      401006:	e8 11 00 00 00       	call   40101c <_test>
      40100b:	e8 04 00 00 00       	call   401014 <_bar>
      401010:	c9                   	leave  
      401011:	c3                   	ret    
      401012:	90                   	nop
      401013:	90                   	nop

    00401014 <_bar>:
      401014:	ff 25 34 20 40 00    	jmp    *0x402034
      40101a:	90                   	nop
      40101b:	90                   	nop

    0040101c <_test>:
      40101c:	55                   	push   %ebp
      40101d:	89 e5                	mov    %esp,%ebp
      40101f:	83 ec 08             	sub    $0x8,%esp
      401022:	e8 d9 ef bf ff       	call   0 <_foo>
      401027:	c9                   	leave  
      401028:	c3                   	ret    
      401029:	90                   	nop
      40102a:	90                   	nop
      40102b:	90                   	nop

Notice that in bad.exe the <_foo> stub is not present, and there is a 'call 0'
in <_test>.
Comment 1 Daniel C. Klauer 2011-04-27 19:11:59 UTC
Hi,

this is my first time hacking the binutils/ld sources, but I think I may have identified the problem.

There is a block of code in src/ld/emultempl/pe.em that tries to detect redundant jump stubs and marks them for exclusion. It checks each symbol in the stub section to find out whether it is used or not. However, only the symbol's u.undef.next field (in struct bfd_link_hash_entry) is checked to determine whether the symbol is in the undefined list or not.

This is not enough if the symbol is at the end of the list, because then the next pointer will be NULL, even though the symbol is in the undefined list. In the attached patch I have added a check against link_info.hash->undefs_tail to cover that case.
Comment 2 Daniel C. Klauer 2011-04-27 19:14:22 UTC
Created attachment 5690 [details]
Proposed patch

ChangeLog ld

	* emultempl/pe.em (gld_${EMULATION_NAME}_after_open): Correctly check
	whether symbol is in undef list.
Comment 3 Alan Modra 2011-04-28 00:44:36 UTC
Your analysis is correct, and the patch looks good except for the overly long line.  I'll commit it with the formatting fix if you provide me with a name to put on the changelog (or go ahead and commit it yourself if you have binutils commit access).
Comment 4 Daniel C. Klauer 2011-04-28 13:19:47 UTC
Created attachment 5693 [details]
Updated patch

2011-04-28  Daniel C. Klauer  <daniel.c.klauer@web.de>

    * emultempl/pe.em (gld_${EMULATION_NAME}_after_open): Correctly check
    whether symbol is in undef list.
Comment 5 Daniel C. Klauer 2011-04-28 13:23:26 UTC
I have updated the patch and also added my name to the changelog item.

Thank you,
Daniel
Comment 6 cvs-commit@gcc.gnu.org 2011-04-28 15:29:46 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2011-04-28 15:29:43

Modified files:
	ld             : ChangeLog 
	ld/emultempl   : pe.em 

Log message:
	PR ld/12614
	* emultempl/pe.em (_after_open): Correctly check whether symbol is
	in undef list.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/ChangeLog.diff?cvsroot=src&r1=1.2323&r2=1.2324
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/emultempl/pe.em.diff?cvsroot=src&r1=1.169&r2=1.170
Comment 7 cvs-commit@gcc.gnu.org 2011-04-28 15:34:01 UTC
CVSROOT:	/cvs/src
Module name:	src
Branch: 	binutils-2_21-branch
Changes by:	amodra@sourceware.org	2011-04-28 15:33:57

Modified files:
	ld             : ChangeLog 
	ld/emultempl   : pe.em 

Log message:
	PR ld/12614
	* emultempl/pe.em (_after_open): Correctly check whether symbol is
	in undef list.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/ChangeLog.diff?cvsroot=src&only_with_tag=binutils-2_21-branch&r1=1.2222.2.19&r2=1.2222.2.20
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/emultempl/pe.em.diff?cvsroot=src&only_with_tag=binutils-2_21-branch&r1=1.162.2.1&r2=1.162.2.2
Comment 8 Alan Modra 2011-04-28 15:34:38 UTC
patch applied