Undefined use of weak symbols in gnulib

Undefined use of weak symbols in gnulib

[Florian Weimer]

lib/glthread/lock.h has this:

| /* The way to test at runtime whether libpthread is present is to test
|    whether a function pointer's value, such as &pthread_mutex_init, is
|    non-NULL.  However, some versions of GCC have a bug through which, in
|    PIC mode, &foo != NULL always evaluates to true if there is a direct
|    call to foo(...) in the same function.  To avoid this, we test the
|    address of a function in libpthread that we don't use.  */
| 
| #  pragma weak pthread_mutex_init
| #  pragma weak pthread_mutex_lock
| #  pragma weak pthread_mutex_unlock
| #  pragma weak pthread_mutex_destroy
| #  pragma weak pthread_rwlock_init
| #  pragma weak pthread_rwlock_rdlock
| #  pragma weak pthread_rwlock_wrlock
| #  pragma weak pthread_rwlock_unlock
| #  pragma weak pthread_rwlock_destroy
| #  pragma weak pthread_once
| […]

And:

| #  if !PTHREAD_IN_USE_DETECTION_HARD
| #   pragma weak pthread_mutexattr_gettype
| #   define pthread_in_use() \
|       (pthread_mutexattr_gettype != NULL || c11_threads_in_use ())
| #  endif

As far as I can tell gnulib uses this macro definition to implement
gl_once on glibc targets:

| #  define glthread_once(ONCE_CONTROL, INITFUNCTION) \
|      (pthread_in_use ()                                                        \
|       ? pthread_once (ONCE_CONTROL, INITFUNCTION)                              \
|       : (glthread_once_singlethreaded (ONCE_CONTROL) ? (INITFUNCTION (), 0) : 0))

So the net effect is this:

  if (pthread_mutexattr_gettype != NULL)
    pthread_once (control, callback);

Dynamic linking with weak symbols is not very well-defined.  On x86-64,
the link editor produces the expected dynamic symbol relocation for the
pthread_once call.  On other targets (notably POWER), no dynamic
relocation is produced, and the code will crash if
pthread_mutexattr_gettype is ever defined.

There is an old thread here covering related issues:

  Specify how undefined weak symbol should be resolved in executable
  <https://sourceware.org/legacy-ml/gnu-gabi/2016-q1/msg00004.html>

On glibc targets, there is another problem: weak references do not carry
symbol versions, so they can bind to base versions unexpectedly.

This will become an urgent issue with glibc 2.34, which defines
pthread_mutexattr_gettype unconditionally.  Certain gnulib modules will
stop working until the binaries are relinked.  I expect the issue is
already visible with earlier glibc versions if libpthread is
unexpectedly present at run time.

I think we can provide an libBrokenGnulib.so preload module which
defines pthread_mutexattr_gettype to zero (as an absolute address), so
there is a kludge to keep old binaries working, but this is really
something that must be fixed in gnulib.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Paul Eggert]

On 4/26/21 10:53 PM, Florian Weimer via Libc-alpha wrote:
> This will become an urgent issue with glibc 2.34, which defines
> pthread_mutexattr_gettype unconditionally.  Certain gnulib modules will
> stop working until the binaries are relinked.

Thanks for mentioning this. But in what sense will the modules stop 
working? I'm a little lost.

glibc 2.34 also defines pthread_once unconditionally, right? So why 
doesn't code like this:

   if (pthread_mutexattr_gettype != NULL)
     pthread_once (control, callback);

continue to work in 2.34?

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Paul Eggert:

> On 4/26/21 10:53 PM, Florian Weimer via Libc-alpha wrote:
>> This will become an urgent issue with glibc 2.34, which defines
>> pthread_mutexattr_gettype unconditionally.  Certain gnulib modules will
>> stop working until the binaries are relinked.
>
> Thanks for mentioning this. But in what sense will the modules stop
> working? I'm a little lost.
>
> glibc 2.34 also defines pthread_once unconditionally, right? So why
> doesn't code like this:
>
>   if (pthread_mutexattr_gettype != NULL)
>     pthread_once (control, callback);
>
> continue to work in 2.34?

The link editor does not emitted a relocation referencing the
pthread_once symbol.  The function address in the GOT is always zero.
It does not matter if a pthread_once symbol exists at run time because
their is no reference to it.  Relinking the executable will of course
fix this and generate a reference to the pthread_once symbol, but
relinking is required.

For example, bison 3.7.4 built against glibc 2.33 has these pthread*
symbol references in its dynamic symbol table on powerpc64le-linux-gnu:

   58: 0000000000000000      0 FUNC    WEAK   DEFAULT    UNDEF pthread_mutex_init@GLIBC_2.17 (2)
  117: 0000000000000000      0 FUNC    WEAK   DEFAULT    UNDEF pthread_mutex_lock@GLIBC_2.17 (2)
  118: 0000000000000000      0 NOTYPE  WEAK   DEFAULT    UNDEF pthread_mutexattr_gettype
  120: 0000000000000000      0 FUNC    WEAK   DEFAULT    UNDEF pthread_mutex_unlock@GLIBC_2.17 (2)

pthread_once is just not there.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Paul Eggert]

I'm still not quite following, but the message I'm getting is "don't 
mess with dynamic detection of whether the pthread functions are linked 
in, because there's no way to do it reliably." If so, I suppose Gnulib 
will have to require pthread to be linked into all libraries that use 
any of these functions, which has some negative consequences (at least 
for glibc 2.33 and earlier).

Bruno's the expert here, though; I hope he has time to weigh in.

Re: Undefined use of weak symbols in gnulib

[Andreas Schwab]

On Apr 27 2021, Florian Weimer via Binutils wrote:

> I think we can provide an libBrokenGnulib.so preload module which
> defines pthread_mutexattr_gettype to zero (as an absolute address), so
> there is a kludge to keep old binaries working, but this is really
> something that must be fixed in gnulib.

It is likely that the use of weak pthread symbols is not confined to
gnulib.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Andreas Schwab:

> On Apr 27 2021, Florian Weimer via Binutils wrote:
>
>> I think we can provide an libBrokenGnulib.so preload module which
>> defines pthread_mutexattr_gettype to zero (as an absolute address), so
>> there is a kludge to keep old binaries working, but this is really
>> something that must be fixed in gnulib.
>
> It is likely that the use of weak pthread symbols is not confined to
> gnulib.

True.

Here's a fairly representative test case, I think.

#include <pthread.h>
#include <stdio.h>

extern __typeof (pthread_key_create) __pthread_key_create __attribute__ ((weak));
extern __typeof (pthread_once) pthread_once __attribute__ ((weak));

void
f1 (void)
{
  puts ("f1 called");
}

pthread_once_t once_var;

void __attribute__ ((weak))
f2 (void)
{
  if (__pthread_key_create != NULL)
    pthread_once (&once_var, f1);
}

int
main (void)
{
  f2 ();
}

Building it with “gcc -O2 -fpie -pie” and linking with binutils 2.30
does not result in a crash with LD_PRELOAD=libpthread.so.0.  That's
because the call-to-address-zero via pthread_once has been stubbed out
with NOPs, I think.  This is still not correct and will undoubtedly
cause problems because pthread_once is usually called for its side
effect.

With binutils 2.35, the call-to-address-zero is not stubbed out anymore,
but there is still no dynamic symbol reference for pthread_once, so
there is a crash when running with LD_PRELOAD=libpthread.so.0.

This looks like a pre-existing toolchain issue on POWER, related to the
thread I quoted at the start.  If we proceed with the glibc libpthread
changes as planned, things might turn out unacceptably bad.

I did a quick experiment and we cannot work around this using a
libpthread.so stub library and compatibility-only symbols in libc.so.6.
Unversioned weak symbols bind to the baseline symbol version, as they
probably should.  Even a __pthread_key_create@GLIBC_2.17 compatibility
symbol triggers this issue.  I have to think of something else to
salvage this.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Hi Florian,

> Here's a fairly representative test case, I think.
> 
> #include <pthread.h>
> #include <stdio.h>
> 
> extern __typeof (pthread_key_create) __pthread_key_create __attribute__ ((weak));
> extern __typeof (pthread_once) pthread_once __attribute__ ((weak));
> 
> void
> f1 (void)
> {
>   puts ("f1 called");
> }
> 
> pthread_once_t once_var;
> 
> void __attribute__ ((weak))
> f2 (void)
> {
>   if (__pthread_key_create != NULL)
>     pthread_once (&once_var, f1);
> }
> 
> int
> main (void)
> {
>   f2 ();
> }
> 
> Building it with “gcc -O2 -fpie -pie” and linking with binutils 2.30
> does not result in a crash with LD_PRELOAD=libpthread.so.0.

