Bug 19935

Summary: [SH] --enable-initfini-array causes failures in GCC testsuite
Product: newlib Reporter: Oleg Endo <olegendo>
Component: libglossAssignee: Jeff Johnston <jjohnstn>
Status: WAITING ---    
Severity: normal CC: hp, jachhunter777, kkojima
Priority: P2    
Version: unspecified   
Target Milestone: ---   
See Also: https://sourceware.org/bugzilla/show_bug.cgi?id=15025
Host: Target: sh*-*-*
Build: Last reconfirmed:
Attachments: 19935

Description Oleg Endo 2016-04-10 12:49:05 UTC
Comparing the GCC testsuite results for binutils 2.26 and current master branch, there are lots of new execution failures on sh-sim.  Just in case, I've tried out a known to be working sh-sim from 2.26 and the errors still persist.  Thus, I guess some change in GAS or LD is causing some wrong code to be generated, although I haven't looked at the details yet.
Comment 1 Mike Frysinger 2016-04-10 14:38:28 UTC
does binutils 2.25 work ?  could you perhaps bisect the two ?
Comment 2 Oleg Endo 2016-04-10 15:12:27 UTC
(In reply to Mike Frysinger from comment #1)
> does binutils 2.25 work ?

AFAIR yes.  Before updating I was using 2.25-something and it was OK.  2.26 also looks OK.

>  could you perhaps bisect the two ?

Yep.
Comment 3 Mike Frysinger 2016-04-11 03:15:38 UTC
(In reply to Oleg Endo from comment #2)

sorry, i thought you were saying 2.26 wasn't working.  i don't think there's any significant differences between 2.26 and master wrt the sim.  but if you could bisect it back, that'd be helpful.
Comment 4 Oleg Endo 2016-04-12 14:57:03 UTC
(In reply to Mike Frysinger from comment #3)
> (In reply to Oleg Endo from comment #2)
> 
> sorry, i thought you were saying 2.26 wasn't working.  i don't think there's
> any significant differences between 2.26 and master wrt the sim.  but if you
> could bisect it back, that'd be helpful.

I've picked one testcase from GCC: testsuite/gcc.dg/constructor-1.c

26e3a0c9ba4a8376fdf9f898637919d144d8b1d8 is the first bad commit
commit 26e3a0c9ba4a8376fdf9f898637919d144d8b1d8
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Dec 2 19:23:41 2015 +1030

    Make --enable-initfini-array the default
    
    	* configure.ac (--enable-initfini-array): Remove run test.  Default
    	to "yes".  Change help string to --disable-initfini-array.
    	* configure: Regenerate.


This makes sense, since most of the new failures are C++ related tests.

Bug 15025 looks somehow related ...
Comment 5 H.J. Lu 2016-04-12 19:39:01 UTC
Does ELF/sh target support the current gABI, like DT_INIT?
Comment 6 Alan Modra 2016-04-12 23:22:58 UTC
We could of course change the default back to --disable-initfini-array for SH, but that would just be hiding the real problem.  Please verify that linking one of the failing testcases with -Wl,--disable-initfini-array produces a working binary with a new linker.  Assuming that is the case, please attach both working and failing binaries.  There may be something obvious to see.
Comment 7 Alan Modra 2016-04-13 08:47:38 UTC
Sorry, that last suggestion of mine wasn't correct.  There is no --disable-initfini-array linker option.  You need to configure binutils with --disable-initfini-array rather than pass an option to ld.
Comment 8 Hans-Peter Nilsson 2016-04-14 17:37:09 UTC
I see this for cris-elf too, sorry I've been silent.  I'm on the fence whether to add the support to libgloss or disable in binutils for cris-elf.  Had no time to do neither.
Comment 9 H.J. Lu 2016-04-14 18:18:28 UTC
There is no excuse not to support .init_array, .fini_array and
.preinit_array.  I will go one step further to say that we should
deprecate any ELF targets without support for .init_array,
.fini_array and .preinit_array in binutils 2.27.
Comment 10 Hans-Peter Nilsson 2016-04-14 23:09:17 UTC
(In reply to H.J. Lu from comment #9)
> There is no excuse not to support .init_array, .fini_array and
> .preinit_array.  I will go one step further to say that we should
> deprecate any ELF targets without support for .init_array,
> .fini_array and .preinit_array in binutils 2.27.

What's with the hyperbole?  It's not like the binutils project is affected by there being no support in *libgloss*.  You effectively want to deprecate lots of, if not most, newlib targets; I don't know what the binutils project would win.
Comment 11 H.J. Lu 2016-04-14 23:48:08 UTC
(In reply to Hans-Peter Nilsson from comment #10)
> 
> What's with the hyperbole?  It's not like the binutils project is affected
> by there being no support in *libgloss*.  You effectively want to deprecate
> lots of, if not most, newlib targets; I don't know what the binutils project
> would win.

.init_array has been in gABI for more than 10 years and newlib supports
.init_array.  You just need to hook it up with your target.  If the target
maintainer doesn't have the time to do it, binutils doesn't have to support
the target.
Comment 12 Oleg Endo 2016-04-15 04:21:01 UTC
(In reply to H.J. Lu from comment #11)
> (In reply to Hans-Peter Nilsson from comment #10)
> > 
> > What's with the hyperbole?  It's not like the binutils project is affected
> > by there being no support in *libgloss*.  You effectively want to deprecate
> > lots of, if not most, newlib targets; I don't know what the binutils project
> > would win.
> 
> .init_array has been in gABI for more than 10 years and newlib supports
> .init_array.  You just need to hook it up with your target.  If the target
> maintainer doesn't have the time to do it, binutils doesn't have to support
> the target.

This sounds like overreacting.
Bare-metal systems often have their own system init code and linker scripts, even when used with newlib for everything else.  Some of them probably don't use newlib at all.  I guess that most of them already support .init_array style initialization in their own way.  It's just that newlib should be updated to support it by default out of the box.

So it's not a binutils issue but a newlib issue.  I've changed the component field accordingly.
Comment 13 Oleg Endo 2016-04-21 11:48:58 UTC
At least on SH/newlib this issue seems a bit more ....  It looks like newlib's init code is not used at all.  Instead a mix of crti.S, crtn.S, crt1.S and crtstuff.c from GCC's libgcc which relies on the .init section.  I guess I'll try to untie that stuff after removing the SH5/SH64 pieces.
Comment 14 Hans-Peter Nilsson 2017-01-29 13:30:02 UTC
I missed the component change (this used to be a binutils PR), but please note that you'll likely *also* want to fix shelf.sh as in binutils commit bf382b306a8db84b4; see <https://sourceware.org/ml/binutils/2017-01/msg00453.html>.  Newlib patch for CRIS coming up too.
Comment 15 Hans-Peter Nilsson 2017-01-29 20:51:06 UTC
Removing the cris*-*-elf target part after newlib commit cd5e7e2d8271d8d4.  See also <https://sourceware.org/ml/newlib/2017/msg00096.html>.
Comment 16 jach hunter 2020-12-17 12:36:07 UTC
Created attachment 13058 [details]
19935