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

Message ID 87v9nwqm10.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series
  • csu: Use ELF constructor instead of _init in libc.so
Related show

Commit Message

Florian Weimer Feb. 24, 2020, 11:09 a.m.
Tested on aarch64-linux-gnu, i386-linux-gnu, powerpc64le-linux-gnu,
x86_64-linux-gnu.  Also built on i686-gnu,
riscv64-linux-gnu-rv64imac-lp64, and s390x-linux-gnu.

Would someone with access to RISC-V hardware please test this patch?
Thanks.

Florian
8<------------------------------------------------------------------8<
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.)

-----
 csu/init-first.c        | 12 ++++++------
 elf/soinit.c            |  2 +-
 include/libc-internal.h |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Andreas Schwab Feb. 24, 2020, 11:13 a.m. | #1
On Feb 24 2020, Florian Weimer wrote:

> Would someone with access to RISC-V hardware please test this patch?


You can use one of the images in
<https://download.opensuse.org/ports/riscv/tumbleweed/images/> and use
it under qemu-linux-user.  See <https://en.opensuse.org/openSUSE:RISC-V>
for details.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Andreas Schwab Feb. 24, 2020, 12:02 p.m. | #2
On Feb 24 2020, Florian Weimer wrote:

> Tested on aarch64-linux-gnu, i386-linux-gnu, powerpc64le-linux-gnu,

> x86_64-linux-gnu.  Also built on i686-gnu,

> riscv64-linux-gnu-rv64imac-lp64, and s390x-linux-gnu.

>

> Would someone with access to RISC-V hardware please test this patch?


I have thrown your patch into a test build
<https://build.opensuse.org/project/show/home:Andreas_Schwab:glibc:test>.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Jim Wilson Feb. 24, 2020, 10:38 p.m. | #3
On Mon, Feb 24, 2020 at 3:09 AM Florian Weimer <fweimer@redhat.com> wrote:
> Would someone with access to RISC-V hardware please test this patch?


Friday, I tested the previous version of this patch on a RISC-V Fedora
rawhide system with a glibc build and check.  The check failed badly
without the patch, it didn't even make it far enough to report
results.  It did succeed with the patch, giving the expected result of
about 17 failures.  It looks like the only changes since the previous
version are to comments.  I can try another check if necessary but I
think the testing Andreas is doing is probably better than my testing.

Jim
Florian Weimer Feb. 25, 2020, 12:34 p.m. | #4
* Andreas Schwab:

> On Feb 24 2020, Florian Weimer wrote:

>

>> Tested on aarch64-linux-gnu, i386-linux-gnu, powerpc64le-linux-gnu,

>> x86_64-linux-gnu.  Also built on i686-gnu,

>> riscv64-linux-gnu-rv64imac-lp64, and s390x-linux-gnu.

>>

>> Would someone with access to RISC-V hardware please test this patch?

>

> I have thrown your patch into a test build

> <https://build.opensuse.org/project/show/home:Andreas_Schwab:glibc:test>.


Would you please help me to interpret the results?  Is the ppc64le
failure spurious?  I can't see any details on the public view.

Thanks,
Florian
Andreas Schwab Feb. 25, 2020, 12:57 p.m. | #5
On Feb 25 2020, Florian Weimer wrote:

> Would you please help me to interpret the results?  Is the ppc64le

> failure spurious?  I can't see any details on the public view.


[ 3013s] /.build/build: line 437: echo: write error: No space left on device

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Florian Weimer Feb. 25, 2020, 12:58 p.m. | #6
* Andreas Schwab:

> On Feb 25 2020, Florian Weimer wrote:

>

>> Would you please help me to interpret the results?  Is the ppc64le

>> failure spurious?  I can't see any details on the public view.

>

> [ 3013s] /.build/build: line 437: echo: write error: No space left on device


Okay, so I guess that's something we can ignore.

Has the riscv64 build proceeded past the critical point yet?

Thanks,
Florian
Andreas Schwab Feb. 25, 2020, 1:18 p.m. | #7
On Feb 25 2020, Florian Weimer wrote:

> Has the riscv64 build proceeded past the critical point yet?


Yes.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Florian Weimer Feb. 25, 2020, 1:44 p.m. | #8
* Andreas Schwab:

> On Feb 25 2020, Florian Weimer wrote:

>

>> Has the riscv64 build proceeded past the critical point yet?

>

> Yes.


Should I commit my patch then?

Thanks,
Florian
Andreas Schwab Feb. 25, 2020, 1:53 p.m. | #9
On Feb 25 2020, Florian Weimer wrote:

> Should I commit my patch then?


Please do.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Alistair Francis Feb. 25, 2020, 10:46 p.m. | #10
On Tue, Feb 25, 2020 at 5:53 AM Andreas Schwab <schwab@suse.de> wrote:
>

> On Feb 25 2020, Florian Weimer wrote:

>

> > Should I commit my patch then?

>

> Please do.


I'm a little late (I missed the original conversation) but this patch
fixes RV32 errors as well.

Alistair

>

> Andreas.

>

> --

> Andreas Schwab, SUSE Labs, schwab@suse.de

> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7

> "And now for something completely different."

Patch

diff --git a/csu/init-first.c b/csu/init-first.c
index 1cd8a75098..264e6f348d 100644
--- a/csu/init-first.c
+++ b/csu/init-first.c
@@ -43,12 +43,11 @@  void
 __libc_init_first (int argc, char **argv, char **envp)
 {
 #ifdef SHARED
-  /* For DSOs we do not need __libc_init_first but instead _init.  */
+  /* For DSOs we do not need __libc_init_first but an ELF constructor.  */
 }
 
-void
-attribute_hidden
-_init (int argc, char **argv, char **envp)
+static void __attribute__ ((constructor))
+_init_first (int argc, char **argv, char **envp)
 {
 #endif
 
@@ -86,8 +85,9 @@  _init (int argc, char **argv, char **envp)
 
 /* This function is defined here so that if this file ever gets into
    ld.so we will get a link error.  Having this file silently included
-   in ld.so causes disaster, because the _init definition above will
-   cause ld.so to gain an init function, which is not a cool thing. */
+   in ld.so causes disaster, because the _init_first definition above
+   will cause ld.so to gain an ELF constructor, which is not a cool
+   thing. */
 
 extern void _dl_start (void) __attribute__ ((noreturn));
 
diff --git a/elf/soinit.c b/elf/soinit.c
index fe9935732b..538eb68186 100644
--- a/elf/soinit.c
+++ b/elf/soinit.c
@@ -20,7 +20,7 @@  run_hooks (void (*const list[]) (void))
     (**list) ();
 }
 
-/* This function will be called from _init in init-first.c.  */
+/* This function will be called from _init_first in init-first.c.  */
 void
 __libc_global_ctors (void)
 {
diff --git a/include/libc-internal.h b/include/libc-internal.h
index fcd21468c0..4c74f6ba60 100644
--- a/include/libc-internal.h
+++ b/include/libc-internal.h
@@ -24,7 +24,7 @@ 
 /* Initialize the `__libc_enable_secure' flag.  */
 extern void __libc_init_secure (void);
 
-/* This function will be called from _init in init-first.c.  */
+/* This function will be called from _init_first in init-first.c.  */
 extern void __libc_global_ctors (void);
 
 /* Discover the tick frequency of the machine if something goes wrong,