Thank you for the test case. It helps the understanding.

But I don't understand
  - why anyone would redeclare 'pthread_once', when it's a standard POSIX
    function,
  - why f2 is declared weak,
  - why the program skips its initializations in single-threaded mode,
  - why libpthread would be loaded through LD_PRELOAD or dlopen, given
    that the long-term statement has been that declaring a symbol weak
    has no effect on the dynamic linker [1][2][3][4]?

How about the following test case instead?

=====================================================================
#include <pthread.h>
#include <stdio.h>

#pragma weak pthread_key_create
#pragma weak pthread_once

void
do_init (void)
{
  puts ("initialization code");
}

pthread_once_t once_var;

void
init (void)
{
  if (pthread_key_create != NULL)
    {
      puts ("multi-threaded initialization");
      pthread_once (&once_var, do_init);
    }
  else
    do_init ();
}

int
main (void)
{
  init ();
}
=====================================================================

$ gcc -Wall -fpie -pie foo.c ; ./a.out 
initialization code

$ gcc -Wall -fpie -pie foo.c -Wl,--no-as-needed -lpthread ; ./a.out
multi-threaded initialization
initialization code

What will change for this program with glibc 2.34?

Bruno

[1] https://sourceware.org/legacy-ml/libc-hacker/2000-06/msg00029.html
[2] https://www.akkadia.org/drepper/dsohowto.pdf page 6
[3] https://stackoverflow.com/questions/21092601/is-pthread-in-glibc-so-implemented-by-weak-symbol-to-provide-pthread-stub-functi/21103255
[4] https://stackoverflow.com/questions/20658809/dynamic-loading-and-weak-symbol-resolution

Re: Undefined use of weak symbols in gnulib

[H.J. Lu]

On Tue, Apr 27, 2021 at 6:57 PM Bruno Haible <bruno@clisp.org> wrote:
>
> Hi Florian,
>
> > Here's a fairly representative test case, I think.
> >
> > #include <pthread.h>
> > #include <stdio.h>
> >
> > extern __typeof (pthread_key_create) __pthread_key_create __attribute__ ((weak));
> > extern __typeof (pthread_once) pthread_once __attribute__ ((weak));
> >
> > void
> > f1 (void)
> > {
> >   puts ("f1 called");
> > }
> >
> > pthread_once_t once_var;
> >
> > void __attribute__ ((weak))
> > f2 (void)
> > {
> >   if (__pthread_key_create != NULL)
> >     pthread_once (&once_var, f1);
> > }
> >
> > int
> > main (void)
> > {
> >   f2 ();
> > }
> >
> > Building it with “gcc -O2 -fpie -pie” and linking with binutils 2.30
> > does not result in a crash with LD_PRELOAD=libpthread.so.0.
>
> Thank you for the test case. It helps the understanding.
>
> But I don't understand
>   - why anyone would redeclare 'pthread_once', when it's a standard POSIX
>     function,
>   - why f2 is declared weak,
>   - why the program skips its initializations in single-threaded mode,
>   - why libpthread would be loaded through LD_PRELOAD or dlopen, given
>     that the long-term statement has been that declaring a symbol weak
>     has no effect on the dynamic linker [1][2][3][4]?
>
> How about the following test case instead?
>
> =====================================================================
> #include <pthread.h>
> #include <stdio.h>
>
> #pragma weak pthread_key_create
> #pragma weak pthread_once
>
> void
> do_init (void)
> {
>   puts ("initialization code");
> }
>
> pthread_once_t once_var;
>
> void
> init (void)
> {
>   if (pthread_key_create != NULL)
>     {
>       puts ("multi-threaded initialization");
>       pthread_once (&once_var, do_init);
>     }
>   else
>     do_init ();
> }
>
> int
> main (void)
> {
>   init ();
> }
> =====================================================================
>
> $ gcc -Wall -fpie -pie foo.c ; ./a.out
> initialization code
>
> $ gcc -Wall -fpie -pie foo.c -Wl,--no-as-needed -lpthread ; ./a.out
> multi-threaded initialization
> initialization code
>
> What will change for this program with glibc 2.34?
>
> Bruno
>
> [1] https://sourceware.org/legacy-ml/libc-hacker/2000-06/msg00029.html
> [2] https://www.akkadia.org/drepper/dsohowto.pdf page 6
> [3] https://stackoverflow.com/questions/21092601/is-pthread-in-glibc-so-implemented-by-weak-symbol-to-provide-pthread-stub-functi/21103255
> [4] https://stackoverflow.com/questions/20658809/dynamic-loading-and-weak-symbol-resolution
>

Does x86 show the same issue?  I fixed several undefined weak symbol
bugs on x86:

https://sourceware.org/bugzilla/show_bug.cgi?id=19636
https://sourceware.org/bugzilla/show_bug.cgi?id=19704
https://sourceware.org/bugzilla/show_bug.cgi?id=19719

with a linker option:

    'dynamic-undefined-weak'
     'nodynamic-undefined-weak'
          Make undefined weak symbols dynamic when building a dynamic
          object, if they are referenced from a regular object file and
          not forced local by symbol visibility or versioning.  Do not
          make them dynamic if 'nodynamic-undefined-weak'.  If neither
          option is given, a target may default to either option being
          in force, or make some other selection of undefined weak
          symbols dynamic.  Not all targets support these options.

Alan extended the fix to PPC:

commit 954b63d4c8645f86e40c7ef6c6d60acd2bf019de
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Apr 19 01:26:57 2017 +0930

    Implement -z dynamic-undefined-weak

    -z nodynamic-undefined-weak is only implemented for x86.  (The sparc
    backend has some support code but doesn't enable the option by
    including ld/emulparams/dynamic_undefined_weak.sh, and since the
    support looks like it may be broken I haven't enabled it.)  This patch
    adds the complementary -z dynamic-undefined-weak, extends both options
    to affect building of shared libraries as well as executables, and
    adds support for the option on powerpc.

-- 
H.J.

Re: Undefined use of weak symbols in gnulib

[H.J. Lu]

On Tue, Apr 27, 2021 at 7:10 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Apr 27, 2021 at 6:57 PM Bruno Haible <bruno@clisp.org> wrote:
> >
> > Hi Florian,
> >
> > > Here's a fairly representative test case, I think.
> > >
> > > #include <pthread.h>
> > > #include <stdio.h>
> > >
> > > extern __typeof (pthread_key_create) __pthread_key_create __attribute__ ((weak));
> > > extern __typeof (pthread_once) pthread_once __attribute__ ((weak));
> > >
> > > void
> > > f1 (void)
> > > {
> > >   puts ("f1 called");
> > > }
> > >
> > > pthread_once_t once_var;
> > >
> > > void __attribute__ ((weak))
> > > f2 (void)
> > > {
> > >   if (__pthread_key_create != NULL)
> > >     pthread_once (&once_var, f1);
> > > }
> > >
> > > int
> > > main (void)
> > > {
> > >   f2 ();
> > > }
> > >
> > > Building it with “gcc -O2 -fpie -pie” and linking with binutils 2.30
> > > does not result in a crash with LD_PRELOAD=libpthread.so.0.
> >
> > Thank you for the test case. It helps the understanding.
> >
> > But I don't understand
> >   - why anyone would redeclare 'pthread_once', when it's a standard POSIX
> >     function,
> >   - why f2 is declared weak,
> >   - why the program skips its initializations in single-threaded mode,
> >   - why libpthread would be loaded through LD_PRELOAD or dlopen, given
> >     that the long-term statement has been that declaring a symbol weak
> >     has no effect on the dynamic linker [1][2][3][4]?
> >
> > How about the following test case instead?
> >
> > =====================================================================
> > #include <pthread.h>
> > #include <stdio.h>
> >
> > #pragma weak pthread_key_create
> > #pragma weak pthread_once
> >
> > void
> > do_init (void)
> > {
> >   puts ("initialization code");
> > }
> >
> > pthread_once_t once_var;
> >
> > void
> > init (void)
> > {
> >   if (pthread_key_create != NULL)
> >     {
> >       puts ("multi-threaded initialization");
> >       pthread_once (&once_var, do_init);
> >     }
> >   else
> >     do_init ();
> > }
> >
> > int
> > main (void)
> > {
> >   init ();
> > }
> > =====================================================================
> >
> > $ gcc -Wall -fpie -pie foo.c ; ./a.out
> > initialization code
> >
> > $ gcc -Wall -fpie -pie foo.c -Wl,--no-as-needed -lpthread ; ./a.out
> > multi-threaded initialization
> > initialization code
> >
> > What will change for this program with glibc 2.34?
> >
> > Bruno
> >
> > [1] https://sourceware.org/legacy-ml/libc-hacker/2000-06/msg00029.html
> > [2] https://www.akkadia.org/drepper/dsohowto.pdf page 6
> > [3] https://stackoverflow.com/questions/21092601/is-pthread-in-glibc-so-implemented-by-weak-symbol-to-provide-pthread-stub-functi/21103255
> > [4] https://stackoverflow.com/questions/20658809/dynamic-loading-and-weak-symbol-resolution
> >
>
> Does x86 show the same issue?  I fixed several undefined weak symbol
> bugs on x86:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=19636
> https://sourceware.org/bugzilla/show_bug.cgi?id=19704
> https://sourceware.org/bugzilla/show_bug.cgi?id=19719
>
> with a linker option:
>
>     'dynamic-undefined-weak'
>      'nodynamic-undefined-weak'
>           Make undefined weak symbols dynamic when building a dynamic
>           object, if they are referenced from a regular object file and
>           not forced local by symbol visibility or versioning.  Do not
>           make them dynamic if 'nodynamic-undefined-weak'.  If neither
>           option is given, a target may default to either option being
>           in force, or make some other selection of undefined weak
>           symbols dynamic.  Not all targets support these options.
>
> Alan extended the fix to PPC:
>
> commit 954b63d4c8645f86e40c7ef6c6d60acd2bf019de
> Author: Alan Modra <amodra@gmail.com>
> Date:   Wed Apr 19 01:26:57 2017 +0930
>
>     Implement -z dynamic-undefined-weak
>
>     -z nodynamic-undefined-weak is only implemented for x86.  (The sparc
>     backend has some support code but doesn't enable the option by
>     including ld/emulparams/dynamic_undefined_weak.sh, and since the
>     support looks like it may be broken I haven't enabled it.)  This patch
>     adds the complementary -z dynamic-undefined-weak, extends both options
>     to affect building of shared libraries as well as executables, and
>     adds support for the option on powerpc.
>

Another undefined weak symbol linker bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=22269

-- 
H.J.

Re: Undefined use of weak symbols in gnulib

[Fangrui Song]

On 2021-04-27, H.J. Lu via Binutils wrote:
>On Tue, Apr 27, 2021 at 7:10 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Tue, Apr 27, 2021 at 6:57 PM Bruno Haible <bruno@clisp.org> wrote:
>> >
>> > Hi Florian,
>> >
>> > > Here's a fairly representative test case, I think.
>> > >
>> > > #include <pthread.h>
>> > > #include <stdio.h>
>> > >
>> > > extern __typeof (pthread_key_create) __pthread_key_create __attribute__ ((weak));
>> > > extern __typeof (pthread_once) pthread_once __attribute__ ((weak));
>> > >
>> > > void
>> > > f1 (void)
>> > > {
>> > >   puts ("f1 called");
>> > > }
>> > >
>> > > pthread_once_t once_var;
>> > >
>> > > void __attribute__ ((weak))
>> > > f2 (void)
>> > > {
>> > >   if (__pthread_key_create != NULL)
>> > >     pthread_once (&once_var, f1);
>> > > }
>> > >
>> > > int
>> > > main (void)
>> > > {
>> > >   f2 ();
>> > > }
>> > >
>> > > Building it with “gcc -O2 -fpie -pie” and linking with binutils 2.30
>> > > does not result in a crash with LD_PRELOAD=libpthread.so.0.
>> >
>> > Thank you for the test case. It helps the understanding.
>> >
>> > But I don't understand
>> >   - why anyone would redeclare 'pthread_once', when it's a standard POSIX
>> >     function,
>> >   - why f2 is declared weak,
>> >   - why the program skips its initializations in single-threaded mode,
>> >   - why libpthread would be loaded through LD_PRELOAD or dlopen, given
>> >     that the long-term statement has been that declaring a symbol weak
>> >     has no effect on the dynamic linker [1][2][3][4]?
>> >
>> > How about the following test case instead?
>> >
>> > =====================================================================
>> > #include <pthread.h>
>> > #include <stdio.h>
>> >
>> > #pragma weak pthread_key_create
>> > #pragma weak pthread_once
>> >
>> > void
>> > do_init (void)
>> > {
>> >   puts ("initialization code");
>> > }
>> >
>> > pthread_once_t once_var;
>> >
>> > void
>> > init (void)
>> > {
>> >   if (pthread_key_create != NULL)
>> >     {
>> >       puts ("multi-threaded initialization");
>> >       pthread_once (&once_var, do_init);
>> >     }
>> >   else
>> >     do_init ();
>> > }
>> >
>> > int
>> > main (void)
>> > {
>> >   init ();
>> > }
>> > =====================================================================
>> >
>> > $ gcc -Wall -fpie -pie foo.c ; ./a.out
>> > initialization code
>> >
>> > $ gcc -Wall -fpie -pie foo.c -Wl,--no-as-needed -lpthread ; ./a.out
>> > multi-threaded initialization
>> > initialization code
>> >
>> > What will change for this program with glibc 2.34?
>> >
>> > Bruno
>> >
>> > [1] https://sourceware.org/legacy-ml/libc-hacker/2000-06/msg00029.html
>> > [2] https://www.akkadia.org/drepper/dsohowto.pdf page 6
>> > [3] https://stackoverflow.com/questions/21092601/is-pthread-in-glibc-so-implemented-by-weak-symbol-to-provide-pthread-stub-functi/21103255
>> > [4] https://stackoverflow.com/questions/20658809/dynamic-loading-and-weak-symbol-resolution
>> >
>>
>> Does x86 show the same issue?  I fixed several undefined weak symbol
>> bugs on x86:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=19636
>> https://sourceware.org/bugzilla/show_bug.cgi?id=19704
>> https://sourceware.org/bugzilla/show_bug.cgi?id=19719

I don't consider the first two bugs.
Whether a dynamic relocation is emitted depends on
(1) whether .dynsym exists (2) architecture (3) relocation type (4)
-no-pie/-pie/-shared (5) -z {,no}dynamic-undefined-weak.

It is unlikely a user can summarize rules which can be relied upon.

We can step back and look at these from a different perspective:
figure out what should be defined, then everything else has no hard rule
and we can choose whatever to simplify rules.

* absolute relocation resolves to 0. There may or may not be dynamic relocations.
* PC-relative relocation doesn't make sense.



I changed LLD to use a simple rule:

* -no-pie and -pie: no dynamic relocation
* -shared: dynamic relocation

https://maskray.me/blog/2021-04-25-weak-symbol

>> with a linker option:
>>
>>     'dynamic-undefined-weak'
>>      'nodynamic-undefined-weak'
>>           Make undefined weak symbols dynamic when building a dynamic
>>           object, if they are referenced from a regular object file and
>>           not forced local by symbol visibility or versioning.  Do not
>>           make them dynamic if 'nodynamic-undefined-weak'.  If neither
>>           option is given, a target may default to either option being
>>           in force, or make some other selection of undefined weak
>>           symbols dynamic.  Not all targets support these options.
>>
>> Alan extended the fix to PPC:
>>
>> commit 954b63d4c8645f86e40c7ef6c6d60acd2bf019de
>> Author: Alan Modra <amodra@gmail.com>
>> Date:   Wed Apr 19 01:26:57 2017 +0930
>>
>>     Implement -z dynamic-undefined-weak
>>
>>     -z nodynamic-undefined-weak is only implemented for x86.  (The sparc
>>     backend has some support code but doesn't enable the option by
>>     including ld/emulparams/dynamic_undefined_weak.sh, and since the
>>     support looks like it may be broken I haven't enabled it.)  This patch
>>     adds the complementary -z dynamic-undefined-weak, extends both options
>>     to affect building of shared libraries as well as executables, and
>>     adds support for the option on powerpc.
>>
>
>Another undefined weak symbol linker bug:
>
>https://sourceware.org/bugzilla/show_bug.cgi?id=22269

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* H. J. Lu via Libc-alpha:

> Alan extended the fix to PPC:
>
> commit 954b63d4c8645f86e40c7ef6c6d60acd2bf019de
> Author: Alan Modra <amodra@gmail.com>
> Date:   Wed Apr 19 01:26:57 2017 +0930
>
>     Implement -z dynamic-undefined-weak
>
>     -z nodynamic-undefined-weak is only implemented for x86.  (The sparc
>     backend has some support code but doesn't enable the option by
>     including ld/emulparams/dynamic_undefined_weak.sh, and since the
>     support looks like it may be broken I haven't enabled it.)  This patch
>     adds the complementary -z dynamic-undefined-weak, extends both options
>     to affect building of shared libraries as well as executables, and
>     adds support for the option on powerpc.

I'm not sure if this option is compatible with all compilers on POWER.

The old binutils behavior allowed the compiler to optimize this common
pattern

void f1 (void) __attribute__ ((weak));

void
f2 (void)
{
  if (f1 != 0)
    f1 ();
}

into an unconditional call to f1 because the linker would replace the
call with a NOP if the symbol is undefined, reproducing the effect of
the if condition.  However, GCC 10 does not appear to perform this
optimization on powerpc64le.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Michael Matz]

Hello,

On Wed, 28 Apr 2021, Florian Weimer via Binutils wrote:

> > commit 954b63d4c8645f86e40c7ef6c6d60acd2bf019de
> > Author: Alan Modra <amodra@gmail.com>
> > Date:   Wed Apr 19 01:26:57 2017 +0930
> >
> >     Implement -z dynamic-undefined-weak
> >
> >     -z nodynamic-undefined-weak is only implemented for x86.  (The sparc
> >     backend has some support code but doesn't enable the option by
> >     including ld/emulparams/dynamic_undefined_weak.sh, and since the
> >     support looks like it may be broken I haven't enabled it.)  This patch
> >     adds the complementary -z dynamic-undefined-weak, extends both options
> >     to affect building of shared libraries as well as executables, and
> >     adds support for the option on powerpc.
> 
> I'm not sure if this option is compatible with all compilers on POWER.
> 
> The old binutils behavior allowed the compiler to optimize this common
> pattern
> 
> void f1 (void) __attribute__ ((weak));
> 
> void
> f2 (void)
> {
>   if (f1 != 0)
>     f1 ();
> }
> 
> into an unconditional call to f1 because the linker would replace the
> call with a NOP if the symbol is undefined, reproducing the effect of
> the if condition.  However, GCC 10 does not appear to perform this
> optimization on powerpc64le.

GCC assumes addresses of functions to be non-null, _except when declared 
weak_.  Over the years we fixed problem in that aspect always into that 
direction (hence the above testcase should always be emitted with an 
actual test, otherwise it would be considered a bug worth fixing).  That 
would be consistent with making undefined weak symbols dynamic in the ELF 
case, so that the ultimate test survives until runtime.

I don't know what other compilers are doing, of course.


Ciao,
Michael.

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Bruno Haible:

> Hi Florian,
>
>> Here's a fairly representative test case, I think.
>> 
>> #include <pthread.h>
>> #include <stdio.h>
>> 
>> extern __typeof (pthread_key_create) __pthread_key_create __attribute__ ((weak));
>> extern __typeof (pthread_once) pthread_once __attribute__ ((weak));
>> 
>> void
>> f1 (void)
>> {
>>   puts ("f1 called");
>> }
>> 
>> pthread_once_t once_var;
>> 
>> void __attribute__ ((weak))
>> f2 (void)
>> {
>>   if (__pthread_key_create != NULL)
>>     pthread_once (&once_var, f1);
>> }
>> 
>> int
>> main (void)
>> {
>>   f2 ();
>> }
>> 
>> Building it with “gcc -O2 -fpie -pie” and linking with binutils 2.30
>> does not result in a crash with LD_PRELOAD=libpthread.so.0.
>
> Thank you for the test case. It helps the understanding.
>
> But I don't understand
>   - why anyone would redeclare 'pthread_once', when it's a standard POSIX
>     function,

I could have used the weak pragma just like gnulib.

>   - why f2 is declared weak,

Oh, sorry for the confusion, it's a quick way to establish a full
compiler barrier with GCC.

>   - why the program skips its initializations in single-threaded mode,

It's a minimal test.

>   - why libpthread would be loaded through LD_PRELOAD or dlopen, given
>     that the long-term statement has been that declaring a symbol weak
>     has no effect on the dynamic linker [1][2][3][4]?

The relevant scenario here is LD_PRELOAD of a library that depends on
libpthread.so.0, so it's about indirect preloading of something that's
more usefull than just libpthread.so.0.  pthread_key_create would still
become available in this case.

>
> How about the following test case instead?
>
> =====================================================================
> #include <pthread.h>
> #include <stdio.h>
>
> #pragma weak pthread_key_create
> #pragma weak pthread_once
>
> void
> do_init (void)
> {
>   puts ("initialization code");
> }
>
> pthread_once_t once_var;
>
> void
> init (void)
> {
>   if (pthread_key_create != NULL)
>     {
>       puts ("multi-threaded initialization");
>       pthread_once (&once_var, do_init);
>     }
>   else
>     do_init ();
> }
>
> int
> main (void)
> {
>   init ();
> }
> =====================================================================
>
> $ gcc -Wall -fpie -pie foo.c ; ./a.out 
> initialization code
>
> $ gcc -Wall -fpie -pie foo.c -Wl,--no-as-needed -lpthread ; ./a.out
> multi-threaded initialization
> initialization code
>
> What will change for this program with glibc 2.34?

If recompiled in this way: The presence of -lpthread will not matter, it
will always behave is if linked with -lpthread.

If not recompiled and linked without -lpthread against glibc 2.33 or
earlier, the behavior with the current glibc 2.34 snapshot is
architecture-dependent and also depends on the binutils version used for
linking the program.  In some cases, calling pthread_once jumps to
address zero (causing a crash), or the call to pthread_once is elided
from the executable.  This scenario can be emulated with older glibc
using LD_PRELOAD=libpthread.so.0.

I will try to come up with a way to preserve the glibc 2.33 behavior for
old binaries.  However, you should really remove those weak symbol
hacks.  They won't have any effect for glibc 2.34, and as explained,
they cause breakage with earlier glibc versions with certain
LD_PRELOAD-ed libraries.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Florian Weimer wrote:
> > $ gcc -Wall -fpie -pie foo.c ; ./a.out 
> > initialization code
> >
> > $ gcc -Wall -fpie -pie foo.c -Wl,--no-as-needed -lpthread ; ./a.out
> > multi-threaded initialization
> > initialization code
> >
> > What will change for this program with glibc 2.34?
> 
> If recompiled in this way: The presence of -lpthread will not matter, it
> will always behave is if linked with -lpthread.

So it will print
  multi-threaded initialization
  initialization code
in all cases. This is perfectly fine.

> The relevant scenario here is LD_PRELOAD of a library that depends on
> libpthread.so.0 ...
> If not recompiled and linked without -lpthread against glibc 2.33 or
> earlier, the behavior with the current glibc 2.34 snapshot is
> architecture-dependent and also depends on the binutils version used for
> linking the program.  In some cases, calling pthread_once jumps to
> address zero (causing a crash), or the call to pthread_once is elided
> from the executable.  This scenario can be emulated with older glibc
> using LD_PRELOAD=libpthread.so.0.

I'm confused again. Are you saying that the crash will occur when old
binaries are being used with glibc 2.34 also *without* LD_PRELOAD?

Bruno

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Bruno Haible:

> I'm confused again. Are you saying that the crash will occur when old
> binaries are being used with glibc 2.34 also *without* LD_PRELOAD?

Yes, that's how we discovered it.  And really is a problem.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Florian Weimer wrote on 2021-04-28:
> However, you should really remove those weak symbol
> hacks.  They won't have any effect for glibc 2.34

I'm trying to do this now. But what is the right condition?

1) I understand that it's only for glibc >= 2.34 on Linux (NPTL),
   right? Not on Hurd (HTL)?

2) /usr/include/gnu/lib-names.h still defines LIBPTHREAD_SO.
   How about not defining LIBPTHREAD_SO, since linking with it is supposed
   to be a no-op in these newer glibc versions?

Bruno

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Bruno Haible:

> Florian Weimer wrote on 2021-04-28:
>> However, you should really remove those weak symbol
>> hacks.  They won't have any effect for glibc 2.34
>
> I'm trying to do this now. But what is the right condition?
>
> 1) I understand that it's only for glibc >= 2.34 on Linux (NPTL),
>    right? Not on Hurd (HTL)?

Yes, Hurd hasn't been integrated.

> 2) /usr/include/gnu/lib-names.h still defines LIBPTHREAD_SO.
>    How about not defining LIBPTHREAD_SO, since linking with it is supposed
>    to be a no-op in these newer glibc versions?

