public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix --disable-threading build
@ 2022-05-09 18:08 Tom Tromey
  2022-05-10 13:54 ` Pedro Alves
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Tromey @ 2022-05-09 18:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

PR build/29110 points out that GDB fails to build on mingw when the
"win32" thread model is in use.  It turns out that the Fedora cross
tools using the "posix" thread model, which somehow manages to support
std::future, whereas the win32 model does not.

While looking into this, I found that the configuring with
--disable-threading will also cause a build failure.

This patch fixes this build by introducing a compatibility wrapper for
std::future.

I am not able to test the win32 thread model build, but I'm going to
ask the reporter to try this patch.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29110
---
 gdb/dwarf2/cooked-index.h |  2 +-
 gdbsupport/parallel-for.h |  4 +--
 gdbsupport/thread-pool.cc |  6 ++--
 gdbsupport/thread-pool.h  | 75 ++++++++++++++++++++++++++++++++++++---
 4 files changed, 75 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index a460351612f..1d229586948 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -330,7 +330,7 @@ class cooked_index_vector : public dwarf_scanner_base
   /* A future that tracks when the 'finalize' method is done.  Note
      that the 'get' method is never called on this future, only
      'wait'.  */
-  std::future<void> m_future;
+  gdb::future<void> m_future;
 };
 
 #endif /* GDB_DWARF2_COOKED_INDEX_H */
diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h
index 713ec660306..7b6891a0dcb 100644
--- a/gdbsupport/parallel-for.h
+++ b/gdbsupport/parallel-for.h
@@ -72,7 +72,7 @@ struct par_for_accumulator
   
   /* A vector of futures coming from the tasks run in the
      background.  */
-  std::vector<std::future<T>> m_futures;
+  std::vector<gdb::future<T>> m_futures;
 };
 
 /* See the generic template.  */
@@ -108,7 +108,7 @@ struct par_for_accumulator<void>
 
 private:
 
-  std::vector<std::future<void>> m_futures;
+  std::vector<gdb::future<void>> m_futures;
 };
 
 }
diff --git a/gdbsupport/thread-pool.cc b/gdbsupport/thread-pool.cc
index 3674e910f1d..ddb76b691c1 100644
--- a/gdbsupport/thread-pool.cc
+++ b/gdbsupport/thread-pool.cc
@@ -192,12 +192,13 @@ thread_pool::set_thread_count (size_t num_threads)
 #endif /* CXX_STD_THREAD */
 }
 
+#if CXX_STD_THREAD
+
 void
 thread_pool::do_post_task (std::packaged_task<void ()> &&func)
 {
   std::packaged_task<void ()> t (std::move (func));
 
-#if CXX_STD_THREAD
   if (m_thread_count != 0)
     {
       std::lock_guard<std::mutex> guard (m_tasks_mutex);
@@ -205,15 +206,12 @@ thread_pool::do_post_task (std::packaged_task<void ()> &&func)
       m_tasks_cv.notify_one ();
     }
   else
-#endif
     {
       /* Just execute it now.  */
       t ();
     }
 }
 
-#if CXX_STD_THREAD
-
 void
 thread_pool::thread_function ()
 {
diff --git a/gdbsupport/thread-pool.h b/gdbsupport/thread-pool.h
index 5e203fd896c..fc35d61faf0 100644
--- a/gdbsupport/thread-pool.h
+++ b/gdbsupport/thread-pool.h
@@ -27,13 +27,69 @@
 #include <thread>
 #include <mutex>
 #include <condition_variable>
-#endif
 #include <future>
+#endif
 #include "gdbsupport/gdb_optional.h"
 
 namespace gdb
 {
 
+#if CXX_STD_THREAD
+
+/* Simply use the standard future.  */
+template<typename T>
+using future = std::future<T>;
+
+#else /* CXX_STD_THREAD */
+
+/* A compatibility wrapper for std::future.  Once <thread> and
+   <future> are available in all GCC builds -- should that ever happen
+   -- this can be removed.
+
+   Meanwhile, in this mode, there are no threads.  Tasks submitted to
+   the thread pool are invoked immediately and their result is stored
+   here.  The base template here simply wraps a T and provides some
+   std::future compatibility methods.  The provided methods are chosen
+   based on what GDB needs presently.  */
+
+template<typename T>
+class future
+{
+public:
+
+  explicit future (T value)
+    : m_value (std::move (value))
+  {
+  }
+
+  future () = default;
+  future (future &&other) = default;
+  future (const future &other) = delete;
+  future &operator= (future &&other) = default;
+  future &operator= (const future &other) = delete;
+
+  void wait () const { }
+
+  T get () { return std::move (m_value); }
+
+private:
+
+  T m_value;
+};
+
+/* A specialization for void.  */
+
+template<>
+class future<void>
+{
+public:
+  void wait () const { }
+  void get () { }
+};
+
+#endif /* CXX_STD_THREAD */
+
+
 /* A thread pool.
 
    There is a single global thread pool, see g_thread_pool.  Tasks can
@@ -64,23 +120,32 @@ class thread_pool
 
   /* Post a task to the thread pool.  A future is returned, which can
      be used to wait for the result.  */
-  std::future<void> post_task (std::function<void ()> &&func)
+  future<void> post_task (std::function<void ()> &&func)
   {
+#if CXX_STD_THREAD
     std::packaged_task<void ()> task (std::move (func));
-    std::future<void> result = task.get_future ();
+    future<void> result = task.get_future ();
     do_post_task (std::packaged_task<void ()> (std::move (task)));
     return result;
+#else
+    func ();
+    return {};
+#endif /* CXX_STD_THREAD */
   }
 
   /* Post a task to the thread pool.  A future is returned, which can
      be used to wait for the result.  */
   template<typename T>
-  std::future<T> post_task (std::function<T ()> &&func)
+  future<T> post_task (std::function<T ()> &&func)
   {
+#if CXX_STD_THREAD
     std::packaged_task<T ()> task (std::move (func));
-    std::future<T> result = task.get_future ();
+    future<T> result = task.get_future ();
     do_post_task (std::packaged_task<void ()> (std::move (task)));
     return result;
+#else
+    return future<T> (func ());
+#endif /* CXX_STD_THREAD */
   }
 
 private:
-- 
2.34.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix --disable-threading build
  2022-05-09 18:08 [PATCH] Fix --disable-threading build Tom Tromey
@ 2022-05-10 13:54 ` Pedro Alves
  0 siblings, 0 replies; 2+ messages in thread
From: Pedro Alves @ 2022-05-10 13:54 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 2022-05-09 19:08, Tom Tromey via Gdb-patches wrote:
> PR build/29110 points out that GDB fails to build on mingw when the
> "win32" thread model is in use.  It turns out that the Fedora cross
> tools using the "posix" thread model, which somehow manages to support
> std::future, whereas the win32 model does not.

I guess it depends on GCC version.  Ubuntu 20.04 ships both win32 and posix
models as alternatives:

"
$ sudo update-alternatives --config x86_64-w64-mingw32-gcc
There are 2 choices for the alternative x86_64-w64-mingw32-gcc (providing /usr/bin/x86_64-w64-mingw32-gcc).

  Selection    Path                                   Priority   Status
------------------------------------------------------------
  0            /usr/bin/x86_64-w64-mingw32-gcc-win32   60        auto mode
* 1            /usr/bin/x86_64-w64-mingw32-gcc-posix   30        manual mode
  2            /usr/bin/x86_64-w64-mingw32-gcc-win32   60        manual mode

Press <enter> to keep the current choice[*], or type selection number: 
"

and there the posix model doesn't have std::future either.  The version is:

 gcc version 9.3-posix 20200320 (GCC) 

Above I have posix selected, but the default is win32.

> 
> While looking into this, I found that the configuring with
> --disable-threading will also cause a build failure.
> 
> This patch fixes this build by introducing a compatibility wrapper for
> std::future.
> 
> I am not able to test the win32 thread model build, but I'm going to
> ask the reporter to try this patch.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29110

I tested this on Ubuntu 20.04, two builds, one with posix model, and another
with win32 model.  Both worked (as in, gdb built).

>  thread_pool::thread_function ()
>  {
> diff --git a/gdbsupport/thread-pool.h b/gdbsupport/thread-pool.h
> index 5e203fd896c..fc35d61faf0 100644
> --- a/gdbsupport/thread-pool.h
> +++ b/gdbsupport/thread-pool.h
> @@ -27,13 +27,69 @@
>  #include <thread>
>  #include <mutex>
>  #include <condition_variable>
> -#endif
>  #include <future>
> +#endif
>  #include "gdbsupport/gdb_optional.h"
>  
>  namespace gdb
>  {
>  
> +#if CXX_STD_THREAD
> +
> +/* Simply use the standard future.  */
> +template<typename T>
> +using future = std::future<T>;
> +
> +#else /* CXX_STD_THREAD */
> +
> +/* A compatibility wrapper for std::future.  Once <thread> and
> +   <future> are available in all GCC builds -- should that ever happen
> +   -- this can be removed.

I think this should mention mingw.

Otherwise LGTM.

Thanks for fixing this.

Pedro Alves

> +
> +   Meanwhile, in this mode, there are no threads.  Tasks submitted to
> +   the thread pool are invoked immediately and their result is stored
> +   here.  The base template here simply wraps a T and provides some
> +   std::future compatibility methods.  The provided methods are chosen
> +   based on what GDB needs presently.  */
> +
> +template<typename T>
> +class future

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-10 13:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 18:08 [PATCH] Fix --disable-threading build Tom Tromey
2022-05-10 13:54 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).