Bug 17739 - Assertion fail ../../bfd/elf32-sh.c:4504 on sh4 when compiling Qt5
Summary: Assertion fail ../../bfd/elf32-sh.c:4504 on sh4 when compiling Qt5
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.25
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-22 15:58 UTC by John Paul Adrian Glaubitz
Modified: 2016-08-04 15:06 UTC (History)
7 users (show)

See Also:
Host:
Target: sh*-*-*
Build:
Last reconfirmed:


Attachments
Example object files (45.29 KB, application/x-compressed-tar)
2015-02-03 23:42 UTC, Michael Karcher
Details
Proposed patch (1.13 KB, patch)
2016-07-27 11:52 UTC, Nick Clifton
Details | Diff
Proposed patch (1.12 KB, patch)
2016-07-27 12:16 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Paul Adrian Glaubitz 2014-12-22 15:58:15 UTC
Hello!

Some components of Qt5 - including qtdeclarative-opensource-src and qtwebkit - currently fail to build from source on the Debian sh4 port due to a linker assertion failing.

The exact linker output is:

g++ -c -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -O2 -std=c++0x -fno-exceptions -Wall -W -D_REENTRANT -fPIE -DQT_NO_CAST_TO_ASCII -DQT_NO_CAST_FROM_ASCII -DQT_NO_EXCEPTIONS -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_NO_DEBUG -DQT_QMLDEVTOOLS_LIB -DQT_CORE_LIB -I/usr/lib/sh4-linux-gnu/qt5/mkspecs/linux-g++ -I. -I../../include -I../../include/QtQml -I../../include/QtQml/5.3.2 -I../../include/QtQml/5.3.2/QtQml -I/usr/include/sh4-linux-gnu/qt5 -I/usr/include/sh4-linux-gnu/qt5/QtCore -I.moc -o .obj/main.o main.cpp
g++ -Wl,-z,relro -Wl,--as-needed -Wl,--gc-sections -Wl,-O1 -o ../../bin/qmlimportscanner .obj/main.o   -L/«PKGBUILDDIR»/lib -lQt5QmlDevTools -L/usr/lib/sh4-linux-gnu -lQt5Core -lpthread 
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
/usr/bin/ld: BFD (GNU Binutils for Debian) 2.24 assertion fail ../../bfd/elf32-sh.c:4504
collect2: error: ld returned 1 exit status

The complete build logs can be found here [1] and here [2]. The build log for qtwebkit which can be found in [2] was built with older versions of the toolchain, but the error is the exact same so I assume it's the same bug. As one of the buildds is currently building an up-to-date version of qtwebkit with the current versions of the toolchain, I will be able to provide an updated build log for qtwebkit shortly. The build takes around two days on sh4, I will provide the build log as soon as the build has finished.

If you need more details or need me to run any tests, please let me know!

Cheers,
Adrian

> [1] http://buildd.debian-ports.org/status/fetch.php?pkg=qtdeclarative-opensource-src&arch=sh4&ver=5.3.2-4&stamp=1417073135
> [2] http://buildd.debian-ports.org/status/fetch.php?pkg=qtwebkit&arch=sh4&ver=2.2.1-7&stamp=1383898686
Comment 1 John Paul Adrian Glaubitz 2015-02-03 21:51:45 UTC
Here is an archive which contains all object files of such a failed build:

> http://users.physik.fu-berlin.de/~glaubitz/qtwebkit-debug.tar.gz

It may help debug the problem.
Comment 2 Michael Karcher 2015-02-03 23:42:14 UTC
Created attachment 8100 [details]
Example object files

The tar file contains three object files, that provoke the assertion when linked together using "ld --gc-sections -shared JSNodeCustom.o JSNode.o JSNotation.o"
Comment 3 Alan Modra 2015-02-04 03:37:10 UTC
This is caused by sh_elf_gc_sweep_hook decrementing h->got.refcount for relocation types which sh_elf_check_relocs does not increment h->got.refcount.  The accounting needs to match exactly.
Comment 4 Michael Karcher 2015-02-04 23:15:20 UTC
Thanks for your comment, which seems to be spot-on. It allowed me to create a minimal example that exhibits the same assertation failure if linked with --gc-sections:

        .section data.kept
        .globl  foo
foo:
        .long   foo@GOT

        .section data.omitted
        .long   foo@GOTOFF