I'm not sure if this is the right way, given that the file still exists
for all currently supported targets.

An alternative would be to add a macro to <pthread.h>, such as
PTHREAD_IN_LIBC.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Hi Florian,

> > 1) I understand that it's only for glibc >= 2.34 on Linux (NPTL),
> >    right? Not on Hurd (HTL)?
> 
> Yes, Hurd hasn't been integrated.
> 
> > 2) /usr/include/gnu/lib-names.h still defines LIBPTHREAD_SO.
> >    How about not defining LIBPTHREAD_SO, since linking with it is supposed
> >    to be a no-op in these newer glibc versions?
> 
> I'm not sure if this is the right way, given that the file still exists
> for all currently supported targets.

Thanks for the rapid answers.

> An alternative would be to add a macro to <pthread.h>, such as
> PTHREAD_IN_LIBC.

This would be useful, yes. Like there is a <gnu/stubs.h> that gives meta-
information about functions that are stubs, it is useful to have a way
to find out whether a library is a stub. For cross-compilation scenarios,
implementing it through a macro in some header file is better than
implementing it through some file in /lib/ on the file system.

Bruno

Re: Undefined use of weak symbols in gnulib

[Joseph Myers]

On Sat, 17 Jul 2021, Bruno Haible wrote:

> 2) /usr/include/gnu/lib-names.h still defines LIBPTHREAD_SO.
>    How about not defining LIBPTHREAD_SO, since linking with it is supposed
>    to be a no-op in these newer glibc versions?

I think LIBPTHREAD_SO is really for use with dlopen (followed by e.g. 
using dlsym to look up a function by name at runtime), not linking against 
(in general you need to link against the *.so name which might be a linker 
script, not directly against the shared library's SONAME).

So if there's any change regarding LIBPTHREAD_SO, I think the natural one 
would be to define it to LIBC_SO (I hope the dlopen/dlsym case works 
regardless of whether that change is made or not).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Joseph Myers:

> On Sat, 17 Jul 2021, Bruno Haible wrote:
>
>> 2) /usr/include/gnu/lib-names.h still defines LIBPTHREAD_SO.
>>    How about not defining LIBPTHREAD_SO, since linking with it is supposed
>>    to be a no-op in these newer glibc versions?
>
> I think LIBPTHREAD_SO is really for use with dlopen (followed by e.g. 
> using dlsym to look up a function by name at runtime), not linking against 
> (in general you need to link against the *.so name which might be a linker 
> script, not directly against the shared library's SONAME).
>
> So if there's any change regarding LIBPTHREAD_SO, I think the natural one 
> would be to define it to LIBC_SO (I hope the dlopen/dlsym case works 
> regardless of whether that change is made or not).

That is in an interesting idea.  I like it.

It doesn't help with Bruno's use case, detecting the integrated
libpthread with the preprocessor.

Carlos, do you think we can still slip in a definition of
PTHREAD_IN_LIBC in <pthread.h> (for __USE_GNU)?

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Paul Eggert]

On 7/27/21 1:19 PM, Florian Weimer wrote:
>> So if there's any change regarding LIBPTHREAD_SO, I think the natural one
>> would be to define it to LIBC_SO (I hope the dlopen/dlsym case works
>> regardless of whether that change is made or not).
> That is in an interesting idea.  I like it.

Me too.

> It doesn't help with Bruno's use case, detecting the integrated
> libpthread with the preprocessor.

It might be good enough for that use case, if the detection of 
LIBPTHREAD_SO==LIBC_SO can be done at 'configure' time instead of at 
preprocessor time. (Bruno would be a better person to opine on this.)

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Florian Weimer wrote on 2021-04-28:
> However, you should really remove those weak symbol
> hacks.  They won't have any effect for glibc 2.34

Done as follows. Tested on a Fedora Rawhide from today.
Thanks for the suggestion.


2021-07-17  Bruno Haible  <bruno@clisp.org>

	Don't use '#pragma weak' for thread functions in Linux/glibc>=2.34.
	Suggested by Florian Weimer <fweimer@redhat.com> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2021-04/msg00211.html>.
	* m4/threadlib.m4 (gl_PTHREADLIB_BODY): Set gl_pthread_in_glibc and set
	LIBPMULTITHREAD accordingly.
	(gl_STDTHREADLIB_BODY): Update comments.
	(gl_THREADLIB_BODY): Define USE_POSIX_THREADS_FROM_LIBC.
	* lib/glthread/thread.h (c11_threads_in_use): Define to 1 if all POSIX
	thread functions are in libc.
	* lib/glthread/lock.h (c11_threads_in_use): Likewise.
	* lib/glthread/cond.h (c11_threads_in_use): Likewise.
	* lib/glthread/tls.h (c11_threads_in_use): Likewise.

diff --git a/lib/glthread/cond.h b/lib/glthread/cond.h
index 9858425..fbab08c 100644
--- a/lib/glthread/cond.h
+++ b/lib/glthread/cond.h
@@ -56,7 +56,9 @@
 #include "glthread/lock.h"
 
 #if !defined c11_threads_in_use
-# if HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
+# if HAVE_THREADS_H && USE_POSIX_THREADS_FROM_LIBC
+#  define c11_threads_in_use() 1
+# elif HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
 #  include <threads.h>
 #  pragma weak thrd_exit
 #  define c11_threads_in_use() (thrd_exit != NULL)
diff --git a/lib/glthread/lock.h b/lib/glthread/lock.h
index 624ce67..cc4c519 100644
--- a/lib/glthread/lock.h
+++ b/lib/glthread/lock.h
@@ -81,7 +81,9 @@
 #include <stdlib.h>
 
 #if !defined c11_threads_in_use
-# if HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
+# if HAVE_THREADS_H && USE_POSIX_THREADS_FROM_LIBC
+#  define c11_threads_in_use() 1
+# elif HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
 #  include <threads.h>
 #  pragma weak thrd_exit
 #  define c11_threads_in_use() (thrd_exit != NULL)
diff --git a/lib/glthread/thread.h b/lib/glthread/thread.h
index 3e84599..44f05f3 100644
--- a/lib/glthread/thread.h
+++ b/lib/glthread/thread.h
@@ -74,7 +74,9 @@
 #include <stdlib.h>
 
 #if !defined c11_threads_in_use
-# if HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
+# if HAVE_THREADS_H && USE_POSIX_THREADS_FROM_LIBC
+#  define c11_threads_in_use() 1
+# elif HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
 #  include <threads.h>
 #  pragma weak thrd_exit
 #  define c11_threads_in_use() (thrd_exit != NULL)
diff --git a/lib/glthread/tls.h b/lib/glthread/tls.h
index 9c2a280..aaaa4b6 100644
--- a/lib/glthread/tls.h
+++ b/lib/glthread/tls.h
@@ -47,7 +47,9 @@
 #include <stdlib.h>
 
 #if !defined c11_threads_in_use
-# if HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
+# if HAVE_THREADS_H && USE_POSIX_THREADS_FROM_LIBC
+#  define c11_threads_in_use() 1
+# elif HAVE_THREADS_H && USE_POSIX_THREADS_WEAK
 #  include <threads.h>
 #  pragma weak thrd_exit
 #  define c11_threads_in_use() (thrd_exit != NULL)
diff --git a/m4/threadlib.m4 b/m4/threadlib.m4
index 8fc3dfd..37b797c 100644
--- a/m4/threadlib.m4
+++ b/m4/threadlib.m4
@@ -1,4 +1,4 @@
-# threadlib.m4 serial 30
+# threadlib.m4 serial 31
 dnl Copyright (C) 2005-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -212,6 +212,27 @@ AC_DEFUN([gl_PTHREADLIB_BODY],
         LIBS=$save_LIBS
         test $gl_pthread_api = yes && break
       done
+      echo "$as_me:__oline__: gl_pthread_api=$gl_pthread_api" >&AS_MESSAGE_LOG_FD
+      echo "$as_me:__oline__: LIBPTHREAD=$LIBPTHREAD" >&AS_MESSAGE_LOG_FD
+
+      gl_pthread_in_glibc=no
+      # On Linux with glibc >= 2.34, libc contains the fully functional
+      # pthread functions.
+      case "$host_os" in
+        linux*)
+          AC_EGREP_CPP([Lucky user],
+            [#include <features.h>
+             #ifdef __GNU_LIBRARY__
+              #if (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 34) || (__GLIBC__ > 2)
+               Lucky user
+              #endif
+             #endif
+            ],
+            [gl_pthread_in_glibc=yes],
+            [])
+          ;;
+      esac
+      echo "$as_me:__oline__: gl_pthread_in_glibc=$gl_pthread_in_glibc" >&AS_MESSAGE_LOG_FD
 
       # Test for libpthread by looking for pthread_kill. (Not pthread_self,
       # since it is defined as a macro on OSF/1.)
