diff mbox

Introduce <elf-initfini.h> and ELF_INITFINI for all architectures

Message ID 87r1ypdrhj.fsf@oldenburg2.str.redhat.com
State Superseded
Headers

Commit Message

Florian Weimer Feb. 20, 2020, 6:48 p.m. UTC 
  * Andreas Schwab:

> environ is empty.

That's because libc.so.6 still has DT_INIT, from which _environ and
other variables are set up.  I assumed binutils would convert that into
DT_INITARRAY because the architecture is not supposed to have DT_INIT.
Without that, it's hard to declare that there is no DT_INIT, and the
patch essentially breaks ABI (because DT_INIT processing is gone).

This should fix the breakage:

Subject: Use ELF constructor instead of _init in libc.so

On !ELF_INITFINI architectures, _init is no longer called by the dynamic
linker.  We can use an ELF constructor instead because the constructor
order does not matter.  (The other constructors are used to set up libio
vtable bypasses and do not depend on this initialization routine.)


But I'm no longer sure if RISC-V is actually an !ELF_INITFINI
architecture.

Thanks,
Florian

Comments

Jim Wilson Feb. 20, 2020, 10:48 p.m. UTC | #1 
On Thu, Feb 20, 2020 at 10:53 AM Florian Weimer <fweimer@redhat.com> wrote:
> That's because libc.so.6 still has DT_INIT, from which _environ and
> other variables are set up.  I assumed binutils would convert that into
> DT_INITARRAY because the architecture is not supposed to have DT_INIT.
> Without that, it's hard to declare that there is no DT_INIT, and the
> patch essentially breaks ABI (because DT_INIT processing is gone).

DT_INIT is a function address.  DT_INITARRAY is a pointer to a table
of addresses.  I don't see how the linker can easily convert between
them.  We would have to increase the size of the init_array table at
link time, and I don't think we have any linker support for that.
Maybe we could modify the startfiles to put a weak reference to _init
at the end of init_array, right before the zero that normally ends the
list, and fix the linker to stop creating DT_INIT for _init.  But that
sounds a little risky, and defeats the purpose of dropping support for
_init to reduce code size.  I would prefer that glibc not create an
_init function in the first place.

> But I'm no longer sure if RISC-V is actually an !ELF_INITFINI
> architecture.

gcc will not create .init sections.  Newlib does not create them or
run them in the startfiles.  The linux kernel does not run DT_INIT.
That is probably why your patch failed for Andreas.

But there is a problem with GNU ld which automatically creates DT_INIT
when it sees a function called _init.  We missed that.  This is in
target dependent code.  The default function name can be overridden
but that isn't quite right for RISC-V either.  Maybe we should give a
linker error if someone gives a _init or _fini function to the linker?
 That way they get a link time error instead of a run-time error.

Jim
Florian Weimer Feb. 21, 2020, 12:41 p.m. UTC | #2 
* Jim Wilson:

> On Thu, Feb 20, 2020 at 10:53 AM Florian Weimer <fweimer@redhat.com> wrote:
>> That's because libc.so.6 still has DT_INIT, from which _environ and
>> other variables are set up.  I assumed binutils would convert that into
>> DT_INITARRAY because the architecture is not supposed to have DT_INIT.
>> Without that, it's hard to declare that there is no DT_INIT, and the
>> patch essentially breaks ABI (because DT_INIT processing is gone).
>
> DT_INIT is a function address.  DT_INITARRAY is a pointer to a table
> of addresses.  I don't see how the linker can easily convert between
> them.  We would have to increase the size of the init_array table at
> link time, and I don't think we have any linker support for that.
> Maybe we could modify the startfiles to put a weak reference to _init
> at the end of init_array, right before the zero that normally ends the
> list, and fix the linker to stop creating DT_INIT for _init.  But that
> sounds a little risky, and defeats the purpose of dropping support for
> _init to reduce code size.  I would prefer that glibc not create an
> _init function in the first place.

I'm not worried about glibc here.  It's easy to make glibc consistent
with itself.  The patch I posted seems to do the trick, without
regressing anywhere else.

What I don't know is whether this binutils bug has resulted in DT_INIT
being used *elsewhere*.  I don't have a RISC-V distribution to analyze,
so I really don't know.  Fedora RISC-V seems dead, this hasn't been
updated in a while:

  <https://dl.fedoraproject.org/pub/alt/risc-v/repo/fedora/rawhide/latest/riscv64/>

(I have existing tools for RPM-based distributions, so having one would
be most useful to me to check for DT_INIT references.)

>> But I'm no longer sure if RISC-V is actually an !ELF_INITFINI
>> architecture.
>
> gcc will not create .init sections.  Newlib does not create them or
> run them in the startfiles.  The linux kernel does not run DT_INIT.

The kernel never runs DT_INIT on any architecture as far as I know, but
currently released glibc does, even on RISC-V.

> That is probably why your patch failed for Andreas.

Yes, looks like it.

> But there is a problem with GNU ld which automatically creates DT_INIT
> when it sees a function called _init.  We missed that.  This is in
> target dependent code.  The default function name can be overridden
> but that isn't quite right for RISC-V either.  Maybe we should give a
> linker error if someone gives a _init or _fini function to the linker?
>  That way they get a link time error instead of a run-time error.

I would expect the toolchain to treat _init just like any other
function, so without DT_INIT support, it would turn into dead code.

Thanks,
Florian
Florian Weimer Feb. 21, 2020, 6:34 p.m. UTC | #3 
* Florian Weimer:

> * Jim Wilson:
>
>> On Thu, Feb 20, 2020 at 10:53 AM Florian Weimer <fweimer@redhat.com> wrote:
>>> That's because libc.so.6 still has DT_INIT, from which _environ and
>>> other variables are set up.  I assumed binutils would convert that into
>>> DT_INITARRAY because the architecture is not supposed to have DT_INIT.
>>> Without that, it's hard to declare that there is no DT_INIT, and the
>>> patch essentially breaks ABI (because DT_INIT processing is gone).
>>
>> DT_INIT is a function address.  DT_INITARRAY is a pointer to a table
>> of addresses.  I don't see how the linker can easily convert between
>> them.  We would have to increase the size of the init_array table at
>> link time, and I don't think we have any linker support for that.
>> Maybe we could modify the startfiles to put a weak reference to _init
>> at the end of init_array, right before the zero that normally ends the
>> list, and fix the linker to stop creating DT_INIT for _init.  But that
>> sounds a little risky, and defeats the purpose of dropping support for
>> _init to reduce code size.  I would prefer that glibc not create an
>> _init function in the first place.
>
> I'm not worried about glibc here.  It's easy to make glibc consistent
> with itself.  The patch I posted seems to do the trick, without
> regressing anywhere else.
>
> What I don't know is whether this binutils bug has resulted in DT_INIT
> being used *elsewhere*.  I don't have a RISC-V distribution to analyze,
> so I really don't know.  Fedora RISC-V seems dead, this hasn't been
> updated in a while:
>
>   <https://dl.fedoraproject.org/pub/alt/risc-v/repo/fedora/rawhide/latest/riscv64/>
>
> (I have existing tools for RPM-based distributions, so having one would
> be most useful to me to check for DT_INIT references.)

I loaded the repository

  <http://fedora.riscv.rocks/repos/rawhide/latest/riscv64/>

and ran this query:

SELECT symboldb.nevra(package), file.name
  FROM symboldb.elf_dynamic dt
    JOIN symboldb.elf_file hdr USING (contents_id)
    JOIN symboldb.file USING (contents_id)
    JOIN symboldb.package USING (package_id)
  WHERE hdr.e_machine = 243 AND dt.tag = 12 ORDER BY 1, 2;

                     nevra                     |                        name                        