It seems like sh_elf_gc_sweep_hook should not decrement the refcount for GOTOFF and GOTPC relocation types. Patch with testcase incoming later
Comment 5 Alan Modra 2015-02-05 02:01:57 UTC
Note that there is more wrong with sh_elf_check_relocs/sh_elf_gc_sweep_hook than just the reloc mismatch.  For instance, this from check_relocs
	case R_SH_GOTPLT32:
#ifdef INCLUDE_SHMEDIA
	case R_SH_GOTPLT_LOW16:
	case R_SH_GOTPLT_MEDLOW16:
	case R_SH_GOTPLT_MEDHI16:
	case R_SH_GOTPLT_HI16:
	case R_SH_GOTPLT10BY4:
	case R_SH_GOTPLT10BY8:
#endif
	  /* If this is a local symbol, we resolve it directly without
	     creating a procedure linkage table entry.  */

	  if (h == NULL
	      || h->forced_local
	      || ! info->shared
	      || info->symbolic
	      || h->dynindx == -1)
	    goto force_got;

You just can't do that, and expect --gc-sections to work.  The trouble is that flags like h->forced_local and h->dynindx may change in the process of loading object files.  check_relocs is called as each file is loaded, but gc_sweep after all files are loaded.  So gc_sweep can't undo check_relocs plt/got counts accurately.

A similar problem is seen here
      if (! info->shared
	  && r_type == R_SH_TLS_IE_32
	  && h != NULL
	  && h->root.type != bfd_link_hash_undefined
	  && h->root.type != bfd_link_hash_undefweak
	  && (h->dynindx == -1
	      || h->def_regular))
	r_type = R_SH_TLS_LE_32;
Again, h->root.type, h->dynindx and h->def_regular may change as object files are loaded, affecting whether h->got.refcount or h->plt.refcount is incremented in check_relocs.
Comment 6 Oleg Endo 2015-02-15 21:36:53 UTC
Kaz, maybe you have an idea?
Comment 7 Kaz Kojima 2015-02-15 23:45:29 UTC
I agree with Alan.  SH port is broken on --gc-sections.
Comment 8 John Paul Adrian Glaubitz 2016-06-19 21:37:02 UTC
Hello!

Is there any chance to get this fixed? Any pointers to what exactly is wrong?

This bug is currently the last big blocker we are seeing on SH, all other issues have been ironed out and both the kernel and the toolchain are generally in good shape. Just this remaining issue which prevents us from building qt5.

Adrian
Comment 9 H.J. Lu 2016-06-20 12:39:32 UTC
(In reply to John Paul Adrian Glaubitz from comment #8)
> Hello!
> 
> Is there any chance to get this fixed? Any pointers to what exactly is wrong?
> 
> This bug is currently the last big blocker we are seeing on SH, all other
> issues have been ironed out and both the kernel and the toolchain are
> generally in good shape. Just this remaining issue which prevents us from
> building qt5.
> 
> Adrian

You can set CHECK_RELOCS_AFTER_OPEN_INPUT to yes in ld for SH and update
SH backend to check relocations after opening all input files.  It will
simplify --gc-sections:

commit e66cdd681f47dc51beaeee3d813f1c9cba27dedf
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Apr 20 17:12:46 2016 -0700

    Remove x86 gc_sweep_hook
    
    Since x86 backends never see the removed sections, there is no need
    for gc_sweep_hook.
    
      * elf32-i386.c (elf_i386_gc_sweep_hook): Removed.
      (elf_backend_gc_sweep_hook): Likewise.
      * elf64-x86-64.c (elf_x86_64_gc_sweep_hook): Likewise.
      (elf_backend_gc_sweep_hook): Likewise.

See how it is done for x86 backends.
Comment 10 Rich Felker 2016-06-27 00:52:46 UTC
From what I can tell, H.J. Lu's proposed fix depends on infrastructure not available in release versions of binutils. Is there any reasonable way to turn this into a fix that works for releases (we're using 2.25.1 now but could move to 2.26 if needed) or do we need some hackish workaround?
Comment 11 Rich Felker 2016-07-27 04:43:55 UTC
Ping?
Comment 12 Nick Clifton 2016-07-27 11:52:43 UTC
Created attachment 9405 [details]
Proposed patch

(In reply to Rich Felker from comment #11)
> Ping?

Right - please could someone try out this potential patch ?  It works with the
small testcase provided.  But I am not an SH expert and I do not have an SH
Linux system in which to test it thoroughly.  So I am asking for help.

The patch implement's H.J.Lu's suggestion of delaying check_relocs until after all the input files have been loaded, and linker garbage collection performed, which means that there is no need for a sweep pass to correct GOT entry counts.

If necessary I can provide a version of the patch made against the 2.26 or 2.25 releases, although I suspect that this version (made against the current mainline sources) should be able to be applied without too much hassle.

Cheers
  Nick
Comment 13 John Paul Adrian Glaubitz 2016-07-27 12:06:07 UTC
Hi Nick!

(In reply to Nick Clifton from comment #12)
> Right - please could someone try out this potential patch ?  It works with
> the
> small testcase provided.  But I am not an SH expert and I do not have an SH
> Linux system in which to test it thoroughly.  So I am asking for help.
> 
> (...)
> 
> If necessary I can provide a version of the patch made against the 2.26 or
> 2.25 releases, although I suspect that this version (made against the
> current mainline sources) should be able to be applied without too much
> hassle.

I would love to test this patch. Could you provide a version which applies against 2.26.1? I will then build a patched version of binutils in Debian and verify whether I will be able to build Qt5.

Adrian
Comment 14 Nick Clifton 2016-07-27 12:16:27 UTC
Created attachment 9406 [details]
Proposed patch

Hi Adrian,

> I would love to test this patch. Could you provide a version which applies 
> against 2.26.1? 

Here you go.

Cheers
  Nick
Comment 15 H.J. Lu 2016-07-27 14:49:40 UTC
(In reply to Nick Clifton from comment #14)
> Created attachment 9406 [details]
> Proposed patch
> 
> Hi Adrian,
> 
> > I would love to test this patch. Could you provide a version which applies 
> > against 2.26.1? 
> 
> Here you go.
> 
> Cheers
>   Nick

Does 2.26.1 support CHECK_RELOCS_AFTER_OPEN_INPUT? I only added it to 2.27.
Comment 16 Nick Clifton 2016-07-27 14:52:50 UTC
Hi H.J.

>>> I would love to test this patch. Could you provide a version which applies 
>>> against 2.26.1? 
>>
>> Here you go.

> Does 2.26.1 support CHECK_RELOCS_AFTER_OPEN_INPUT? I only added it to 2.27.

*B......* <rude word>.  No, it does not.  So my 2.26.1 patch will be useless.
*sigh*

Adrian - is there any chance that you could switch to 2.27 and apply the original patch ?

Cheers
  Nick
Comment 17 John Paul Adrian Glaubitz 2016-07-27 15:23:46 UTC
(In reply to Nick Clifton from comment #16)
> *B......* <rude word>.  No, it does not.  So my 2.26.1 patch will be useless.
> *sigh*

It does seem to build fine, however. The binutils Debian package with the patch applied has been building on qemu-sh4 for a while now. I still want to let it finish.

> Adrian - is there any chance that you could switch to 2.27 and apply the
> original patch ?

Not sure whether I can build an upstream version of binutils on Debian without further patching. I will give it a try, but it may take a few days (including the test build of qtwebkit) before I will be able to provide feedback.

Thanks,
Adrian
Comment 18 John Paul Adrian Glaubitz 2016-07-31 18:31:47 UTC
Ok, the results are in.

binutils-2_27-branch, without the patch:

(sid-sh4-sbuild)root@ikarus:/tmp/qtwebkit-debug/qtwebkit-2.3.4.dfsg/WebKitBuild/Release/Source/WebCore/obj/release# /usr/src/binutils-2.27.51/ld/ld-new --gc-sections -shared JSNodeCustom.o JSNode.o JSNotation.o
/usr/src/binutils-2.27.51/ld/ld-new: BFD (GNU Binutils) 2.26.90.20160728 assertion fail elf32-sh.c:4521
(sid-sh4-sbuild)root@ikarus:/tmp/qtwebkit-debug/qtwebkit-2.3.4.dfsg/WebKitBuild/Release/Source/WebCore/obj/release#

binutils-2_27-branch, with the patch:

(sid-sh4-sbuild)root@ikarus:/tmp/qtwebkit-debug/qtwebkit-2.3.4.dfsg/WebKitBuild/Release/Source/WebCore/obj/release# /usr/src/binutils-2.27.51/ld/ld-new --gc-sections -shared JSNodeCustom.o JSNode.o JSNotation.o
(sid-sh4-sbuild)root@ikarus:/tmp/qtwebkit-debug/qtwebkit-2.3.4.dfsg/WebKitBuild/Release/Source/WebCore/obj/release#

So the patch seems to work. I will do another full rebuild and see whether I can build qtwebkit now.
Comment 19 John Paul Adrian Glaubitz 2016-08-01 22:06:41 UTC
(In reply to John Paul Adrian Glaubitz from comment #18)
> So the patch seems to work. I will do another full rebuild and see whether I
> can build qtwebkit now.

I can confirm that qtwebkit builds fine now, full build log in [1].

Please apply the patch!

Thanks,
Adrian

> [1] https://people.debian.org/~glaubitz/qtwebkit_2.3.4.dfsg-8_sh4-20160731-2121.build
Comment 20 cvs-commit@gcc.gnu.org 2016-08-02 10:58:16 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit a94d834c9d0108f0bb50ddc311554d1bed320f54
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Aug 2 11:56:55 2016 +0100

    Fix SH GOT allocation in the presence of linker garbage collection.
    
    	PR ld/17739
    ld	* emulparams/shelf.sh (CHECK_RELOCS_AFTER_OPEN_INPUT): Define with
    	valye 'yes'.
    	* emulparams/shelf32.sh: Likewise.
    	* emulparams/shelf32.sh: Likewise.
    	* emulparams/shelf_nto.sh: Likewise.
    	* emulparams/shelf_nto.sh: Likewise.
    	* emulparams/shelf_vxworks.sh: Likewise.
    	* emulparams/shelf_vxworks.sh: Likewise.
    	* emulparams/shlelf32_linux.sh: Likewise.
    	* emulparams/shlelf32_linux.sh: Likewise.
    	* emulparams/shlelf_linux.sh: Likewise.
    	* emulparams/shlelf_linux.sh: Likewise.
    	* emulparams/shlelf_nto.sh: Likewise.
    	* emulparams/shlelf_nto.sh: Likewise.
    
    bfd	* elf32-sh.c (sh_elf_gc_sweep_hook): Delete.
    	(elf_backend_sweep_hook): Delete.
Comment 21 John Paul Adrian Glaubitz 2016-08-02 11:01:28 UTC
I think we can mark this as RESOLVED now.
Comment 22 John Paul Adrian Glaubitz 2016-08-03 02:09:25 UTC
(In reply to cvs-commit@gcc.gnu.org from comment #20)
> The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

Btw, can you make sure this gets backported to binutils-2_27-branch as well?

We need to make sure it's going to be in the 2.27 release.

Thanks,
Adrian
Comment 23 cvs-commit@gcc.gnu.org 2016-08-04 15:01:55 UTC
The binutils-2_27-branch branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit dccb2ff522d2afba77301a046b37949356c29f88
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Aug 4 16:00:27 2016 +0100

    Fix the generation of GOT table entries for the SH in the presence of linker garbage collection.
    
    	PR ld/17739
    	* emulparams/shelf.sh (CHECK_RELOCS_AFTER_OPEN_INPUT): Define with
    ld	valye 'yes'.
    	* emulparams/shelf32.sh: Likewise.
    	* emulparams/shelf32.sh: Likewise.
    	* emulparams/shelf_nto.sh: Likewise.
    	* emulparams/shelf_nto.sh: Likewise.
    	* emulparams/shelf_vxworks.sh: Likewise.
    	* emulparams/shelf_vxworks.sh: Likewise.
    	* emulparams/shlelf32_linux.sh: Likewise.
    	* emulparams/shlelf32_linux.sh: Likewise.
    	* emulparams/shlelf_linux.sh: Likewise.
    	* emulparams/shlelf_linux.sh: Likewise.
    	* emulparams/shlelf_nto.sh: Likewise.
    	* emulparams/shlelf_nto.sh: Likewise.
    
    bfd	* elf32-sh.c (sh_elf_gc_sweep_hook): Delete.
    	(elf_backend_sweep_hook): Delete.
Comment 24 Nick Clifton 2016-08-04 15:03:58 UTC
(In reply to John Paul Adrian Glaubitz from comment #22)
> Btw, can you make sure this gets backported to binutils-2_27-branch as well?


Sorry - we missed the release window.

I have checked the patch in to the 2.27 branch so that when/if a point release
is made the patch will be present there.

 Cheers
   Nick
Comment 25 John Paul Adrian Glaubitz 2016-08-04 15:06:01 UTC
(In reply to Nick Clifton from comment #24)
> Sorry - we missed the release window.

Yeah, 2.27 came quite unexpected. I knew it was announced for a while but I was expecting a much later release.

> I have checked the patch in to the 2.27 branch so that when/if a point
> release
> is made the patch will be present there.

That's still a good thing since Debian usually adds the latest branch updates to the releases of gcc and binutils, so hopefully this will be available soon.

Thanks,
Adrian