@@ -219,18 +240,22 @@ AC_DEFUN([gl_PTHREADLIB_BODY],
         # The program links fine without libpthread. But it may actually
         # need to link with libpthread in order to create multiple threads.
         AC_CHECK_LIB([pthread], [pthread_kill],
-          [LIBPMULTITHREAD=-lpthread
-           # On Solaris and HP-UX, most pthread functions exist also in libc.
-           # Therefore pthread_in_use() needs to actually try to create a
-           # thread: pthread_create from libc will fail, whereas
-           # pthread_create will actually create a thread.
-           # On Solaris 10 or newer, this test is no longer needed, because
-           # libc contains the fully functional pthread functions.
-           case "$host_os" in
-             solaris | solaris2.[1-9] | solaris2.[1-9].* | hpux*)
-               AC_DEFINE([PTHREAD_IN_USE_DETECTION_HARD], [1],
-                 [Define if the pthread_in_use() detection is hard.])
-           esac
+          [if test $gl_pthread_in_glibc = yes; then
+             LIBPMULTITHREAD=
+           else
+             LIBPMULTITHREAD=-lpthread
+             # On Solaris and HP-UX, most pthread functions exist also in libc.
+             # Therefore pthread_in_use() needs to actually try to create a
+             # thread: pthread_create from libc will fail, whereas
+             # pthread_create will actually create a thread.
+             # On Solaris 10 or newer, this test is no longer needed, because
+             # libc contains the fully functional pthread functions.
+             case "$host_os" in
+               solaris | solaris2.[1-9] | solaris2.[1-9].* | hpux*)
+                 AC_DEFINE([PTHREAD_IN_USE_DETECTION_HARD], [1],
+                   [Define if the pthread_in_use() detection is hard.])
+             esac
+           fi
           ])
       elif test $gl_pthread_api != yes; then
         # Some library is needed. Try libpthread and libc_r.
@@ -246,6 +271,7 @@ AC_DEFUN([gl_PTHREADLIB_BODY],
              LIBPMULTITHREAD=-lc_r])
         fi
       fi
+      echo "$as_me:__oline__: LIBPMULTITHREAD=$LIBPMULTITHREAD" >&AS_MESSAGE_LOG_FD
     fi
     AC_MSG_CHECKING([whether POSIX threads API is available])
     AC_MSG_RESULT([$gl_pthread_api])
@@ -311,7 +337,8 @@ AC_DEFUN([gl_STDTHREADLIB_BODY],
           dnl glibc >= 2.29 has thrd_create in libpthread.
           dnl FreeBSD >= 10 has thrd_create in libstdthreads; this library depends
           dnl on libpthread (for the symbol 'pthread_mutexattr_gettype').
-          dnl AIX >= 7.1 and Solaris >= 11.4 have thrd_create in libc.
+          dnl glibc >= 2.34, AIX >= 7.1, and Solaris >= 11.4 have thrd_create in
+          dnl libc.
           AC_CHECK_FUNCS([thrd_create])
           if test $ac_cv_func_thrd_create = yes; then
             LIBSTDTHREAD=
@@ -481,7 +508,10 @@ AC_DEFUN([gl_THREADLIB_BODY],
           gl_threads_api=posix
           AC_DEFINE([USE_POSIX_THREADS], [1],
             [Define if the POSIX multithreading library can be used.])
-          if test -n "$LIBMULTITHREAD" || test -n "$LTLIBMULTITHREAD"; then
+          if test -z "$LIBMULTITHREAD" && test -z "$LTLIBMULTITHREAD"; then
+            AC_DEFINE([USE_POSIX_THREADS_FROM_LIBC], [1],
+              [Define if references to the POSIX multithreading library are satisfied by libc.])
+          else
             if case "$gl_cv_have_weak" in *yes) true;; *) false;; esac; then
               AC_DEFINE([USE_POSIX_THREADS_WEAK], [1],
                 [Define if references to the POSIX multithreading library should be made weak.])
@@ -576,7 +606,9 @@ dnl                    flavours   option      weak       result
 dnl ---------------    ---------  ---------   --------   ---------
 dnl Linux 2.4/glibc    posix      -lpthread       Y      OK
 dnl
-dnl GNU Hurd/glibc     posix
+dnl Linux/glibc 2.34   posix                      Y      OK
+dnl
+dnl GNU Hurd/glibc     posix      -lpthread       Y      OK
 dnl
 dnl Ubuntu 14.04       posix      -pthread        Y      OK
 dnl

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Andreas Schwab wrote:
> It is likely that the use of weak pthread symbols is not confined to
> gnulib.

Yes. Between 2000 and 2010 I heard a couple of times "Multithreading
support is only available through libpthread. It is a common technique
to use weak symbols to distinguish a program linked with -lpthread
from a program not linked with -lpthread (and such a program is
single-threaded)".

Also, people were saying "dlopen of libpthread does not work" [1][2].

Bruno

[1] https://jmmv.dev/2004/11/dynamic-open-of-libpthread-considered.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=18192

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Hi Florian,

> This will become an urgent issue with glibc 2.34

Thank you for the early heads-up.

I would like to understand the scope and the severity of the issue.

You write:
> Dynamic linking with weak symbols is not very well-defined.  ...
> the code will crash if pthread_mutexattr_gettype is ever defined.

In which situations will it crash?

  (a) when the code is in an executable, that gets linked with '-lpthread'
      and that does not use dlopen()?
  (b) when the code is in an executable, that gets linked WITHOUT
      '-lpthread' and that does not use dlopen()?
  (c) when the code is in an executable, that gets linked WITHOUT
      '-lpthread' and that does a dlopen("libpthread.so.X")?

Under which conditions will it crash?

  ($) when the executable was built before glibc 2.34 and is run
      with glibc 2.34 ?
  (%) when the executable is built against glibc 2.34 and is run
      with glibc 2.34 ?

And if it crashes, will setting the environment variable LD_DYNAMIC_WEAK [1]
avoid the crash?

Bruno

[1] https://sourceware.org/legacy-ml/libc-hacker/2000-06/msg00029.html

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Bruno Haible:

> You write:
>> Dynamic linking with weak symbols is not very well-defined.  ...
>> the code will crash if pthread_mutexattr_gettype is ever defined.
>
> In which situations will it crash?
>
>   (a) when the code is in an executable, that gets linked with '-lpthread'
>       and that does not use dlopen()?

The pthread_mutexattr_gettype is defined, but also pthread_once and the
weak symbols, so there is no problem because the link editor doesn't do
funny things.

>   (b) when the code is in an executable, that gets linked WITHOUT
>       '-lpthread' and that does not use dlopen()?

Yes, it will crash or behave incorrectly on most architectures *if*
pthread_mutexattr_gettype becomes available for some reason.

>   (c) when the code is in an executable, that gets linked WITHOUT
>       '-lpthread' and that does a dlopen("libpthread.so.X")?

This will probably work because pthread_mutexattr_gettype is not rebound
to the definition.

> Under which conditions will it crash?
>
>   ($) when the executable was built before glibc 2.34 and is run
>       with glibc 2.34 ?

Yes.

>   (%) when the executable is built against glibc 2.34 and is run
>       with glibc 2.34 ?

No.  glibc 2.34 will behave as if an implicit -lpthread is present on
the linker program line.

> And if it crashes, will setting the environment variable LD_DYNAMIC_WEAK [1]
> avoid the crash?

No, it's unrelated.  The crash or other undefined behavior is a
consequence of actions of the link editor and cannot be reverted at run
time.  The best we can do is to hide definitions of symbols like
pthread_mutexattr_gettype, therefore masking the existence of those
corrupted code paths (like glibc 2.33 and earlier do).

Thanks for looking into this.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Hi Florian,

Thank you for the details.