-----------------------------------------------+----------------------------------------------------
 glibc-2.30.9000-31.fc32.riscv64               | /lib64/libc-2.30.9000.so
 jamin-0.97.16-19.20111031cvs.fc31.riscv64     | /usr/lib64/ladspa/jamincont_1912.so
 ladspa-1.13-23.fc31.riscv64                   | /usr/lib64/ladspa/amp.so
 ladspa-1.13-23.fc31.riscv64                   | /usr/lib64/ladspa/delay.so
 ladspa-1.13-23.fc31.riscv64                   | /usr/lib64/ladspa/filter.so
 ladspa-1.13-23.fc31.riscv64                   | /usr/lib64/ladspa/noise.so
 ladspa-autotalent-plugins-0.2-20.fc31.riscv64 | /usr/lib64/ladspa/autotalent.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/adsr_1653.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/adsr_1680.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/amp_1654.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/blop_files/parabola_1649_data.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/blop_files/sawtooth_1641_data.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/blop_files/square_1643_data.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/branch_1673.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/dahdsr_2021.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/difference_2030.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/fmod_1656.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/interpolator_1660.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/lp4pole_1671.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/product_1668.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/pulse_1645.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/quantiser100_2029.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/quantiser20_2027.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/quantiser50_2028.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/random_1661.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/ratio_2034.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sawtooth_1641.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sequencer16_1677.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sequencer32_1676.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sequencer64_1675.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/square_1643.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sum_1665.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sync_pulse_2023.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/sync_square_1678.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/tracker_2025.so
 ladspa-blop-plugins-0.2.8-26.fc31.riscv64     | /usr/lib64/ladspa/triangle_1649.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_autopan.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_chorusflanger.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_deesser.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_doubler.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_dynamics_m.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_dynamics_st.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_echo.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_eq.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_eqbw.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_limiter.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_pinknoise.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_pitch.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_reflector.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_reverb.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_rotspeak.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_sigmoid.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_tremolo.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_tubewarmth.so
 ladspa-tap-plugins-0.7.0-24.fc31.riscv64      | /usr/lib64/ladspa/tap_vibrato.so
 ladspa-wasp-plugins-0.9.7-10.fc31.riscv64     | /usr/lib64/ladspa/libwasp_booster.so
 ladspa-wasp-plugins-0.9.7-10.fc31.riscv64     | /usr/lib64/ladspa/libwasp_noisifier.so
 ladspa-wasp-plugins-0.9.7-10.fc31.riscv64     | /usr/lib64/ladspa/libwasp_xshaper.so
 nekobee-dssi-0.1.7-22.fc31.riscv64            | /usr/lib64/dssi/nekobee.so
 numactl-libs-2.0.12-3.fc32.riscv64            | /usr/lib64/libnuma.so.1.0.0
 sblim-cmpi-rpm-1.0.1-25.fc31.riscv64          | /usr/lib64/libcimrpm.so.0.0.0
 sblim-cmpi-rpm-1.0.1-25.fc31.riscv64          | /usr/lib64/libcimrpmv4.so.0.0.0
(62 rows)

So there are a few packages which have been built in such a way that
they have a DT_INIT tag.

I don't know if this qualifies as use, and I haven't disassembled the
objects to see if the init routine does anything meaningful.

Thoughts?  Should we still label RISC-V as an !ELF_INITFINI
architecture?

Thanks,
Florian
Jim Wilson Feb. 21, 2020, 9:11 p.m. UTC | #4 
On Fri, Feb 21, 2020 at 10:34 AM Florian Weimer <fweimer@redhat.com> wrote:
> So there are a few packages which have been built in such a way that
> they have a DT_INIT tag.

I looked at numactl.  In the Makefile.am I see
    -Wl,-init,numa_init -Wl,-fini,numa_fini
So the DT_INIT and DT_FINI are doing something useful, but can be
fixed by porting the package to use init_array/fini_array instead, or
maybe adding some trickiness to the linker.  But I'd rather the
package be fixed, and that the linker should give an error when -init
or -fini are used.

It is at least the intent that we don't have init/fini support for
RISC-V, and we should drop DT_INIT if we can do so without causing an
ABI break.  It looks like glibc is the only important package with
DT_INIT so maybe that is the only one we have to worry about.

> Thoughts?  Should we still label RISC-V as an !ELF_INITFINI
> architecture?

I created an ABI issue to start a discussion with other RISC-V developers
    https://github.com/riscv/riscv-elf-psabi-doc/issues/132

Jim
David Abdurachmanov Feb. 21, 2020, 9:20 p.m. UTC | #5 
On Fri, Feb 21, 2020 at 2:42 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Jim Wilson:
>
> > On Thu, Feb 20, 2020 at 10:53 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> That's because libc.so.6 still has DT_INIT, from which _environ and
> >> other variables are set up.  I assumed binutils would convert that into
> >> DT_INITARRAY because the architecture is not supposed to have DT_INIT.
> >> Without that, it's hard to declare that there is no DT_INIT, and the
> >> patch essentially breaks ABI (because DT_INIT processing is gone).
> >
> > DT_INIT is a function address.  DT_INITARRAY is a pointer to a table
> > of addresses.  I don't see how the linker can easily convert between
> > them.  We would have to increase the size of the init_array table at
> > link time, and I don't think we have any linker support for that.
> > Maybe we could modify the startfiles to put a weak reference to _init
> > at the end of init_array, right before the zero that normally ends the
> > list, and fix the linker to stop creating DT_INIT for _init.  But that
> > sounds a little risky, and defeats the purpose of dropping support for
> > _init to reduce code size.  I would prefer that glibc not create an
> > _init function in the first place.
>
> I'm not worried about glibc here.  It's easy to make glibc consistent
> with itself.  The patch I posted seems to do the trick, without
> regressing anywhere else.
>
> What I don't know is whether this binutils bug has resulted in DT_INIT
> being used *elsewhere*.  I don't have a RISC-V distribution to analyze,
> so I really don't know.  Fedora RISC-V seems dead, this hasn't been
> updated in a while:
>
>   <https://dl.fedoraproject.org/pub/alt/risc-v/repo/fedora/rawhide/latest/riscv64/>
>

We are still alive, but the mirroring of the repository to
dl.fedoraproject.org was disabled.
I will look into enabling it again.
Andreas Schwab Feb. 24, 2020, 9:12 a.m. UTC | #6 
Can you please revert your change until the issue is resolved one way or
the other?

Andreas.
Andreas Schwab Feb. 24, 2020, 9:13 a.m. UTC | #7 
On Feb 21 2020, Florian Weimer wrote:

> What I don't know is whether this binutils bug has resulted in DT_INIT
> being used *elsewhere*.  I don't have a RISC-V distribution to analyze,
> so I really don't know.  Fedora RISC-V seems dead, this hasn't been
> updated in a while:
>
>   <https://dl.fedoraproject.org/pub/alt/risc-v/repo/fedora/rawhide/latest/riscv64/>
>
> (I have existing tools for RPM-based distributions, so having one would
> be most useful to me to check for DT_INIT references.)

You can take a look here:

https://download.opensuse.org/ports/riscv/tumbleweed/repo/oss/

Andreas.
Florian Weimer Feb. 24, 2020, 10:21 a.m. UTC | #8 
* Andreas Schwab:

> Can you please revert your change until the issue is resolved one way or
> the other?

I'm going resubmit the actual fix after testing on more architectures.

Thanks,
Florian
Florian Weimer Feb. 24, 2020, 12:31 p.m. UTC | #9 
* Andreas Schwab:

> On Feb 21 2020, Florian Weimer wrote:
>
>> What I don't know is whether this binutils bug has resulted in DT_INIT
>> being used *elsewhere*.  I don't have a RISC-V distribution to analyze,
>> so I really don't know.  Fedora RISC-V seems dead, this hasn't been
>> updated in a while:
>>
>>   <https://dl.fedoraproject.org/pub/alt/risc-v/repo/fedora/rawhide/latest/riscv64/>
>>
>> (I have existing tools for RPM-based distributions, so having one would
>> be most useful to me to check for DT_INIT references.)
>
> You can take a look here:
>
> https://download.opensuse.org/ports/riscv/tumbleweed/repo/oss/

