Re: Bug in src-netbsd
- From: Francesco Lattanzio <franz.lattanzio@xxxxxxxxx>
- To: rumpkernel-users@xxxxxxxxxxxxx
- Date: Tue, 28 Mar 2017 19:19:13 +0200
On Sun, Mar 26, 2017 at 07:07:55PM +0000, Antti Kantee wrote:
On 26/03/17 17:49, Francesco Lattanzio wrote:
On Sun, Mar 26, 2017 at 03:56:18PM +0000, Antti Kantee wrote:
On 26/03/17 09:11, Francesco Lattanzio wrote:
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 don't see a pull req in buildrump.sh?
Sorry, I got confused and answered with the wrong answer.
The correct answer is: I couldn't reproduce this issue (that is, gcc
used to check for pthread_setname_np() and Clang to compile
librumpuser). I think I mixed the config.log and the buildrump.sh output
from different runs.
ic
Well, while testing the attached patch I stumbled again in this issue.
You can reproduce it if you have both Clang and GCC installed -- the
former as cc and the latter as gcc -- and you run buildrump.sh without
specifying a value for CC -- buildrump.sh will set CC=cc whereas
lib/librumuser/configure will set CC=gcc. Exporting CC before invoking
configure fixes the issue.
I've filed a pull request.
But speaking of your patch to librumpuser, please just add the necessary
things for FreeBSD using the existing conventions. For example, you
cannot add a requirement for -lpthread, since some systems have the same
functionality in libc without supplying a libpthread at all, and they
would now be broken.
You're right -- I was mislead by the LIBS="${LIBS} -lpthread" line to
think libpthread to be a required library. I'll fix it.
Thinking about it, that might be the reason the
configure script uses AC_TRY_COMPILE instead of LINK to check for
pthread stuff -- can't remember for sure anymore.
Using AC_TRY_COMPILE implies that the LIBS="${LIBS} -lpthread" is
superfluos. Ain't it?
Yes, it looks like a bug at least in the "mislead" sense, as you pointed
out.
See below for a new patch.
-- >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 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 option
disabled.
---
lib/librumpuser/configure | 70 ++++++++++++++++++++++++++----------
lib/librumpuser/configure.ac | 59 +++++++++++++++++++-----------
lib/librumpuser/rumpuser_config.h.in | 10 +++---
lib/librumpuser/rumpuser_pth.c | 12 +++----
4 files changed, 101 insertions(+), 50 deletions(-)
diff --git a/lib/librumpuser/configure b/lib/librumpuser/configure
index 6b6e7f18..265d8769 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"
@@ -4095,7 +4095,7 @@ fi
SAVE_CFLAGS="${CFLAGS}"
-CFLAGS="${SAVE_CFLAGS} -Werror"
+CFLAGS="${SAVE_CFLAGS} -Werror -Wimplicit"
for ac_header in sys/cdefs.h
do :
@@ -4111,19 +4111,20 @@ fi
done
-SAVE_LIBS="${LIBS}"
-LIBS="${LIBS} -lpthread"
{ $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. */
#define _GNU_SOURCE
#include <pthread.h>
+ #ifdef HAVE_PTHREAD_NP_H
+ #include <pthread_np.h>
+ #endif
int
main ()
{
pthread_t pt;
- pthread_setname_np(pt, "x");return 0;
+ pthread_setname_np(pt, "x");
;
return 0;
}
@@ -4134,27 +4135,27 @@ 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(a, b) pthread_setname_np(a, b)"
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
/* end confdefs.h. */
#define _GNU_SOURCE
#include <pthread.h>
+ #ifdef HAVE_PTHREAD_NP_H
+ #include <pthread_np.h>
+ #endif
int
main ()
{
pthread_t pt;
- pthread_setname_np(pt, "X", (void *)0);return 0;
+ pthread_setname_np(pt, "X", (void *)0);
;
return 0;
}
@@ -4164,18 +4165,52 @@ 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 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 "$as_me:${as_lineno-$LINENO}: checking for pthread_set_name_np()"
&5
+$as_echo_n "checking for pthread_set_name_np()... " >&6; }
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#define _GNU_SOURCE
+ #include <pthread.h>
+ #ifdef HAVE_PTHREAD_NP_H
+ #include <pthread_np.h>
+ #endif
+int
+main ()
+{
+pthread_t pt;
+ pthread_set_name_np(pt, "x");
+ ;
+ return 0;
+}
-$as_echo "#define HAVE_PTHREAD_SETNAME3 1" >>confdefs.h
+_ACEOF
+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 pthread_setname_npx(a, b)
pthread_set_name_np(a, b)" >>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
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-LIBS="${SAVELIBS}"
{ $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 +4223,6 @@ int
main ()
{
- return 0;
;
return 0;
}
diff --git a/lib/librumpuser/configure.ac b/lib/librumpuser/configure.ac
index 14efa054..d076212d 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
@@ -51,52 +51,71 @@ dnl
dnl pthread_setname() sillyness is a bit longer; we need the signature
dnl
SAVE_CFLAGS="${CFLAGS}"
-CFLAGS="${SAVE_CFLAGS} -Werror"
+CFLAGS="${SAVE_CFLAGS} -Werror -Wimplicit"
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_LIBS="${LIBS}"
-LIBS="${LIBS} -lpthread"
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;]])
+ #include <pthread.h>
+ #ifdef HAVE_PTHREAD_NP_H
+ #include <pthread_np.h>
+ #endif]],
+ [[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(a, b)],
+ [pthread_setname_np(a, b)],
+ [Replacement for 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;]])
+ #include <pthread.h>
+ #ifdef HAVE_PTHREAD_NP_H
+ #include <pthread_np.h>
+ #endif]],
+ [[pthread_t pt;
+ pthread_setname_np(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_MSG_CHECKING([for pthread_set_name_np()])
+AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#define _GNU_SOURCE
+ #include <pthread.h>
+ #ifdef HAVE_PTHREAD_NP_H
+ #include <pthread_np.h>
+ #endif]],
+ [[pthread_t pt;
+ pthread_set_name_np(pt, "x");]])
+ ],[
+ AC_MSG_RESULT([yes])
+ AC_DEFINE([pthread_setname_npx(a, b)],
+ [pthread_set_name_np(a, b)])
+ ],[
+ AC_MSG_RESULT([no])
+ AC_DEFINE([pthread_setname_npx(a, b)],
+ [])
+])])])
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..cfb0b839 100644
--- a/lib/librumpuser/rumpuser_config.h.in
+++ b/lib/librumpuser/rumpuser_config.h.in
@@ -57,11 +57,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 +151,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_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
Other related posts:
