diff mbox series

Stop 'configure --enable-threading' if std::thread doesn't work

Message ID 20240514184341.1399428-1-pedro@palves.net
State New
Headers
Series Stop 'configure --enable-threading' if std::thread doesn't work |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Pedro Alves May 14, 2024, 6:43 p.m. UTC 
  Currently, if you configure gdb with explicit --enable-threading, but
then configure detects std::thread does not work, configure silently
disables threading support and continues configuring.

This patch makes that scenario cause a configuration error, like so:

 $ /home/pedro/gdb/src/configure --enable-threading && make
 ...
 configure: error: std::thread does not work; disable threading
 make[1]: *** [Makefile:11225: configure-gdbsupport] Error 1
 make[1]: Leaving directory '/home/pedro/gdb/build-windows-threads'
 make: *** [Makefile:1041: all] Error 2
 $

Additionally, if you don't explicitly pass --enable-threading, and
std::thread does not work, we will now get a warning (and the build
continues):

 $ /home/pedro/gdb/src/configure && make
 ...
 configure: WARNING: std::thread does not work; disabling threading
 ...

This is similar to how we handle --enable-tui and missing curses.  The
code and error/warning messages were borrowed from there.

Change-Id: I73a8b580d1e2a796b23136920c0e181408ae1b22
---
 gdb/configure        | 14 +++++++++++---
 gdbserver/configure  | 14 +++++++++++---
 gdbsupport/common.m4 | 13 ++++++++++---
 gdbsupport/configure | 14 +++++++++++---
 4 files changed, 43 insertions(+), 12 deletions(-)


base-commit: 414aa6987f21a814851e5f3113388a3616993fa3

Comments

Bernd Edlinger May 15, 2024, 11:27 a.m. UTC | #1 
Hi Pedro,

On 5/14/24 20:43, Pedro Alves wrote:
> Currently, if you configure gdb with explicit --enable-threading, but
> then configure detects std::thread does not work, configure silently
> disables threading support and continues configuring.
> 
> This patch makes that scenario cause a configuration error, like so:
> 
>  $ /home/pedro/gdb/src/configure --enable-threading && make
>  ...
>  configure: error: std::thread does not work; disable threading
>  make[1]: *** [Makefile:11225: configure-gdbsupport] Error 1
>  make[1]: Leaving directory '/home/pedro/gdb/build-windows-threads'
>  make: *** [Makefile:1041: all] Error 2
>  $
> 
> Additionally, if you don't explicitly pass --enable-threading, and
> std::thread does not work, we will now get a warning (and the build
> continues):
> 
>  $ /home/pedro/gdb/src/configure && make
>  ...
>  configure: WARNING: std::thread does not work; disabling threading
>  ...
> 
> This is similar to how we handle --enable-tui and missing curses.  The
> code and error/warning messages were borrowed from there.
> 

Thanks for taking up the ball.

I wonder if we should declare --enable-threading an experimental feature
for the time being, given how many open issues it created so far,
and thus simply make it disabled by default?

That way it would no longer delay the 15.1 release.
What do you think?


Thanks
Bernd.
Tom Tromey May 15, 2024, 3:53 p.m. UTC | #2 
>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

Bernd> I wonder if we should declare --enable-threading an experimental feature
Bernd> for the time being, given how many open issues it created so far,
Bernd> and thus simply make it disabled by default?

No, we should not do that.

First, threading has been enabled for several releases now.  The build
issue here is that GCC also changed, but GCC and GDB aren't coordinated
on which Windows versions are supported.  So, that's caused some
friction.

The races on the other hand are caused by the background reading
series -- not threading in general.  Entirely disabling threading would
be a big step backward.

I think fixing the background reading problem is as simple as changing
'dwarf_synchronous' to default to 'true'.  Though I still am holding out
hope to fix the problems for real.

Tom
Tom Tromey May 15, 2024, 3:54 p.m. UTC | #3 
>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Currently, if you configure gdb with explicit --enable-threading, but
Pedro> then configure detects std::thread does not work, configure silently
Pedro> disables threading support and continues configuring.

Pedro> This patch makes that scenario cause a configuration error, like so:

Thanks for doing this.  This is ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
Pedro Alves May 16, 2024, 12:08 p.m. UTC | #4 
On 2024-05-15 16:54, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> Currently, if you configure gdb with explicit --enable-threading, but
> Pedro> then configure detects std::thread does not work, configure silently
> Pedro> disables threading support and continues configuring.
> 
> Pedro> This patch makes that scenario cause a configuration error, like so:
> 
> Thanks for doing this.  This is ok.
> Approved-By: Tom Tromey <tom@tromey.com>

Thank you.  I merged it, with the comment tweaked to say GCC 13 instead GCC 14.

Also, courtesy of Eric Botcazou, since today, the GCC 13 release news page documents
the win32 model improvements:

  https://gcc.gnu.org/gcc-13/changes.html
diff mbox series

Patch

diff --git a/gdb/configure b/gdb/configure
index 98cd488a737..15c9cfe0937 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -20181,12 +20181,13 @@  if test "${enable_threading+set}" = set; then :
     *) as_fn_error $? "bad value $enableval for threading" "$LINENO" 5 ;;
     esac
 else
-  want_threading=yes
+  want_threading=auto
 fi
 
 
   # Check for std::thread.  This does not work on some platforms, like