Nice, I could just point my tool at it and it got some data (despite not
having ported it to openSUSE or riscv64 at all):

              nevra               |                 name                 
----------------------------------+--------------------------------------
 authbind-2.1.2-2.1.riscv64       | /usr/lib/authbind/libauthbind.so.1.0
 glibc-2.31-2.1.riscv64           | /lib64/libc-2.31.so
 jamin-0.95.0-259.1.riscv64       | /usr/lib64/ladspa/jamincont_1912.so
 ladspa-caps-0.4.4-4.2.riscv64    | /usr/lib64/ladspa/caps.so
 ladspa-pvoc-0.1.12-3.2.riscv64   | /usr/lib64/ladspa/pvoc.so
 libnuma1-2.0.13-1.1.riscv64      | /usr/lib64/libnuma.so.1.0.0
 segv_handler-0.0.1-161.2.riscv64 | /usr/lib64/segv_handler.so
 vde2-2.3.2+svn587-3.2.riscv64    | /usr/lib64/vde2/libvdetap.so
(8 rows)

This still looks rather manageable to me, so perhaps we can go ahead
with the removal of DT_INIT support on riscv64.

Thanks,
Florian
Vineet Gupta Feb. 26, 2020, 7:10 p.m. UTC | #10 
Hi Florian,

On 2/20/20 10:48 AM, Florian Weimer wrote:
> * Andreas Schwab:

> 

>> environ is empty.

> 

> That's because libc.so.6 still has DT_INIT, from which _environ and

> other variables are set up.  I assumed binutils would convert that into

> DT_INITARRAY because the architecture is not supposed to have DT_INIT.

> Without that, it's hard to declare that there is no DT_INIT, and the

> patch essentially breaks ABI (because DT_INIT processing is gone).

> 

> This should fix the breakage:

> 

> Subject: Use ELF constructor instead of _init in libc.so

> 

> On !ELF_INITFINI architectures, _init is no longer called by the dynamic

> linker.  We can use an ELF constructor instead because the constructor

> order does not matter.  (The other constructors are used to set up libio

> vtable bypasses and do not depend on this initialization routine.)

> 

> diff --git a/csu/init-first.c b/csu/init-first.c

> index 1cd8a75098..1fa1633657 100644

> --- a/csu/init-first.c

> +++ b/csu/init-first.c

> @@ -46,9 +46,8 @@ __libc_init_first (int argc, char **argv, char **envp)

>    /* For DSOs we do not need __libc_init_first but instead _init.  */

>  }

>  

> -void

> -attribute_hidden

> -_init (int argc, char **argv, char **envp)

> +static void __attribute__ ((constructor))

> +init (int argc, char **argv, char **envp)