> > In which situations will it crash?
> >
> >   (a) when the code is in an executable, that gets linked with '-lpthread'
> >       and that does not use dlopen()?
> 
> The pthread_mutexattr_gettype is defined, but also pthread_once and the
> weak symbols, so there is no problem because the link editor doesn't do
> funny things.
> 
> >   (b) when the code is in an executable, that gets linked WITHOUT
> >       '-lpthread' and that does not use dlopen()?
> 
> Yes, it will crash or behave incorrectly on most architectures *if*
> pthread_mutexattr_gettype becomes available for some reason.
> 
> >   (c) when the code is in an executable, that gets linked WITHOUT
> >       '-lpthread' and that does a dlopen("libpthread.so.X")?
> 
> This will probably work because pthread_mutexattr_gettype is not rebound
> to the definition.

So, in the normal cases (link with '-lpthread', link without '-lpthread',
and even with dlopen()), everything will work fine. The only problematic
case thus is the the use of LD_PRELOAD. Right?

I think few packages in a distro will be affected. And few users are
using LD_PRELOAD on their own, because since the time when glibc
started to use 'internal' calls to system calls where possible, there
are not a lot of uses of LD_PRELOAD that still work.

> > Under which conditions will it crash?
> >
> >   ($) when the executable was built before glibc 2.34 and is run
> >       with glibc 2.34 ?
> 
> Yes.
> 
> >   (%) when the executable is built against glibc 2.34 and is run
> >       with glibc 2.34 ?
> 
> No.  glibc 2.34 will behave as if an implicit -lpthread is present on
> the linker program line.

Good. This means a bullet-proof way for a distro to avoid the problem
is to "rebuild the world" after importing glibc 2.34.

> > And if it crashes, will setting the environment variable LD_DYNAMIC_WEAK [1]
> > avoid the crash?
> 
> No, it's unrelated.  The crash or other undefined behavior is a
> consequence of actions of the link editor and cannot be reverted at run
> time.

In other words, the problem is that
  - there are some/many binaries out there, that were produced by an 'ld'
    that did not anticipate the changes in glibc 2.34, and
  - these binaries have a problem not when run directly, but only when
    run with LD_PRELOAD.

Right?

Bruno

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Bruno Haible:

> So, in the normal cases (link with '-lpthread', link without '-lpthread',
> and even with dlopen()), everything will work fine. The only problematic
> case thus is the the use of LD_PRELOAD. Right?

LD_PRELOAD and glibc 2.34 as originally planned.

> I think few packages in a distro will be affected. And few users are
> using LD_PRELOAD on their own, because since the time when glibc
> started to use 'internal' calls to system calls where possible, there
> are not a lot of uses of LD_PRELOAD that still work.

We get the occasional bug report when these things break.  We have not
seen much of that yet because our gnulib-using programs are still at
older versions for most of our users.

Here's an example of such a bug report, although not for libpthread:

  powerpc: libc segfaults when LD_PRELOADed with libgcc
  <https://sourceware.org/bugzilla/show_bug.cgi?id=26615>

I think in this bug, libc.so.6 was invoked during some build process
(which could easily run bison as well, so it has to work with
LD_PRELOAD).

>> No.  glibc 2.34 will behave as if an implicit -lpthread is present on
>> the linker program line.
>
> Good. This means a bullet-proof way for a distro to avoid the problem
> is to "rebuild the world" after importing glibc 2.34.

Yeah, but that's not good enough.  So I spent today on coming up with a
workaround in glibc.

>> No, it's unrelated.  The crash or other undefined behavior is a
>> consequence of actions of the link editor and cannot be reverted at run
>> time.
>
> In other words, the problem is that
>   - there are some/many binaries out there, that were produced by an 'ld'
>     that did not anticipate the changes in glibc 2.34, and
>   - these binaries have a problem not when run directly, but only when
>     run with LD_PRELOAD.
>
> Right?

No, glibc 2.34 won't need LD_PRELOAD to expose the bug.  LD_PRELOAD is
just a development aid that reveals the problem with glibc 2.33 and
earlier.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Bruno Haible]

Hi Florian,

> > So, in the normal cases (link with '-lpthread', link without '-lpthread',
> > and even with dlopen()), everything will work fine. The only problematic
> > case thus is the the use of LD_PRELOAD. Right?
> 
> LD_PRELOAD and glibc 2.34 as originally planned.
> ...
> > In other words, the problem is that
> >   - there are some/many binaries out there, that were produced by an 'ld'
> >     that did not anticipate the changes in glibc 2.34, and
> >   - these binaries have a problem not when run directly, but only when
> >     run with LD_PRELOAD.
> >
> > Right?
> 
> No, glibc 2.34 won't need LD_PRELOAD to expose the bug.  LD_PRELOAD is
> just a development aid that reveals the problem with glibc 2.33 and
> earlier.

Ouch, then it means that a significant number of binaries out there
will crash when the system's libc is upgraded to 2.34.

Many programs use "#pragma weak ..." in the way it was advertised 20 years
ago. In fact, it was advertised to be useful for backward compatibility [1] —
and now we face a problem with it, precisely regarding backward compatibility.

Here are just some occurrences of "#pragma weak pthread_*":

- Gnulib - embedded in many GNU programs. [2]
- GCC / libstdc++ - gthr-posix.[hc] (gcc-4.6.4)
- libxml2 [3]
- prelude-siem [4]

> I spent today on coming up with a workaround in glibc.

These are the workarounds I can see:
  - Delay the planned changes in glibc by 5 years or so, to minimize
    the number of binaries out there that would crash. (Probably not what
    you want.)
  - Change glibc's ld.so to deal with the binaries that are out there
    and that have been produced by existing binutils (with or without the
    patches that H.J. Lu listed).
  - Play tricks with the preprocessor, such as
    '#define pthread_create pthread_create_in_libc'. (Probably not POSIX
    compliant.)
  - Make use of symbol versioning. Symbol versioning was invented to
    allow making big changes to libc without breaking binary backward
    compatibility. (I don't know about the interplay between weak and
    versioned symbols.)

> >> No, it's unrelated.  The crash or other undefined behavior is a
> >> consequence of actions of the link editor and cannot be reverted at run
> >> time.

I fear that you will need to touch the dynamic linker. The binaries that
could crash are out in the wild.

> However, you should really remove those weak symbol
> hacks.  They won't have any effect for glibc 2.34, and as explained

Yes, I will remove them from gnulib, when a glibc >= 2.34 is found,
simply to reduce complexity on glibc systems.
But it will not help fixing the crashes when glibc 2.34 is released,
because it takes time until
  1. the gnulib changes are integrated into the packages that use gnulib,
  2. these packages have had a tarball release on ftp.gnu.org,
  3. users have replaced their old self-compiled binaries with newer ones.
I am talking about a time frame from 3 to 10 years here.

Bruno

[1] https://en.wikipedia.org/wiki/Weak_symbol#Use_cases
[2] https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/glthread/lock.h;h=a0b24b63224cf44f862bd8e181e79b037c969d58;hb=HEAD#l242
[3] https://gitlab.gnome.org/GNOME/libxml2/-/blob/master/threads.c#L50
[4] https://www.prelude-siem.org/attachments/download/119/pthread-weak.diff

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Bruno Haible:

>> I spent today on coming up with a workaround in glibc.
>
> These are the workarounds I can see:
>   - Delay the planned changes in glibc by 5 years or so, to minimize
>     the number of binaries out there that would crash. (Probably not what
>     you want.)

Nah, those binaries won't go away in just five years.

>   - Change glibc's ld.so to deal with the binaries that are out there
>     and that have been produced by existing binutils (with or without the
>     patches that H.J. Lu listed).

This is what I've tried to implement:

  [RFC] elf: Implement filtering of symbols historically defined in libpthread
  <https://sourceware.org/pipermail/libc-alpha/2021-April/125564.html>

>   - Play tricks with the preprocessor, such as
>     '#define pthread_create pthread_create_in_libc'. (Probably not POSIX
>     compliant.)

It doesn't solve the problem, even for new binaries.

>   - Make use of symbol versioning. Symbol versioning was invented to
>     allow making big changes to libc without breaking binary backward
>     compatibility. (I don't know about the interplay between weak and
>     versioned symbols.)

If the symbol is weak and undefined at build time, there is no symbol
version attached to it.  My patch uses that to make sure the filtering
does not happen on the fast path (because symbols bound to libc.so.6
usually have versions), but I don't think symbol versioning is all that
useful in this context.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Ben Pfaff]

