bug#20136: 25.0.50; [PATCH] Fix broken libpthread detection

bug#20136: 25.0.50; [PATCH] Fix broken libpthread detection

[Wolfgang Jenkner]

On FreeBSD, configure emacs (trunk) with, say,

./configure --without-all --with-x --with-x-toolkit=no

and look at what configure thinks about pthread support:

checking for library containing pthread_atfork... none required
[...]
checking whether pthread_sigmask works without -lpthread... no
checking whether pthread_sigmask returns error numbers... yes
checking whether pthread_sigmask unblocks signals correctly... no

The correct results are `-lpthread' instead of `none required' (that is,
if the test were pertinent at all, see below) and `yes' instead of the
last `no'.  The bug is due to 93ca4887 and is not present in emacs-24.

The reason is that while pthread_atfork and some other pthread_ stubs
are indeed present in libc, -lpthread or something equivalent is still
needed for correct behaviour.

The issue is not peculiar to FreeBSD, as GNU libc also contains some
pthread_ symbols, but not those that configure tests, so things happen
to work there.  Allegedly, older versions of Solaris libc contain stubs
for (almost) all pthread_ functions whereas in newer versions those are
not stubs but the real stuff...

Next, the pthread_ functions can really be macros (at least in theory).

Finally, it would be nice (I imagine) to use -pthread instead
of -lpthread if possible.

So I wonder if it wouldn't be better to use the ax_pthread.m4 module
from the autoconf archive, which tackles those problems, see

https://www.gnu.org/software/autoconf-archive/ax_pthread.html

It contains some historical cruft, like tests for linuxthreads or
FreeBSD kthreads, but that could easily be trimmed.

Also, guile uses this module, so I guess that copyright assignment to
the FSF either has already been dealt with or is not an issue here.

In any case, I've written the necessary glue to integrate it into
configure.ac (to test it, you have also to download the latest version
of the module from the link above, put it in m4/ and run ./autogen.sh
after the new file and the patch below are in place):

The subtle art of writing tests which can be compiled and linked but
would break at run time is new to me, though (but ax_pthread.m4 does
this, so why not imitate it).

-- >8 --
Subject: [PATCH] configure.ac: Integrate m4/ax_pthread.m4.

---
 configure.ac | 59 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/configure.ac b/configure.ac
index 85e04e9..6ce0c6a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2182,33 +2182,38 @@ LIB_PTHREAD=
 if test "$opsys" != "mingw32"; then
 AC_CHECK_HEADERS_ONCE(pthread.h)
 if test "$ac_cv_header_pthread_h"; then
