On Sun, Mar 19, 2017 at 08:25:38PM +0000, Antti Kantee wrote:
> On 19/03/17 18:58, Francesco Lattanzio wrote:
> > First be known that I'm building on FreeBSD 11.0.
> > When I wrote this e-mail I thought there was no pthread_setname_np() at
> > all, but searching more carefully I found out that there is indeed a
> > pthread_setname_np() function but its name is pthread_set_name_np() (see
> > https://www.freebsd.org/cgi/man.cgi?query=pthread_set_name_np&apropos=0&sektion=0&manpath=FreeBSD+11.0-RELEASE+and+Ports&arch=default&format=html).
> >
> > Using Clang (version 3.8.0) or a recent gcc (version 6.3.0), both tests
> > fail and none of the two versions of pthread_setname_np() is included,
> > which allows for a clean link.
> >
> > Using gcc 4.9.4 (the default gcc in FreeBSD) what happen is that both
> > checks succeed and I get both versions of pthread_setname_np() included
> > in rumpuser_pth.c, but neither is defined in libpthread and the
> > compilation terminates with the following error:
>
> Ah, the pthread_setname_np() variant is tested only with
> AC_COMPILE_IFELSE, which just depends on compiler warnings/errors, not
> the link result. So, your initial analysis was correct.
>
> > --- rumpuser_pth.pico ---
> > # compile librumpuser/rumpuser_pth.pico
> > /usr/home/fltt/tmp/rump/buildrump.sh/obj/tooldir/bin/i486--netbsdelf-clang
> > -O2 -g -std=gnu99 -Wno-sign-compare -Wno-pointer-sign -Wall
> > -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-sign-compare
> > -Wa,--fatal-warnings -Wreturn-type -Wswitch -Wshadow -Wcast-qual
> > -Wwrite-strings -Wextra -Wno-unused-parameter -Wno-sign-compare
> > -Wold-style-definition -Wsign-compare -Wformat=2 -Wpointer-sign
> > -Wmissing-noreturn -Werror -Wno-format -DRUMPUSER_CONFIG=yes
> > -I/usr/home/fltt/tmp/rump/buildrump.sh/obj/tooldir/autoconf
> > -I/usr/home/fltt/tmp/rump/buildrump.sh/obj/tooldir/compat/include
> > -I/usr/home/fltt/tmp/rump/buildrump.sh/obj/dest.stage/usr/include
> > -DLIBRUMPUSER -D_KERNTYPES -D_REENTRANT -c -fPIC -g
> > /usr/home/fltt/tmp/rump/buildrump.sh/src/lib/librumpuser/rumpuser_pth.c -o
> > rumpuser_pth.pico
> > --- rumpuser_pth.pico ---
> > /usr/home/fltt/tmp/rump/buildrump.sh/src/lib/librumpuser/rumpuser_pth.c:85:3:
> > error: implicit declaration of function 'pthread_setname_np' is invalid in
> > C99 [-Werror,-Wimplicit-function-declaration]
> > pthread_setname_np(*ptidp, thrname, NULL);
> >
> > Note that gcc is used to check for the presence of pthread_setname_np()
> > but Clang was used to compile the code. I don't know if this may be of
> > some relevance.
>
> Yes and no. Yes in that it's a bug, but no in that configure.ac is also
> wrong. exporting CC in buildrump.sh in the snippet that runs configure
> should fix it, at least assuming you're running buildrump.sh with
> "CC=clang ./buildrump.sh". Want to try it (and submit a pull req if so)?
Done. See below.
I've tested it on FreeBSD 11.0 (32 bit, all four possible combination of
cc and gcc as CC and HOST_CC) and on ArchLinux (64 bit, with
gcc). Should be tested on NetBSD too and the content of rumpuser_port.h
checked.
NOTE: You don't really need the -Wimplicit-function-declaration option
to build on FreeBSD, as the presence of the pthread_set_name_np()
function will make configure skip the 2/3 args checks on
pthread_setname_np(). However, I kept it as it solves issues that would
arise with old-yet-still-available versions of GCC on Linux (i.e., the 2
and the 3 args checks both successfully passed).
> > It looks like the correct solution would be to check for
> > pthread_set_name_np() too. And to avoid using the
> > -Wimplicit-function-declaration option you may try to actually link
> > against the pthread_setname_np() functions by means of the AC_TRY_LINK
> > macro.
>
> Yes. Well, easier to change to AC_LINK_IFELSE, but generally yes. Want
> to prepare a patch for that, especially the pthread_set_name_np() part
> which I can't easily test?
-- >8 --
Subject: [PATCH] Improve support for non-posix pthread functions
FreeBSD's pthread_setname_np() is named pthread_set_name_np() and its
declaration is stored in "pthread_np.h" (which contains all the
non-posix pthread function declarations).
We also need to add the -Wimplicit-function-declaration options
explicitly for those Linux distributions (e.g., Debian 8) that install,
by default, old versions of gcc (e.g., 4.9.x) which, by default, have
the -Wimplicit-function-declaration option disabled.
---
lib/librumpuser/configure | 116 +++++++++++++++++++++++++----------
lib/librumpuser/configure.ac | 51 ++++++++-------
lib/librumpuser/rumpuser_config.h.in | 13 ++--
lib/librumpuser/rumpuser_port.h | 3 +-
lib/librumpuser/rumpuser_pth.c | 12 ++--
5 files changed, 128 insertions(+), 67 deletions(-)
diff --git a/lib/librumpuser/configure b/lib/librumpuser/configure
index 6b6e7f18..83d37f40 100755
--- a/lib/librumpuser/configure
+++ b/lib/librumpuser/configure
@@ -3073,7 +3073,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3119,7 +3119,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3143,7 +3143,7 @@ rm -f core conftest.err conftest.$ac_objext
conftest.$ac_ext
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3188,7 +3188,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3212,7 +3212,7 @@ rm -f core conftest.err conftest.$ac_objext
conftest.$ac_ext
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3647,7 +3647,7 @@ done
for ac_header in sys/param.h sys/sysctl.h sys/disk.h \
- sys/disklabel.h sys/dkio.h sys/atomic.h paths.h
+ sys/disklabel.h sys/dkio.h sys/atomic.h paths.h pthread_np.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header"
"$ac_includes_default"
@@ -4081,6 +4081,53 @@ rm -f core conftest.err conftest.$ac_objext \
conftest$ac_exeext conftest.$ac_ext
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for pthread_mutex_lock in
-lpthread" >&5
+$as_echo_n "checking for pthread_mutex_lock in -lpthread... " >&6; }
+if ${ac_cv_lib_pthread_pthread_mutex_lock+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_check_lib_save_LIBS=$LIBS
+LIBS="-lpthread $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char pthread_mutex_lock ();
+int
+main ()
+{
+return pthread_mutex_lock ();
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+ ac_cv_lib_pthread_pthread_mutex_lock=yes
+else
+ ac_cv_lib_pthread_pthread_mutex_lock=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
$ac_cv_lib_pthread_pthread_mutex_lock" >&5
+$as_echo "$ac_cv_lib_pthread_pthread_mutex_lock" >&6; }
+if test "x$ac_cv_lib_pthread_pthread_mutex_lock" = xyes; then :
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_LIBPTHREAD 1
+_ACEOF
+
+ LIBS="-lpthread $LIBS"
+
+else
+ as_fn_error $? "can't build without libpthread" "$LINENO" 5
+fi
+
ac_fn_c_check_member "$LINENO" "struct sockaddr_in" "sin_len"
"ac_cv_member_struct_sockaddr_in_sin_len" "#include <netinet/in.h>
"
@@ -4094,26 +4141,21 @@ _ACEOF
fi
-SAVE_CFLAGS="${CFLAGS}"
-CFLAGS="${SAVE_CFLAGS} -Werror"
+if test "x$ac_cv_header_pthread_np_h" = xyes; then :
+ ac_fn_c_check_func "$LINENO" "pthread_set_name_np"
"ac_cv_func_pthread_set_name_np"
+if test "x$ac_cv_func_pthread_set_name_np" = xyes; then :
-for ac_header in sys/cdefs.h
-do :
- ac_fn_c_check_header_compile "$LINENO" "sys/cdefs.h"
"ac_cv_header_sys_cdefs_h" "#include <sys/cdefs.h>
-"
-if test "x$ac_cv_header_sys_cdefs_h" = xyes; then :
- cat >>confdefs.h <<_ACEOF
-#define HAVE_SYS_CDEFS_H 1
-_ACEOF
+$as_echo "#define pthread_setname_npx pthread_set_name_np" >>confdefs.h
fi
-done
+fi
+SAVE_CFLAGS="${CFLAGS}"
+CFLAGS="${SAVE_CFLAGS} -Werror -Wimplicit-function-declaration"
-SAVE_LIBS="${LIBS}"
-LIBS="${LIBS} -lpthread"
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for two-argument
pthread_setname_np()" >&5
+if test "x$ac_cv_func_pthread_set_name_np" != xyes; then :
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for two-argument
pthread_setname_np()" >&5
$as_echo_n "checking for two-argument pthread_setname_np()... " >&6; }
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
@@ -4123,7 +4165,7 @@ int
main ()
{
pthread_t pt;
- pthread_setname_np(pt, "x");return 0;
+ pthread_setname_np(pt, "x");
;
return 0;
}
@@ -4133,17 +4175,13 @@ if ac_fn_c_try_compile "$LINENO"; then :
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
$as_echo "yes" >&6; }
-
-$as_echo "#define HAVE_PTHREAD_SETNAME2 1" >>confdefs.h
+ $as_echo "#define pthread_setname_npx pthread_setname_np"
>>confdefs.h
else
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
-
-fi
-rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for three-argument
pthread_setname_np()" >&5
$as_echo_n "checking for three-argument pthread_setname_np()... " >&6; }
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -4154,7 +4192,7 @@ int
main ()
{
pthread_t pt;
- pthread_setname_np(pt, "X", (void *)0);return 0;
+ pthread_setname_npx(pt, "X", (void *)0);
;
return 0;
}
@@ -4164,18 +4202,35 @@ if ac_fn_c_try_compile "$LINENO"; then :
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
$as_echo "yes" >&6; }
-
-$as_echo "#define HAVE_PTHREAD_SETNAME3 1" >>confdefs.h
+ $as_echo "#define pthread_setname_npx(a, b)
pthread_setname_np(a, b, NULL)" >>confdefs.h
else
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
+ $as_echo "#define pthread_setname_npx(a, b) /**/" >>confdefs.h
+
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-LIBS="${SAVELIBS}"
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+
+for ac_header in sys/cdefs.h
+do :
+ ac_fn_c_check_header_compile "$LINENO" "sys/cdefs.h"
"ac_cv_header_sys_cdefs_h" "#include <sys/cdefs.h>
+"
+if test "x$ac_cv_header_sys_cdefs_h" = xyes; then :
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_SYS_CDEFS_H 1
+_ACEOF
+
+fi
+
+done
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for ioctl cmd being int" >&5
$as_echo_n "checking for ioctl cmd being int... " >&6; }
@@ -4188,7 +4243,6 @@ int
main ()
{
- return 0;
;
return 0;
}
diff --git a/lib/librumpuser/configure.ac b/lib/librumpuser/configure.ac
index 14efa054..5818e6f5 100644
--- a/lib/librumpuser/configure.ac
+++ b/lib/librumpuser/configure.ac
@@ -20,7 +20,7 @@ AC_LANG([C])
AC_SYS_LARGEFILE
AC_CHECK_HEADERS([sys/param.h sys/sysctl.h sys/disk.h \
- sys/disklabel.h sys/dkio.h sys/atomic.h paths.h])
+ sys/disklabel.h sys/dkio.h sys/atomic.h paths.h pthread_np.h])
AC_CANONICAL_TARGET
@@ -44,59 +44,64 @@ AC_CHECK_LIB([rt], [clock_nanosleep],
AC_CHECK_LIB([dl], [dlinfo],
AC_DEFINE([HAVE_DLINFO], 1, [dlinfo]),
AC_TRY_LINK_FUNC([dlinfo], AC_DEFINE([HAVE_DLINFO], 1, [dlinfo])))
+AC_CHECK_LIB([pthread], [pthread_mutex_lock], [],
+ [AC_MSG_ERROR([can't build without libpthread])])
AC_CHECK_MEMBERS([struct sockaddr_in.sin_len],,,[#include <netinet/in.h>])
dnl
dnl pthread_setname() sillyness is a bit longer; we need the signature
dnl
-SAVE_CFLAGS="${CFLAGS}"
-CFLAGS="${SAVE_CFLAGS} -Werror"
+AS_IF([test "x$ac_cv_header_pthread_np_h" = xyes],
+ [AC_CHECK_FUNC([pthread_set_name_np],
+ [AC_DEFINE([pthread_setname_npx],
+ [pthread_set_name_np],
+ [Replacement for pthread_setname_np()])])])
-dnl check sys/cdefs.h creatively to process only with cc, not cpp
-dnl (sys/cdefs.h in at least in musl contains a #warning)
-AC_CHECK_HEADERS([sys/cdefs.h], [], [], [#include <sys/cdefs.h>])
+SAVE_CFLAGS="${CFLAGS}"
+CFLAGS="${SAVE_CFLAGS} -Werror -Wimplicit-function-declaration"
-SAVE_LIBS="${LIBS}"
-LIBS="${LIBS} -lpthread"
-AC_MSG_CHECKING([for two-argument pthread_setname_np()])
+AS_IF([test "x$ac_cv_func_pthread_set_name_np" != xyes],
+ [AC_MSG_CHECKING([for two-argument pthread_setname_np()])
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM(
[[#define _GNU_SOURCE
#include <pthread.h>]],
- [[pthread_t pt;]
- [pthread_setname_np(pt, "x");return 0;]])
+ [[pthread_t pt;
+ pthread_setname_np(pt, "x");]])
],[
AC_MSG_RESULT([yes])
- AC_DEFINE(HAVE_PTHREAD_SETNAME2, [1],
- [Define to 1 if you have 2-arg pthread_setname_np()])
+ AC_DEFINE([pthread_setname_npx],
+ [pthread_setname_np])
],[
AC_MSG_RESULT([no])
-])
AC_MSG_CHECKING([for three-argument pthread_setname_np()])
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM(
[[#define _GNU_SOURCE
#include <pthread.h>]],
- [[pthread_t pt;]
- [pthread_setname_np(pt, "X", (void *)0);return 0;]])
+ [[pthread_t pt;
+ pthread_setname_npx(pt, "X", (void *)0);]])
],[
AC_MSG_RESULT([yes])
- AC_DEFINE(HAVE_PTHREAD_SETNAME3, [1],
- [Define to 1 if you have 3-arg pthread_setname_np()])
+ AC_DEFINE([pthread_setname_npx(a, b)],
+ [pthread_setname_np(a, b, NULL)])
],[
AC_MSG_RESULT([no])
-])
-LIBS="${SAVELIBS}"
+ AC_DEFINE([pthread_setname_npx(a, b)],
+ [])
+])])])
+
+dnl check sys/cdefs.h creatively to process only with cc, not cpp
+dnl (sys/cdefs.h in at least in musl contains a #warning)
+AC_CHECK_HEADERS([sys/cdefs.h], [], [], [#include <sys/cdefs.h>])
AC_MSG_CHECKING([for ioctl cmd being int])
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM(
[[#include <sys/ioctl.h>
#include <unistd.h>
- int ioctl(int fd, int, ...);]],
- [[]
- [return 0;]])
+ int ioctl(int fd, int, ...);]])
],[
AC_MSG_RESULT([yes])
AC_DEFINE(HAVE_IOCTL_CMD_INT, [1],
diff --git a/lib/librumpuser/rumpuser_config.h.in
b/lib/librumpuser/rumpuser_config.h.in
index b008c062..1b5ac701 100644
--- a/lib/librumpuser/rumpuser_config.h.in
+++ b/lib/librumpuser/rumpuser_config.h.in
@@ -42,6 +42,9 @@
/* Define to 1 if you have the `kqueue' function. */
#undef HAVE_KQUEUE
+/* Define to 1 if you have the `pthread' library (-lpthread). */
+#undef HAVE_LIBPTHREAD
+
/* Define to 1 if you have the `rt' library (-lrt). */
#undef HAVE_LIBRT
@@ -57,11 +60,8 @@
/* Define to 1 if you have the `posix_memalign' function. */
#undef HAVE_POSIX_MEMALIGN
-/* Define to 1 if you have 2-arg pthread_setname_np() */
-#undef HAVE_PTHREAD_SETNAME2
-
-/* Define to 1 if you have 3-arg pthread_setname_np() */
-#undef HAVE_PTHREAD_SETNAME3
+/* Define to 1 if you have the <pthread_np.h> header file. */
+#undef HAVE_PTHREAD_NP_H
/* Define to 1 if the system has the type `register_t'. */
#undef HAVE_REGISTER_T
@@ -154,3 +154,6 @@
/* Define for large files, on AIX-style hosts. */
#undef _LARGE_FILES
+
+/* Replacement for pthread_setname_np() */
+#undef pthread_setname_npx
diff --git a/lib/librumpuser/rumpuser_port.h b/lib/librumpuser/rumpuser_port.h
index fb645ab4..1a4686ef 100644
--- a/lib/librumpuser/rumpuser_port.h
+++ b/lib/librumpuser/rumpuser_port.h
@@ -25,10 +25,10 @@
#define HAVE_GETSUBOPT 1
#define HAVE_INTTYPES_H 1
#define HAVE_KQUEUE 1
+#define HAVE_LIBPTHREAD 1
#define HAVE_MEMORY_H 1
#define HAVE_PATHS_H 1
#define HAVE_POSIX_MEMALIGN 1
-#define HAVE_PTHREAD_SETNAME3 1
#define HAVE_REGISTER_T 1
#define HAVE_SETPROGNAME 1
#define HAVE_STDINT_H 1
@@ -59,6 +59,7 @@
#ifndef _DARWIN_USE_64_BIT_INODE
# define _DARWIN_USE_64_BIT_INODE 1
#endif
+#define pthread_setname_npx(a, b) pthread_setname_np(a, b, NULL)
#else /* RUMPUSER_CONFIG */
#include "rumpuser_config.h"
diff --git a/lib/librumpuser/rumpuser_pth.c b/lib/librumpuser/rumpuser_pth.c
index faf45dc1..096fb3ef 100644
--- a/lib/librumpuser/rumpuser_pth.c
+++ b/lib/librumpuser/rumpuser_pth.c
@@ -47,6 +47,10 @@ __RCSID("$NetBSD: rumpuser_pth.c,v 1.45 2015/09/18 10:56:25
pooka Exp $");
#include <stdint.h>
#include <unistd.h>
+#if defined(HAVE_PTHREAD_NP_H)
+#include <pthread_np.h>
+#endif
+
#include <rump/rumpuser.h>
#include "rumpuser_int.h"
@@ -80,15 +84,9 @@ rumpuser_thread_create(void *(*f)(void *), void *arg, const
char *thrname,
nanosleep(&ts, NULL);
}
-#if defined(HAVE_PTHREAD_SETNAME3)
if (rv == 0 && thrname) {
- pthread_setname_np(*ptidp, thrname, NULL);
+ pthread_setname_npx(*ptidp, thrname);
}
-#elif defined(HAVE_PTHREAD_SETNAME2)
- if (rv == 0 && thrname) {
- pthread_setname_np(*ptidp, thrname);
- }
-#endif
if (joinable) {
assert(ptcookie);
--
Francesco Lattanzio