On Wed, Apr 28, 2021 at 7:40 AM Bruno Haible <bruno@clisp.org> wrote:
> So, in the normal cases (link with '-lpthread', link without '-lpthread',
> and even with dlopen()), everything will work fine. The only problematic
> case thus is the the use of LD_PRELOAD. Right?
>
> I think few packages in a distro will be affected. And few users are
> using LD_PRELOAD on their own, because since the time when glibc
> started to use 'internal' calls to system calls where possible, there
> are not a lot of uses of LD_PRELOAD that still work.

One important use of LD_PRELOAD is for the "fakeroot" program that
Debian and derived distributions tend to use to build packages.  I haven't
followed along carefully enough to know whether this is likely to cause
problems for what we're talking about now.

Re: Undefined use of weak symbols in gnulib

[Alan Modra]

On Tue, Apr 27, 2021 at 07:53:16AM +0200, Florian Weimer via Binutils wrote:
> So the net effect is this:
> 
>   if (pthread_mutexattr_gettype != NULL)
>     pthread_once (control, callback);
> 
> Dynamic linking with weak symbols is not very well-defined.  On x86-64,
> the link editor produces the expected dynamic symbol relocation for the
> pthread_once call.  On other targets (notably POWER), no dynamic
> relocation is produced, and the code will crash if
> pthread_mutexattr_gettype is ever defined.

Yes, this construct is not handled in ppc32 and ppc64 ld.  It also
doesn't work with some combination of compiler flags on x86.  Don't
allow yourself to be fooled by only testing with compilers that
default to -fpie..

For example, this on x86_64 with gcc-11 (and previous versions).
$ cat ~/src/tmp/weakfunc2.c 
extern void func1 (void) __attribute__((weak));
extern void func2 (void) __attribute__((weak));

int
main (void)
{
  if (func1)
    func2 ();
  return 0;
}
$ gcc -O2 -c -fno-pie ~/src/tmp/weakfunc2.c 
$ gas/as-new -o empty.o /dev/null
$ ld/ld-new -shared -o empty.so empty.o
$ ld/ld-new -o weakfunc2 weakfunc2.o empty.so 
ld/ld-new: warning: cannot find entry symbol _start; defaulting to 0000000000401020
$ binutils/nm-new --dynamic weakfunc2
                 w func2

So func1 is resolved to zero at link time.  Well, I guess that isn't
unexpected.  powerpc-linux and powerpc64-linux (both ABIs) have
further problems that I'll fix.  In particular the above testcase
ought to work even though it looks wrong to test func1 non-NULL and
then call func2.  Slightly less weird is

  if (func1)
    {
      func1 ();
      func2 ();
    }

which might be reasonable user code when it is known that func1 and
func2 both exist or are both NULL.  ppc gets that one wrong too.

-- 
Alan Modra
Australia Development Lab, IBM

Re: Undefined use of weak symbols in gnulib

[Michael Hudson-Doyle]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On Tue, 27 Apr 2021 at 17:53, Florian Weimer <fweimer@redhat.com> wrote:

> lib/glthread/lock.h has this:
>
> | /* The way to test at runtime whether libpthread is present is to test
> |    whether a function pointer's value, such as &pthread_mutex_init, is
> |    non-NULL.  However, some versions of GCC have a bug through which, in
> |    PIC mode, &foo != NULL always evaluates to true if there is a direct
> |    call to foo(...) in the same function.  To avoid this, we test the
> |    address of a function in libpthread that we don't use.  */
>

[snip]

This will become an urgent issue with glibc 2.34, which defines
> pthread_mutexattr_gettype unconditionally.  Certain gnulib modules will
> stop working until the binaries are relinked.  I expect the issue is
> already visible with earlier glibc versions if libpthread is
> unexpectedly present at run time.
>

Did this thread ever reach a conclusion? I'm testing a snapshot of glibc
2.34 in ubuntu and running into this issue -- bison segfaults on startup on
ppc64el. There is some talk of 'rebuilding the world' once 2.34 lands in a
distro but that might be hard because I suspect the world might be too
broken to do that (maybe it's not that bad really... but it doesn't sound
like fun).

Cheers,
mwh

[-- Attachment #2: Type: text/html, Size: 1853 bytes --]

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Michael Hudson-Doyle:

> Did this thread ever reach a conclusion? I'm testing a snapshot of
> glibc 2.34 in ubuntu and running into this issue -- bison segfaults on
> startup on ppc64el.

For us it got resolved with a binutils fix:

commit b293661219c36e72acb80502a86b51160bb88cfd
Author: Alan Modra <amodra@gmail.com>
Date:   Mon May 3 10:03:06 2021 +0930

    PPC: ensure_undef_dynamic on weak undef only in plt
    
    It's slightly weird to have a call to a weak function not protected by
    a test of that function being non-NULL, but the non-NULL test might be
    covered by a test of another function.  For example:
      if (func1)
        {
          func1 ();
          func2 ();
        }
    where func2 is known to exist if func1 exists.
    
            * elf32-ppc.c (allocate_dynrelocs): Call ensure_undef_dynamic for
            weak undefined symols that only appear on PLT relocs.
            * elf64-ppc.c (allocate_dynrelocs): Likewise.

We rebuilt bison and a couple of other packages that looked like it
would be affected by this before putting glibc 2.34 snapshots into the
buildroot, and that worked quite well for us.  (Thanks to Andreas Schwab
for identifying the issue so early.)

We don't know yet whether there are user binaries out there which will
be incompatible with glibc 2.34.  I have posted a glibc patch which
alters symbol resolution to increase compatibility with old binaries (so
it's technically feasible to get this working again), but in the review
discussion, I was asked to break *more* older binaries, including every
i386 binary from the late 1990s/early 2000s, so I dropped that patch and
did not pursue this approach further.  But I guess we can get back to it
if feedback from end users indicates that the current approach doesn't
work for them.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Matthias Klose]

On 7/12/21 5:03 PM, Florian Weimer via Binutils wrote:
> * Michael Hudson-Doyle:
> 
>> Did this thread ever reach a conclusion? I'm testing a snapshot of
>> glibc 2.34 in ubuntu and running into this issue -- bison segfaults on
>> startup on ppc64el.

> We rebuilt bison and a couple of other packages

do you have a list of these packages?

Matthias

Re: Undefined use of weak symbols in gnulib

[Florian Weimer]

* Matthias Klose:

> On 7/12/21 5:03 PM, Florian Weimer via Binutils wrote:
>> * Michael Hudson-Doyle:
>> 
>>> Did this thread ever reach a conclusion? I'm testing a snapshot of
>>> glibc 2.34 in ubuntu and running into this issue -- bison segfaults on
>>> startup on ppc64el.
>
>> We rebuilt bison and a couple of other packages
>
> do you have a list of these packages?

I rebuilt everything that had a weak symbol reference to
pthread_mutexattr_gettype or thread_exit because those two symbols are
used for the single-thread optimization in current gnulib.  The presence
of these symbols largely depends on at which point the upstream sources
you use last imported the relevant modules from gnulib, so it's probably
best to check each distribution individually.  According to my notes,
for us, that was bison, findutils, nano, and gnulib itself.

Thanks,
Florian

Re: Undefined use of weak symbols in gnulib

[Michael Hudson-Doyle]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Tue, 13 Jul 2021 at 03:37, Florian Weimer <fweimer@redhat.com> wrote:

> * Matthias Klose:
>
> > On 7/12/21 5:03 PM, Florian Weimer via Binutils wrote:
> >> * Michael Hudson-Doyle:
> >>
> >>> Did this thread ever reach a conclusion? I'm testing a snapshot of
> >>> glibc 2.34 in ubuntu and running into this issue -- bison segfaults on
> >>> startup on ppc64el.
> >
> >> We rebuilt bison and a couple of other packages
> >
> > do you have a list of these packages?
>
> I rebuilt everything that had a weak symbol reference to
> pthread_mutexattr_gettype or thread_exit because those two symbols are
> used for the single-thread optimization in current gnulib.  The presence
> of these symbols largely depends on at which point the upstream sources
> you use last imported the relevant modules from gnulib, so it's probably
> best to check each distribution individually.  According to my notes,
> for us, that was bison, findutils, nano, and gnulib itself.
>

Thanks for the list and the other reply. It doesn't seem quite so bad if
it's only a few packages and we can fix them by rebuilding before the new
libc lands.

Cheers,
mwh

[-- Attachment #2: Type: text/html, Size: 1634 bytes --]