-  dnl gmalloc.c uses pthread_atfork, which is not available on older-style
-  dnl hosts such as MirBSD 10, so test for pthread_atfork instead of merely
-  dnl testing for pthread_kill if Emacs uses gmalloc.c.
-  if test "$GMALLOC_OBJ" = gmalloc.o; then
-    emacs_pthread_function=pthread_atfork
-  else
-    emacs_pthread_function=pthread_kill
-  fi
-  OLD_LIBS=$LIBS
-  AC_SEARCH_LIBS([$emacs_pthread_function], [pthread],
-    [AC_DEFINE([HAVE_PTHREAD], [1],
-       [Define to 1 if you have pthread (-lpthread).])
-     # Some systems optimize for single-threaded programs by default, and
-     # need special flags to disable these optimizations. For example, the
-     # definition of 'errno' in <errno.h>.
-     case $opsys in
-       sol*)
-         AC_DEFINE([_REENTRANT], 1,
-   	[Define to 1 if your system requires this in multithreaded code.]);;
-       aix4-2)
-         AC_DEFINE([_THREAD_SAFE], 1,
-   	[Define to 1 if your system requires this in multithreaded code.]);;
-     esac])
- if test "X$LIBS" != "X$OLD_LIBS"; then
-    eval LIB_PTHREAD=\$ac_cv_search_$emacs_pthread_function
-  fi
-  LIBS=$OLD_LIBS
+  OLD_CC="$CC"
+  OLD_CFLAGS="$CFLAGS"
+  OLD_LDFLAGS="$LDFLAGS"
+  OLD_LIBS="$LIBS"
+  ACX_PTHREAD([CC="$PTHREAD_CC"
+    CFLAGS="$PTHREAD_CFLAGS $CFLAGS"
+    dnl XXX Might not always work?
+    LDFLAGS="$PTHREAD_CFLAGS $LDFLAGS"
+    LIBS="$PTHREAD_LIBS $LIBS"
+    dnl gmalloc.c uses pthread_atfork, which is not available on older-style
+    dnl hosts such as MirBSD 10, so test for pthread_atfork instead of merely
+    dnl testing for pthread_kill if Emacs uses gmalloc.c.
+    if test "$GMALLOC_OBJ" = gmalloc.o; then
+      emacs_pthread_function=pthread_atfork
+      emacs_pthread_args="NULL, NULL, NULL"
+    else
+      emacs_pthread_function=pthread_kill
+      emacs_pthread_args="tid, 0"
+    fi
+    AC_MSG_CHECKING([for $emacs_pthread_function])
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include <pthread.h>
+pthread_t tid;]],
+      [[pthread_create (&tid, NULL, NULL, NULL);
+$emacs_pthread_function ($emacs_pthread_args)]])],
+      [AC_MSG_RESULT([yes])
+       AC_DEFINE([HAVE_PTHREAD], [1], [Define to 1 if you have pthread.])
+       LIB_PTHREAD="$PTHREAD_LIBS"],
+      [AC_MSG_RESULT([no])
+       CC="$OLD_CC"
+       CFLAGS="$OLD_CFLAGS"
+       LDFLAGS="$OLD_LDFLAGS"])
+    LIBS="$OLD_LIBS"])
 fi
 AC_SUBST([LIB_PTHREAD])
 fi
-- 
2.3.3

bug#20136: 25.0.50; [PATCH] Fix broken libpthread detection

[Paul Eggert]

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

Thanks for the bug report and suggestion.  However, it's probably better 
to keep Emacs's pthread configuration simple.  We're already bypassing 
both of Gnulib's pthread configuration strategies, so it's not too much 
of a stretch to also bypass the strategy in the Autoconf archive.  All 
these strategies are hacky and are showing signs of age (Sequent! wow! 
it's been a while) and I'm sort of hoping that FreeBSD will change its 
default to be pthread-friendly instead of pthread-hostile so that Emacs 
and other apps can stop worrying about this gorp.  So I installed the 
attached patch instead, which I tested on Fedora 21 x86-64 and on 
FreeBSD 9.1 x86-64.

> it would be nice (I imagine) to use -pthread instead
> of -lpthread if possible.
>

What does -pthread do that the current approach doesn't?

More generally, if we're worried only about Emacs's runtime behavior, is 
there anything wrong with compiling with the equivalent of 
-D_THREAD_SAFE -lpthread?

[-- Attachment #2: 0001-Better-port-of-pthread-usage-to-FreeBSD.patch --]
[-- Type: text/x-patch, Size: 5467 bytes --]

From 2e32ff90a6af9695d812700e9347f15b50d829e5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 19 Mar 2015 14:14:07 -0700
Subject: [PATCH] Better port of pthread usage to FreeBSD

* configure.ac (ac_func_list): Omit pthread_sigmask, since
we check for that ourselves rather than relying on gnulib.
(HAVE_PTHREAD, LIB_PTHREAD, _THREAD_SAFE): Port better to FreeBSD,
by also checking for pthread_create, pthread_self, pthread_sigmask.
Tighten the test for pthread_atfork while we're at it.
Fixes: bug#20136
---
 ChangeLog    |  8 ++++++
 configure.ac | 87 ++++++++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9d7ba93..d268ba0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2015-03-19  Paul Eggert  <eggert@cs.ucla.edu>
 
+	Better port of pthread usage to FreeBSD
+	* configure.ac (ac_func_list): Omit pthread_sigmask, since
+	we check for that ourselves rather than relying on gnulib.
+	(HAVE_PTHREAD, LIB_PTHREAD, _THREAD_SAFE): Port better to FreeBSD,
+	by also checking for pthread_create, pthread_self, pthread_sigmask.
+	Tighten the test for pthread_atfork while we're at it.
+	Fixes: bug#20136
+
 	Merge from gnulib
 	This incorporates:
 	2015-03-19 fdopendir: port better to MinGW
diff --git a/configure.ac b/configure.ac
index d65494a..520816b1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -780,6 +780,12 @@ AC_DEFUN([gl_CRYPTO_CHECK])
 # Avoid gnulib's tests for HAVE_WORKING_O_NOATIME and HAVE_WORKING_O_NOFOLLOW,
 # as we don't use them.
 AC_DEFUN([gl_FCNTL_O_FLAGS])
+# Avoid gnulib's test for pthread_sigmask.
+funcs=
+for func in $ac_func_list; do
+  test $func = pthread_sigmask || AS_VAR_APPEND([funcs], [" $func"])
+done
+ac_func_list=$funcs
 # Use the system putenv even if it lacks GNU features, as we don't need them,
 # and the gnulib replacement runs afoul of a FreeBSD 10.1 bug; see Bug#19874.
 AC_CHECK_FUNCS_ONCE([putenv])
@@ -2179,39 +2185,62 @@ AC_CHECK_LIB(Xbsd, main, LD_SWITCH_X_SITE="$LD_SWITCH_X_SITE -lXbsd")
 
 dnl Check for the POSIX thread library.
 LIB_PTHREAD=
-if test "$opsys" != "mingw32"; then
 AC_CHECK_HEADERS_ONCE(pthread.h)
-if test "$ac_cv_header_pthread_h"; then
-  dnl gmalloc.c uses pthread_atfork, which is not available on older-style
-  dnl hosts such as MirBSD 10, so test for pthread_atfork instead of merely
-  dnl testing for pthread_kill if Emacs uses gmalloc.c.
-  if test "$GMALLOC_OBJ" = gmalloc.o; then
-    emacs_pthread_function=pthread_atfork
-  else
-    emacs_pthread_function=pthread_kill
-  fi
-  OLD_LIBS=$LIBS
-  AC_SEARCH_LIBS([$emacs_pthread_function], [pthread],
-    [AC_DEFINE([HAVE_PTHREAD], [1],
-       [Define to 1 if you have pthread (-lpthread).])
-     # Some systems optimize for single-threaded programs by default, and
-     # need special flags to disable these optimizations. For example, the
-     # definition of 'errno' in <errno.h>.
-     case $opsys in
-       sol*)
-         AC_DEFINE([_REENTRANT], 1,
-   	[Define to 1 if your system requires this in multithreaded code.]);;
-       aix4-2)
-         AC_DEFINE([_THREAD_SAFE], 1,
-   	[Define to 1 if your system requires this in multithreaded code.]);;
-     esac])
- if test "X$LIBS" != "X$OLD_LIBS"; then
-    eval LIB_PTHREAD=\$ac_cv_search_$emacs_pthread_function
+if test "$ac_cv_header_pthread_h" && test "$opsys" != "mingw32"; then
+  AC_CACHE_CHECK([for pthread library],
+    [emacs_cv_pthread_lib],
+    [emacs_cv_pthread_lib=no
+     OLD_CPPFLAGS=$CPPFLAGS
+     OLD_LIBS=$LIBS
+     for emacs_pthread_lib in 'none needed' -lpthread; do
+       case $emacs_pthread_lib in
+	 -*) LIBS="$OLD_LIBS $emacs_pthread_lib";;
+       esac
+       AC_LINK_IFELSE(
+	 [AC_LANG_PROGRAM(
+	    [[#include <pthread.h>
+	      #include <signal.h>
+	      sigset_t old_mask, new_mask;
+	      void noop (void) {}]],
+	    [[pthread_t th = pthread_self ();
+	      int status = 0;
+	      status += pthread_create (&th, 0, 0, 0);
+	      status += pthread_sigmask (SIG_BLOCK, &new_mask, &old_mask);
+	      status += pthread_kill (th, 0);
+	      #if ! (defined SYSTEM_MALLOC || defined HYBRID_MALLOC \
+		     || defined DOUG_LEA_MALLOC)
+	      /* Test for pthread_atfork only if gmalloc uses it,
+		 as older-style hosts like MirBSD 10 lack it.  */
+	      status += pthread_atfork (noop, noop, noop);
+	      #endif
+	      return status;]])],
+	 [emacs_cv_pthread_lib=$emacs_pthread_lib])
+       LIBS=$OLD_LIBS
+       if test "$emacs_cv_pthread_lib" != no; then
+	 break
+       fi
+     done
+     CPPFLAGS=$OLD_CPPFLAGS])
+  if test "$emacs_cv_pthread_lib" != no; then
+    AC_DEFINE([HAVE_PTHREAD], 1, [Define to 1 if you have POSIX threads.])
+    case $emacs_cv_pthread_lib in
+      -*) LIB_PTHREAD=$emacs_cv_pthread_lib;;
+    esac
+    ac_cv_func_pthread_sigmask=yes
+    # Some systems optimize for single-threaded programs by default, and
+    # need special flags to disable these optimizations. For example, the
+    # definition of 'errno' in <errno.h>.
+    case $opsys in
+      hpux* | sol*)
+	AC_DEFINE([_REENTRANT], 1,
+	  [Define to 1 if your system requires this in multithreaded code.]);;
+      aix4-2 | darwin | freebsd)
+	AC_DEFINE([_THREAD_SAFE], 1,
+	  [Define to 1 if your system requires this in multithreaded code.]);;
+    esac
   fi
-  LIBS=$OLD_LIBS
 fi
 AC_SUBST([LIB_PTHREAD])
-fi
 
 dnl Check for need for bigtoc support on IBM AIX
 
-- 
2.1.0

bug#20136: 25.0.50; [PATCH] Fix broken libpthread detection

[Wolfgang Jenkner]

On Thu, Mar 19 2015, Paul Eggert wrote:

> So I installed the attached patch instead, which I tested
> on Fedora 21 x86-64 and on FreeBSD 9.1 x86-64.

Thanks, this works on FreeBSD 10.1-STABLE, too.

> More generally, if we're worried only about Emacs's runtime behavior,
> is there anything wrong with compiling with the equivalent
> of -D_THREAD_SAFE -lpthread?

IIUC, -D_THREAD_SAFE or other flags are not needed on FreeBSD (since 7,
at least), linking with -lpthread should be enough since all pthread_
symbols in the shared libc are weak.

bug#20136: 25.0.50; [PATCH] Fix broken libpthread detection

[Paul Eggert]

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

Wolfgang Jenkner wrote:
> -D_THREAD_SAFE or other flags are not needed on FreeBSD (since 7,
> at least)

OK, thanks, FreeBSD 6 was end-of-lifed some time ago and I assume Darwin is 
similar so I installed the attached simplification.

[-- Attachment #2: 0001-configure.ac-_THREAD_SAFE-Simplify-Bug-20136.patch --]
[-- Type: text/x-patch, Size: 1362 bytes --]

From 84c6c65cb46bb9190fdd098e06e2109cdc7507d9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 19 Mar 2015 18:33:59 -0700
Subject: [PATCH] * configure.ac (_THREAD_SAFE): Simplify (Bug#20136).

---
 ChangeLog    | 2 +-
 configure.ac | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d268ba0..a79863a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,7 +3,7 @@
 	Better port of pthread usage to FreeBSD
 	* configure.ac (ac_func_list): Omit pthread_sigmask, since
 	we check for that ourselves rather than relying on gnulib.
-	(HAVE_PTHREAD, LIB_PTHREAD, _THREAD_SAFE): Port better to FreeBSD,
+	(HAVE_PTHREAD, LIB_PTHREAD): Port better to FreeBSD,
 	by also checking for pthread_create, pthread_self, pthread_sigmask.
 	Tighten the test for pthread_atfork while we're at it.
 	Fixes: bug#20136
diff --git a/configure.ac b/configure.ac
index 520816b1..40c8d2b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2234,7 +2234,7 @@ if test "$ac_cv_header_pthread_h" && test "$opsys" != "mingw32"; then
       hpux* | sol*)
 	AC_DEFINE([_REENTRANT], 1,
 	  [Define to 1 if your system requires this in multithreaded code.]);;
-      aix4-2 | darwin | freebsd)
+      aix4-2)
 	AC_DEFINE([_THREAD_SAFE], 1,
 	  [Define to 1 if your system requires this in multithreaded code.]);;
     esac
-- 
2.1.0