Re: [PATCH] jobserver: detect properly O_NONBLOCK

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Aug 18, 2022 at 2:43 PM Martin Liška <mliska@suse.cz> wrote:
>
> That handles systems that don't have O_NONBLOCK, in that case
> WPA streaming is not using jobserver if --jobserver-auth uses 'fifo'.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> Tested with mingw cross compiler as well.
>
> Ready to be installed?

libstdc++ uses

# For Networking TS.
AC_CHECK_HEADERS([fcntl.h sys/ioctl.h sys/socket.h sys/uio.h poll.h
netdb.h arpa/inet.h netinet/in.h netinet/tcp.h])
AC_CHECK_DECL(F_GETFL,,,[#include <fcntl.h>])
AC_CHECK_DECL(F_SETFL,,,[#include <fcntl.h>])
if test "$ac_cv_have_decl_F_GETFL$ac_cv_have_decl_F_SETFL" = yesyes ; then
  AC_CHECK_DECL(O_NONBLOCK,,,[#include <fcntl.h>])
fi

I'd rather not invent sth fancy with /dev/null (that seems very unixy)

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
>         * configure.ac: Detect O_NONBLOCK flag for open.
>         * config.in: Regenerate.
>         * configure: Regenerate.
>         * opts-common.cc (jobserver_info::connect): Set is_connected
>           properly based on O_NONBLOCK.
>         * opts-jobserver.h (struct jobserver_info): Add is_connected
>           member variable.
>
> gcc/lto/ChangeLog:
>
>         * lto.cc (wait_for_child): Ask if we are connected to jobserver.
>         (stream_out_partitions): Likewise.
> ---
>  gcc/config.in        |  6 ++++++
>  gcc/configure        | 39 +++++++++++++++++++++++++++++++++++++--
>  gcc/configure.ac     | 11 +++++++++++
>  gcc/lto/lto.cc       | 12 ++++++------
>  gcc/opts-common.cc   | 11 ++++++++++-
>  gcc/opts-jobserver.h |  2 ++
>  6 files changed, 72 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/config.in b/gcc/config.in
> index 413b2bd36cb..abab9bf5024 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -2148,6 +2148,12 @@
>  #endif
>
>
> +/* Define if O_NONBLOCK supported by fcntl. */
> +#ifndef USED_FOR_TARGET
> +#undef HOST_HAS_O_NONBLOCK
> +#endif
> +
> +
>  /* Define which stat syscall is able to handle 64bit indodes. */
>  #ifndef USED_FOR_TARGET
>  #undef HOST_STAT_FOR_64BIT_INODES
> diff --git a/gcc/configure b/gcc/configure
> index da7a45066b5..8b416c1a142 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -12460,6 +12460,41 @@ $as_echo "#define HOST_HAS_O_CLOEXEC 1" >>confdefs.h
>
>  fi
>
> +# Check if O_NONBLOCK is defined by fcntl
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for O_NONBLOCK" >&5
> +$as_echo_n "checking for O_NONBLOCK... " >&6; }
> +if ${ac_cv_o_nonblock+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +
> +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +
> +#include <fcntl.h>
> +int
> +main ()
> +{
> +
> +return open ("/dev/null", O_RDONLY | O_NONBLOCK);
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_cxx_try_compile "$LINENO"; then :
> +  ac_cv_o_nonblock=yes
> +else
> +  ac_cv_o_nonblock=no
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_o_nonblock" >&5
> +$as_echo "$ac_cv_o_nonblock" >&6; }
> +if test $ac_cv_o_nonblock = yes; then
> +
> +$as_echo "#define HOST_HAS_O_NONBLOCK 1" >>confdefs.h
> +
> +fi
> +
>  # C++ Modules would like some networking features to provide the mapping
>  # server.  You can still use modules without them though.
>  # The following network-related checks could probably do with some
> @@ -19678,7 +19713,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 19681 "configure"
> +#line 19716 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -19784,7 +19819,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 19787 "configure"
> +#line 19822 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index f70b6c24fda..4ebdad38b9b 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -1707,6 +1707,17 @@ if test $ac_cv_o_cloexec = yes; then
>    [Define if O_CLOEXEC supported by fcntl.])
>  fi
>
> +# Check if O_NONBLOCK is defined by fcntl
> +AC_CACHE_CHECK(for O_NONBLOCK, ac_cv_o_nonblock, [
> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> +#include <fcntl.h>]], [[
> +return open ("/dev/null", O_RDONLY | O_NONBLOCK);]])],
> +[ac_cv_o_nonblock=yes],[ac_cv_o_nonblock=no])])
> +if test $ac_cv_o_nonblock = yes; then
> +  AC_DEFINE(HOST_HAS_O_NONBLOCK, 1,
> +  [Define if O_NONBLOCK supported by fcntl.])
> +fi
> +
>  # C++ Modules would like some networking features to provide the mapping
>  # server.  You can still use modules without them though.
>  # The following network-related checks could probably do with some
> diff --git a/gcc/lto/lto.cc b/gcc/lto/lto.cc
> index c82307f4f7e..3a9147b01b5 100644
> --- a/gcc/lto/lto.cc
> +++ b/gcc/lto/lto.cc
> @@ -213,11 +213,11 @@ wait_for_child ()
>      }
>    while (!WIFEXITED (status) && !WIFSIGNALED (status));
>
> -    --nruns;
> +  --nruns;
>
> -    /* Return token to the jobserver if active.  */
> -    if (jinfo != NULL && jinfo->is_active)
> -      jinfo->return_token ();
> +  /* Return token to the jobserver if active.  */
> +  if (jinfo != NULL && jinfo->is_connected)
> +    jinfo->return_token ();
>  }
>  #endif
>
> @@ -254,7 +254,7 @@ stream_out_partitions (char *temp_filename, int blen, int min, int max,
>       streaming process.  */
>    if (!last)
>      {
> -      if (jinfo != NULL && jinfo->is_active)
> +      if (jinfo != NULL && jinfo->is_connected)
>         while (true)
>           {
>             if (jinfo->get_token ())
> @@ -291,7 +291,7 @@ stream_out_partitions (char *temp_filename, int blen, int min, int max,
>        while (nruns > 0)
>         wait_for_child ();
>
> -      if (jinfo != NULL && jinfo->is_active)
> +      if (jinfo != NULL && jinfo->is_connected)
>         jinfo->disconnect ();
>      }
>    asm_nodes_output = true;
> diff --git a/gcc/opts-common.cc b/gcc/opts-common.cc
> index 4dec9f94447..bff2be02d38 100644
> --- a/gcc/opts-common.cc
> +++ b/gcc/opts-common.cc
> @@ -2064,7 +2064,16 @@ void
>  jobserver_info::connect ()
>  {
>    if (!pipe_path.empty ())
> -    pipefd = open (pipe_path.c_str (), O_RDWR | O_NONBLOCK);
> +    {
> +#if HOST_HAS_O_NONBLOCK
> +      pipefd = open (pipe_path.c_str (), O_RDWR | O_NONBLOCK);
> +      is_connected = true;
> +#else
> +      is_connected = false;
> +#endif
> +    }
> +  else
> +    is_connected = true;
>  }
>
>  void
> diff --git a/gcc/opts-jobserver.h b/gcc/opts-jobserver.h
> index 76c1d9b2882..284b2417837 100644
> --- a/gcc/opts-jobserver.h
> +++ b/gcc/opts-jobserver.h
> @@ -55,6 +55,8 @@ struct jobserver_info
>    int pipefd = -1;
>    /* Return true if jobserver is active.  */
>    bool is_active = false;
> +  /* Return true if communication with jobserver is working.  */
> +  bool is_connected = false;
>  };
>
>  #endif /* GCC_JOBSERVER_H */
> --
> 2.37.1
>