[musl] [RFC PATCH] Fix __libc_start_main prototype in [r]crt1.c to match the caller

[m***@megous.com]

From: Ondrej Jirman <***@megous.com>

__libc_start_main function is not using the last three arguments.

GCC in LTO mode complains about mismatch.
---
crt/crt1.c | 6 +++---
crt/rcrt1.c | 7 ++++---
2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/crt/crt1.c b/crt/crt1.c
index 7b12665f..81ecf118 100644
--- a/crt/crt1.c
+++ b/crt/crt1.c
@@ -8,12 +8,12 @@
int main();
weak void _init();
weak void _fini();
-_Noreturn int __libc_start_main(int (*)(), int, char **,
- void (*)(), void(*)(), void(*)());
+_Noreturn
+int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv);

void _start_c(long *p)
{
int argc = p[0];
char **argv = (void *)(p+1);
- __libc_start_main(main, argc, argv, _init, _fini, 0);
+ __libc_start_main(main, argc, argv);
}
diff --git a/crt/rcrt1.c b/crt/rcrt1.c
index 7bb3322f..52928b3e 100644
--- a/crt/rcrt1.c
+++ b/crt/rcrt1.c
@@ -5,10 +5,11 @@
int main();
weak void _init();
weak void _fini();
-_Noreturn int __libc_start_main(int (*)(), int, char **,
- void (*)(), void(*)(), void(*)());
+
+_Noreturn
+int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv);

hidden _Noreturn void __dls2(unsigned char *base, size_t *sp)
{
- __libc_start_main(main, *sp, (void *)(sp+1), _init, _fini, 0);
+ __libc_start_main(main, *sp, (void *)(sp+1));
}

[Szabolcs Nagy]

Post by m***@megous.com
__libc_start_main function is not using the last three arguments.
GCC in LTO mode complains about mismatch.

fix it in the other way then.

Post by m***@megous.com
- __libc_start_main(main, argc, argv, _init, _fini, 0);
+ __libc_start_main(main, argc, argv);

you just completely broke everything there didnt you?

how will the _init/_fini code of executables with
DT_INIT, DT_FINI dynamic tags run?

i think gcc still havent fixed weak object symbol alias
bugs with lto so e.g. you will get incorrect environ if
you lto link the libc.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69271

[Szabolcs Nagy]

Post by Szabolcs Nagy

Post by m***@megous.com
__libc_start_main function is not using the last three arguments.
GCC in LTO mode complains about mismatch.

fix it in the other way then.

Post by m***@megous.com
- __libc_start_main(main, argc, argv, _init, _fini, 0);
+ __libc_start_main(main, argc, argv);

you just completely broke everything there didnt you?

sorry the _init, _fini there is only needed for glibc compat
(i.e. executable linked with musl crt1.o, but using glibc
to run it, which should not be a common use case)

Post by Szabolcs Nagy
how will the _init/_fini code of executables with
DT_INIT, DT_FINI dynamic tags run?
i think gcc still havent fixed weak object symbol alias
bugs with lto so e.g. you will get incorrect environ if
you lto link the libc.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69271

these lto issues still apply.

[Rich Felker]

Post by Szabolcs Nagy

Post by Szabolcs Nagy

Post by m***@megous.com
__libc_start_main function is not using the last three arguments.
GCC in LTO mode complains about mismatch.

fix it in the other way then.

Post by m***@megous.com
- __libc_start_main(main, argc, argv, _init, _fini, 0);
+ __libc_start_main(main, argc, argv);

you just completely broke everything there didnt you?

sorry the _init, _fini there is only needed for glibc compat
(i.e. executable linked with musl crt1.o, but using glibc
to run it, which should not be a common use case)

This is not remotely a supported case. However the history here is
more subtle. At one point, analysis was done, and the following
conclusions were drawn:

- For static linking, there's no advantage of having the _init and
_fini pointers passed from crt to libc since there are no external
interfaces involved. The symbolic binding from __libc_start_main.c
will resolve to the same thing the symbolic binding in crt1.o would.

- For dynamic linking, DT_INIT/DT_FINI are available and should be
preferred over the symbolic binding. Historically the symbolic
binding of _init and _fini in crt1 rather than from
__libc_start_main.c prevented these two names from being ABI
(they're not supposed to be ABI), but using the _DYNAMIC entries is
a strictly better way to achieve the same.

However, at some point in the distant past, musl did take the _init
and _fini pointers as arguments to __libc_start_main from crt1. By
keeping the code that would pass them, but omitting the code that
would receive and use them, compatibility was preserved in case we
ever wanted to go back to the old way of doing things. If any crt1
ever shipped without passing them, going back would be impossible,
since there would be binaries out in the wild that would be passing
junk in those argument slots.

It seems increasingly implausible that we would ever want to go back
to the old way of doing things, and I'm pretty sure we actually can't,
because I think there were historically some archs where the crt1 used
to be written in asm, and the asm did not pass the _init and _fini
pointers. If this is really the case, we should probably just throw
away the pretense that we still support the possibility of going back.

Aside from that, there is some argument to be made that crt1 should
not be musl-specific and should be written such that it works with any
libc respecting the __libc_start_main entry point ABI. However this is
not currently true -- the crt code loses the register-passed argument
ldso is allowed to pass to request registration of an atexit handler
-- and thus it's not something we really need to consider as a
constraint that must be met.

If for some reason it turns out we don't want to drop the convention
of passing the extra 3 unused arguments, we could just update
__libc_start_main to accept and ignore them. I forget why this was
removed to begin with, but it seems like it would be nice; in general,
having extra unused incoming arguments is desirable in that it
improves the ability to tail-call, though I don't think it helps here
anyway.

Post by Szabolcs Nagy

Post by Szabolcs Nagy
how will the _init/_fini code of executables with
DT_INIT, DT_FINI dynamic tags run?
i think gcc still havent fixed weak object symbol alias
bugs with lto so e.g. you will get incorrect environ if
you lto link the libc.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69271

these lto issues still apply.

This affects building *of* libc.so with LTO, but does not affect
linking *to* libc.a with LTO. The latter should be expected to work,
unless _start_c is still getting wrongly optimized out...

Rich

[Ondřej Jirman]

Post by Szabolcs Nagy

Post by Szabolcs Nagy

Post by m***@megous.com
__libc_start_main function is not using the last three arguments.
GCC in LTO mode complains about mismatch.

fix it in the other way then.

Post by m***@megous.com
- __libc_start_main(main, argc, argv, _init, _fini, 0);
+ __libc_start_main(main, argc, argv);

you just completely broke everything there didnt you?

sorry the _init, _fini there is only needed for glibc compat
(i.e. executable linked with musl crt1.o, but using glibc
to run it, which should not be a common use case)

I had a hunch that there was some reason outside of musl for these arguments,
therefore just the RFC. I guess, it's better to ignore the patch then.

I just saw a difference of prototypes, so I sent the patch.

Post by Szabolcs Nagy

Post by Szabolcs Nagy
how will the _init/_fini code of executables with
DT_INIT, DT_FINI dynamic tags run?
i think gcc still havent fixed weak object symbol alias
bugs with lto so e.g. you will get incorrect environ if
you lto link the libc.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69271

these lto issues still apply.

I'm attempting a static build, for which this issue should not apply, AFAIK.

Though, I'm trying on mips, and it leads to GOT related error. So LTO of musl+my
code doesn't work anyway, because ld fails. I'll try with some other target just
to see if I can get something that works, without the limitations of mips arch.

regards,
o.