>  {

>  #endif

>  


This (or lack thereof) turned out to be the nasty busbox sh crash (and kernel init
panic) on ARC based off rebased upstream (latest RV32 from Alistair) containing
the original patch f4349837d93b4df. I tried this fixup and it seems to work. The
DT_INIT entry goes away and init does run as part of init_array.

It seems that commit also removed init_array from sysdeps/{riscv,csky}/Implies -
so newer arches. I suppose I need to do that for ARC as well - but could you
please explain (or point to documentation) which explains how this the Implies
stuff works.

> But I'm no longer sure if RISC-V is actually an !ELF_INITFINI

> architecture.


And an arch is !ELF_INITFINI if it supports initarray ?
I did switch ARC gcc [1] / binutils [2]to initarray last year

[1] http://lists.infradead.org/pipermail/linux-snps-arc/2019-January/005318.html
[2] http://lists.infradead.org/pipermail/linux-snps-arc/2019-February/005388.html

Thx,
-Vineet
Florian Weimer Feb. 26, 2020, 7:27 p.m. UTC | #11 
* Vineet Gupta:

> It seems that commit also removed init_array from
> sysdeps/{riscv,csky}/Implies - so newer arches. I suppose I need to do
> that for ARC as well - but could you please explain (or point to
> documentation) which explains how this the Implies stuff works.

You are very lucky.  The mechanism is explained in the section Porting
the GNU C Library in the manual:

  <https://www.gnu.org/software/libc/manual/html_node/Porting.html>

It's one of the few aspects of our build systems that's documented.

However, in commit f4349837d93b4dfe9ba09791e280ee2d6c99919f ("Introduce
<elf-initfini.h> and ELF_INITFINI for all architectures") I replaced the
use of the implies mechanism with a new header file, <elf-initfini.h>.
This allows us to control the default for new targets in a more explicit
way.  Previously, new ports had to include initarray explicitly, and we
know from past experience (the lack of removal of the sysctl function
comes to my mind) that this does not work.

>> But I'm no longer sure if RISC-V is actually an !ELF_INITFINI
>> architecture.
>
> And an arch is !ELF_INITFINI if it supports initarray ?

Not necessarily.  glibc always supports DT_INITARRAY for all targets,
but on some targets, it is necessary to honor DT_INIT/DT_FINI for legacy
binaries at least.  (I hope no targets are left where binutils produces
DT_INIT for regular ELF constructors, but I haven't checked.)

> I did switch ARC gcc [1] / binutils [2]to initarray last year
>
> [1] http://lists.infradead.org/pipermail/linux-snps-arc/2019-January/005318.html
> [2] http://lists.infradead.org/pipermail/linux-snps-arc/2019-February/005388.html

I haven't followed the ARC contribution process closely, sorry.  Do you
plan to contribute the port with a GLIBC_2.32 ABI baseline, or do you
want to support older binaries for an earlier non-upstream port,
backdating the baseline?

This matters because in the GLIBC_2.32 case, old binaries will not work
anyway, so we may as well require that they are rebuilt without
DT_INIT/DT_FINI.  In this case, the glibc master defaults should work.

If you want to support old binaries (which use older symbol versions
such as GLIBC_2.17), it may make sense to keep DT_INIT/DT_FINI support
as well.  To achieve this, you need to add an <elf-initifini.h> header
file with

/* Enable DT_INIT/DT_FINI support.  */
#define ELF_INITFINI 1

and keep the crti.S and crtn.S files you already have.

Thanks,
Florian
Vineet Gupta Feb. 26, 2020, 8:02 p.m. UTC | #12 
On 2/26/20 11:27 AM, Florian Weimer wrote:

> I haven't followed the ARC contribution process closely, sorry.  Do you

> plan to contribute the port with a GLIBC_2.32 ABI baseline, or do you

> want to support older binaries for an earlier non-upstream port,

> backdating the baseline?


No we don't need to support old binaries: 2.32 will be baseline

> This matters because in the GLIBC_2.32 case, old binaries will not work

> anyway, so we may as well require that they are rebuilt without

> DT_INIT/DT_FINI.  In this case, the glibc master defaults should work.


Understood !

> If you want to support old binaries (which use older symbol versions

> such as GLIBC_2.17), it may make sense to keep DT_INIT/DT_FINI support

> as well.  To achieve this, you need to add an <elf-initifini.h> header

> file with

> 

> /* Enable DT_INIT/DT_FINI support.  */

> #define ELF_INITFINI 1

> 

> and keep the crti.S and crtn.S files you already have.


Per the review comments on ARC port last year [1], I was advised to remove the ARC
specific crt*.S so those are anyhow gone.

[1] https://sourceware.org/ml/libc-alpha/2019-01/msg00654.html
diff mbox

Patch

diff --git a/csu/init-first.c b/csu/init-first.c
index 1cd8a75098..1fa1633657 100644
--- a/csu/init-first.c
+++ b/csu/init-first.c
@@ -46,9 +46,8 @@  __libc_init_first (int argc, char **argv, char **envp)
   /* For DSOs we do not need __libc_init_first but instead _init.  */
 }
 
-void
-attribute_hidden
-_init (int argc, char **argv, char **envp)
+static void __attribute__ ((constructor))
+init (int argc, char **argv, char **envp)
 {
 #endif