-  # mingw and DJGPP.
+  # mingw using the win32 threads model with gcc older than 14, and
+  # DJGPP.
   ac_ext=cpp
 ac_cpp='$CXXCPP $CPPFLAGS'
 ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
@@ -20893,11 +20894,18 @@  done
   LIBS="$save_LIBS"
   CXXFLAGS="$save_CXXFLAGS"
 
-  if test "$want_threading" = "yes"; then
+  if test "$want_threading" != "no"; then
     if test "$gdb_cv_cxx_std_thread" = "yes"; then
 
 $as_echo "#define CXX_STD_THREAD 1" >>confdefs.h
 
+    else
+	if test "$want_threading" = "yes"; then
+	    as_fn_error $? "std::thread does not work; disable threading" "$LINENO" 5
+	else
+	    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: std::thread does not work; disabling threading" >&5
+$as_echo "$as_me: WARNING: std::thread does not work; disabling threading" >&2;}
+	fi
     fi
   fi
   ac_ext=c
diff --git a/gdbserver/configure b/gdbserver/configure
index 2da525ebf3b..538ffd8acd2 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -8888,12 +8888,13 @@  if test "${enable_threading+set}" = set; then :
     *) as_fn_error $? "bad value $enableval for threading" "$LINENO" 5 ;;
     esac
 else
-  want_threading=yes
+  want_threading=auto
 fi
 
 
   # Check for std::thread.  This does not work on some platforms, like
-  # mingw and DJGPP.
+  # mingw using the win32 threads model with gcc older than 14, and
+  # DJGPP.
   ac_ext=cpp
 ac_cpp='$CXXCPP $CPPFLAGS'
 ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
@@ -9600,11 +9601,18 @@  done
   LIBS="$save_LIBS"
   CXXFLAGS="$save_CXXFLAGS"
 
-  if test "$want_threading" = "yes"; then
+  if test "$want_threading" != "no"; then
     if test "$gdb_cv_cxx_std_thread" = "yes"; then
 
 $as_echo "#define CXX_STD_THREAD 1" >>confdefs.h
 
+    else
+	if test "$want_threading" = "yes"; then
+	    as_fn_error $? "std::thread does not work; disable threading" "$LINENO" 5
+	else
+	    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: std::thread does not work; disabling threading" >&5
+$as_echo "$as_me: WARNING: std::thread does not work; disabling threading" >&2;}
+	fi
     fi
   fi
   ac_ext=c
diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
index bef396445ba..f4fc81d1e0a 100644
--- a/gdbsupport/common.m4
+++ b/gdbsupport/common.m4
@@ -89,10 +89,11 @@  AC_DEFUN([GDB_AC_COMMON], [
     no) want_threading=no ;;
     *) AC_MSG_ERROR([bad value $enableval for threading]) ;;
     esac],
-    [want_threading=yes])
+    [want_threading=auto])
 
   # Check for std::thread.  This does not work on some platforms, like
-  # mingw and DJGPP.
+  # mingw using the win32 threads model with gcc older than 14, and
+  # DJGPP.
   AC_LANG_PUSH([C++])
   AX_PTHREAD([threads=yes], [threads=no])
   save_LIBS="$LIBS"
@@ -128,10 +129,16 @@  AC_DEFUN([GDB_AC_COMMON], [
   LIBS="$save_LIBS"
   CXXFLAGS="$save_CXXFLAGS"
 
-  if test "$want_threading" = "yes"; then
+  if test "$want_threading" != "no"; then
     if test "$gdb_cv_cxx_std_thread" = "yes"; then
       AC_DEFINE(CXX_STD_THREAD, 1,
 		[Define to 1 if std::thread works.])
+    else
+	if test "$want_threading" = "yes"; then
+	    AC_MSG_ERROR([std::thread does not work; disable threading])
+	else
+	    AC_MSG_WARN([std::thread does not work; disabling threading])
+	fi
     fi
   fi
   AC_LANG_POP
diff --git a/gdbsupport/configure b/gdbsupport/configure
index a218b06ce28..52d14c18740 100755
--- a/gdbsupport/configure
+++ b/gdbsupport/configure
@@ -11662,12 +11662,13 @@  if test "${enable_threading+set}" = set; then :
     *) as_fn_error $? "bad value $enableval for threading" "$LINENO" 5 ;;
     esac
 else
-  want_threading=yes
+  want_threading=auto
 fi
 
 
   # Check for std::thread.  This does not work on some platforms, like
-  # mingw and DJGPP.
+  # mingw using the win32 threads model with gcc older than 14, and
+  # DJGPP.
   ac_ext=cpp
 ac_cpp='$CXXCPP $CPPFLAGS'
 ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
@@ -12374,11 +12375,18 @@  done
   LIBS="$save_LIBS"
   CXXFLAGS="$save_CXXFLAGS"
 
-  if test "$want_threading" = "yes"; then
+  if test "$want_threading" != "no"; then
     if test "$gdb_cv_cxx_std_thread" = "yes"; then
 
 $as_echo "#define CXX_STD_THREAD 1" >>confdefs.h
 
+    else
+	if test "$want_threading" = "yes"; then
+	    as_fn_error $? "std::thread does not work; disable threading" "$LINENO" 5
+	else
+	    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: std::thread does not work; disabling threading" >&5
+$as_echo "$as_me: WARNING: std::thread does not work; disabling threading" >&2;}
+	fi
     fi
   fi
   ac_ext=c