Re: glthread_create() failure in test-rwlock1

[Bruno Haible]

Pádraig Brady wrote:
> Your test failure was:
>   FAIL: test-rwlock1
>   ==================
>   Unexpected outcome 3
>   FAIL test-rwlock1 (exit status: 134)
> which is due to glthread_create() failing.
> CC'ing gnulib.

The patch below fixes it.

The issue is caused by three factors:

  1) The gnulib 'lock' module links against pthread symbols only in a
     weak manner. So that most programs can use multithread-safe code,
     without actually linking in -lpthread. BUT test-rwlock1 is one of the
     few programs that does require linking with -lpthread.

  2) gcc's configuration enables --as-needed by default, which eliminates
     -l options to libraries that appear to be unneeded. However, this
     detection whether a library is needed or not does not consider weak
     symbols, only undefined symbols.

     This issue already occurred on Ubuntu 16.04. You would have seen the
     issue if LIBMULTITHREAD had been "-lpthread". But it happened to be
     "-pthread", and through some fortuitous circumstances in the GCC
     configuration, the linker command line ended up being something like

       collect2 ... --as-needed ... -o test-rwlock1 ... \
       test-rwlock1.o \
       libtests.a ../lib/libcoreutils.a libtests.a \
       -lrt -lgcc \
       --as-needed -lgcc_s --no-as-needed \
       -lpthread \
       -lc -lgcc \
       --as-needed -lgcc_s --no-as-needed \
       ...

     which means that -lpthread comes at a point where --no-as-needed is in
     effect.

  3) The new thing in Ubuntu 18.04 is that its GCC configuration uses the
     relatively new linker options --push-state/--pop-state:

       collect2 ... --as-needed ... -o test-rwlock1 ... \
       test-rwlock1.o \
       libtests.a ../lib/libcoreutils.a libtests.a \
       -lrt -lgcc \
       ---push-state --as-needed -lgcc_s --pop-state \
       -lpthread \
       -lc -lgcc \
       --push-state --as-needed -lgcc_s --pop-state \
       ...

     which means that here -lpthread comes at a point where --as-needed is in
     effect. Thus it gets optimized out. Thus &pthread_create evaluates to
     NULL, and the program cannot create threads.

The fix is to make use of --no-as-needed.

It has the side effect that on not-so-new glibc systems (which don't have
--push-state/--pop-state) not only -lpthread, but also other libraries like
-lrt are linked in as well (when they were optimized out before). But this
does not cause bugs, only slightly longer program start times. And it won't
affect new distros, which all support --push-state/--pop-state.

I verified it causes no regression on CentOS 5, 6, 7, musl libc, FreeBSD,
OpenBSD.


2018-06-25  Bruno Haible  <address@hidden>

        threadlib: Fix LIBMULTITHREAD on platforms where --as-needed is enabled.
        Reported by Erik Auerswald <address@hidden>
        in <https://lists.gnu.org/archive/html/coreutils/2018-06/msg00063.html>.
        * m4/threadlib.m4 (gl_THREADLIB_BODY): Check whether the linker supports
        --as-needed/--no-as-needed and --push-state/--pop-state. When defining
        USE_POSIX_THREADS_WEAK or USE_SOLARIS_THREADS_WEAK or
        USE_PTH_THREADS_WEAK, define LIBMULTITHREAD in such a way that -lpthread
        / -lthread / -lpth does not get optimized away by a preceding
        --as-needed option.

diff --git a/m4/threadlib.m4 b/m4/threadlib.m4
index 1f7b134..54a57e9 100644
--- a/m4/threadlib.m4
+++ b/m4/threadlib.m4
@@ -1,4 +1,4 @@
-# threadlib.m4 serial 13
+# threadlib.m4 serial 14
 dnl Copyright (C) 2005-2018 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -153,6 +153,36 @@ int main ()
          *" -static "*) gl_cv_have_weak=no ;;
        esac
       ])
