Message ID | mvmwp6em7uq.fsf@suse.de |
---|---|
State | New |
Headers | show |
On 08/08/2017 04:50 PM, Andreas Schwab wrote: > Unlike the vfork forwarder and like the fork forwarder as in bug 19861, > there won't be a problem when the compiler does not turn this into a tail > call. > > Andreas. > > * nptl/pt-longjmp.c (longjmp, siglongjmp): Don't use IFUNC resolver. > * nptl/pt-system.c (system): Likewise. Looks good to me. Thanks, Florian
On Tue, Aug 8, 2017 at 7:50 AM, Andreas Schwab <schwab@suse.de> wrote: > Unlike the vfork forwarder and like the fork forwarder as in bug 19861, > there won't be a problem when the compiler does not turn this into a tail > call. > > Andreas. > > * nptl/pt-longjmp.c (longjmp, siglongjmp): Don't use IFUNC resolver. > * nptl/pt-system.c (system): Likewise. Missing BZ #. Can you add a testcase from https://sourceware.org/bugzilla/show_bug.cgi?id=21041 > diff --git a/nptl/pt-longjmp.c b/nptl/pt-longjmp.c > index 2ef757e687..8f3c6b3a09 100644 > --- a/nptl/pt-longjmp.c > +++ b/nptl/pt-longjmp.c > @@ -25,21 +25,14 @@ > symbol in libpthread, but the historical ABI requires it. For static > linking, there is no need to provide anything here--the libc version > will be linked in. For shared library ABI compatibility, there must be > - longjmp and siglongjmp symbols in libpthread.so; so we define them using > - IFUNC to redirect to the libc function. */ > + longjmp and siglongjmp symbols in libpthread.so. > > -#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) > - > -# if HAVE_IFUNC > - > -# undef INIT_ARCH > -# define INIT_ARCH() > -# define DEFINE_LONGJMP(name) libc_ifunc (name, &__libc_longjmp) > - > -extern __typeof(longjmp) longjmp_ifunc; > -extern __typeof(siglongjmp) siglongjmp_ifunc; > + With an IFUNC resolver, it would be possible to avoid the indirection, > + but the IFUNC resolver might run before the __libc_longjmp symbol has > + been relocated, in which case the IFUNC resolver would not be able to > + provide the correct address. */ > > -# else /* !HAVE_IFUNC */ > +#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) > > static void __attribute__ ((noreturn, used)) > longjmp_compat (jmp_buf env, int val) > @@ -47,14 +40,10 @@ longjmp_compat (jmp_buf env, int val) > __libc_longjmp (env, val); > } > > -# define DEFINE_LONGJMP(name) strong_alias (longjmp_compat, name) > - > -# endif /* HAVE_IFUNC */ > - > -DEFINE_LONGJMP (longjmp_ifunc) > -compat_symbol (libpthread, longjmp_ifunc, longjmp, GLIBC_2_0); > +strong_alias (longjmp_compat, longjmp_alias) > +compat_symbol (libpthread, longjmp_alias, longjmp, GLIBC_2_0); > > -strong_alias (longjmp_ifunc, siglongjmp_ifunc) > -compat_symbol (libpthread, siglongjmp_ifunc, siglongjmp, GLIBC_2_0); > +strong_alias (longjmp_alias, siglongjmp_alias) > +compat_symbol (libpthread, siglongjmp_alias, siglongjmp, GLIBC_2_0); > > #endif > diff --git a/nptl/pt-system.c b/nptl/pt-system.c > index f8ca6ba0d9..b30ddf2b39 100644 > --- a/nptl/pt-system.c > +++ b/nptl/pt-system.c > @@ -25,29 +25,21 @@ > libpthread, but the historical ABI requires it. For static linking, > there is no need to provide anything here--the libc version will be > linked in. For shared library ABI compatibility, there must be a > - 'system' symbol in libpthread.so; so we define it using IFUNC to > - redirect to the libc function. */ > + 'system' symbol in libpthread.so. > > -#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) > - > -# if HAVE_IFUNC > - > -extern __typeof(system) system_ifunc; > -# undef INIT_ARCH > -# define INIT_ARCH() > -libc_ifunc (system_ifunc, &__libc_system) > + With an IFUNC resolver, it would be possible to avoid the indirection, > + but the IFUNC resolver might run before the __libc_system symbol has > + been relocated, in which case the IFUNC resolver would not be able to > + provide the correct address. */ > > -# else /* !HAVE_IFUNC */ > +#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) > > static int __attribute__ ((used)) > system_compat (const char *line) > { > return __libc_system (line); > } > -strong_alias (system_compat, system_ifunc) > - > -# endif /* HAVE_IFUNC */ > - > -compat_symbol (libpthread, system_ifunc, system, GLIBC_2_0); > +strong_alias (system_compat, system_alias) > +compat_symbol (libpthread, system_alias, system, GLIBC_2_0); > > #endif > -- > 2.14.0 > > > -- > 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."
I suspect this of having caused s390 build failures. s390x-glibc-linux-gnu-gcc -m31 ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c -c -std=gnu11 -fgnu89-inline -O2 -Wall -Werror -Wundef -Wwrite-strings -fmerge-all-constants -fno-stack-protector -frounding-math -g -Wstrict-prototypes -Wold-style-definition -mlong-double-128 -fPIC -ftls-model=initial-exec -I../include -I/scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/nptl -I/scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc -I../sysdeps/unix/sysv/linux/s390/s390-32 -I../sysdeps/unix/sysv/linux/s390/fpu -I../sysdeps/s390/fpu -I../sysdeps/unix/sysv/linux/s390 -I../sysdeps/s390/nptl -I../sysdeps/ieee754/ldbl-64-128 -I../sysdeps/ieee754/ldbl-opt -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux -I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu -I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix -I../sysdeps/posix -I../sysdeps/s390/s390-32/multiarch -I../sysdeps/s390/s390-32 -I../sysdeps/wordsize-32 -I../sysdeps/s390/multiarch -I../sysdeps/s390 -I../sysdeps/ieee754/ldbl-128 -I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32 -I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I. -D_LIBC_REENTRANT -include /scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/libc-modules.h -DMODULE_NAME=libpthread -include ../include/libc-symbols.h -DPIC -DSHARED -DTOP_NAMESPACE=glibc -o /scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/nptl/pt-longjmp.os -MD -MP -MF /scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/nptl/pt-longjmp.os.dt -MT /scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/nptl/pt-longjmp.os In file included from <command-line>:0:0: ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:29:15: error: 'longjmp_ifunc' undeclared here (not in a function); did you mean 'longjmp_alias'? strong_alias (longjmp_ifunc, __v2longjmp) ^ ./../include/libc-symbols.h:127:20: note: in definition of macro '_strong_alias' extern __typeof (name) aliasname __attribute__ ((alias (#name))); ^~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:29:1: note: in expansion of macro 'strong_alias' strong_alias (longjmp_ifunc, __v2longjmp) ^~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:31:15: error: 'siglongjmp_ifunc' undeclared here (not in a function); did you mean 'longjmp_ifunc'? strong_alias (siglongjmp_ifunc, __v2siglongjmp) ^ ./../include/libc-symbols.h:127:20: note: in definition of macro '_strong_alias' extern __typeof (name) aliasname __attribute__ ((alias (#name))); ^~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:31:1: note: in expansion of macro 'strong_alias' strong_alias (siglongjmp_ifunc, __v2siglongjmp) ^~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:31:33: error: '__v2siglongjmp' aliased to undefined symbol 'siglongjmp_ifunc' strong_alias (siglongjmp_ifunc, __v2siglongjmp) ^ ./../include/libc-symbols.h:127:20: note: in definition of macro '_strong_alias' extern __typeof (name) aliasname __attribute__ ((alias (#name))); ^~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:31:1: note: in expansion of macro 'strong_alias' strong_alias (siglongjmp_ifunc, __v2siglongjmp) ^~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:31:33: error: '__v2siglongjmp' aliased to undefined symbol 'siglongjmp_ifunc' strong_alias (siglongjmp_ifunc, __v2siglongjmp) ^ ./../include/libc-symbols.h:127:26: note: in definition of macro '_strong_alias' extern __typeof (name) aliasname __attribute__ ((alias (#name))); ^~~~~~~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:31:1: note: in expansion of macro 'strong_alias' strong_alias (siglongjmp_ifunc, __v2siglongjmp) ^~~~~~~~~~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:29:30: error: '__v2longjmp' aliased to undefined symbol 'longjmp_ifunc' strong_alias (longjmp_ifunc, __v2longjmp) ^ ./../include/libc-symbols.h:127:26: note: in definition of macro '_strong_alias' extern __typeof (name) aliasname __attribute__ ((alias (#name))); ^~~~~~~~~ ../sysdeps/unix/sysv/linux/s390/pt-longjmp.c:29:1: note: in expansion of macro 'strong_alias' strong_alias (longjmp_ifunc, __v2longjmp) ^~~~~~~~~~~~ /scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/sysd-rules:151: recipe for target '/scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/nptl/pt-longjmp.os' failed make[3]: *** [/scratch/jmyers/glibc-bot/build/glibcs/s390-linux-gnu/glibc/nptl/pt-longjmp.os] Error 1
Sorry, fixed now. Andreas.
Hi All, Le 08/08/2017 à 16:50, Andreas Schwab a écrit : > Unlike the vfork forwarder and like the fork forwarder as in bug 19861, > there won't be a problem when the compiler does not turn this into a tail > call. > > Andreas. > > * nptl/pt-longjmp.c (longjmp, siglongjmp): Don't use IFUNC resolver. > * nptl/pt-system.c (system): Likewise. > What's the status of this issue ? This patch has been merged in master (upcoming 2.27) but the ticket is still "NEW" [1]. The issue has been reported on the Buildroot mailing list by a user using glibc 2.25. I'm able to reproduce it using glibc 2.26-73. It's safe to backport this patch to glibc 2.25 and 2.26 stable branches ? Best regards, Romain [1] https://sourceware.org/bugzilla/show_bug.cgi?id=21041 [2] http://lists.busybox.net/pipermail/buildroot/2017-October/205719.html
On Nov 01 2017, Romain Naour <romain.naour@gmail.com> wrote: > Hi All, > > Le 08/08/2017 à 16:50, Andreas Schwab a écrit : >> Unlike the vfork forwarder and like the fork forwarder as in bug 19861, >> there won't be a problem when the compiler does not turn this into a tail >> call. >> >> Andreas. >> >> * nptl/pt-longjmp.c (longjmp, siglongjmp): Don't use IFUNC resolver. >> * nptl/pt-system.c (system): Likewise. >> > > What's the status of this issue ? > This patch has been merged in master (upcoming 2.27) but the ticket is still > "NEW" [1]. The bug is about moving a symbol between libraries, as such still unresolved. > It's safe to backport this patch to glibc 2.25 and 2.26 stable branches ? You can try. Andreas.
On 2017-11-06 09:48, Andreas Schwab wrote: > On Nov 01 2017, Romain Naour <romain.naour@gmail.com> wrote: > >> Hi All, >> >> Le 08/08/2017 à 16:50, Andreas Schwab a écrit : >>> Unlike the vfork forwarder and like the fork forwarder as in bug 19861, >>> there won't be a problem when the compiler does not turn this into a tail >>> call. >>> >>> Andreas. >>> >>> * nptl/pt-longjmp.c (longjmp, siglongjmp): Don't use IFUNC resolver. >>> * nptl/pt-system.c (system): Likewise. >>> >> >> What's the status of this issue ? >> This patch has been merged in master (upcoming 2.27) but the ticket is still >> "NEW" [1]. > > The bug is about moving a symbol between libraries, as such still > unresolved. > >> It's safe to backport this patch to glibc 2.25 and 2.26 stable branches ? > > You can try. > > Andreas. > I backported this to 2.26 in Arch some time ago. Looks fine on my end. Bartłomiej
Hi Andreas, Bartłomiej, Le 06/11/2017 à 14:30, Bartłomiej Piotrowski a écrit : > On 2017-11-06 09:48, Andreas Schwab wrote: >> On Nov 01 2017, Romain Naour <romain.naour@gmail.com> wrote: >> >>> Hi All, >>> >>> Le 08/08/2017 à 16:50, Andreas Schwab a écrit : >>>> Unlike the vfork forwarder and like the fork forwarder as in bug 19861, >>>> there won't be a problem when the compiler does not turn this into a tail >>>> call. >>>> >>>> Andreas. >>>> >>>> * nptl/pt-longjmp.c (longjmp, siglongjmp): Don't use IFUNC resolver. >>>> * nptl/pt-system.c (system): Likewise. >>>> >>> >>> What's the status of this issue ? >>> This patch has been merged in master (upcoming 2.27) but the ticket is still >>> "NEW" [1]. >> >> The bug is about moving a symbol between libraries, as such still >> unresolved. >> >>> It's safe to backport this patch to glibc 2.25 and 2.26 stable branches ? >> >> You can try. >> >> Andreas. >> > > I backported this to 2.26 in Arch some time ago. Looks fine on my end. Thank you both for the feedback. I did a runtime test with ffmpeg [1] and the "Relink" message is gone. No regression so far. [1] http://lists.busybox.net/pipermail/buildroot/2017-October/205865.html Best regards, Romain > > Bartłomiej >
diff --git a/nptl/pt-longjmp.c b/nptl/pt-longjmp.c index 2ef757e687..8f3c6b3a09 100644 --- a/nptl/pt-longjmp.c +++ b/nptl/pt-longjmp.c @@ -25,21 +25,14 @@ symbol in libpthread, but the historical ABI requires it. For static linking, there is no need to provide anything here--the libc version will be linked in. For shared library ABI compatibility, there must be - longjmp and siglongjmp symbols in libpthread.so; so we define them using - IFUNC to redirect to the libc function. */ + longjmp and siglongjmp symbols in libpthread.so. -#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) - -# if HAVE_IFUNC - -# undef INIT_ARCH -# define INIT_ARCH() -# define DEFINE_LONGJMP(name) libc_ifunc (name, &__libc_longjmp) - -extern __typeof(longjmp) longjmp_ifunc; -extern __typeof(siglongjmp) siglongjmp_ifunc; + With an IFUNC resolver, it would be possible to avoid the indirection, + but the IFUNC resolver might run before the __libc_longjmp symbol has + been relocated, in which case the IFUNC resolver would not be able to + provide the correct address. */ -# else /* !HAVE_IFUNC */ +#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) static void __attribute__ ((noreturn, used)) longjmp_compat (jmp_buf env, int val) @@ -47,14 +40,10 @@ longjmp_compat (jmp_buf env, int val) __libc_longjmp (env, val); } -# define DEFINE_LONGJMP(name) strong_alias (longjmp_compat, name) - -# endif /* HAVE_IFUNC */ - -DEFINE_LONGJMP (longjmp_ifunc) -compat_symbol (libpthread, longjmp_ifunc, longjmp, GLIBC_2_0); +strong_alias (longjmp_compat, longjmp_alias) +compat_symbol (libpthread, longjmp_alias, longjmp, GLIBC_2_0); -strong_alias (longjmp_ifunc, siglongjmp_ifunc) -compat_symbol (libpthread, siglongjmp_ifunc, siglongjmp, GLIBC_2_0); +strong_alias (longjmp_alias, siglongjmp_alias) +compat_symbol (libpthread, siglongjmp_alias, siglongjmp, GLIBC_2_0); #endif diff --git a/nptl/pt-system.c b/nptl/pt-system.c index f8ca6ba0d9..b30ddf2b39 100644 --- a/nptl/pt-system.c +++ b/nptl/pt-system.c @@ -25,29 +25,21 @@ libpthread, but the historical ABI requires it. For static linking, there is no need to provide anything here--the libc version will be linked in. For shared library ABI compatibility, there must be a - 'system' symbol in libpthread.so; so we define it using IFUNC to - redirect to the libc function. */ + 'system' symbol in libpthread.so. -#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) - -# if HAVE_IFUNC - -extern __typeof(system) system_ifunc; -# undef INIT_ARCH -# define INIT_ARCH() -libc_ifunc (system_ifunc, &__libc_system) + With an IFUNC resolver, it would be possible to avoid the indirection, + but the IFUNC resolver might run before the __libc_system symbol has + been relocated, in which case the IFUNC resolver would not be able to + provide the correct address. */ -# else /* !HAVE_IFUNC */ +#if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) static int __attribute__ ((used)) system_compat (const char *line) { return __libc_system (line); } -strong_alias (system_compat, system_ifunc) - -# endif /* HAVE_IFUNC */ - -compat_symbol (libpthread, system_ifunc, system, GLIBC_2_0); +strong_alias (system_compat, system_alias) +compat_symbol (libpthread, system_alias, system, GLIBC_2_0); #endif