On 3/15/23 18:25, Eric Blake wrote:
> On Wed, Mar 15, 2023 at 12:01:57PM +0100, Laszlo Ersek wrote:
>> Don't try to test async-signal-safety, only that
>> NBD_INTERNAL_FORK_SAFE_ASSERT() works similarly to assert():
>>
>> - it prints diagnostics to stderr,
>>
>> - it calls abort().
>>
>> Some unfortunate gymnastics are necessary to avoid littering the system
>> with unwanted core dumps.
>>
>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>> ---
>
>> diff --git a/configure.ac b/configure.ac
>> index b6d60c3df6a1..62fe470b6cd5 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -132,11 +132,16 @@ dnl Check for various libc functions, all optional.
>> dnl
>> dnl posix_fadvise helps to optimise linear reads and writes.
>> dnl
>> +dnl When /proc/sys/kernel/core_pattern starts with a pipe (|) symbol, Linux
>> +dnl ignores "ulimit -c" and (equivalent) setrlimit(RLIMIT_CORE) actions, for
>> +dnl disabling core dumping. Only prctl() can be used then, for that purpose.
>> +dnl
>> dnl strerrordesc_np (glibc only) is preferred over sys_errlist:
>> dnl
>> https://lists.fedoraproject.org/archives/list/gl...@lists.fedoraproject.org/thread/WJHGG2OO7ABNAYICGA5WQZ2Q34Q2FEHU/
>> AC_CHECK_FUNCS([\
>> posix_fadvise \
>> posix_memalign \
>> + prctl \
>> strerrordesc_np \
>> valloc])
>
> AC_CHECK_FUNCS looks for whether the given entry point can be linked
> with, which is okay for functions in the common headers (<stdlib.h>,
> <unistd.h>, ...) that autoconf includes in its test programs by
> default. But...
>
>>
>> diff --git a/lib/test-fork-safe-assert.c b/lib/test-fork-safe-assert.c
>> new file mode 100644
>> index 000000000000..4a4f6e88ce65
>> --- /dev/null
>> +++ b/lib/test-fork-safe-assert.c
>> @@ -0,0 +1,66 @@
>
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#ifdef HAVE_PRCTL
>> +#include <sys/prctl.h>
>> +#endif
>
> ...the fact that prctl() is in a non-standard header makes me wonder
> if we might fail to detect the function merely because we didn't
> include the right header, rather than because its symbol was not
> exported.
>
> On the other hand, prctl() is definitely Linux-specific, so I think
> you are quite safe in assuming that <sys/prctl.h> exists if and only
> if prctl() is a linkable entry point. If it does turn out to break
> someone, we can fix it in a followup patch, so no change needed in
> your usage at this time.
*shudder*
Another hideous piece of complexity that's orthogonal to what one wants
to do :)
So, I investigated a bit.
If I understand correctly, your point is that we could get a *false
negative* here, because AC_CHECK_FUNCS might not find prctl() due to the
autoconf-generated test program not #including <sys/prctl.h>.
Assuming I got that right, I have two comments on it:
(1) A false negative in this case would not be a huge problem; we'd miss
out on prctl(), i.e. the test program would remain dumpable on Linux.
The test would still function as needed, just litter the user's machine
with a coredump during "make check". Not ideal, but also not tragic.
(2) I believe I disagree with the idea that AC_CHECK_FUNCS might not
find an otherwise existent prctl() *due to* AC_CHECK_FUNCS not
generating "#include <sys/prctl.h>" into the test program.
On my RHEL-9.1 install (using autoconf-2.69-38.el9.noarch), I've checked
the generated "configure" script. First, we have
18509 for ac_func in \
18510 posix_fadvise \
18511 posix_memalign \
18512 prctl \
18513 strerrordesc_np \
18514 valloc
18515 do :
18516 as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
18517 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
18518 if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
18519 cat >>confdefs.h <<_ACEOF
18520 #define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
18521 _ACEOF
18522
18523 fi
18524 done
I.e., we call "ac_fn_c_check_func" with "prctl" passed as second
argument.
Then, that function is defined as follows:
2001 # ac_fn_c_check_func LINENO FUNC VAR
2002 # ----------------------------------
2003 # Tests whether FUNC exists, setting the cache variable VAR accordingly
2004 ac_fn_c_check_func ()
2005 {
2006 as_lineno=${as_lineno-"$1"}
as_lineno_stack=as_lineno_stack=$as_lineno_stack
2007 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
2008 $as_echo_n "checking for $2... " >&6; }
2009 if eval \${$3+:} false; then :
2010 $as_echo_n "(cached) " >&6
2011 else
2012 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
2013 /* end confdefs.h. */
2014 /* Define $2 to an innocuous variant, in case <limits.h> declares $2.
2015 For example, HP-UX 11i <limits.h> declares gettimeofday. */
2016 #define $2 innocuous_$2
2017
2018 /* System header to define __stub macros and hopefully few prototypes,
2019 which can conflict with char $2 (); below.
2020 Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
2021 <limits.h> exists even on freestanding compilers. */
2022
2023 #ifdef __STDC__
2024 # include <limits.h>
2025 #else
2026 # include <assert.h>
2027 #endif
2028
2029 #undef $2
2030
2031 /* Override any GCC internal prototype to avoid an error.
2032 Use char because int might match the return type of a GCC
2033 builtin and then its argument prototype would still apply. */
2034 #ifdef __cplusplus
2035 extern "C"
2036 #endif
2037 char $2 ();
2038 /* The GNU C library defines this for functions which it implements
2039 to always fail with ENOSYS. Some functions are actually named
2040 something starting with __ and the normal name is an alias. */
2041 #if defined __stub_$2 || defined __stub___$2
2042 choke me
2043 #endif
2044
2045 int
2046 main ()
2047 {
2048 return $2 ();
2049 ;
2050 return 0;
2051 }
2052 _ACEOF
2053 if ac_fn_c_try_link "$LINENO"; then :
2054 eval "$3=yes"
2055 else
2056 eval "$3=no"
2057 fi
2058 rm -f core conftest.err conftest.$ac_objext \
2059 conftest$ac_exeext conftest.$ac_ext
2060 fi
2061 eval ac_res=\$$3
2062 { $as_echo "$as_me:${as_lineno-$LINENO}: result:
$ac_res" >&5
2063 $as_echo "$ac_res" >&6; }
2064 eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
2065
2066 } # ac_fn_c_check_func
As far as I can tell, the test program provides its own declaration for
prctl() on line 2037, so it does not depend on any system header
providing a real prototype.
... I figured autoconf should have a "header check" too, and indeed it
does: AC_CHECK_HEADER, AC_CHECK_HEADERS, at
<https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Generic-Headers.html>.
We do use AC_CHECK_HEADERS in libnbd.
The question is now whether checking for <sys/prctl.h> with
AC_CHECK_HEADERS is "more robust" than checking for prctl() with
AC_CHECK_FUNCS(). I'd say: AC_CHECK_FUNCS() is a tiny bit stronger (we
actually want to be able to call the particular prctl() function), but
they should be mostly *interchangeable*. I'm saying that because prctl()
is "Linux-specific" (per prctl(2) manual), so:
(a) <sys/prctl.h> existing (per AC_CHECK_HEADERS), but not exposing the
real -- and linkable -- prctl() prototype,
(b) a call to prctl() being linkable (via the bogus declaration used by
AC_CHECK_HEADERS), but <sys/prctl.h> not exposing a proper prctl()
prototype,
are *equally* Linux userspace bugs.
So indeed I'll stick with the AC_CHECK_FUNCS approach.
>
>> +++ b/lib/test-fork-safe-assert.sh
>> @@ -0,0 +1,32 @@
>> +#!/usr/bin/env bash
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
Thank you!
Laszlo
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