+    dnl Check whether the linker supports the --as-needed/--no-as-needed 
options.
+    dnl Assume GCC, so that we can use the -Wl option.
+    AC_CACHE_CHECK([whether the linker supports --as-needed],
+      [gl_cv_linker_have_as_needed],
+      [if test -n "$GCC"; then
+         gl_saved_ldflags="$LDFLAGS"
+         LDFLAGS="$gl_saved_ldflags -Wl,--as-needed -Wl,--no-as-needed"
+         AC_LINK_IFELSE([AC_LANG_PROGRAM()],
+           [gl_cv_linker_have_as_needed=yes],
+           [gl_cv_linker_have_as_needed=no])
+         LDFLAGS="$gl_saved_ldflags"
+       else
+         gl_cv_linker_have_as_needed=no
+       fi
+      ])
+    dnl Check whether the linker supports the --push-state/--pop-state options.
+    dnl Assume GCC, so that we can use the -Wl option.
+    AC_CACHE_CHECK([whether the linker supports --push-state],
+      [gl_cv_linker_have_push_state],
+      [if test -n "$GCC"; then
+         gl_saved_ldflags="$LDFLAGS"
+         LDFLAGS="$gl_saved_ldflags -Wl,--push-state -Wl,--pop-state"
+         AC_LINK_IFELSE([AC_LANG_PROGRAM()],
+           [gl_cv_linker_have_push_state=yes],
+           [gl_cv_linker_have_push_state=no])
+         LDFLAGS="$gl_saved_ldflags"
+       else
+         gl_cv_linker_have_push_state=no
+       fi
+      ])
     if test "$gl_use_threads" = yes || test "$gl_use_threads" = posix; then
       # On OSF/1, the compiler needs the flag -pthread or -D_REENTRANT so that
       # it groks <pthread.h>. It's added above, in gl_THREADLIB_EARLY_BODY.
@@ -231,6 +261,32 @@ int main ()
                 [Define if references to the POSIX multithreading library 
should be made weak.])
               LIBTHREAD=
               LTLIBTHREAD=
+              dnl On platforms where GCC enables --as-needed by default, 
attempt
+              dnl to make sure that LIBMULTITHREAD really links with -lpthread.
+              dnl Otherwise linking with LIBMULTITHREAD has no effect; then
+              dnl the weak symbols are not defined and thus evaluate to NULL.
+              case "$LIBMULTITHREAD" in
+                "") ;;
+                -pthread)
+                  if test $gl_cv_linker_have_as_needed = yes; then
+                    if test $gl_cv_linker_have_push_state = yes; then
+                      LIBMULTITHREAD="$LIBMULTITHREAD -Wl,--push-state 
-Wl,--no-as-needed -lpthread -Wl,--pop-state"
+                    else
+                      LIBMULTITHREAD="$LIBMULTITHREAD -Wl,--no-as-needed 
-lpthread"
+                    fi
+                  fi
+                  ;;
+                *)
+                  if test $gl_cv_linker_have_as_needed = yes; then
+                    if test $gl_cv_linker_have_push_state = yes; then
+                      LIBMULTITHREAD="-Wl,--push-state -Wl,--no-as-needed 
$LIBMULTITHREAD -Wl,--pop-state"
+                    else
+                      LIBMULTITHREAD="-Wl,--no-as-needed $LIBMULTITHREAD"
+                    fi
+                  fi
+                  ;;
+              esac
+              # TODO: May need to modify LTLIBMULTITHREAD similarly.
             fi
           fi
         fi
@@ -263,6 +319,18 @@ int main ()
               [Define if references to the old Solaris multithreading library 
should be made weak.])
             LIBTHREAD=
             LTLIBTHREAD=
+            dnl On platforms where GCC enables --as-needed by default, attempt
+            dnl to make sure that LIBMULTITHREAD really links with -lthread.
+            dnl Otherwise linking with LIBMULTITHREAD has no effect; then
+            dnl the weak symbols are not defined and thus evaluate to NULL.
+            if test $gl_cv_linker_have_as_needed = yes; then
+              if test $gl_cv_linker_have_push_state = yes; then
+                LIBMULTITHREAD="-Wl,--push-state -Wl,--no-as-needed 
$LIBMULTITHREAD -Wl,--pop-state"
+              else
+                LIBMULTITHREAD="-Wl,--no-as-needed $LIBMULTITHREAD"
+              fi
+            fi
+            # TODO: May need to modify LTLIBMULTITHREAD similarly.
           fi
         fi
       fi
@@ -291,6 +359,18 @@ int main ()
               [Define if references to the GNU Pth multithreading library 
should be made weak.])
             LIBTHREAD=
             LTLIBTHREAD=
+            dnl On platforms where GCC enables --as-needed by default, attempt
+            dnl to make sure that LIBMULTITHREAD really links with -lpth.
+            dnl Otherwise linking with LIBMULTITHREAD has no effect; then
+            dnl the weak symbols are not defined and thus evaluate to NULL.
+            if test $gl_cv_linker_have_as_needed = yes; then
+              if test $gl_cv_linker_have_push_state = yes; then
+                LIBMULTITHREAD="-Wl,--push-state -Wl,--no-as-needed 
$LIBMULTITHREAD -Wl,--pop-state"
+              else
+                LIBMULTITHREAD="-Wl,--no-as-needed $LIBMULTITHREAD"
+              fi
+            fi
+            # TODO: May need to modify LTLIBMULTITHREAD similarly.
           fi
         fi
       else