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.

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"

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.

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

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.

Kaz, maybe you have an idea?

I agree with Alan.  SH port is broken on --gc-sections.

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

(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.

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?

Ping?

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

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

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

(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.

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

(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

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.

(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

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.

I think we can mark this as RESOLVED now.

(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

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.

(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

(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