public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 5/8] Introduce run_on_main_thread
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 1/8] Defer minimal symbol name-setting Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 4/8] Lock the demangled hash table Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 2/8] Remove static buffer from ada_decode Tom Tromey
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces a way for a callback to be run on the main thread.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* ser-event.h (run_on_main_thread): Declare.
	* ser-event.c (runnable_event, runnables, runnable_mutex): New
	globals.
	(run_events, run_on_main_thread, _initialize_ser_event): New
	functions.
---
 gdb/ChangeLog   |  8 ++++++
 gdb/ser-event.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/ser-event.h |  6 +++++
 3 files changed, 83 insertions(+)

diff --git a/gdb/ser-event.c b/gdb/ser-event.c
index d3956346246..4870433b287 100644
--- a/gdb/ser-event.c
+++ b/gdb/ser-event.c
@@ -20,6 +20,10 @@
 #include "ser-event.h"
 #include "serial.h"
 #include "common/filestuff.h"
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
+#include "event-loop.h"
 
 /* On POSIX hosts, a serial_event is basically an abstraction for the
    classical self-pipe trick.
@@ -217,3 +221,68 @@ serial_event_clear (struct serial_event *event)
   ResetEvent (state->event);
 #endif
 }
+
+\f
+
+/* The serial event used when posting runnables.  */
+
+static struct serial_event *runnable_event;
+
+/* Runnables that have been posted.  */
+
+static std::vector<std::function<void ()>> runnables;
+
+#if CXX_STD_THREAD
+
+/* Mutex to hold when handling runnable_event or runnables.  */
+
+static std::mutex runnable_mutex;
+
+#endif
+
+/* Run all the queued runnables.  */
+
+static void
+run_events (int error, gdb_client_data client_data)
+{
+  std::vector<std::function<void ()>> local;
+
+  /* Hold the lock while changing the globals, but not while running
+     the runnables.  */
+  {
+#if CXX_STD_THREAD
+    std::lock_guard<std::mutex> lock (runnable_mutex);
+#endif
+
+    /* Clear the event fd.  Do this before flushing the events list,
+       so that any new event post afterwards is sure to re-awaken the
+       event loop.  */
+    serial_event_clear (runnable_event);
+
+    /* Move the vector in case running a runnable pushes a new
+       runnable.  */
+    std::swap (local, runnables);
+  }
+
+  for (auto &item : local)
+    item ();
+}
+
+/* See ser-event.h.  */
+
+void
+run_on_main_thread (std::function<void ()> &&func)
+{
+#if CXX_STD_THREAD
+  std::lock_guard<std::mutex> lock (runnable_mutex);
+#endif
+  runnables.emplace_back (std::move (func));
+  serial_event_set (runnable_event);
+}
+
+void
+_initialize_ser_event ()
+{
+  runnable_event = make_serial_event ();
+  add_file_handler (serial_event_fd (runnable_event), run_events, nullptr);
+}
diff --git a/gdb/ser-event.h b/gdb/ser-event.h
index 137348557f9..61a84f9cc79 100644
--- a/gdb/ser-event.h
+++ b/gdb/ser-event.h
@@ -19,6 +19,8 @@
 #ifndef SER_EVENT_H
 #define SER_EVENT_H
 
+#include <functional>
+
 /* This is used to be able to signal the event loop (or any other
    select/poll) of events, in a race-free manner.
 
@@ -48,4 +50,8 @@ extern void serial_event_set (struct serial_event *event);
    call is made.  */
 extern void serial_event_clear (struct serial_event *event);
 
+/* Send a runnable to the main thread.  */
+
+extern void run_on_main_thread (std::function<void ()> &&);
+
 #endif
-- 
2.17.2

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

* [PATCH v2 8/8] Add maint set/show enable-threads
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
                   ` (6 preceding siblings ...)
  2019-05-18 21:00 ` [PATCH v2 7/8] Demangle minsyms in parallel Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-22  5:01   ` Eli Zaretskii
  2019-05-19 13:59 ` [PATCH v2 0/8] Demangle minimal symbol names in worker threads Philippe Waroquiers
  8 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds maint commands to control whether gdb is able to use
threads.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* NEWS: Add entry.
	* maint.c (_initialize_maint_cmds): Add "enable-threads" maint
	commands.

gdb/doc/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Maintenance Commands): Document new maint
	commands.
---
 gdb/ChangeLog       |  6 ++++++
 gdb/NEWS            |  4 ++++
 gdb/doc/ChangeLog   |  5 +++++
 gdb/doc/gdb.texinfo | 11 +++++++++++
 gdb/maint.c         | 10 ++++++++++
 5 files changed, 36 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 1e92a2b52c2..dc278b5ee68 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -46,6 +46,10 @@ show print max-depth
   The default max-depth is 20, but this can be set to unlimited to get
   the old behavior back.
 
+maint set enable-threads
+maint show enable-threads
+  Control whether GDB can use threads.  The default is "on".
+
 * New MI commands
 
 -complete
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 37e2f14ad0f..7e34cd3179c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36925,6 +36925,17 @@ with the DWARF frame unwinders enabled.
 
 If DWARF frame unwinders are not supported for a particular target
 architecture, then enabling this flag does not cause them to be used.
+
+@kindex maint set enable-threads
+@kindex maint show enable-threads
+@item maint set enable-threads
+@item maint show enable-threads
+Control whether @value{GDBN} uses threads.  On capable hosts,
+@value{GDBN} will use multiple threads to speed up certain operations.
+However, when debugging @value{GDBN} itself, it is sometimes
+convenient to disable this feature.  That can be done using this
+command.  The default is @samp{on}.
+
 @kindex maint set profile
 @kindex maint show profile
 @cindex profiling GDB
diff --git a/gdb/maint.c b/gdb/maint.c
index 328d6026a34..0368eff338b 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -39,6 +39,7 @@
 #include "top.h"
 #include "maint.h"
 #include "common/selftest.h"
+#include "common/parallel-for.h"
 
 #include "cli/cli-decode.h"
 #include "cli/cli-utils.h"
@@ -1143,4 +1144,13 @@ When enabled GDB is profiled."),
 			   show_maintenance_profile_p,
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
+
+  add_setshow_boolean_cmd ("enable-threads", class_maintenance,
+			   &gdb::enable_threads, _("\
+Set whether gdb can use multiple threads."), _("\
+Show whether gdb can use multiple threads."), _("\
+If enabled, gdb will use multiple threads when possible."),
+			   NULL, NULL,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
 }
-- 
2.17.2

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

* [PATCH v2 0/8] Demangle minimal symbol names in worker threads
@ 2019-05-18 21:00 Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 1/8] Defer minimal symbol name-setting Tom Tromey
                   ` (8 more replies)
  0 siblings, 9 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches

This is v2 of my patch series to demangle minimal symbol names in
worker threads.

v1 was here:

    https://sourceware.org/ml/gdb-patches/2019-03/msg00211.html

I think this version addresses all the comments, in particular it adds
a simple way to disable threading, and adds some configury.

Regression tested by the buildbot.  At one point this was failing most
tests on the i686 builder, but I couldn't reproduce this with my own
i686 build (on the GCC compile farm), and various checks involving
running parts of the test suite on the buildbot using valgrind did not
show anything.  In the end, the problem seems to have gone away
(though there are many gdb.arch failures on i686 -- assertion failures
in libopcodes).  So, I'm not totally sure what to make of this.

Tom


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

* [PATCH v2 7/8] Demangle minsyms in parallel
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
                   ` (5 preceding siblings ...)
  2019-05-18 21:00 ` [PATCH v2 6/8] Introduce thread-safe way to handle SIGSEGV Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 8/8] Add maint set/show enable-threads Tom Tromey
  2019-05-19 13:59 ` [PATCH v2 0/8] Demangle minimal symbol names in worker threads Philippe Waroquiers
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* common/parallel-for.h: New file.
	* common/parallel-for.c: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add common/parallel-for.h.
	(COMMON_SFILES): Add common/parallel-for.c.
---
 gdb/ChangeLog             |  9 +++++
 gdb/Makefile.in           |  2 +
 gdb/common/parallel-for.c | 27 +++++++++++++
 gdb/common/parallel-for.h | 79 +++++++++++++++++++++++++++++++++++++++
 gdb/minsyms.c             | 23 +++++++-----
 5 files changed, 130 insertions(+), 10 deletions(-)
 create mode 100644 gdb/common/parallel-for.c
 create mode 100644 gdb/common/parallel-for.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 2dd69f3f0ba..15c7a6e2536 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -971,6 +971,7 @@ COMMON_SFILES = \
 	common/gdb_vecs.c \
 	common/netstuff.c \
 	common/new-op.c \
+	common/parallel-for.c \
 	common/pathstuff.c \
 	common/print-utils.c \
 	common/ptid.c \
@@ -1471,6 +1472,7 @@ HFILES_NO_SRCDIR = \
 	common/common-inferior.h \
 	common/netstuff.h \
 	common/host-defs.h \
+	common/parallel-for.h \
 	common/pathstuff.h \
 	common/print-utils.h \
 	common/ptid.h \
diff --git a/gdb/common/parallel-for.c b/gdb/common/parallel-for.c
new file mode 100644
index 00000000000..acec77c9fa4
--- /dev/null
+++ b/gdb/common/parallel-for.c
@@ -0,0 +1,27 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common/common-defs.h"
+#include "common/parallel-for.h"
+
+namespace gdb
+{
+/* See parallel-for.h.  */
+int enable_threads = 1;
+}
diff --git a/gdb/common/parallel-for.h b/gdb/common/parallel-for.h
new file mode 100644
index 00000000000..db90137127d
--- /dev/null
+++ b/gdb/common/parallel-for.h
@@ -0,0 +1,79 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef COMMON_PARALLEL_FOR_H
+#define COMMON_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <thread>
+#endif
+
+namespace gdb
+{
+
+/* True if threading should be enabled.  */
+
+extern int enable_threads;
+
+/* A very simple "parallel for".  This iterates over the elements
+   given by the range of iterators, which must be random access
+   iterators.  For each element, it calls the callback function.  The
+   work may or may not be done by separate threads.  */
+
+template<class RandomIt, class UnaryFunction>
+void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
+{
+#if CXX_STD_THREAD
+  unsigned n_threads = std::thread::hardware_concurrency ();
+  /* So we can use a local array below.  */
+  const unsigned max_threads = 16;
+  if (n_threads > max_threads)
+    n_threads = max_threads;
+
+  if (!enable_threads || n_threads == 0 || last - first < 2 * n_threads)
+#endif /* CXX_STD_THREAD */
+    {
+      /* Don't bother.  */
+      std::for_each (first, last, f);
+      return;
+    }
+
+#if CXX_STD_THREAD
+  auto body = [&] (RandomIt start)
+    {
+      for (; start < last; start += n_threads)
+	f (*start);
+    };
+
+  std::thread threads[max_threads];
+  for (unsigned i = 0; i < n_threads; ++i)
+    {
+      threads[i] = std::thread (body, first);
+      ++first;
+    }
+
+  for (unsigned i = 0; i < n_threads; ++i)
+    threads[i].join ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* COMMON_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 488201a1886..4c0ee11c47e 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,7 @@
 #include "common/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "common/parallel-for.h"
 
 /* See minsyms.h.  */
 
@@ -1372,16 +1373,18 @@ minimal_symbol_reader::install ()
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				strlen (msymbols[i].name), 0,
-				m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol &msym)
+	 {
+	   if (!msym.name_set)
+	     {
+	       symbol_set_names (&msym, msym.name,
+				 strlen (msym.name), 0,
+				 m_objfile->per_bfd);
+	       msym.name_set = 1;
+	     }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
-- 
2.17.2

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

* [PATCH v2 6/8] Introduce thread-safe way to handle SIGSEGV
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
                   ` (4 preceding siblings ...)
  2019-05-18 21:00 ` [PATCH v2 3/8] Add configure check for std::thread Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 7/8] Demangle minsyms in parallel Tom Tromey
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The gdb demangler installs a SIGSEGV handler in order to protect gdb
from demangler bugs.  However, this is not thread-safe, as signal
handlers are global to the process.

This patch changes gdb to always install a global SIGSEGV handler, and
then lets thread indicate their interest in handling the signal by
setting a thread-local variable.

This patch then arranges for the demangler code to use this; being
sure to arrange for calls to warning and the like to be done on the
main thread.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* event-top.h (segv_handler): Declare.
	* event-top.c (segv_handler): New global.
	(handle_sigsegv): New function.
	(async_init_signals): Install SIGSEGV handler.
	* cp-support.c (gdb_demangle_jmp_buf): Change type.  Now
	thread-local.
	(report_failed_demangle): New function.
	(gdb_demangle): Make core_dump_allowed atomic.  Remove signal
	handler-setting code, instead use segv_handler.  Run warning code
	on main thread.
---
 gdb/ChangeLog    |  13 ++++++
 gdb/cp-support.c | 114 +++++++++++++++++++++++------------------------
 gdb/event-top.c  |  37 +++++++++++++++
 gdb/event-top.h  |   3 ++
 4 files changed, 109 insertions(+), 58 deletions(-)

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 4afb79a4ea9..60bbbc23ec3 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -37,6 +37,9 @@
 #include "common/gdb_setjmp.h"
 #include "safe-ctype.h"
 #include "common/selftest.h"
+#include <atomic>
+#include "event-top.h"
+#include "ser-event.h"
 
 #define d_left(dc) (dc)->u.s_binary.left
 #define d_right(dc) (dc)->u.s_binary.right
@@ -1480,7 +1483,7 @@ static int catch_demangler_crashes = 1;
 
 /* Stack context and environment for demangler crash recovery.  */
 
-static SIGJMP_BUF gdb_demangle_jmp_buf;
+static thread_local SIGJMP_BUF *gdb_demangle_jmp_buf;
 
 /* If nonzero, attempt to dump core from the signal handler.  */
 
@@ -1499,7 +1502,43 @@ gdb_demangle_signal_handler (int signo)
       gdb_demangle_attempt_core_dump = 0;
     }
 
-  SIGLONGJMP (gdb_demangle_jmp_buf, signo);
+  SIGLONGJMP (*gdb_demangle_jmp_buf, signo);
+}
+
+/* A helper for gdb_demangle that reports a demangling failure.  */
+
+static void
+report_failed_demangle (const char *name, int core_dump_allowed,
+			int crash_signal)
+{
+  static int error_reported = 0;
+
+  if (!error_reported)
+    {
+      std::string short_msg
+	= string_printf (_("unable to demangle '%s' "
+			   "(demangler failed with signal %d)"),
+			 name, crash_signal);
+
+      std::string long_msg
+	= string_printf ("%s:%d: %s: %s", __FILE__, __LINE__,
+			 "demangler-warning", short_msg.c_str ());
+
+      target_terminal::scoped_restore_terminal_state term_state;
+      target_terminal::ours_for_output ();
+
+      begin_line ();
+      if (core_dump_allowed)
+	fprintf_unfiltered (gdb_stderr,
+			    _("%s\nAttempting to dump core.\n"),
+			    long_msg.c_str ());
+      else
+	warn_cant_dump_core (long_msg.c_str ());
+
+      demangler_warning (__FILE__, __LINE__, "%s", short_msg.c_str ());
+
+      error_reported = 1;
+    }
 }
 
 #endif
@@ -1513,12 +1552,7 @@ gdb_demangle (const char *name, int options)
   int crash_signal = 0;
 
 #ifdef HAVE_WORKING_FORK
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-  struct sigaction sa, old_sa;
-#else
-  sighandler_t ofunc;
-#endif
-  static int core_dump_allowed = -1;
+  static std::atomic<int> core_dump_allowed (-1);
 
   if (core_dump_allowed == -1)
     {
@@ -1528,23 +1562,15 @@ gdb_demangle (const char *name, int options)
 	gdb_demangle_attempt_core_dump = 0;
     }
 
-  if (catch_demangler_crashes)
-    {
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-      sa.sa_handler = gdb_demangle_signal_handler;
-      sigemptyset (&sa.sa_mask);
-#ifdef HAVE_SIGALTSTACK
-      sa.sa_flags = SA_ONSTACK;
-#else
-      sa.sa_flags = 0;
-#endif
-      sigaction (SIGSEGV, &sa, &old_sa);
-#else
-      ofunc = signal (SIGSEGV, gdb_demangle_signal_handler);
-#endif
+  scoped_restore restore_segv
+    = make_scoped_restore (&segv_handler,
+			   catch_demangler_crashes
+			   ? gdb_demangle_signal_handler
+			   : nullptr);
 
-      crash_signal = SIGSETJMP (gdb_demangle_jmp_buf);
-    }
+  SIGJMP_BUF jmp_buf;
+  if (catch_demangler_crashes)
+    crash_signal = SIGSETJMP (jmp_buf);
 #endif
 
   if (crash_signal == 0)
@@ -1553,42 +1579,14 @@ gdb_demangle (const char *name, int options)
 #ifdef HAVE_WORKING_FORK
   if (catch_demangler_crashes)
     {
-#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
-      sigaction (SIGSEGV, &old_sa, NULL);
-#else
-      signal (SIGSEGV, ofunc);
-#endif
-
       if (crash_signal != 0)
 	{
-	  static int error_reported = 0;
-
-	  if (!error_reported)
-	    {
-	      std::string short_msg
-		= string_printf (_("unable to demangle '%s' "
-				   "(demangler failed with signal %d)"),
-				 name, crash_signal);
-
-	      std::string long_msg
-		= string_printf ("%s:%d: %s: %s", __FILE__, __LINE__,
-				 "demangler-warning", short_msg.c_str ());
-
-	      target_terminal::scoped_restore_terminal_state term_state;
-	      target_terminal::ours_for_output ();
-
-	      begin_line ();
-	      if (core_dump_allowed)
-		fprintf_unfiltered (gdb_stderr,
-				    _("%s\nAttempting to dump core.\n"),
-				    long_msg.c_str ());
-	      else
-		warn_cant_dump_core (long_msg.c_str ());
-
-	      demangler_warning (__FILE__, __LINE__, "%s", short_msg.c_str ());
-
-	      error_reported = 1;
-	    }
+	  std::string copy = name;
+	  run_on_main_thread ([=] ()
+            {
+	      report_failed_demangle (copy.c_str (), core_dump_allowed,
+				      crash_signal);
+	    });
 
 	  result = NULL;
 	}
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3ccf136ff12..abc624d9f9c 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -849,6 +849,29 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
 }
 \f
 
+/* See event-top.h.  */
+
+thread_local void (*segv_handler) (int);
+
+/* Handler for SIGSEGV.  */
+
+static void
+handle_sigsegv (int sig)
+{
+  if (segv_handler == nullptr)
+    {
+      signal (sig, nullptr);
+      raise (sig);
+    }
+  else
+    {
+      signal (sig, handle_sigsegv);
+      segv_handler (sig);
+    }
+}
+
+\f
+
 /* The serial event associated with the QUIT flag.  set_quit_flag sets
    this, and check_quit_flag clears it.  Used by interruptible_select
    to be able to do interruptible I/O with no race with the SIGINT
@@ -916,6 +939,20 @@ async_init_signals (void)
   sigtstp_token =
     create_async_signal_handler (async_sigtstp_handler, NULL);
 #endif
+
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+  struct sigaction sa, old_sa;
+  sa.sa_handler = handle_sigsegv;
+  sigemptyset (&sa.sa_mask);
+#ifdef HAVE_SIGALTSTACK
+  sa.sa_flags = SA_ONSTACK;
+#else
+  sa.sa_flags = 0;
+#endif
+  sigaction (SIGSEGV, &sa, &old_sa);
+#else
+  signal (SIGSEGV, handle_sigsegv);
+#endif
 }
 
 /* See defs.h.  */
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 4c3e6cb8612..849f40d9192 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -70,4 +70,7 @@ extern void gdb_rl_callback_handler_install (const char *prompt);
    currently installed.  */
 extern void gdb_rl_callback_handler_reinstall (void);
 
+/* The SIGSEGV handler for this thread, or NULL if there is none.  */
+extern thread_local void (*segv_handler) (int);
+
 #endif
-- 
2.17.2

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

* [PATCH v2 4/8] Lock the demangled hash table
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 1/8] Defer minimal symbol name-setting Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 5/8] Introduce run_on_main_thread Tom Tromey
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces a lock that is used when modifying the demangled hash
table.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* symtab.c (demangled_mutex): New global.
	(symbol_set_names): Use a lock_guard.
---
 gdb/ChangeLog |   5 ++
 gdb/symtab.c  | 134 ++++++++++++++++++++++++++++----------------------
 2 files changed, 81 insertions(+), 58 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 130d5cd48ff..6ad024a8a29 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,6 +69,9 @@
 #include "arch-utils.h"
 #include <algorithm>
 #include "common/pathstuff.h"
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* Forward declarations for local functions.  */
 
@@ -709,6 +712,11 @@ symbol_set_language (struct general_symbol_info *gsymbol,
 
 /* Functions to initialize a symbol's mangled name.  */
 
+#if CXX_STD_THREAD
+/* Mutex that is used when modifying the demangled hash table.  */
+static std::mutex demangled_mutex;
+#endif
+
 /* Objects of this type are stored in the demangled name hash table.  */
 struct demangled_name_entry
 {
@@ -837,9 +845,6 @@ symbol_set_names (struct general_symbol_info *gsymbol,
       return;
     }
 
-  if (per_bfd->demangled_names_hash == NULL)
-    create_demangled_names_hash (per_bfd);
-
   if (linkage_name[len] != '\0')
     {
       char *alloc_name;
@@ -858,64 +863,77 @@ symbol_set_names (struct general_symbol_info *gsymbol,
     = symbol_find_demangled_name (gsymbol, linkage_name_copy);
   gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
 
-  entry.mangled = linkage_name_copy;
-  slot = ((struct demangled_name_entry **)
-	  htab_find_slot (per_bfd->demangled_names_hash.get (),
-			  &entry, INSERT));
-
-  /* If this name is not in the hash table, add it.  */
-  if (*slot == NULL
-      /* A C version of the symbol may have already snuck into the table.
-	 This happens to, e.g., main.init (__go_init_main).  Cope.  */
-      || (gsymbol->language == language_go
-	  && (*slot)->demangled[0] == '\0'))
-    {
-      int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
-
-      /* Suppose we have demangled_name==NULL, copy_name==0, and
-	 linkage_name_copy==linkage_name.  In this case, we already have the
-	 mangled name saved, and we don't have a demangled name.  So,
-	 you might think we could save a little space by not recording
-	 this in the hash table at all.
+  struct demangled_name_entry *found_entry;
+
+  {
+#if CXX_STD_THREAD
+    std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+
+    if (per_bfd->demangled_names_hash == NULL)
+      create_demangled_names_hash (per_bfd);
+
+    entry.mangled = linkage_name_copy;
+    slot = ((struct demangled_name_entry **)
+	    htab_find_slot (per_bfd->demangled_names_hash.get (),
+			    &entry, INSERT));
+
+    /* If this name is not in the hash table, add it.  */
+    if (*slot == NULL
+	/* A C version of the symbol may have already snuck into the table.
+	   This happens to, e.g., main.init (__go_init_main).  Cope.  */
+	|| (gsymbol->language == language_go
+	    && (*slot)->demangled[0] == '\0'))
+      {
+	int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
+
+	/* Suppose we have demangled_name==NULL, copy_name==0, and
+	   linkage_name_copy==linkage_name.  In this case, we already have the
+	   mangled name saved, and we don't have a demangled name.  So,
+	   you might think we could save a little space by not recording
+	   this in the hash table at all.
 	 
-	 It turns out that it is actually important to still save such
-	 an entry in the hash table, because storing this name gives
-	 us better bcache hit rates for partial symbols.  */
-      if (!copy_name && linkage_name_copy == linkage_name)
-	{
-	  *slot
-	    = ((struct demangled_name_entry *)
-	       obstack_alloc (&per_bfd->storage_obstack,
-			      offsetof (struct demangled_name_entry, demangled)
-			      + demangled_len + 1));
-	  (*slot)->mangled = linkage_name;
-	}
-      else
-	{
-	  char *mangled_ptr;
-
-	  /* If we must copy the mangled name, put it directly after
-	     the demangled name so we can have a single
-	     allocation.  */
-	  *slot
-	    = ((struct demangled_name_entry *)
-	       obstack_alloc (&per_bfd->storage_obstack,
-			      offsetof (struct demangled_name_entry, demangled)
-			      + len + demangled_len + 2));
-	  mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
-	  strcpy (mangled_ptr, linkage_name_copy);
-	  (*slot)->mangled = mangled_ptr;
-	}
+	   It turns out that it is actually important to still save such
+	   an entry in the hash table, because storing this name gives
+	   us better bcache hit rates for partial symbols.  */
+	if (!copy_name && linkage_name_copy == linkage_name)
+	  {
+	    *slot
+	      = ((struct demangled_name_entry *)
+		 obstack_alloc (&per_bfd->storage_obstack,
+				offsetof (struct demangled_name_entry, demangled)
+				+ demangled_len + 1));
+	    (*slot)->mangled = linkage_name;
+	  }
+	else
+	  {
+	    char *mangled_ptr;
+
+	    /* If we must copy the mangled name, put it directly after
+	       the demangled name so we can have a single
+	       allocation.  */
+	    *slot
+	      = ((struct demangled_name_entry *)
+		 obstack_alloc (&per_bfd->storage_obstack,
+				offsetof (struct demangled_name_entry, demangled)
+				+ len + demangled_len + 2));
+	    mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
+	    strcpy (mangled_ptr, linkage_name_copy);
+	    (*slot)->mangled = mangled_ptr;
+	  }
 
-      if (demangled_name != NULL)
-	strcpy ((*slot)->demangled, demangled_name.get());
-      else
-	(*slot)->demangled[0] = '\0';
-    }
+	if (demangled_name != NULL)
+	  strcpy ((*slot)->demangled, demangled_name.get());
+	else
+	  (*slot)->demangled[0] = '\0';
+      }
+
+    found_entry = *slot;
+  }
 
-  gsymbol->name = (*slot)->mangled;
-  if ((*slot)->demangled[0] != '\0')
-    symbol_set_demangled_name (gsymbol, (*slot)->demangled,
+  gsymbol->name = found_entry->mangled;
+  if (found_entry->demangled[0] != '\0')
+    symbol_set_demangled_name (gsymbol, found_entry->demangled,
 			       &per_bfd->storage_obstack);
   else
     symbol_set_demangled_name (gsymbol, NULL, &per_bfd->storage_obstack);
-- 
2.17.2

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

* [PATCH v2 1/8] Defer minimal symbol name-setting
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 4/8] Lock the demangled hash table Tom Tromey
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Currently the demangled name of a minimal symbol is set when creating
the symbol.  However, there is no intrinsic need to do this.  This
patch instead arranges for the demangling to be done just before the
minsym hash tables are filled.  This will be useful in a later patch.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* symtab.h (struct minimal_symbol) <name_set>: New member.
	* minsyms.c (minimal_symbol_reader::record_full): Copy name.
	Don't call symbol_set_names.
	(minimal_symbol_reader::install): Call symbol_set_names.
---
 gdb/ChangeLog |  7 +++++++
 gdb/minsyms.c | 18 +++++++++++++++++-
 gdb/symtab.h  |  4 ++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 9d29d880aab..488201a1886 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1140,7 +1140,11 @@ minimal_symbol_reader::record_full (const char *name, int name_len,
   msymbol = &m_msym_bunch->contents[m_msym_bunch_index];
   symbol_set_language (msymbol, language_auto,
 		       &m_objfile->per_bfd->storage_obstack);
-  symbol_set_names (msymbol, name, name_len, copy_name, m_objfile->per_bfd);
+
+  if (copy_name)
+    name = (char *) obstack_copy0 (&m_objfile->per_bfd->storage_obstack,
+				   name, name_len);
+  msymbol->name = name;
 
   SET_MSYMBOL_VALUE_ADDRESS (msymbol, address);
   MSYMBOL_SECTION (msymbol) = section;
@@ -1367,6 +1371,18 @@ minimal_symbol_reader::install ()
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+      msymbols = m_objfile->per_bfd->msymbols.get ();
+      for (int i = 0; i < mcount; ++i)
+	{
+	  if (!msymbols[i].name_set)
+	    {
+	      symbol_set_names (&msymbols[i], msymbols[i].name,
+				strlen (msymbols[i].name), 0,
+				m_objfile->per_bfd);
+	      msymbols[i].name_set = 1;
+	    }
+	}
+
       build_minimal_symbol_hash_tables (m_objfile);
     }
 }
diff --git a/gdb/symtab.h b/gdb/symtab.h
index e4ee7271a15..cb06fe01c59 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -669,6 +669,10 @@ struct minimal_symbol : public general_symbol_info
      the object file format may not carry that piece of information.  */
   unsigned int has_size : 1;
 
+  /* Non-zero if this symbol ever had its demangled name set (even if
+     it was set to NULL).  */
+  unsigned int name_set : 1;
+
   /* Minimal symbols with the same hash key are kept on a linked
      list.  This is the link.  */
 
-- 
2.17.2

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

* [PATCH v2 2/8] Remove static buffer from ada_decode
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
                   ` (2 preceding siblings ...)
  2019-05-18 21:00 ` [PATCH v2 5/8] Introduce run_on_main_thread Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 3/8] Add configure check for std::thread Tom Tromey
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

ada_decode has a static buffer, which means it is not thread-safe.
This patch removes the buffer and adds a parameter so that the storage
can be managed by the caller.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* ada-varobj.c (ada_varobj_describe_simple_array_child): Update.
	* ada-lang.h (ada_decode): Add "storage" parameter.
	* ada-lang.c (ada_decode): Add "storage" parameter.
	(ada_decode_symbol, ada_la_decode, ada_sniff_from_mangled_name)
	(is_valid_name_for_wild_match, name_matches_regex)
	(ada_add_global_exceptions): Update.
	* ada-exp.y (write_object_renaming): Update.
---
 gdb/ChangeLog    | 10 +++++++++
 gdb/ada-exp.y    |  6 +++++-
 gdb/ada-lang.c   | 55 ++++++++++++++++++++++++++++++------------------
 gdb/ada-lang.h   |  3 ++-
 gdb/ada-varobj.c |  3 ++-
 5 files changed, 54 insertions(+), 23 deletions(-)

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index 92461dbcb69..279d8db61f7 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -816,7 +816,11 @@ write_object_renaming (struct parser_state *par_state,
 				 renamed_entity_len);
   ada_lookup_encoded_symbol (name, orig_left_context, VAR_DOMAIN, &sym_info);
   if (sym_info.symbol == NULL)
-    error (_("Could not find renamed variable: %s"), ada_decode (name));
+    {
+      gdb::unique_xmalloc_ptr<char> storage;
+      error (_("Could not find renamed variable: %s"),
+	     ada_decode (name, &storage));
+    }
   else if (SYMBOL_CLASS (sym_info.symbol) == LOC_TYPEDEF)
     /* We have a renaming of an old-style renaming symbol.  Don't
        trust the block information.  */
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 23197f60340..4e6b1965557 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -1134,22 +1134,18 @@ ada_remove_Xbn_suffix (const char *encoded, int *len)
 
 /* If ENCODED follows the GNAT entity encoding conventions, then return
    the decoded form of ENCODED.  Otherwise, return "<%s>" where "%s" is
-   replaced by ENCODED.
-
-   The resulting string is valid until the next call of ada_decode.
-   If the string is unchanged by decoding, the original string pointer
-   is returned.  */
+   replaced by ENCODED.  */
 
 const char *
-ada_decode (const char *encoded)
+ada_decode (const char *encoded, gdb::unique_xmalloc_ptr<char> *storage)
 {
   int i, j;
   int len0;
   const char *p;
   char *decoded;
   int at_start_name;
-  static char *decoding_buffer = NULL;
-  static size_t decoding_buffer_size = 0;
+  char *decoding_buffer = NULL;
+  size_t decoding_buffer_size = 0;
 
   /* With function descriptors on PPC64, the value of a symbol named
      ".FN", if it exists, is the entry point of the function "FN".  */
@@ -1377,9 +1373,15 @@ ada_decode (const char *encoded)
       goto Suppress;
 
   if (strcmp (decoded, encoded) == 0)
-    return encoded;
+    {
+      xfree (decoding_buffer);
+      return encoded;
+    }
   else
-    return decoded;
+    {
+      storage->reset (decoded);
+      return decoded;
+    }
 
 Suppress:
   GROW_VECT (decoding_buffer, decoding_buffer_size, strlen (encoded) + 3);
@@ -1388,8 +1390,8 @@ Suppress:
     strcpy (decoded, encoded);
   else
     xsnprintf (decoded, decoding_buffer_size, "<%s>", encoded);
+  storage->reset (decoded);
   return decoded;
-
 }
 
 /* Table for keeping permanent unique copies of decoded names.  Once
@@ -1418,7 +1420,8 @@ ada_decode_symbol (const struct general_symbol_info *arg)
 
   if (!gsymbol->ada_mangled)
     {
-      const char *decoded = ada_decode (gsymbol->name);
+      gdb::unique_xmalloc_ptr<char> storage;
+      const char *decoded = ada_decode (gsymbol->name, &storage);
       struct obstack *obstack = gsymbol->language_specific.obstack;
 
       gsymbol->ada_mangled = 1;
@@ -1448,7 +1451,8 @@ ada_decode_symbol (const struct general_symbol_info *arg)
 static char *
 ada_la_decode (const char *encoded, int options)
 {
-  return xstrdup (ada_decode (encoded));
+  gdb::unique_xmalloc_ptr<char> storage;
+  return xstrdup (ada_decode (encoded, &storage));
 }
 
 /* Implement la_sniff_from_mangled_name for Ada.  */
@@ -1456,7 +1460,8 @@ ada_la_decode (const char *encoded, int options)
 static int
 ada_sniff_from_mangled_name (const char *mangled, char **out)
 {
-  const char *demangled = ada_decode (mangled);
+  gdb::unique_xmalloc_ptr<char> storage;
+  const char *demangled = ada_decode (mangled, &storage);
 
   *out = NULL;
 
@@ -6091,7 +6096,8 @@ is_name_suffix (const char *str)
 static int
 is_valid_name_for_wild_match (const char *name0)
 {
-  const char *decoded_name = ada_decode (name0);
+  gdb::unique_xmalloc_ptr<char> storage;
+  const char *decoded_name = ada_decode (name0, &storage);
   int i;
 
   /* If the decoded name starts with an angle bracket, it means that
@@ -6339,8 +6345,9 @@ ada_lookup_name_info::matches
          is not a suitable completion.  */
       const char *sym_name_copy = sym_name;
       bool has_angle_bracket;
+      gdb::unique_xmalloc_ptr<char> storage;
 
-      sym_name = ada_decode (sym_name);
+      sym_name = ada_decode (sym_name, &storage);
       has_angle_bracket = (sym_name[0] == '<');
       match = (has_angle_bracket == m_verbatim_p);
       sym_name = sym_name_copy;
@@ -6362,12 +6369,14 @@ ada_lookup_name_info::matches
 
   /* Second: Try wild matching...  */
 
+  gdb::unique_xmalloc_ptr<char> sym_name_storage;
   if (!match && m_wild_match_p)
     {
       /* Since we are doing wild matching, this means that TEXT
          may represent an unqualified symbol name.  We therefore must
          also compare TEXT against the unqualified name of the symbol.  */
-      sym_name = ada_unqualified_name (ada_decode (sym_name));
+      sym_name = ada_unqualified_name (ada_decode (sym_name,
+						   &sym_name_storage));
 
       if (strncmp (sym_name, text, text_len) == 0)
 	match = true;
@@ -6383,7 +6392,10 @@ ada_lookup_name_info::matches
       std::string &match_str = comp_match_res->match.storage ();
 
       if (!m_encoded_p)
-	match_str = ada_decode (sym_name);
+	{
+	  gdb::unique_xmalloc_ptr<char> storage;
+	  match_str = ada_decode (sym_name, &storage);
+	}
       else
 	{
 	  if (m_verbatim_p)
@@ -13575,8 +13587,9 @@ ada_add_exceptions_from_frame (compiled_regex *preg,
 static bool
 name_matches_regex (const char *name, compiled_regex *preg)
 {
+  gdb::unique_xmalloc_ptr<char> storage;
   return (preg == NULL
-	  || preg->exec (ada_decode (name), 0, NULL, 0) == 0);
+	  || preg->exec (ada_decode (name, &storage), 0, NULL, 0) == 0);
 }
 
 /* Add all exceptions defined globally whose name name match
@@ -13609,7 +13622,9 @@ ada_add_global_exceptions (compiled_regex *preg,
 			   lookup_name_info::match_any (),
 			   [&] (const char *search_name)
 			   {
-			     const char *decoded = ada_decode (search_name);
+			     gdb::unique_xmalloc_ptr<char> storage;
+			     const char *decoded = ada_decode (search_name,
+							       &storage);
 			     return name_matches_regex (decoded, preg);
 			   },
 			   NULL,
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 52d597e04c1..4bcca1f9de3 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -227,7 +227,8 @@ extern struct type *ada_get_decoded_type (struct type *type);
 
 extern const char *ada_decode_symbol (const struct general_symbol_info *);
 
-extern const char *ada_decode (const char*);
+extern const char *ada_decode (const char *,
+			       gdb::unique_xmalloc_ptr<char> *storage);
 
 extern enum language ada_update_initial_language (enum language);
 
diff --git a/gdb/ada-varobj.c b/gdb/ada-varobj.c
index a4d553d3786..29a4c86cc5f 100644
--- a/gdb/ada-varobj.c
+++ b/gdb/ada-varobj.c
@@ -624,6 +624,7 @@ ada_varobj_describe_simple_array_child (struct value *parent_value,
 	 of the array index type when such type qualification is
 	 needed.  */
       const char *index_type_name = NULL;
+      gdb::unique_xmalloc_ptr<char> storage;
 
       /* If the index type is a range type, find the base type.  */
       while (TYPE_CODE (index_type) == TYPE_CODE_RANGE)
@@ -634,7 +635,7 @@ ada_varobj_describe_simple_array_child (struct value *parent_value,
 	{
 	  index_type_name = ada_type_name (index_type);
 	  if (index_type_name)
-	    index_type_name = ada_decode (index_type_name);
+	    index_type_name = ada_decode (index_type_name, &storage);
 	}
 
       if (index_type_name != NULL)
-- 
2.17.2

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

* [PATCH v2 3/8] Add configure check for std::thread
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
                   ` (3 preceding siblings ...)
  2019-05-18 21:00 ` [PATCH v2 2/8] Remove static buffer from ada_decode Tom Tromey
@ 2019-05-18 21:00 ` Tom Tromey
  2019-05-18 21:00 ` [PATCH v2 6/8] Introduce thread-safe way to handle SIGSEGV Tom Tromey
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-18 21:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds a configure check for std::thread.  This is needed because
std::thread is not available in mingw.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* acinclude.m4: Include ax_pthread.m4.
	* Makefile.in (PTHREAD_CFLAGS, PTHREAD_LIBS): New variables.
	(INTERNAL_CFLAGS_BASE): Use PTHREAD_CFLAGS.
	(CLIBS): Use PTHREAD_LIBS.
	(aclocal_m4_deps): Add ax_pthread.m4.
	* config.in, configure: Rebuild.
	* common/common.m4 (GDB_AC_COMMON): Check for std::thread.

gdb/gdbserver/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* acinclude.m4: Include ax_pthread.m4.
	* config.in, configure: Rebuild.
---
 gdb/ChangeLog              |  10 +
 gdb/Makefile.in            |  12 +-
 gdb/acinclude.m4           |   2 +
 gdb/common/common.m4       |  25 ++
 gdb/config.in              |   3 +
 gdb/configure              | 766 +++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog    |   5 +
 gdb/gdbserver/acinclude.m4 |   2 +
 gdb/gdbserver/config.in    |   3 +
 gdb/gdbserver/configure    | 766 +++++++++++++++++++++++++++++++++++++
 10 files changed, 1590 insertions(+), 4 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0f495783600..2dd69f3f0ba 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -203,6 +203,9 @@ WERROR_CFLAGS = @WERROR_CFLAGS@
 GDB_WARN_CFLAGS = $(WARN_CFLAGS)
 GDB_WERROR_CFLAGS = $(WERROR_CFLAGS)
 
+PTHREAD_CFLAGS = @PTHREAD_CFLAGS@
+PTHREAD_LIBS = @PTHREAD_LIBS@
+
 RDYNAMIC = @RDYNAMIC@
 
 # Where is the INTL library?  Typically in ../intl.
@@ -572,7 +575,7 @@ INTERNAL_CFLAGS_BASE = \
 	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) $(ZLIBINC) \
 	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) $(LIBDECNUMBER_CFLAGS) \
 	$(INTL_CFLAGS) $(INCGNU) $(ENABLE_CFLAGS) $(INTERNAL_CPPFLAGS) \
-	$(SRCHIGH_CFLAGS)
+	$(SRCHIGH_CFLAGS) $(PTHREAD_CFLAGS)
 INTERNAL_WARN_CFLAGS = $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS)
 INTERNAL_CFLAGS = $(INTERNAL_WARN_CFLAGS) $(GDB_WERROR_CFLAGS)
 
@@ -586,7 +589,7 @@ LDFLAGS = @LDFLAGS@
 # PROFILE_CFLAGS is _not_ included, however, because we use monstartup.
 INTERNAL_LDFLAGS = \
 	$(CXXFLAGS) $(GLOBAL_CFLAGS) $(MH_LDFLAGS) \
-	$(LDFLAGS) $(CONFIG_LDFLAGS)
+	$(LDFLAGS) $(CONFIG_LDFLAGS) $(PTHREAD_CFLAGS)
 
 # Libraries and corresponding dependencies for compiling gdb.
 # XM_CLIBS, defined in *config files, have host-dependent libs.
@@ -596,7 +599,7 @@ CLIBS = $(SIM) $(READLINE) $(OPCODES) $(BFD) $(ZLIB) $(INTL) $(LIBIBERTY) $(LIBD
 	@LIBS@ @GUILE_LIBS@ @PYTHON_LIBS@ \
 	$(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) $(LIBIPT) \
 	$(LIBIBERTY) $(WIN32LIBS) $(LIBGNU) $(LIBICONV) $(LIBMPFR) \
-	$(SRCHIGH_LIBS)
+	$(SRCHIGH_LIBS) $(PTHREAD_LIBS)
 CDEPS = $(NAT_CDEPS) $(SIM) $(BFD) $(READLINE_DEPS) \
 	$(OPCODES) $(INTL_DEPS) $(LIBIBERTY) $(CONFIG_DEPS) $(LIBGNU)
 
@@ -2081,7 +2084,8 @@ aclocal_m4_deps = \
 	../config/depstand.m4 \
 	../config/lcmessage.m4 \
 	../config/codeset.m4 \
-	../config/zlib.m4
+	../config/zlib.m4 \
+	../config/ax_pthread.m4
 
 $(srcdir)/aclocal.m4: @MAINTAINER_MODE_TRUE@ $(aclocal_m4_deps)
 	cd $(srcdir) && $(ACLOCAL) $(ACLOCAL_AMFLAGS)
diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
index 0719d422a71..ff463ea3595 100644
--- a/gdb/acinclude.m4
+++ b/gdb/acinclude.m4
@@ -76,6 +76,8 @@ m4_include(ptrace.m4)
 
 m4_include(ax_cxx_compile_stdcxx.m4)
 
+sinclude([../config/ax_pthread.m4])
+
 ## ----------------------------------------- ##
 ## ANSIfy the C compiler whenever possible.  ##
 ## From Franc,ois Pinard                     ##
diff --git a/gdb/common/common.m4 b/gdb/common/common.m4
index 5701dd98293..90a0913c96e 100644
--- a/gdb/common/common.m4
+++ b/gdb/common/common.m4
@@ -35,6 +35,31 @@ AC_DEFUN([GDB_AC_COMMON], [
 
   AC_CHECK_DECLS([strerror, strstr])
 
+  # Check for std::thread.  This does not work on mingw.
+  AC_LANG_PUSH([C++])
+  AX_PTHREAD([threads=yes], [threads=no])
+  if test "$threads" = "yes"; then
+    save_LIBS="$LIBS"
+    LIBS="$PTHREAD_LIBS $LIBS"
+    save_CXXFLAGS="$CXXFLAGS"
+    CXXFLAGS="$PTHREAD_CFLAGS $save_CFLAGS"
+    AC_CACHE_CHECK([for std::thread],
+		   gdb_cv_cxx_std_thread,
+		   [AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
+    [[#include <thread>
+      void callback() { }]],
+    [[std::thread t(callback);]])],
+				  gdb_cv_cxx_std_thread=yes,
+				  gdb_cv_cxx_std_thread=no)])
+    LIBS="$save_LIBS"
+    CXXFLAGS="$save_CXXFLAGS"
+  fi
+  if test $gdb_cv_cxx_std_thread = yes; then
+    AC_DEFINE(CXX_STD_THREAD, 1,
+	      [Define to 1 if std::thread works.])
+  fi
+  AC_LANG_POP
+
   dnl Check if sigsetjmp is available.  Using AC_CHECK_FUNCS won't
   dnl do since sigsetjmp might only be defined as a macro.
 AC_CACHE_CHECK([for sigsetjmp], gdb_cv_func_sigsetjmp,
diff --git a/gdb/config.in b/gdb/config.in
index c0291fbd9c5..8cb9289302d 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -17,6 +17,9 @@
    */
 #undef CRAY_STACKSEG_END
 
+/* Define to 1 if std::thread works. */
+#undef CXX_STD_THREAD
+
 /* Define to 1 if using `alloca.c'. */
 #undef C_ALLOCA
 
diff --git a/gdb/configure b/gdb/configure
index 15a96afcca8..5efdfe60911 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -697,6 +697,11 @@ SYSTEM_GDBINIT
 TARGET_SYSTEM_ROOT
 CONFIG_LDFLAGS
 RDYNAMIC
+PTHREAD_CFLAGS
+PTHREAD_LIBS
+PTHREAD_CC
+ax_pthread_config
+SED
 ALLOCA
 LTLIBIPT
 LIBIPT
@@ -13347,6 +13352,75 @@ _ACEOF
 
 fi
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a sed that does not truncate output" >&5
+$as_echo_n "checking for a sed that does not truncate output... " >&6; }
+if ${ac_cv_path_SED+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+            ac_script=s/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/
+     for ac_i in 1 2 3 4 5 6 7; do
+       ac_script="$ac_script$as_nl$ac_script"
+     done
+     echo "$ac_script" 2>/dev/null | sed 99q >conftest.sed
+     { ac_script=; unset ac_script;}
+     if test -z "$SED"; then
+  ac_path_SED_found=false
+  # Loop through the user's path and test for each of PROGNAME-LIST
+  as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_prog in sed gsed; do
+    for ac_exec_ext in '' $ac_executable_extensions; do
+      ac_path_SED="$as_dir/$ac_prog$ac_exec_ext"
+      as_fn_executable_p "$ac_path_SED" || continue
+# Check for GNU ac_path_SED and select it if it is found.
+  # Check for GNU $ac_path_SED
+case `"$ac_path_SED" --version 2>&1` in
+*GNU*)
+  ac_cv_path_SED="$ac_path_SED" ac_path_SED_found=:;;
+*)
+  ac_count=0
+  $as_echo_n 0123456789 >"conftest.in"
+  while :
+  do
+    cat "conftest.in" "conftest.in" >"conftest.tmp"
+    mv "conftest.tmp" "conftest.in"
+    cp "conftest.in" "conftest.nl"
+    $as_echo '' >> "conftest.nl"
+    "$ac_path_SED" -f conftest.sed < "conftest.nl" >"conftest.out" 2>/dev/null || break
+    diff "conftest.out" "conftest.nl" >/dev/null 2>&1 || break
+    as_fn_arith $ac_count + 1 && ac_count=$as_val
+    if test $ac_count -gt ${ac_path_SED_max-0}; then
+      # Best one so far, save it but keep looking for a better one
+      ac_cv_path_SED="$ac_path_SED"
+      ac_path_SED_max=$ac_count
+    fi
+    # 10*(2^10) chars as input seems more than enough
+    test $ac_count -gt 10 && break
+  done
+  rm -f conftest.in conftest.tmp conftest.nl conftest.out;;
+esac
+
+      $ac_path_SED_found && break 3
+    done
+  done
+  done
+IFS=$as_save_IFS
+  if test -z "$ac_cv_path_SED"; then
+    as_fn_error $? "no acceptable sed could be found in \$PATH" "$LINENO" 5
+  fi
+else
+  ac_cv_path_SED=$SED
+fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_path_SED" >&5
+$as_echo "$ac_cv_path_SED" >&6; }
+ SED="$ac_cv_path_SED"
+  rm -f conftest.sed
+
 
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ANSI C header files" >&5
 $as_echo_n "checking for ANSI C header files... " >&6; }
@@ -13731,6 +13805,698 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
+  # Check for std::thread.  This does not work on mingw.
+  ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+
+
+
+
+ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+ax_pthread_ok=no
+
+# We used to check for pthread.h first, but this fails if pthread.h
+# requires special compiler flags (e.g. on Tru64 or Sequent).
+# It gets checked for in the link test anyway.
+
+# First of all, check if the user has set any of the PTHREAD_LIBS,
+# etcetera environment variables, and if threads linking works using
+# them:
+if test "x$PTHREAD_CFLAGS$PTHREAD_LIBS" != "x"; then
+        ax_pthread_save_CC="$CC"
+        ax_pthread_save_CFLAGS="$CFLAGS"
+        ax_pthread_save_LIBS="$LIBS"
+        if test "x$PTHREAD_CC" != "x"; then :
+  CC="$PTHREAD_CC"
+fi
+        CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+        LIBS="$PTHREAD_LIBS $LIBS"
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pthread_join using $CC $PTHREAD_CFLAGS $PTHREAD_LIBS" >&5
+$as_echo_n "checking for pthread_join using $CC $PTHREAD_CFLAGS $PTHREAD_LIBS... " >&6; }
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char pthread_join ();
+int
+main ()
+{
+return pthread_join ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_pthread_ok=yes
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_pthread_ok" >&5
+$as_echo "$ax_pthread_ok" >&6; }
+        if test "x$ax_pthread_ok" = "xno"; then
+                PTHREAD_LIBS=""
+                PTHREAD_CFLAGS=""
+        fi
+        CC="$ax_pthread_save_CC"
+        CFLAGS="$ax_pthread_save_CFLAGS"
+        LIBS="$ax_pthread_save_LIBS"
+fi
+
+# We must check for the threads library under a number of different
+# names; the ordering is very important because some systems
+# (e.g. DEC) have both -lpthread and -lpthreads, where one of the
+# libraries is broken (non-POSIX).
+
+# Create a list of thread flags to try.  Items starting with a "-" are
+# C compiler flags, and other items are library names, except for "none"
+# which indicates that we try without any flags at all, and "pthread-config"
+# which is a program returning the flags for the Pth emulation library.
+
+ax_pthread_flags="pthreads none -Kthread -pthread -pthreads -mthreads pthread --thread-safe -mt pthread-config"
+
+# The ordering *is* (sometimes) important.  Some notes on the
+# individual items follow:
+
+# pthreads: AIX (must check this before -lpthread)
+# none: in case threads are in libc; should be tried before -Kthread and
+#       other compiler flags to prevent continual compiler warnings
+# -Kthread: Sequent (threads in libc, but -Kthread needed for pthread.h)
+# -pthread: Linux/gcc (kernel threads), BSD/gcc (userland threads), Tru64
+#           (Note: HP C rejects this with "bad form for `-t' option")
+# -pthreads: Solaris/gcc (Note: HP C also rejects)
+# -mt: Sun Workshop C (may only link SunOS threads [-lthread], but it
+#      doesn't hurt to check since this sometimes defines pthreads and
+#      -D_REENTRANT too), HP C (must be checked before -lpthread, which
+#      is present but should not be used directly; and before -mthreads,
+#      because the compiler interprets this as "-mt" + "-hreads")
+# -mthreads: Mingw32/gcc, Lynx/gcc
+# pthread: Linux, etcetera
+# --thread-safe: KAI C++
+# pthread-config: use pthread-config program (for GNU Pth library)
+
+case $host_os in
+
+        freebsd*)
+
+        # -kthread: FreeBSD kernel threads (preferred to -pthread since SMP-able)
+        # lthread: LinuxThreads port on FreeBSD (also preferred to -pthread)
+
+        ax_pthread_flags="-kthread lthread $ax_pthread_flags"
+        ;;
+
+        hpux*)
+
+        # From the cc(1) man page: "[-mt] Sets various -D flags to enable
+        # multi-threading and also sets -lpthread."
+
+        ax_pthread_flags="-mt -pthread pthread $ax_pthread_flags"
+        ;;
+
+        openedition*)
+
+        # IBM z/OS requires a feature-test macro to be defined in order to
+        # enable POSIX threads at all, so give the user a hint if this is
+        # not set. (We don't define these ourselves, as they can affect
+        # other portions of the system API in unpredictable ways.)
+
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#            if !defined(_OPEN_THREADS) && !defined(_UNIX03_THREADS)
+             AX_PTHREAD_ZOS_MISSING
+#            endif
+
+_ACEOF
+if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
+  $EGREP "AX_PTHREAD_ZOS_MISSING" >/dev/null 2>&1; then :
+  { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: IBM z/OS requires -D_OPEN_THREADS or -D_UNIX03_THREADS to enable pthreads support." >&5
+$as_echo "$as_me: WARNING: IBM z/OS requires -D_OPEN_THREADS or -D_UNIX03_THREADS to enable pthreads support." >&2;}
+fi
+rm -f conftest*
+
+        ;;
+
+        solaris*)
+
+        # On Solaris (at least, for some versions), libc contains stubbed
+        # (non-functional) versions of the pthreads routines, so link-based
+        # tests will erroneously succeed. (N.B.: The stubs are missing
+        # pthread_cleanup_push, or rather a function called by this macro,
+        # so we could check for that, but who knows whether they'll stub
+        # that too in a future libc.)  So we'll check first for the
+        # standard Solaris way of linking pthreads (-mt -lpthread).
+
+        ax_pthread_flags="-mt,pthread pthread $ax_pthread_flags"
+        ;;
+esac
+
+# GCC generally uses -pthread, or -pthreads on some platforms (e.g. SPARC)
+
+if test "x$GCC" = "xyes"; then :
+  ax_pthread_flags="-pthread -pthreads $ax_pthread_flags"
+fi
+
+# The presence of a feature test macro requesting re-entrant function
+# definitions is, on some systems, a strong hint that pthreads support is
+# correctly enabled
+
+case $host_os in
+        darwin* | hpux* | linux* | osf* | solaris*)
+        ax_pthread_check_macro="_REENTRANT"
+        ;;
+
+        aix*)
+        ax_pthread_check_macro="_THREAD_SAFE"
+        ;;
+
+        *)
+        ax_pthread_check_macro="--"
+        ;;
+esac
+if test "x$ax_pthread_check_macro" = "x--"; then :
+  ax_pthread_check_cond=0
+else
+  ax_pthread_check_cond="!defined($ax_pthread_check_macro)"
+fi
+
+# Are we compiling with Clang?
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC is Clang" >&5
+$as_echo_n "checking whether $CC is Clang... " >&6; }
+if ${ax_cv_PTHREAD_CLANG+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_CLANG=no
+     # Note that Autoconf sets GCC=yes for Clang as well as GCC
+     if test "x$GCC" = "xyes"; then
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Note: Clang 2.7 lacks __clang_[a-z]+__ */
+#            if defined(__clang__) && defined(__llvm__)
+             AX_PTHREAD_CC_IS_CLANG
+#            endif
+
+_ACEOF
+if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
+  $EGREP "AX_PTHREAD_CC_IS_CLANG" >/dev/null 2>&1; then :
+  ax_cv_PTHREAD_CLANG=yes
+fi
+rm -f conftest*
+
+     fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_CLANG" >&5
+$as_echo "$ax_cv_PTHREAD_CLANG" >&6; }
+ax_pthread_clang="$ax_cv_PTHREAD_CLANG"
+
+ax_pthread_clang_warning=no
+
+# Clang needs special handling, because older versions handle the -pthread
+# option in a rather... idiosyncratic way
+
+if test "x$ax_pthread_clang" = "xyes"; then
+
+        # Clang takes -pthread; it has never supported any other flag
+
+        # (Note 1: This will need to be revisited if a system that Clang
+        # supports has POSIX threads in a separate library.  This tends not
+        # to be the way of modern systems, but it's conceivable.)
+
+        # (Note 2: On some systems, notably Darwin, -pthread is not needed
+        # to get POSIX threads support; the API is always present and
+        # active.  We could reasonably leave PTHREAD_CFLAGS empty.  But
+        # -pthread does define _REENTRANT, and while the Darwin headers
+        # ignore this macro, third-party headers might not.)
+
+        PTHREAD_CFLAGS="-pthread"
+        PTHREAD_LIBS=
+
+        ax_pthread_ok=yes
+
+        # However, older versions of Clang make a point of warning the user
+        # that, in an invocation where only linking and no compilation is
+        # taking place, the -pthread option has no effect ("argument unused
+        # during compilation").  They expect -pthread to be passed in only
+        # when source code is being compiled.
+        #
+        # Problem is, this is at odds with the way Automake and most other
+        # C build frameworks function, which is that the same flags used in
+        # compilation (CFLAGS) are also used in linking.  Many systems
+        # supported by AX_PTHREAD require exactly this for POSIX threads
+        # support, and in fact it is often not straightforward to specify a
+        # flag that is used only in the compilation phase and not in
+        # linking.  Such a scenario is extremely rare in practice.
+        #
+        # Even though use of the -pthread flag in linking would only print
+        # a warning, this can be a nuisance for well-run software projects
+        # that build with -Werror.  So if the active version of Clang has
+        # this misfeature, we search for an option to squash it.
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether Clang needs flag to prevent \"argument unused\" warning when linking with -pthread" >&5
+$as_echo_n "checking whether Clang needs flag to prevent \"argument unused\" warning when linking with -pthread... " >&6; }
+if ${ax_cv_PTHREAD_CLANG_NO_WARN_FLAG+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_CLANG_NO_WARN_FLAG=unknown
+             # Create an alternate version of $ac_link that compiles and
+             # links in two steps (.c -> .o, .o -> exe) instead of one
+             # (.c -> exe), because the warning occurs only in the second
+             # step
+             ax_pthread_save_ac_link="$ac_link"
+             ax_pthread_sed='s/conftest\.\$ac_ext/conftest.$ac_objext/g'
+             ax_pthread_link_step=`$as_echo "$ac_link" | sed "$ax_pthread_sed"`
+             ax_pthread_2step_ac_link="($ac_compile) && (echo ==== >&5) && ($ax_pthread_link_step)"
+             ax_pthread_save_CFLAGS="$CFLAGS"
+             for ax_pthread_try in '' -Qunused-arguments -Wno-unused-command-line-argument unknown; do
+                if test "x$ax_pthread_try" = "xunknown"; then :
+  break
+fi
+                CFLAGS="-Werror -Wunknown-warning-option $ax_pthread_try -pthread $ax_pthread_save_CFLAGS"
+                ac_link="$ax_pthread_save_ac_link"
+                cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+int main(void){return 0;}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ac_link="$ax_pthread_2step_ac_link"
+                     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+int main(void){return 0;}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  break
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+             done
+             ac_link="$ax_pthread_save_ac_link"
+             CFLAGS="$ax_pthread_save_CFLAGS"
+             if test "x$ax_pthread_try" = "x"; then :
+  ax_pthread_try=no
+fi
+             ax_cv_PTHREAD_CLANG_NO_WARN_FLAG="$ax_pthread_try"
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" >&5
+$as_echo "$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" >&6; }
+
+        case "$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" in
+                no | unknown) ;;
+                *) PTHREAD_CFLAGS="$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG $PTHREAD_CFLAGS" ;;
+        esac
+
+fi # $ax_pthread_clang = yes
+
+if test "x$ax_pthread_ok" = "xno"; then
+for ax_pthread_try_flag in $ax_pthread_flags; do
+
+        case $ax_pthread_try_flag in
+                none)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether pthreads work without any flags" >&5
+$as_echo_n "checking whether pthreads work without any flags... " >&6; }
+                ;;
+
+                -mt,pthread)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether pthreads work with -mt -lpthread" >&5
+$as_echo_n "checking whether pthreads work with -mt -lpthread... " >&6; }
+                PTHREAD_CFLAGS="-mt"
+                PTHREAD_LIBS="-lpthread"
+                ;;
+
+                -*)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether pthreads work with $ax_pthread_try_flag" >&5
+$as_echo_n "checking whether pthreads work with $ax_pthread_try_flag... " >&6; }
+                PTHREAD_CFLAGS="$ax_pthread_try_flag"
+                ;;
+
+                pthread-config)
+                # Extract the first word of "pthread-config", so it can be a program name with args.
+set dummy pthread-config; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_ax_pthread_config+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$ax_pthread_config"; then
+  ac_cv_prog_ax_pthread_config="$ax_pthread_config" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_ax_pthread_config="yes"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+  test -z "$ac_cv_prog_ax_pthread_config" && ac_cv_prog_ax_pthread_config="no"
+fi
+fi
+ax_pthread_config=$ac_cv_prog_ax_pthread_config
+if test -n "$ax_pthread_config"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_pthread_config" >&5
+$as_echo "$ax_pthread_config" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+                if test "x$ax_pthread_config" = "xno"; then :
+  continue
+fi
+                PTHREAD_CFLAGS="`pthread-config --cflags`"
+                PTHREAD_LIBS="`pthread-config --ldflags` `pthread-config --libs`"
+                ;;
+
+                *)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking for the pthreads library -l$ax_pthread_try_flag" >&5
+$as_echo_n "checking for the pthreads library -l$ax_pthread_try_flag... " >&6; }
+                PTHREAD_LIBS="-l$ax_pthread_try_flag"
+                ;;
+        esac
+
+        ax_pthread_save_CFLAGS="$CFLAGS"
+        ax_pthread_save_LIBS="$LIBS"
+        CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+        LIBS="$PTHREAD_LIBS $LIBS"
+
+        # Check for various functions.  We must include pthread.h,
+        # since some functions may be macros.  (On the Sequent, we
+        # need a special flag -Kthread to make this header compile.)
+        # We check for pthread_join because it is in -lpthread on IRIX
+        # while pthread_create is in libc.  We check for pthread_attr_init
+        # due to DEC craziness with -lpthreads.  We check for
+        # pthread_cleanup_push because it is one of the few pthread
+        # functions on Solaris that doesn't have a non-functional libc stub.
+        # We try pthread_create on general principles.
+
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pthread.h>
+#                       if $ax_pthread_check_cond
+#                        error "$ax_pthread_check_macro must be defined"
+#                       endif
+                        static void routine(void *a) { a = 0; }
+                        static void *start_routine(void *a) { return a; }
+int
+main ()
+{
+pthread_t th; pthread_attr_t attr;
+                        pthread_create(&th, 0, start_routine, 0);
+                        pthread_join(th, 0);
+                        pthread_attr_init(&attr);
+                        pthread_cleanup_push(routine, 0);
+                        pthread_cleanup_pop(0) /* ; */
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_pthread_ok=yes
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+
+        CFLAGS="$ax_pthread_save_CFLAGS"
+        LIBS="$ax_pthread_save_LIBS"
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_pthread_ok" >&5
+$as_echo "$ax_pthread_ok" >&6; }
+        if test "x$ax_pthread_ok" = "xyes"; then :
+  break
+fi
+
+        PTHREAD_LIBS=""
+        PTHREAD_CFLAGS=""
+done
+fi
+
+# Various other checks:
+if test "x$ax_pthread_ok" = "xyes"; then
+        ax_pthread_save_CFLAGS="$CFLAGS"
+        ax_pthread_save_LIBS="$LIBS"
+        CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+        LIBS="$PTHREAD_LIBS $LIBS"
+
+        # Detect AIX lossage: JOINABLE attribute is called UNDETACHED.
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking for joinable pthread attribute" >&5
+$as_echo_n "checking for joinable pthread attribute... " >&6; }
+if ${ax_cv_PTHREAD_JOINABLE_ATTR+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_JOINABLE_ATTR=unknown
+             for ax_pthread_attr in PTHREAD_CREATE_JOINABLE PTHREAD_CREATE_UNDETACHED; do
+                 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pthread.h>
+int
+main ()
+{
+int attr = $ax_pthread_attr; return attr /* ; */
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_cv_PTHREAD_JOINABLE_ATTR=$ax_pthread_attr; break
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+             done
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_JOINABLE_ATTR" >&5
+$as_echo "$ax_cv_PTHREAD_JOINABLE_ATTR" >&6; }
+        if test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xunknown" && \
+               test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xPTHREAD_CREATE_JOINABLE" && \
+               test "x$ax_pthread_joinable_attr_defined" != "xyes"; then :
+
+cat >>confdefs.h <<_ACEOF
+#define PTHREAD_CREATE_JOINABLE $ax_cv_PTHREAD_JOINABLE_ATTR
+_ACEOF
+
+               ax_pthread_joinable_attr_defined=yes
+
+fi
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether more special flags are required for pthreads" >&5
+$as_echo_n "checking whether more special flags are required for pthreads... " >&6; }
+if ${ax_cv_PTHREAD_SPECIAL_FLAGS+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_SPECIAL_FLAGS=no
+             case $host_os in
+             solaris*)
+             ax_cv_PTHREAD_SPECIAL_FLAGS="-D_POSIX_PTHREAD_SEMANTICS"
+             ;;
+             esac
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_SPECIAL_FLAGS" >&5
+$as_echo "$ax_cv_PTHREAD_SPECIAL_FLAGS" >&6; }
+        if test "x$ax_cv_PTHREAD_SPECIAL_FLAGS" != "xno" && \
+               test "x$ax_pthread_special_flags_added" != "xyes"; then :
+  PTHREAD_CFLAGS="$ax_cv_PTHREAD_SPECIAL_FLAGS $PTHREAD_CFLAGS"
+               ax_pthread_special_flags_added=yes
+fi
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PTHREAD_PRIO_INHERIT" >&5
+$as_echo_n "checking for PTHREAD_PRIO_INHERIT... " >&6; }
+if ${ax_cv_PTHREAD_PRIO_INHERIT+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pthread.h>
+int
+main ()
+{
+int i = PTHREAD_PRIO_INHERIT;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_cv_PTHREAD_PRIO_INHERIT=yes
+else
+  ax_cv_PTHREAD_PRIO_INHERIT=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_PRIO_INHERIT" >&5
+$as_echo "$ax_cv_PTHREAD_PRIO_INHERIT" >&6; }
+        if test "x$ax_cv_PTHREAD_PRIO_INHERIT" = "xyes" && \
+               test "x$ax_pthread_prio_inherit_defined" != "xyes"; then :
+
+$as_echo "#define HAVE_PTHREAD_PRIO_INHERIT 1" >>confdefs.h
+
+               ax_pthread_prio_inherit_defined=yes
+
+fi
+
+        CFLAGS="$ax_pthread_save_CFLAGS"
+        LIBS="$ax_pthread_save_LIBS"
+
+        # More AIX lossage: compile with *_r variant
+        if test "x$GCC" != "xyes"; then
+            case $host_os in
+                aix*)
+                case "x/$CC" in #(
+  x*/c89|x*/c89_128|x*/c99|x*/c99_128|x*/cc|x*/cc128|x*/xlc|x*/xlc_v6|x*/xlc128|x*/xlc128_v6) :
+    #handle absolute path differently from PATH based program lookup
+                     case "x$CC" in #(
+  x/*) :
+    if as_fn_executable_p ${CC}_r; then :
+  PTHREAD_CC="${CC}_r"
+fi ;; #(
+  *) :
+    for ac_prog in ${CC}_r
+do
+  # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_PTHREAD_CC+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$PTHREAD_CC"; then
+  ac_cv_prog_PTHREAD_CC="$PTHREAD_CC" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_PTHREAD_CC="$ac_prog"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+fi
+fi
+PTHREAD_CC=$ac_cv_prog_PTHREAD_CC
+if test -n "$PTHREAD_CC"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PTHREAD_CC" >&5
+$as_echo "$PTHREAD_CC" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+  test -n "$PTHREAD_CC" && break
+done
+test -n "$PTHREAD_CC" || PTHREAD_CC="$CC"
+ ;;
+esac ;; #(
+  *) :
+     ;;
+esac
+                ;;
+            esac
+        fi
+fi
+
+test -n "$PTHREAD_CC" || PTHREAD_CC="$CC"
+
+
+
+
+
+# Finally, execute ACTION-IF-FOUND/ACTION-IF-NOT-FOUND:
+if test "x$ax_pthread_ok" = "xyes"; then
+        threads=yes
+        :
+else
+        ax_pthread_ok=no
+        threads=no
+fi
+ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+
+  if test "$threads" = "yes"; then
+    save_LIBS="$LIBS"
+    LIBS="$PTHREAD_LIBS $LIBS"
+    save_CXXFLAGS="$CXXFLAGS"
+    CXXFLAGS="$PTHREAD_CFLAGS $save_CFLAGS"
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for std::thread" >&5
+$as_echo_n "checking for std::thread... " >&6; }
+if ${gdb_cv_cxx_std_thread+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <thread>
+      void callback() { }
+int
+main ()
+{
+std::thread t(callback);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  gdb_cv_cxx_std_thread=yes
+else
+  gdb_cv_cxx_std_thread=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_cxx_std_thread" >&5
+$as_echo "$gdb_cv_cxx_std_thread" >&6; }
+    LIBS="$save_LIBS"
+    CXXFLAGS="$save_CXXFLAGS"
+  fi
+  if test $gdb_cv_cxx_std_thread = yes; then
+
+$as_echo "#define CXX_STD_THREAD 1" >>confdefs.h
+
+  fi
+  ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+
     { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sigsetjmp" >&5
 $as_echo_n "checking for sigsetjmp... " >&6; }
 if ${gdb_cv_func_sigsetjmp+:} false; then :
diff --git a/gdb/gdbserver/acinclude.m4 b/gdb/gdbserver/acinclude.m4
index fc3e344a611..7ea43b5e762 100644
--- a/gdb/gdbserver/acinclude.m4
+++ b/gdb/gdbserver/acinclude.m4
@@ -34,6 +34,8 @@ m4_include(../ax_cxx_compile_stdcxx.m4)
 dnl For GDB_AC_SELFTEST.
 m4_include(../selftest.m4)
 
+sinclude([../../config/ax_pthread.m4])
+
 dnl Check for existence of a type $1 in libthread_db.h
 dnl Based on BFD_HAVE_SYS_PROCFS_TYPE in bfd/bfd.m4.
 
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index 05537df81e3..2d041ed2470 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -5,6 +5,9 @@
    */
 #undef CRAY_STACKSEG_END
 
+/* Define to 1 if std::thread works. */
+#undef CXX_STD_THREAD
+
 /* Define to 1 if using `alloca.c'. */
 #undef C_ALLOCA
 
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 1ddbd6b27e0..5b50f2d249f 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -636,6 +636,11 @@ WERROR_CFLAGS
 WARN_CFLAGS
 ustinc
 ustlibs
+PTHREAD_CFLAGS
+PTHREAD_LIBS
+PTHREAD_CC
+ax_pthread_config
+SED
 ALLOCA
 CCDEPMODE
 CONFIG_SRC_SUBDIR
@@ -6539,6 +6544,75 @@ _ACEOF
 
 fi
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a sed that does not truncate output" >&5
+$as_echo_n "checking for a sed that does not truncate output... " >&6; }
+if ${ac_cv_path_SED+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+            ac_script=s/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/
+     for ac_i in 1 2 3 4 5 6 7; do
+       ac_script="$ac_script$as_nl$ac_script"
+     done
+     echo "$ac_script" 2>/dev/null | sed 99q >conftest.sed
+     { ac_script=; unset ac_script;}
+     if test -z "$SED"; then
+  ac_path_SED_found=false
+  # Loop through the user's path and test for each of PROGNAME-LIST
+  as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_prog in sed gsed; do
+    for ac_exec_ext in '' $ac_executable_extensions; do
+      ac_path_SED="$as_dir/$ac_prog$ac_exec_ext"
+      as_fn_executable_p "$ac_path_SED" || continue
+# Check for GNU ac_path_SED and select it if it is found.
+  # Check for GNU $ac_path_SED
+case `"$ac_path_SED" --version 2>&1` in
+*GNU*)
+  ac_cv_path_SED="$ac_path_SED" ac_path_SED_found=:;;
+*)
+  ac_count=0
+  $as_echo_n 0123456789 >"conftest.in"
+  while :
+  do
+    cat "conftest.in" "conftest.in" >"conftest.tmp"
+    mv "conftest.tmp" "conftest.in"
+    cp "conftest.in" "conftest.nl"
+    $as_echo '' >> "conftest.nl"
+    "$ac_path_SED" -f conftest.sed < "conftest.nl" >"conftest.out" 2>/dev/null || break
+    diff "conftest.out" "conftest.nl" >/dev/null 2>&1 || break
+    as_fn_arith $ac_count + 1 && ac_count=$as_val
+    if test $ac_count -gt ${ac_path_SED_max-0}; then
+      # Best one so far, save it but keep looking for a better one
+      ac_cv_path_SED="$ac_path_SED"
+      ac_path_SED_max=$ac_count
+    fi
+    # 10*(2^10) chars as input seems more than enough
+    test $ac_count -gt 10 && break
+  done
+  rm -f conftest.in conftest.tmp conftest.nl conftest.out;;
+esac
+
+      $ac_path_SED_found && break 3
+    done
+  done
+  done
+IFS=$as_save_IFS
+  if test -z "$ac_cv_path_SED"; then
+    as_fn_error $? "no acceptable sed could be found in \$PATH" "$LINENO" 5
+  fi
+else
+  ac_cv_path_SED=$SED
+fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_path_SED" >&5
+$as_echo "$ac_cv_path_SED" >&6; }
+ SED="$ac_cv_path_SED"
+  rm -f conftest.sed
+
 
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ANSI C header files" >&5
 $as_echo_n "checking for ANSI C header files... " >&6; }
@@ -6923,6 +6997,698 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
+  # Check for std::thread.  This does not work on mingw.
+  ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+
+
+
+
+ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+ax_pthread_ok=no
+
+# We used to check for pthread.h first, but this fails if pthread.h
+# requires special compiler flags (e.g. on Tru64 or Sequent).
+# It gets checked for in the link test anyway.
+
+# First of all, check if the user has set any of the PTHREAD_LIBS,
+# etcetera environment variables, and if threads linking works using
+# them:
+if test "x$PTHREAD_CFLAGS$PTHREAD_LIBS" != "x"; then
+        ax_pthread_save_CC="$CC"
+        ax_pthread_save_CFLAGS="$CFLAGS"
+        ax_pthread_save_LIBS="$LIBS"
+        if test "x$PTHREAD_CC" != "x"; then :
+  CC="$PTHREAD_CC"
+fi
+        CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+        LIBS="$PTHREAD_LIBS $LIBS"
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking for pthread_join using $CC $PTHREAD_CFLAGS $PTHREAD_LIBS" >&5
+$as_echo_n "checking for pthread_join using $CC $PTHREAD_CFLAGS $PTHREAD_LIBS... " >&6; }
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char pthread_join ();
+int
+main ()
+{
+return pthread_join ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_pthread_ok=yes
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_pthread_ok" >&5
+$as_echo "$ax_pthread_ok" >&6; }
+        if test "x$ax_pthread_ok" = "xno"; then
+                PTHREAD_LIBS=""
+                PTHREAD_CFLAGS=""
+        fi
+        CC="$ax_pthread_save_CC"
+        CFLAGS="$ax_pthread_save_CFLAGS"
+        LIBS="$ax_pthread_save_LIBS"
+fi
+
+# We must check for the threads library under a number of different
+# names; the ordering is very important because some systems
+# (e.g. DEC) have both -lpthread and -lpthreads, where one of the
+# libraries is broken (non-POSIX).
+
+# Create a list of thread flags to try.  Items starting with a "-" are
+# C compiler flags, and other items are library names, except for "none"
+# which indicates that we try without any flags at all, and "pthread-config"
+# which is a program returning the flags for the Pth emulation library.
+
+ax_pthread_flags="pthreads none -Kthread -pthread -pthreads -mthreads pthread --thread-safe -mt pthread-config"
+
+# The ordering *is* (sometimes) important.  Some notes on the
+# individual items follow:
+
+# pthreads: AIX (must check this before -lpthread)
+# none: in case threads are in libc; should be tried before -Kthread and
+#       other compiler flags to prevent continual compiler warnings
+# -Kthread: Sequent (threads in libc, but -Kthread needed for pthread.h)
+# -pthread: Linux/gcc (kernel threads), BSD/gcc (userland threads), Tru64
+#           (Note: HP C rejects this with "bad form for `-t' option")
+# -pthreads: Solaris/gcc (Note: HP C also rejects)
+# -mt: Sun Workshop C (may only link SunOS threads [-lthread], but it
+#      doesn't hurt to check since this sometimes defines pthreads and
+#      -D_REENTRANT too), HP C (must be checked before -lpthread, which
+#      is present but should not be used directly; and before -mthreads,
+#      because the compiler interprets this as "-mt" + "-hreads")
+# -mthreads: Mingw32/gcc, Lynx/gcc
+# pthread: Linux, etcetera
+# --thread-safe: KAI C++
+# pthread-config: use pthread-config program (for GNU Pth library)
+
+case $host_os in
+
+        freebsd*)
+
+        # -kthread: FreeBSD kernel threads (preferred to -pthread since SMP-able)
+        # lthread: LinuxThreads port on FreeBSD (also preferred to -pthread)
+
+        ax_pthread_flags="-kthread lthread $ax_pthread_flags"
+        ;;
+
+        hpux*)
+
+        # From the cc(1) man page: "[-mt] Sets various -D flags to enable
+        # multi-threading and also sets -lpthread."
+
+        ax_pthread_flags="-mt -pthread pthread $ax_pthread_flags"
+        ;;
+
+        openedition*)
+
+        # IBM z/OS requires a feature-test macro to be defined in order to
+        # enable POSIX threads at all, so give the user a hint if this is
+        # not set. (We don't define these ourselves, as they can affect
+        # other portions of the system API in unpredictable ways.)
+
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#            if !defined(_OPEN_THREADS) && !defined(_UNIX03_THREADS)
+             AX_PTHREAD_ZOS_MISSING
+#            endif
+
+_ACEOF
+if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
+  $EGREP "AX_PTHREAD_ZOS_MISSING" >/dev/null 2>&1; then :
+  { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: IBM z/OS requires -D_OPEN_THREADS or -D_UNIX03_THREADS to enable pthreads support." >&5
+$as_echo "$as_me: WARNING: IBM z/OS requires -D_OPEN_THREADS or -D_UNIX03_THREADS to enable pthreads support." >&2;}
+fi
+rm -f conftest*
+
+        ;;
+
+        solaris*)
+
+        # On Solaris (at least, for some versions), libc contains stubbed
+        # (non-functional) versions of the pthreads routines, so link-based
+        # tests will erroneously succeed. (N.B.: The stubs are missing
+        # pthread_cleanup_push, or rather a function called by this macro,
+        # so we could check for that, but who knows whether they'll stub
+        # that too in a future libc.)  So we'll check first for the
+        # standard Solaris way of linking pthreads (-mt -lpthread).
+
+        ax_pthread_flags="-mt,pthread pthread $ax_pthread_flags"
+        ;;
+esac
+
+# GCC generally uses -pthread, or -pthreads on some platforms (e.g. SPARC)
+
+if test "x$GCC" = "xyes"; then :
+  ax_pthread_flags="-pthread -pthreads $ax_pthread_flags"
+fi
+
+# The presence of a feature test macro requesting re-entrant function
+# definitions is, on some systems, a strong hint that pthreads support is
+# correctly enabled
+
+case $host_os in
+        darwin* | hpux* | linux* | osf* | solaris*)
+        ax_pthread_check_macro="_REENTRANT"
+        ;;
+
+        aix*)
+        ax_pthread_check_macro="_THREAD_SAFE"
+        ;;
+
+        *)
+        ax_pthread_check_macro="--"
+        ;;
+esac
+if test "x$ax_pthread_check_macro" = "x--"; then :
+  ax_pthread_check_cond=0
+else
+  ax_pthread_check_cond="!defined($ax_pthread_check_macro)"
+fi
+
+# Are we compiling with Clang?
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC is Clang" >&5
+$as_echo_n "checking whether $CC is Clang... " >&6; }
+if ${ax_cv_PTHREAD_CLANG+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_CLANG=no
+     # Note that Autoconf sets GCC=yes for Clang as well as GCC
+     if test "x$GCC" = "xyes"; then
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+/* Note: Clang 2.7 lacks __clang_[a-z]+__ */
+#            if defined(__clang__) && defined(__llvm__)
+             AX_PTHREAD_CC_IS_CLANG
+#            endif
+
+_ACEOF
+if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
+  $EGREP "AX_PTHREAD_CC_IS_CLANG" >/dev/null 2>&1; then :
+  ax_cv_PTHREAD_CLANG=yes
+fi
+rm -f conftest*
+
+     fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_CLANG" >&5
+$as_echo "$ax_cv_PTHREAD_CLANG" >&6; }
+ax_pthread_clang="$ax_cv_PTHREAD_CLANG"
+
+ax_pthread_clang_warning=no
+
+# Clang needs special handling, because older versions handle the -pthread
+# option in a rather... idiosyncratic way
+
+if test "x$ax_pthread_clang" = "xyes"; then
+
+        # Clang takes -pthread; it has never supported any other flag
+
+        # (Note 1: This will need to be revisited if a system that Clang
+        # supports has POSIX threads in a separate library.  This tends not
+        # to be the way of modern systems, but it's conceivable.)
+
+        # (Note 2: On some systems, notably Darwin, -pthread is not needed
+        # to get POSIX threads support; the API is always present and
+        # active.  We could reasonably leave PTHREAD_CFLAGS empty.  But
+        # -pthread does define _REENTRANT, and while the Darwin headers
+        # ignore this macro, third-party headers might not.)
+
+        PTHREAD_CFLAGS="-pthread"
+        PTHREAD_LIBS=
+
+        ax_pthread_ok=yes
+
+        # However, older versions of Clang make a point of warning the user
+        # that, in an invocation where only linking and no compilation is
+        # taking place, the -pthread option has no effect ("argument unused
+        # during compilation").  They expect -pthread to be passed in only
+        # when source code is being compiled.
+        #
+        # Problem is, this is at odds with the way Automake and most other
+        # C build frameworks function, which is that the same flags used in
+        # compilation (CFLAGS) are also used in linking.  Many systems
+        # supported by AX_PTHREAD require exactly this for POSIX threads
+        # support, and in fact it is often not straightforward to specify a
+        # flag that is used only in the compilation phase and not in
+        # linking.  Such a scenario is extremely rare in practice.
+        #
+        # Even though use of the -pthread flag in linking would only print
+        # a warning, this can be a nuisance for well-run software projects
+        # that build with -Werror.  So if the active version of Clang has
+        # this misfeature, we search for an option to squash it.
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether Clang needs flag to prevent \"argument unused\" warning when linking with -pthread" >&5
+$as_echo_n "checking whether Clang needs flag to prevent \"argument unused\" warning when linking with -pthread... " >&6; }
+if ${ax_cv_PTHREAD_CLANG_NO_WARN_FLAG+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_CLANG_NO_WARN_FLAG=unknown
+             # Create an alternate version of $ac_link that compiles and
+             # links in two steps (.c -> .o, .o -> exe) instead of one
+             # (.c -> exe), because the warning occurs only in the second
+             # step
+             ax_pthread_save_ac_link="$ac_link"
+             ax_pthread_sed='s/conftest\.\$ac_ext/conftest.$ac_objext/g'
+             ax_pthread_link_step=`$as_echo "$ac_link" | sed "$ax_pthread_sed"`
+             ax_pthread_2step_ac_link="($ac_compile) && (echo ==== >&5) && ($ax_pthread_link_step)"
+             ax_pthread_save_CFLAGS="$CFLAGS"
+             for ax_pthread_try in '' -Qunused-arguments -Wno-unused-command-line-argument unknown; do
+                if test "x$ax_pthread_try" = "xunknown"; then :
+  break
+fi
+                CFLAGS="-Werror -Wunknown-warning-option $ax_pthread_try -pthread $ax_pthread_save_CFLAGS"
+                ac_link="$ax_pthread_save_ac_link"
+                cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+int main(void){return 0;}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ac_link="$ax_pthread_2step_ac_link"
+                     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+int main(void){return 0;}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  break
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+             done
+             ac_link="$ax_pthread_save_ac_link"
+             CFLAGS="$ax_pthread_save_CFLAGS"
+             if test "x$ax_pthread_try" = "x"; then :
+  ax_pthread_try=no
+fi
+             ax_cv_PTHREAD_CLANG_NO_WARN_FLAG="$ax_pthread_try"
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" >&5
+$as_echo "$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" >&6; }
+
+        case "$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG" in
+                no | unknown) ;;
+                *) PTHREAD_CFLAGS="$ax_cv_PTHREAD_CLANG_NO_WARN_FLAG $PTHREAD_CFLAGS" ;;
+        esac
+
+fi # $ax_pthread_clang = yes
+
+if test "x$ax_pthread_ok" = "xno"; then
+for ax_pthread_try_flag in $ax_pthread_flags; do
+
+        case $ax_pthread_try_flag in
+                none)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether pthreads work without any flags" >&5
+$as_echo_n "checking whether pthreads work without any flags... " >&6; }
+                ;;
+
+                -mt,pthread)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether pthreads work with -mt -lpthread" >&5
+$as_echo_n "checking whether pthreads work with -mt -lpthread... " >&6; }
+                PTHREAD_CFLAGS="-mt"
+                PTHREAD_LIBS="-lpthread"
+                ;;
+
+                -*)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether pthreads work with $ax_pthread_try_flag" >&5
+$as_echo_n "checking whether pthreads work with $ax_pthread_try_flag... " >&6; }
+                PTHREAD_CFLAGS="$ax_pthread_try_flag"
+                ;;
+
+                pthread-config)
+                # Extract the first word of "pthread-config", so it can be a program name with args.
+set dummy pthread-config; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_ax_pthread_config+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$ax_pthread_config"; then
+  ac_cv_prog_ax_pthread_config="$ax_pthread_config" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_ax_pthread_config="yes"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+  test -z "$ac_cv_prog_ax_pthread_config" && ac_cv_prog_ax_pthread_config="no"
+fi
+fi
+ax_pthread_config=$ac_cv_prog_ax_pthread_config
+if test -n "$ax_pthread_config"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_pthread_config" >&5
+$as_echo "$ax_pthread_config" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+                if test "x$ax_pthread_config" = "xno"; then :
+  continue
+fi
+                PTHREAD_CFLAGS="`pthread-config --cflags`"
+                PTHREAD_LIBS="`pthread-config --ldflags` `pthread-config --libs`"
+                ;;
+
+                *)
+                { $as_echo "$as_me:${as_lineno-$LINENO}: checking for the pthreads library -l$ax_pthread_try_flag" >&5
+$as_echo_n "checking for the pthreads library -l$ax_pthread_try_flag... " >&6; }
+                PTHREAD_LIBS="-l$ax_pthread_try_flag"
+                ;;
+        esac
+
+        ax_pthread_save_CFLAGS="$CFLAGS"
+        ax_pthread_save_LIBS="$LIBS"
+        CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+        LIBS="$PTHREAD_LIBS $LIBS"
+
+        # Check for various functions.  We must include pthread.h,
+        # since some functions may be macros.  (On the Sequent, we
+        # need a special flag -Kthread to make this header compile.)
+        # We check for pthread_join because it is in -lpthread on IRIX
+        # while pthread_create is in libc.  We check for pthread_attr_init
+        # due to DEC craziness with -lpthreads.  We check for
+        # pthread_cleanup_push because it is one of the few pthread
+        # functions on Solaris that doesn't have a non-functional libc stub.
+        # We try pthread_create on general principles.
+
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pthread.h>
+#                       if $ax_pthread_check_cond
+#                        error "$ax_pthread_check_macro must be defined"
+#                       endif
+                        static void routine(void *a) { a = 0; }
+                        static void *start_routine(void *a) { return a; }
+int
+main ()
+{
+pthread_t th; pthread_attr_t attr;
+                        pthread_create(&th, 0, start_routine, 0);
+                        pthread_join(th, 0);
+                        pthread_attr_init(&attr);
+                        pthread_cleanup_push(routine, 0);
+                        pthread_cleanup_pop(0) /* ; */
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_pthread_ok=yes
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+
+        CFLAGS="$ax_pthread_save_CFLAGS"
+        LIBS="$ax_pthread_save_LIBS"
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_pthread_ok" >&5
+$as_echo "$ax_pthread_ok" >&6; }
+        if test "x$ax_pthread_ok" = "xyes"; then :
+  break
+fi
+
+        PTHREAD_LIBS=""
+        PTHREAD_CFLAGS=""
+done
+fi
+
+# Various other checks:
+if test "x$ax_pthread_ok" = "xyes"; then
+        ax_pthread_save_CFLAGS="$CFLAGS"
+        ax_pthread_save_LIBS="$LIBS"
+        CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+        LIBS="$PTHREAD_LIBS $LIBS"
+
+        # Detect AIX lossage: JOINABLE attribute is called UNDETACHED.
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking for joinable pthread attribute" >&5
+$as_echo_n "checking for joinable pthread attribute... " >&6; }
+if ${ax_cv_PTHREAD_JOINABLE_ATTR+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_JOINABLE_ATTR=unknown
+             for ax_pthread_attr in PTHREAD_CREATE_JOINABLE PTHREAD_CREATE_UNDETACHED; do
+                 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pthread.h>
+int
+main ()
+{
+int attr = $ax_pthread_attr; return attr /* ; */
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_cv_PTHREAD_JOINABLE_ATTR=$ax_pthread_attr; break
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+             done
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_JOINABLE_ATTR" >&5
+$as_echo "$ax_cv_PTHREAD_JOINABLE_ATTR" >&6; }
+        if test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xunknown" && \
+               test "x$ax_cv_PTHREAD_JOINABLE_ATTR" != "xPTHREAD_CREATE_JOINABLE" && \
+               test "x$ax_pthread_joinable_attr_defined" != "xyes"; then :
+
+cat >>confdefs.h <<_ACEOF
+#define PTHREAD_CREATE_JOINABLE $ax_cv_PTHREAD_JOINABLE_ATTR
+_ACEOF
+
+               ax_pthread_joinable_attr_defined=yes
+
+fi
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether more special flags are required for pthreads" >&5
+$as_echo_n "checking whether more special flags are required for pthreads... " >&6; }
+if ${ax_cv_PTHREAD_SPECIAL_FLAGS+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ax_cv_PTHREAD_SPECIAL_FLAGS=no
+             case $host_os in
+             solaris*)
+             ax_cv_PTHREAD_SPECIAL_FLAGS="-D_POSIX_PTHREAD_SEMANTICS"
+             ;;
+             esac
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_SPECIAL_FLAGS" >&5
+$as_echo "$ax_cv_PTHREAD_SPECIAL_FLAGS" >&6; }
+        if test "x$ax_cv_PTHREAD_SPECIAL_FLAGS" != "xno" && \
+               test "x$ax_pthread_special_flags_added" != "xyes"; then :
+  PTHREAD_CFLAGS="$ax_cv_PTHREAD_SPECIAL_FLAGS $PTHREAD_CFLAGS"
+               ax_pthread_special_flags_added=yes
+fi
+
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PTHREAD_PRIO_INHERIT" >&5
+$as_echo_n "checking for PTHREAD_PRIO_INHERIT... " >&6; }
+if ${ax_cv_PTHREAD_PRIO_INHERIT+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <pthread.h>
+int
+main ()
+{
+int i = PTHREAD_PRIO_INHERIT;
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+  ax_cv_PTHREAD_PRIO_INHERIT=yes
+else
+  ax_cv_PTHREAD_PRIO_INHERIT=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_PTHREAD_PRIO_INHERIT" >&5
+$as_echo "$ax_cv_PTHREAD_PRIO_INHERIT" >&6; }
+        if test "x$ax_cv_PTHREAD_PRIO_INHERIT" = "xyes" && \
+               test "x$ax_pthread_prio_inherit_defined" != "xyes"; then :
+
+$as_echo "#define HAVE_PTHREAD_PRIO_INHERIT 1" >>confdefs.h
+
+               ax_pthread_prio_inherit_defined=yes
+
+fi
+
+        CFLAGS="$ax_pthread_save_CFLAGS"
+        LIBS="$ax_pthread_save_LIBS"
+
+        # More AIX lossage: compile with *_r variant
+        if test "x$GCC" != "xyes"; then
+            case $host_os in
+                aix*)
+                case "x/$CC" in #(
+  x*/c89|x*/c89_128|x*/c99|x*/c99_128|x*/cc|x*/cc128|x*/xlc|x*/xlc_v6|x*/xlc128|x*/xlc128_v6) :
+    #handle absolute path differently from PATH based program lookup
+                     case "x$CC" in #(
+  x/*) :
+    if as_fn_executable_p ${CC}_r; then :
+  PTHREAD_CC="${CC}_r"
+fi ;; #(
+  *) :
+    for ac_prog in ${CC}_r
+do
+  # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_PTHREAD_CC+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$PTHREAD_CC"; then
+  ac_cv_prog_PTHREAD_CC="$PTHREAD_CC" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_PTHREAD_CC="$ac_prog"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+fi
+fi
+PTHREAD_CC=$ac_cv_prog_PTHREAD_CC
+if test -n "$PTHREAD_CC"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PTHREAD_CC" >&5
+$as_echo "$PTHREAD_CC" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+  test -n "$PTHREAD_CC" && break
+done
+test -n "$PTHREAD_CC" || PTHREAD_CC="$CC"
+ ;;
+esac ;; #(
+  *) :
+     ;;
+esac
+                ;;
+            esac
+        fi
+fi
+
+test -n "$PTHREAD_CC" || PTHREAD_CC="$CC"
+
+
+
+
+
+# Finally, execute ACTION-IF-FOUND/ACTION-IF-NOT-FOUND:
+if test "x$ax_pthread_ok" = "xyes"; then
+        threads=yes
+        :
+else
+        ax_pthread_ok=no
+        threads=no
+fi
+ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+
+  if test "$threads" = "yes"; then
+    save_LIBS="$LIBS"
+    LIBS="$PTHREAD_LIBS $LIBS"
+    save_CXXFLAGS="$CXXFLAGS"
+    CXXFLAGS="$PTHREAD_CFLAGS $save_CFLAGS"
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for std::thread" >&5
+$as_echo_n "checking for std::thread... " >&6; }
+if ${gdb_cv_cxx_std_thread+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <thread>
+      void callback() { }
+int
+main ()
+{
+std::thread t(callback);
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  gdb_cv_cxx_std_thread=yes
+else
+  gdb_cv_cxx_std_thread=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_cxx_std_thread" >&5
+$as_echo "$gdb_cv_cxx_std_thread" >&6; }
+    LIBS="$save_LIBS"
+    CXXFLAGS="$save_CXXFLAGS"
+  fi
+  if test $gdb_cv_cxx_std_thread = yes; then
+
+$as_echo "#define CXX_STD_THREAD 1" >>confdefs.h
+
+  fi
+  ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+
     { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sigsetjmp" >&5
 $as_echo_n "checking for sigsetjmp... " >&6; }
 if ${gdb_cv_func_sigsetjmp+:} false; then :
-- 
2.17.2

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
                   ` (7 preceding siblings ...)
  2019-05-18 21:00 ` [PATCH v2 8/8] Add maint set/show enable-threads Tom Tromey
@ 2019-05-19 13:59 ` Philippe Waroquiers
  2019-05-19 18:55   ` Tom Tromey
  8 siblings, 1 reply; 30+ messages in thread
From: Philippe Waroquiers @ 2019-05-19 13:59 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On Sat, 2019-05-18 at 15:00 -0600, Tom Tromey wrote:
> This is v2 of my patch series to demangle minimal symbol names in
> worker threads.
> 
> v1 was here:
> 
>     https://sourceware.org/ml/gdb-patches/2019-03/msg00211.html
> 
> I think this version addresses all the comments, in particular it adds
> a simple way to disable threading, and adds some configury.
> 
> Regression tested by the buildbot.  At one point this was failing most
> tests on the i686 builder, but I couldn't reproduce this with my own
> i686 build (on the GCC compile farm), and various checks involving
> running parts of the test suite on the buildbot using valgrind did not
> show anything.  In the end, the problem seems to have gone away
> (though there are many gdb.arch failures on i686 -- assertion failures
> in libopcodes).  So, I'm not totally sure what to make of this.
That looks like a nice startup speed improvement, will be much appreciated
when debugging big executables at work :).

HEAD:
time ../build_binutils-gdb/gdb/gdb --nx -batch -ex 'info var tructructruc' ./gdb/gdb
...
real	0m3.902s
user	0m3.068s
sys	0m0.836s

worker thread patch:
time ./gdb/gdb --nx -batch -ex 'info var tructructruc' ./gdb/gdb
...
real	0m2.379s
user	0m2.172s
sys	0m0.284s


I however do not observe much parallel CPU being used.
What type of operations will be mostly helped by parallel threads ?


One comment about 'maint set|show enable-threads' :
what is the reason to have this as a maintenance command ?
Also, maybe it would be better to have this setting being the
maximum nr of threads to use.  In some environments (e.g.
operational environments), one might want to limit the nr of threads
used by GDB.
So, maybe 'maint set enable-threads' might be replaced by something like:
   set max-gdb-threads (NUMBER | unlimited)
where unlimited means to use the max permitted by the implementation
(or similar wording as in std::thread::hardware_concurrency),
and where 0 will disable threading.

Note also that I see GDB is starting some threads by default
when guile is configured in, and 'maint set enable-threads off'
seems to not disable this type of threading.

Thanks

Philippe


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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-19 13:59 ` [PATCH v2 0/8] Demangle minimal symbol names in worker threads Philippe Waroquiers
@ 2019-05-19 18:55   ` Tom Tromey
  2019-05-21  0:35     ` Philippe Waroquiers
                       ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Tom Tromey @ 2019-05-19 18:55 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: Tom Tromey, gdb-patches

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> That looks like a nice startup speed improvement, will be much appreciated
Philippe> when debugging big executables at work :).

Philippe> I however do not observe much parallel CPU being used.
Philippe> What type of operations will be mostly helped by parallel threads ?

With this series, only demangling of minimal symbols is parallelized.
This does not dominate startup except in unusual situations, maybe a
very large C++ library that has no debuginfo and is unstripped.

Locally I only saw utilization of 1.5 CPUs or so -- so, not very
parallel yet.

The real point of this series is to flush out some of the lower-level
problems with having threading in gdb, so we can use it in more places.
For example, in this series, the SEGV handler stuff; but also this was
the reason for the big exception handling changes.

A better win, I think, would be to parallelize partial symbol reading in
dwarf2read.c.  This is a bit tricky... there are cross-CU references to
consider, and also the bcache seems like a point of contention.  I've
been wondering if, instead, it would make sense to change the DWARF
reader not to make partial symbols at all.

Philippe> One comment about 'maint set|show enable-threads' :
Philippe> what is the reason to have this as a maintenance command ?

I see it as a setting for gdb developers, not gdb users.  In fact, v1
didn't have this, but John wanted a way to disable threading for
debugging gdb.

Philippe> Also, maybe it would be better to have this setting being the
Philippe> maximum nr of threads to use.  In some environments (e.g.
Philippe> operational environments), one might want to limit the nr of threads
Philippe> used by GDB.

I think that most users don't know what most programs do under the hood;
nor should they need to.  I suppose in some extreme situation maybe
someone would want to do this, but disabling threading in this case
should be good enough.  Users like this should probably use gdbserver
instead though.

Philippe> Note also that I see GDB is starting some threads by default
Philippe> when guile is configured in, and 'maint set enable-threads off'
Philippe> seems to not disable this type of threading.

Guile creates threads by default.  In general gdb can't control the
threads made by either Guile or Python, because those can be made by
scripts created at runtime.

Tom

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-19 18:55   ` Tom Tromey
@ 2019-05-21  0:35     ` Philippe Waroquiers
  2019-05-21  7:35     ` Andrew Burgess
  2019-05-31  2:48     ` Tom Tromey
  2 siblings, 0 replies; 30+ messages in thread
From: Philippe Waroquiers @ 2019-05-21  0:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Sun, 2019-05-19 at 12:55 -0600, Tom Tromey wrote:
> Philippe> One comment about 'maint set|show enable-threads' :
> Philippe> what is the reason to have this as a maintenance command ?
> 
> I see it as a setting for gdb developers, not gdb users.  In fact, v1
> didn't have this, but John wanted a way to disable threading for
> debugging gdb.
> 
> Philippe> Also, maybe it would be better to have this setting being the
> Philippe> maximum nr of threads to use.  In some environments (e.g.
> Philippe> operational environments), one might want to limit the nr of threads
> Philippe> used by GDB.
> 
> I think that most users don't know what most programs do under the hood;
> nor should they need to.  I suppose in some extreme situation maybe
> someone would want to do this, but disabling threading in this case
> should be good enough.  Users like this should probably use gdbserver
> instead though.
It looks relatively common to let the end user control the level of parallelism.
E.g. xz, make, gnatmake, firefox all have a way to let the user tune
the parallelism (nr of processes or threads).

Philippe

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-19 18:55   ` Tom Tromey
  2019-05-21  0:35     ` Philippe Waroquiers
@ 2019-05-21  7:35     ` Andrew Burgess
  2019-05-21 15:45       ` Tom Tromey
  2019-05-31  2:48     ` Tom Tromey
  2 siblings, 1 reply; 30+ messages in thread
From: Andrew Burgess @ 2019-05-21  7:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Philippe Waroquiers, gdb-patches

* Tom Tromey <tom@tromey.com> [2019-05-19 12:55:04 -0600]:

> >>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> That looks like a nice startup speed improvement, will be much appreciated
> Philippe> when debugging big executables at work :).
> 
> Philippe> I however do not observe much parallel CPU being used.
> Philippe> What type of operations will be mostly helped by parallel threads ?
> 
> With this series, only demangling of minimal symbols is parallelized.
> This does not dominate startup except in unusual situations, maybe a
> very large C++ library that has no debuginfo and is unstripped.
> 
> Locally I only saw utilization of 1.5 CPUs or so -- so, not very
> parallel yet.
> 
> The real point of this series is to flush out some of the lower-level
> problems with having threading in gdb, so we can use it in more places.
> For example, in this series, the SEGV handler stuff; but also this was
> the reason for the big exception handling changes.
> 
> A better win, I think, would be to parallelize partial symbol reading in
> dwarf2read.c.  This is a bit tricky... there are cross-CU references to
> consider, and also the bcache seems like a point of contention.  I've
> been wondering if, instead, it would make sense to change the DWARF
> reader not to make partial symbols at all.
> 
> Philippe> One comment about 'maint set|show enable-threads' :
> Philippe> what is the reason to have this as a maintenance command ?
> 
> I see it as a setting for gdb developers, not gdb users.  In fact, v1
> didn't have this, but John wanted a way to disable threading for
> debugging gdb.
> 
> Philippe> Also, maybe it would be better to have this setting being the
> Philippe> maximum nr of threads to use.  In some environments (e.g.
> Philippe> operational environments), one might want to limit the nr of threads
> Philippe> used by GDB.
> 
> I think that most users don't know what most programs do under the hood;
> nor should they need to.  I suppose in some extreme situation maybe
> someone would want to do this, but disabling threading in this case
> should be good enough.  Users like this should probably use gdbserver
> instead though.

Sorry for being really slow, but how does gdbserver help here?  The
threads are to help GDB parse the symbols/debug information from the
ELF, right?  In most cases GDB will still be parsing the symbols/debug
from the ELF at the GDB end, not the gdbserver end.

> 
> Philippe> Note also that I see GDB is starting some threads by default
> Philippe> when guile is configured in, and 'maint set enable-threads off'
> Philippe> seems to not disable this type of threading.
> 
> Guile creates threads by default.  In general gdb can't control the
> threads made by either Guile or Python, because those can be made by
> scripts created at runtime.

Sure, but I think there's a difference between threads the user has
specifically _asked_ GDB to create (through a script) and threads that
are started to support some builtin GDB feature.

Most users don't build their own GDB (I assume) so don't choose to
enable guile support or not, so I think it probably is worth
mentioning that the guile threads can't be disabled with this flag.

Thanks,
Andrew

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-21  7:35     ` Andrew Burgess
@ 2019-05-21 15:45       ` Tom Tromey
  2019-05-21 16:21         ` Andrew Burgess
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2019-05-21 15:45 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Tom Tromey, Philippe Waroquiers, gdb-patches

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

>> I think that most users don't know what most programs do under the hood;
>> nor should they need to.  I suppose in some extreme situation maybe
>> someone would want to do this, but disabling threading in this case
>> should be good enough.  Users like this should probably use gdbserver
>> instead though.

Andrew> Sorry for being really slow, but how does gdbserver help here?  The
Andrew> threads are to help GDB parse the symbols/debug information from the
Andrew> ELF, right?  In most cases GDB will still be parsing the symbols/debug
Andrew> from the ELF at the GDB end, not the gdbserver end.

My understanding is that the scenario is wanting to limit threads so
that one can run gdb on some resource-constrained machine.  In this
situation it's better to run gdbserver on the limited machine and run
gdb on one's desktop.

>> Guile creates threads by default.  In general gdb can't control the
>> threads made by either Guile or Python, because those can be made by
>> scripts created at runtime.

Andrew> Sure, but I think there's a difference between threads the user has
Andrew> specifically _asked_ GDB to create (through a script) and threads that
Andrew> are started to support some builtin GDB feature.

It's mildly incorrect to say that the user asked for these threads.  The
guile ones are created at startup regardless.  Also, due to
auto-loading, the user may not be aware of precisely what Python is
running in gdb or what it is doing; though in this case it does seem
pretty unlikely for threading to be used.

I'll change the setting to control the number of threads.

Tom

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-21 15:45       ` Tom Tromey
@ 2019-05-21 16:21         ` Andrew Burgess
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Burgess @ 2019-05-21 16:21 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Philippe Waroquiers, gdb-patches

* Tom Tromey <tom@tromey.com> [2019-05-21 11:45:21 -0400]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> >> I think that most users don't know what most programs do under the hood;
> >> nor should they need to.  I suppose in some extreme situation maybe
> >> someone would want to do this, but disabling threading in this case
> >> should be good enough.  Users like this should probably use gdbserver
> >> instead though.
> 
> Andrew> Sorry for being really slow, but how does gdbserver help here?  The
> Andrew> threads are to help GDB parse the symbols/debug information from the
> Andrew> ELF, right?  In most cases GDB will still be parsing the symbols/debug
> Andrew> from the ELF at the GDB end, not the gdbserver end.
> 
> My understanding is that the scenario is wanting to limit threads so
> that one can run gdb on some resource-constrained machine.  In this
> situation it's better to run gdbserver on the limited machine and run
> gdb on one's desktop.

Told you I was being slow :) that makes perfect sense.

Thanks,
Andrew

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

* Re: [PATCH v2 8/8] Add maint set/show enable-threads
  2019-05-18 21:00 ` [PATCH v2 8/8] Add maint set/show enable-threads Tom Tromey
@ 2019-05-22  5:01   ` Eli Zaretskii
  2019-05-26 20:46     ` Tom Tromey
  0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2019-05-22  5:01 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Sat, 18 May 2019 15:00:10 -0600
> 
> This adds maint commands to control whether gdb is able to use
> threads.
> 
> gdb/ChangeLog
> 2019-05-18  Tom Tromey  <tom@tromey.com>
> 
> 	* NEWS: Add entry.
> 	* maint.c (_initialize_maint_cmds): Add "enable-threads" maint
> 	commands.
> 
> gdb/doc/ChangeLog
> 2019-05-18  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.texinfo (Maintenance Commands): Document new maint
> 	commands.

Thanks.

> +maint set enable-threads
> +maint show enable-threads
> +  Control whether GDB can use threads.  The default is "on".

I'd suggest to say what will threads be used for.

> +  add_setshow_boolean_cmd ("enable-threads", class_maintenance,
> +			   &gdb::enable_threads, _("\
> +Set whether gdb can use multiple threads."), _("\
> +Show whether gdb can use multiple threads."), _("\
> +If enabled, gdb will use multiple threads when possible."),

maybe also here.

The documentation parts are OK with this nit fixed.

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

* Re: [PATCH v2 8/8] Add maint set/show enable-threads
  2019-05-22  5:01   ` Eli Zaretskii
@ 2019-05-26 20:46     ` Tom Tromey
  2019-05-27  2:32       ` Eli Zaretskii
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2019-05-26 20:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches

>> +maint set enable-threads
>> +maint show enable-threads
>> +  Control whether GDB can use threads.  The default is "on".

Eli> I'd suggest to say what will threads be used for.

In NEWS I guess it's ok, but elsewhere I'd prefer to be a bit vague
about the precise use of threads.  That way the documentation won't go
stale if we add more uses.

Tom

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

* Re: [PATCH v2 8/8] Add maint set/show enable-threads
  2019-05-26 20:46     ` Tom Tromey
@ 2019-05-27  2:32       ` Eli Zaretskii
  0 siblings, 0 replies; 30+ messages in thread
From: Eli Zaretskii @ 2019-05-27  2:32 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sun, 26 May 2019 14:46:12 -0600
> 
> >> +maint set enable-threads
> >> +maint show enable-threads
> >> +  Control whether GDB can use threads.  The default is "on".
> 
> Eli> I'd suggest to say what will threads be used for.
> 
> In NEWS I guess it's ok, but elsewhere I'd prefer to be a bit vague
> about the precise use of threads.  That way the documentation won't go
> stale if we add more uses.

That's okay, we can be vague.  How about saying that "threads are used
for some CPU-extensive processing, such as ..." and give one example?

My point is that leaving it entirely unsaid is way too vague,
especially for readers who might be debugging programs that themselves
use threads.

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-19 18:55   ` Tom Tromey
  2019-05-21  0:35     ` Philippe Waroquiers
  2019-05-21  7:35     ` Andrew Burgess
@ 2019-05-31  2:48     ` Tom Tromey
  2019-05-31 17:13       ` Philippe Waroquiers
  2 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2019-05-31  2:48 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Philippe Waroquiers, gdb-patches

Tom> Locally I only saw utilization of 1.5 CPUs or so -- so, not very
Tom> parallel yet.

I found the "mutrace" tool and it looks like the new mutex is heavily
contended.  This is pretty much what I suspected, but still unfortunate.

$ mutrace -d ./gdb -nx -batch ./gdb
[...]
 Mutex #   Locked  Changed    Cont. tot.Time[ms] avg.Time[ms] max.Time[ms]  Flags
       2  3714780    45723    26425      975.514        0.000       22.141 M-.--.

I'm not really sure what to do about it.

I do wonder if there are many duplicates when demangling minsym names.
One idea would be to demangle in parallel and only then fill in the
table.

A similar problem will occur when/if we want to parallelize psymbol
reading.  Those use the demangle hash table, but also the bcache.  (The
bcache is important, but I found recently that the demangling isn't
really needed -- the names are demangled due to the physname work, and
the demangle hash table is only used to ensure that the bcache gives
good results.  Removing the demangling step here can also provide a nice
speedup...)

Tom

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

* Re: [PATCH v2 0/8] Demangle minimal symbol names in worker threads
  2019-05-31  2:48     ` Tom Tromey
@ 2019-05-31 17:13       ` Philippe Waroquiers
  2019-09-29  0:35         ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 30+ messages in thread
From: Philippe Waroquiers @ 2019-05-31 17:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Thu, 2019-05-30 at 20:48 -0600, Tom Tromey wrote:
> Tom> Locally I only saw utilization of 1.5 CPUs or so -- so, not very
> Tom> parallel yet.
> 
> I found the "mutrace" tool and it looks like the new mutex is heavily
> contended.  This is pretty much what I suspected, but still unfortunate.
> 
> $ mutrace -d ./gdb -nx -batch ./gdb
> [...]
>  Mutex #   Locked  Changed    Cont. tot.Time[ms] avg.Time[ms] max.Time[ms]  Flags
>        2  3714780    45723    26425      975.514        0.000       22.141 M-.--.
> 
> I'm not really sure what to do about it.
Use lockless data structures (e.g. concurrencykit.org) ?

But I am quite sure such lockless libraries/data structures will not
be available on all supported GDB platforms, so it might not be very
cheap to change and maintain GDB to use e.g. lockless hash tables
on some platforms and some other hash tables in less common platforms.

Alternatively, develop a few GDB specific lockless data structures
that are protected by a mutex and/or are single threads on less
common platforms ?

> 
> I do wonder if there are many duplicates when demangling minsym names.
> One idea would be to demangle in parallel and only then fill in the
> table.
> 
> A similar problem will occur when/if we want to parallelize psymbol
> reading.  Those use the demangle hash table, but also the bcache.  (The
> bcache is important, but I found recently that the demangling isn't
> really needed -- the names are demangled due to the physname work, and
> the demangle hash table is only used to ensure that the bcache gives
> good results.  Removing the demangling step here can also provide a nice
> speedup...)
> 
> Tom

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

* [PATCH] Don't use the mutex for each symbol_set_names call
  2019-05-31 17:13       ` Philippe Waroquiers
@ 2019-09-29  0:35         ` Christian Biesinger via gdb-patches
  2019-09-30 14:18           ` Tom Tromey
  0 siblings, 1 reply; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-09-29  0:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

[So how's this version to avoid the lock overhead? In case threading
doesn't work correctly for some reason, this is in response to
https://sourceware.org/ml/gdb-patches/2019-05/msg00753.html and
applies on top of that patchset.

Also available, rebased to trunk, on
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/cbiesinger/parallel-minsyms-mutex
]

This avoids the lock contention that was seen with tromey's patch (it
moves it to a once- per-thread lock call from a once-per-symbol call).

It speeds up "attach to Chrome's content_shell binary" from 44 sec -> 30
sec (32%) (compared to GDB trunk), or from 37 sec compared to this
patchset before this patch.

gdb/ChangeLog:

2019-09-28  Christian Biesinger  <cbiesinger@google.com>

	* minsyms.c (minimal_symbol_reader::install): Only do
	demangling on the background thread; still call
	symbol_set_names on the main thread.
	* symtab.c (symbol_find_demangled_name): Make public,
	so that minsyms.c can call it.
	(symbol_set_names): Remove mutex. Avoid demangle call
	if the demangled name is already set.
	* symtab.h (symbol_find_demangled_name): Make public.
---
 gdb/gdbsupport/parallel-for.h |   5 +-
 gdb/minsyms.c                 |  48 +++++++---
 gdb/symtab.c                  | 162 +++++++++++++++-------------------
 gdb/symtab.h                  |  10 +++
 4 files changed, 123 insertions(+), 102 deletions(-)

diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
index d63a8c4e82..9041435d7a 100644
--- a/gdb/gdbsupport/parallel-for.h
+++ b/gdb/gdbsupport/parallel-for.h
@@ -38,8 +38,8 @@ extern int max_threads;
    iterators.  For each element, it calls the callback function.  The
    work may or may not be done by separate threads.  */
 
-template<class RandomIt, class UnaryFunction>
-void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
+template<class RandomIt, class UnaryFunction, class PerThreadFinishFunction>
+void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f, PerThreadFinishFunction per_thread_finish)
 {
   auto body = [&] (RandomIt start, RandomIt end)
     {
@@ -53,6 +53,7 @@ void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
 	    /* Just ignore exceptions.  Each item here must be
 	       independent.  */
 	  }
+      per_thread_finish (start, end);
     };
 
 #if CXX_STD_THREAD
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 10e3c8a548..4802b0bea0 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -55,6 +55,20 @@
 #include "safe-ctype.h"
 #include "gdbsupport/parallel-for.h"
 
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
+
+#if CXX_STD_THREAD
+/* Mutex that is used when modifying or accessing the demangled hash
+   table.  This is a global mutex simply because the only current
+   multi-threaded user of the hash table does not process multiple
+   objfiles in parallel.  The mutex could easily live on the per-BFD
+   object, but this approach avoids using extra memory when it is not
+   needed.  */
+static std::mutex demangled_mutex;
+#endif
+
 /* See minsyms.h.  */
 
 bool
@@ -1340,17 +1354,29 @@ minimal_symbol_reader::install ()
 
       msymbols = m_objfile->per_bfd->msymbols.get ();
       gdb::parallel_for_each
-	(&msymbols[0], &msymbols[mcount],
-	 [&] (minimal_symbol &msym)
-	 {
-	   if (!msym.name_set)
-	     {
-	       symbol_set_names (&msym, msym.name,
-				 strlen (msym.name), 0,
-				 m_objfile->per_bfd);
-	       msym.name_set = 1;
-	     }
-	 });
+	(&msymbols[0], &msymbols[mcount], [&] (minimal_symbol &msym)
+	  {
+	    if (!msym.name_set)
+	      {
+	        if (msym.language != language_ada)
+		  {
+		    /* This will be freed later, by symbol_set_names.  */
+		    char* demangled_name = symbol_find_demangled_name (&msym,
+								       msym.name);
+		    symbol_set_demangled_name (&msym, demangled_name,
+					       &m_objfile->per_bfd->storage_obstack);
+		    msym.name_set = 1;
+		  }
+	      }
+	  },
+	  [&] (minimal_symbol *first, minimal_symbol* last) {
+	    std::lock_guard<std::mutex> guard (demangled_mutex);
+	    for (; first < last; ++first) {
+	      symbol_set_names (first, first->name,
+		  strlen (first->name), 0,
+		  m_objfile->per_bfd);
+	    }
+	  });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8a7ac6f036..3600e0b0ff 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,9 +69,6 @@
 #include "arch-utils.h"
 #include <algorithm>
 #include "gdbsupport/pathstuff.h"
-#if CXX_STD_THREAD
-#include <mutex>
-#endif
 
 /* Forward declarations for local functions.  */
 
@@ -713,16 +710,6 @@ symbol_set_language (struct general_symbol_info *gsymbol,
 
 /* Functions to initialize a symbol's mangled name.  */
 
-#if CXX_STD_THREAD
-/* Mutex that is used when modifying or accessing the demangled hash
-   table.  This is a global mutex simply because the only current
-   multi-threaded user of the hash table does not process multiple
-   objfiles in parallel.  The mutex could easily live on the per-BFD
-   object, but this approach avoids using extra memory when it is not
-   needed.  */
-static std::mutex demangled_mutex;
-#endif
-
 /* Objects of this type are stored in the demangled name hash table.  */
 struct demangled_name_entry
 {
@@ -772,13 +759,9 @@ create_demangled_names_hash (struct objfile_per_bfd_storage *per_bfd)
      NULL, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -867,78 +850,79 @@ symbol_set_names (struct general_symbol_info *gsymbol,
 
   struct demangled_name_entry *found_entry;
 
-  {
-#if CXX_STD_THREAD
-    std::lock_guard<std::mutex> guard (demangled_mutex);
-#endif
-
-    if (per_bfd->demangled_names_hash == NULL)
-      create_demangled_names_hash (per_bfd);
-
-    entry.mangled = linkage_name_copy;
-    slot = ((struct demangled_name_entry **)
-	    htab_find_slot (per_bfd->demangled_names_hash.get (),
-			    &entry, INSERT));
-
-    /* If this name is not in the hash table, add it.  */
-    if (*slot == NULL
-	/* A C version of the symbol may have already snuck into the table.
-	   This happens to, e.g., main.init (__go_init_main).  Cope.  */
-	|| (gsymbol->language == language_go
-	    && (*slot)->demangled[0] == '\0'))
-      {
-	char *demangled_name_ptr
-	  = symbol_find_demangled_name (gsymbol, linkage_name_copy);
-	gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
-	int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
-
-	/* Suppose we have demangled_name==NULL, copy_name==0, and
-	   linkage_name_copy==linkage_name.  In this case, we already have the
-	   mangled name saved, and we don't have a demangled name.  So,
-	   you might think we could save a little space by not recording
-	   this in the hash table at all.
-	   
-	   It turns out that it is actually important to still save such
-	   an entry in the hash table, because storing this name gives
-	   us better bcache hit rates for partial symbols.  */
-	if (!copy_name && linkage_name_copy == linkage_name)
-	  {
-	    *slot
-	      = ((struct demangled_name_entry *)
-		 obstack_alloc (&per_bfd->storage_obstack,
-				offsetof (struct demangled_name_entry, demangled)
-				+ demangled_len + 1));
-	    (*slot)->mangled = linkage_name;
-	  }
-	else
-	  {
-	    char *mangled_ptr;
-
-	    /* If we must copy the mangled name, put it directly after
-	       the demangled name so we can have a single
-	       allocation.  */
-	    *slot
-	      = ((struct demangled_name_entry *)
-		 obstack_alloc (&per_bfd->storage_obstack,
-				offsetof (struct demangled_name_entry, demangled)
-				+ len + demangled_len + 2));
-	    mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
-	    strcpy (mangled_ptr, linkage_name_copy);
-	    (*slot)->mangled = mangled_ptr;
-	  }
-	(*slot)->language = gsymbol->language;
+  if (per_bfd->demangled_names_hash == NULL)
+    create_demangled_names_hash (per_bfd);
+
+  entry.mangled = linkage_name_copy;
+  slot = ((struct demangled_name_entry **)
+          htab_find_slot (per_bfd->demangled_names_hash.get (),
+      		    &entry, INSERT));
+
+  /* If this name is not in the hash table, add it.  */
+  if (*slot == NULL
+      /* A C version of the symbol may have already snuck into the table.
+         This happens to, e.g., main.init (__go_init_main).  Cope.  */
+      || (gsymbol->language == language_go
+          && (*slot)->demangled[0] == '\0'))
+    {
+      /* The const_cast is safe because the only reason it is already initialized
+         is if we purposefully set it from a background thread to avoid doing the
+         work here. However, it is still allocated from the heap and needs to
+         be freed by us, just like if we called symbol_find_demangled_name
+         here.  */
+      char *demangled_name_ptr
+        = gsymbol->language_specific.demangled_name
+        ? const_cast<char*>(gsymbol->language_specific.demangled_name)
+        : symbol_find_demangled_name (gsymbol, linkage_name_copy);
+      gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
+      int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
+
+      /* Suppose we have demangled_name==NULL, copy_name==0, and
+         linkage_name_copy==linkage_name.  In this case, we already have the
+         mangled name saved, and we don't have a demangled name.  So,
+         you might think we could save a little space by not recording
+         this in the hash table at all.
+         
+         It turns out that it is actually important to still save such
+         an entry in the hash table, because storing this name gives
+         us better bcache hit rates for partial symbols.  */
+      if (!copy_name && linkage_name_copy == linkage_name)
+        {
+          *slot
+            = ((struct demangled_name_entry *)
+      	 obstack_alloc (&per_bfd->storage_obstack,
+      			offsetof (struct demangled_name_entry, demangled)
+      			+ demangled_len + 1));
+          (*slot)->mangled = linkage_name;
+        }
+      else
+        {
+          char *mangled_ptr;
+
+          /* If we must copy the mangled name, put it directly after
+             the demangled name so we can have a single
+             allocation.  */
+          *slot
+            = ((struct demangled_name_entry *)
+      	 obstack_alloc (&per_bfd->storage_obstack,
+      			offsetof (struct demangled_name_entry, demangled)
+      			+ len + demangled_len + 2));
+          mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
+          strcpy (mangled_ptr, linkage_name_copy);
+          (*slot)->mangled = mangled_ptr;
+        }
+      (*slot)->language = gsymbol->language;
 
-	if (demangled_name != NULL)
-	  strcpy ((*slot)->demangled, demangled_name.get ());
-	else
-	  (*slot)->demangled[0] = '\0';
-      }
-    else if (gsymbol->language == language_unknown
-	     || gsymbol->language == language_auto)
-      gsymbol->language = (*slot)->language;
+      if (demangled_name != NULL)
+        strcpy ((*slot)->demangled, demangled_name.get ());
+      else
+        (*slot)->demangled[0] = '\0';
+    }
+  else if (gsymbol->language == language_unknown
+           || gsymbol->language == language_auto)
+    gsymbol->language = (*slot)->language;
 
-    found_entry = *slot;
-  }
+  found_entry = *slot;
 
   gsymbol->name = found_entry->mangled;
   if (found_entry->demangled[0] != '\0')
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c3918a85af..17903df92d 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -483,6 +483,16 @@ extern void symbol_set_language (struct general_symbol_info *symbol,
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must
-- 
2.23.0.444.g18eeb5a265-goog

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

* Re: [PATCH] Don't use the mutex for each symbol_set_names call
  2019-09-29  0:35         ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
@ 2019-09-30 14:18           ` Tom Tromey
  2019-09-30 16:55             ` Christian Biesinger via gdb-patches
  2019-09-30 21:45             ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
  0 siblings, 2 replies; 30+ messages in thread
From: Tom Tromey @ 2019-09-30 14:18 UTC (permalink / raw)
  To: Christian Biesinger via gdb-patches; +Cc: Christian Biesinger

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> Also available, rebased to trunk, on
Christian> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/cbiesinger/parallel-minsyms-mutex
Christian> ]

I didn't read the patch yet, but this weekend I did rebase my branch and
fixed most of the review issues.  So, you may want to consider rebasing
on that.

Tom

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

* [PATCH] Don't use the mutex for each symbol_set_names call
  2019-09-30 14:18           ` Tom Tromey
@ 2019-09-30 16:55             ` Christian Biesinger via gdb-patches
  2019-10-02 17:18               ` Tom Tromey
  2019-09-30 21:45             ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
  1 sibling, 1 reply; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-09-30 16:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

[Thanks Tom -- here's a rebased version, with also a small improvement
to parallel-for.h]

This avoids the lock contention that was seen with tromey's patch (it
moves it to a once- per-thread lock call from a once-per-symbol call).

It speeds up "attach to Chrome's content_shell binary" from 44 sec -> 30
sec (32%) (compared to GDB trunk), or from 37 sec compared to this
patchset before this patch.

gdb/ChangeLog:

2019-09-28  Christian Biesinger  <cbiesinger@google.com>

	* gdbsupport/parallel-for.h (parallel_for_each): Add a way to
	run a function on each thread after the elements are processed,
	to "finish up" computations.
	* minsyms.c (minimal_symbol_reader::install): Only do
	demangling on the background thread; still call
	symbol_set_names on the main thread.
	* symtab.c (symbol_find_demangled_name): Make public,
	so that minsyms.c can call it.
	(symbol_set_names): Remove mutex. Avoid demangle call
	if the demangled name is already set.
	* symtab.h (symbol_find_demangled_name): Make public.
---
 gdb/gdbsupport/parallel-for.h |  12 ++-
 gdb/minsyms.c                 |  48 +++++++---
 gdb/symtab.c                  | 162 +++++++++++++++-------------------
 gdb/symtab.h                  |  10 +++
 4 files changed, 130 insertions(+), 102 deletions(-)

diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
index e8bfced475..e5b8d33989 100644
--- a/gdb/gdbsupport/parallel-for.h
+++ b/gdb/gdbsupport/parallel-for.h
@@ -36,13 +36,20 @@ namespace gdb
 
 extern int max_threads;
 
+template <class It>
+void DoNothing(It a, It b)
+{
+}
+
 /* A very simple "parallel for".  This iterates over the elements
    given by the range of iterators, which must be random access
    iterators.  For each element, it calls the callback function.  The
    work may or may not be done by separate threads.  */
 
-template<class RandomIt, class UnaryFunction>
-void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
+template<class RandomIt, class UnaryFunction, class PerThreadFinishFunction>
+void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f,
+			PerThreadFinishFunction per_thread_finish
+			  = DoNothing<RandomIt>)
 {
   auto body = [&] (RandomIt start, RandomIt end)
     {
@@ -60,6 +67,7 @@ void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
 	    /* Just ignore exceptions.  Each item here must be
 	       independent.  */
 	  }
+      per_thread_finish (start, end);
     };
 
 #if CXX_STD_THREAD
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 10e3c8a548..4802b0bea0 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -55,6 +55,20 @@
 #include "safe-ctype.h"
 #include "gdbsupport/parallel-for.h"
 
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
+
+#if CXX_STD_THREAD
+/* Mutex that is used when modifying or accessing the demangled hash
+   table.  This is a global mutex simply because the only current
+   multi-threaded user of the hash table does not process multiple
+   objfiles in parallel.  The mutex could easily live on the per-BFD
+   object, but this approach avoids using extra memory when it is not
+   needed.  */
+static std::mutex demangled_mutex;
+#endif
+
 /* See minsyms.h.  */
 
 bool
@@ -1340,17 +1354,29 @@ minimal_symbol_reader::install ()
 
       msymbols = m_objfile->per_bfd->msymbols.get ();
       gdb::parallel_for_each
-	(&msymbols[0], &msymbols[mcount],
-	 [&] (minimal_symbol &msym)
-	 {
-	   if (!msym.name_set)
-	     {
-	       symbol_set_names (&msym, msym.name,
-				 strlen (msym.name), 0,
-				 m_objfile->per_bfd);
-	       msym.name_set = 1;
-	     }
-	 });
+	(&msymbols[0], &msymbols[mcount], [&] (minimal_symbol &msym)
+	  {
+	    if (!msym.name_set)
+	      {
+	        if (msym.language != language_ada)
+		  {
+		    /* This will be freed later, by symbol_set_names.  */
+		    char* demangled_name = symbol_find_demangled_name (&msym,
+								       msym.name);
+		    symbol_set_demangled_name (&msym, demangled_name,
+					       &m_objfile->per_bfd->storage_obstack);
+		    msym.name_set = 1;
+		  }
+	      }
+	  },
+	  [&] (minimal_symbol *first, minimal_symbol* last) {
+	    std::lock_guard<std::mutex> guard (demangled_mutex);
+	    for (; first < last; ++first) {
+	      symbol_set_names (first, first->name,
+		  strlen (first->name), 0,
+		  m_objfile->per_bfd);
+	    }
+	  });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8adcff7cf2..3600e0b0ff 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,9 +69,6 @@
 #include "arch-utils.h"
 #include <algorithm>
 #include "gdbsupport/pathstuff.h"
-#if CXX_STD_THREAD
-#include <mutex>
-#endif
 
 /* Forward declarations for local functions.  */
 
@@ -713,16 +710,6 @@ symbol_set_language (struct general_symbol_info *gsymbol,
 
 /* Functions to initialize a symbol's mangled name.  */
 
-#if CXX_STD_THREAD
-/* Mutex that is used when modifying or accessing the demangled hash
-   table.  This is a global mutex simply because the only current
-   multi-threaded user of the hash table does not process multiple
-   objfiles in parallel.  The mutex could easily live on the per-BFD
-   object, but this approach avoids using extra memory when it is not
-   needed.  */
-static std::mutex demangled_mutex;
-#endif
-
 /* Objects of this type are stored in the demangled name hash table.  */
 struct demangled_name_entry
 {
@@ -772,13 +759,9 @@ create_demangled_names_hash (struct objfile_per_bfd_storage *per_bfd)
      NULL, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -867,78 +850,79 @@ symbol_set_names (struct general_symbol_info *gsymbol,
 
   struct demangled_name_entry *found_entry;
 
-  {
-#if CXX_STD_THREAD
-    std::lock_guard<std::mutex> guard (demangled_mutex);
-#endif
-
-    if (per_bfd->demangled_names_hash == NULL)
-      create_demangled_names_hash (per_bfd);
-
-    entry.mangled = linkage_name_copy;
-    slot = ((struct demangled_name_entry **)
-	    htab_find_slot (per_bfd->demangled_names_hash.get (),
-			    &entry, INSERT));
-
-    /* If this name is not in the hash table, add it.  */
-    if (*slot == NULL
-	/* A C version of the symbol may have already snuck into the table.
-	   This happens to, e.g., main.init (__go_init_main).  Cope.  */
-	|| (gsymbol->language == language_go
-	    && (*slot)->demangled[0] == '\0'))
-      {
-	char *demangled_name_ptr
-	  = symbol_find_demangled_name (gsymbol, linkage_name_copy);
-	gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
-	int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
-
-	/* Suppose we have demangled_name==NULL, copy_name==0, and
-	   linkage_name_copy==linkage_name.  In this case, we already have the
-	   mangled name saved, and we don't have a demangled name.  So,
-	   you might think we could save a little space by not recording
-	   this in the hash table at all.
-	 
-	   It turns out that it is actually important to still save such
-	   an entry in the hash table, because storing this name gives
-	   us better bcache hit rates for partial symbols.  */
-	if (!copy_name && linkage_name_copy == linkage_name)
-	  {
-	    *slot
-	      = ((struct demangled_name_entry *)
-		 obstack_alloc (&per_bfd->storage_obstack,
-				offsetof (struct demangled_name_entry, demangled)
-				+ demangled_len + 1));
-	    (*slot)->mangled = linkage_name;
-	  }
-	else
-	  {
-	    char *mangled_ptr;
-
-	    /* If we must copy the mangled name, put it directly after
-	       the demangled name so we can have a single
-	       allocation.  */
-	    *slot
-	      = ((struct demangled_name_entry *)
-		 obstack_alloc (&per_bfd->storage_obstack,
-				offsetof (struct demangled_name_entry, demangled)
-				+ len + demangled_len + 2));
-	    mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
-	    strcpy (mangled_ptr, linkage_name_copy);
-	    (*slot)->mangled = mangled_ptr;
-	  }
-	(*slot)->language = gsymbol->language;
+  if (per_bfd->demangled_names_hash == NULL)
+    create_demangled_names_hash (per_bfd);
+
+  entry.mangled = linkage_name_copy;
+  slot = ((struct demangled_name_entry **)
+          htab_find_slot (per_bfd->demangled_names_hash.get (),
+      		    &entry, INSERT));
+
+  /* If this name is not in the hash table, add it.  */
+  if (*slot == NULL
+      /* A C version of the symbol may have already snuck into the table.
+         This happens to, e.g., main.init (__go_init_main).  Cope.  */
+      || (gsymbol->language == language_go
+          && (*slot)->demangled[0] == '\0'))
+    {
+      /* The const_cast is safe because the only reason it is already initialized
+         is if we purposefully set it from a background thread to avoid doing the
+         work here. However, it is still allocated from the heap and needs to
+         be freed by us, just like if we called symbol_find_demangled_name
+         here.  */
+      char *demangled_name_ptr
+        = gsymbol->language_specific.demangled_name
+        ? const_cast<char*>(gsymbol->language_specific.demangled_name)
+        : symbol_find_demangled_name (gsymbol, linkage_name_copy);
+      gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
+      int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
+
+      /* Suppose we have demangled_name==NULL, copy_name==0, and
+         linkage_name_copy==linkage_name.  In this case, we already have the
+         mangled name saved, and we don't have a demangled name.  So,
+         you might think we could save a little space by not recording
+         this in the hash table at all.
+         
+         It turns out that it is actually important to still save such
+         an entry in the hash table, because storing this name gives
+         us better bcache hit rates for partial symbols.  */
+      if (!copy_name && linkage_name_copy == linkage_name)
+        {
+          *slot
+            = ((struct demangled_name_entry *)
+      	 obstack_alloc (&per_bfd->storage_obstack,
+      			offsetof (struct demangled_name_entry, demangled)
+      			+ demangled_len + 1));
+          (*slot)->mangled = linkage_name;
+        }
+      else
+        {
+          char *mangled_ptr;
+
+          /* If we must copy the mangled name, put it directly after
+             the demangled name so we can have a single
+             allocation.  */
+          *slot
+            = ((struct demangled_name_entry *)
+      	 obstack_alloc (&per_bfd->storage_obstack,
+      			offsetof (struct demangled_name_entry, demangled)
+      			+ len + demangled_len + 2));
+          mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
+          strcpy (mangled_ptr, linkage_name_copy);
+          (*slot)->mangled = mangled_ptr;
+        }
+      (*slot)->language = gsymbol->language;
 
-	if (demangled_name != NULL)
-	  strcpy ((*slot)->demangled, demangled_name.get ());
-	else
-	  (*slot)->demangled[0] = '\0';
-      }
-    else if (gsymbol->language == language_unknown
-	     || gsymbol->language == language_auto)
-      gsymbol->language = (*slot)->language;
+      if (demangled_name != NULL)
+        strcpy ((*slot)->demangled, demangled_name.get ());
+      else
+        (*slot)->demangled[0] = '\0';
+    }
+  else if (gsymbol->language == language_unknown
+           || gsymbol->language == language_auto)
+    gsymbol->language = (*slot)->language;
 
-    found_entry = *slot;
-  }
+  found_entry = *slot;
 
   gsymbol->name = found_entry->mangled;
   if (found_entry->demangled[0] != '\0')
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c3918a85af..17903df92d 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -483,6 +483,16 @@ extern void symbol_set_language (struct general_symbol_info *symbol,
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must
-- 
2.23.0.444.g18eeb5a265-goog

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

* Re: [PATCH] Don't use the mutex for each symbol_set_names call
  2019-09-30 14:18           ` Tom Tromey
  2019-09-30 16:55             ` Christian Biesinger via gdb-patches
@ 2019-09-30 21:45             ` Christian Biesinger via gdb-patches
  2019-10-01 17:02               ` Tom Tromey
  1 sibling, 1 reply; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-09-30 21:45 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Christian Biesinger via gdb-patches

Hi Tom,

On Mon, Sep 30, 2019 at 9:18 AM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
>
> Christian> Also available, rebased to trunk, on
> Christian> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/cbiesinger/parallel-minsyms-mutex
> Christian> ]
>
> I didn't read the patch yet, but this weekend I did rebase my branch and
> fixed most of the review issues.  So, you may want to consider rebasing
> on that.

Looking at your current patch version, I noticed this line in parallel-for.h:
+  if (n_threads > 1 && 2 * n_threads >= n_elements)

I am fairly sure this should be <= in the threads comparison,
otherwise you only use threads if there are few elements, which is the
opposite of what is desired (perhaps a merge error?)

Christian

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

* Re: [PATCH] Don't use the mutex for each symbol_set_names call
  2019-09-30 21:45             ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
@ 2019-10-01 17:02               ` Tom Tromey
  0 siblings, 0 replies; 30+ messages in thread
From: Tom Tromey @ 2019-10-01 17:02 UTC (permalink / raw)
  To: Christian Biesinger via gdb-patches; +Cc: Tom Tromey, Christian Biesinger

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> I am fairly sure this should be <= in the threads comparison,
Christian> otherwise you only use threads if there are few elements, which is the
Christian> opposite of what is desired (perhaps a merge error?)

I probably messed it up when adding the "maint" control.

Thanks very much for pointing this out.

Tom

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

* Re: [PATCH] Don't use the mutex for each symbol_set_names call
  2019-09-30 16:55             ` Christian Biesinger via gdb-patches
@ 2019-10-02 17:18               ` Tom Tromey
  2019-10-02 18:20                 ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Tromey @ 2019-10-02 17:18 UTC (permalink / raw)
  To: Christian Biesinger via gdb-patches; +Cc: Christian Biesinger

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> It speeds up "attach to Chrome's content_shell binary" from 44 sec -> 30
Christian> sec (32%) (compared to GDB trunk), or from 37 sec compared to this
Christian> patchset before this patch.

Nice.

Christian> +	  [&] (minimal_symbol *first, minimal_symbol* last) {
Christian> +	    std::lock_guard<std::mutex> guard (demangled_mutex);
Christian> +	    for (; first < last; ++first) {
Christian> +	      symbol_set_names (first, first->name,
Christian> +		  strlen (first->name), 0,
Christian> +		  m_objfile->per_bfd);
Christian> +	    }
Christian> +	  });

IIUC the idea is to separate the demangling from updating the
demangled_names_hash.

A couple of thoughts on that...

Christian> +          *slot
Christian> +            = ((struct demangled_name_entry *)
Christian> +      	 obstack_alloc (&per_bfd->storage_obstack,
Christian> +      			offsetof (struct demangled_name_entry, demangled)
Christian> +      			+ len + demangled_len + 2));
Christian> +          mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
Christian> +          strcpy (mangled_ptr, linkage_name_copy);
Christian> +          (*slot)->mangled = mangled_ptr;

There's no deep reason that these things have to be stored on the
per-BFD obstack -- and requiring this means an extra copy.  Instead we
could change the hash table to use ordinary heap allocation, and I think
this would be more efficient when demangling in worker threads, because
we could just reuse the existing allocation.

Also, it seems fine to serialize the calls to symbol_set_names.  There's
no need for a lock at all, then.

One idea I had was to parallelize build_minimal_symbol_hash_tables as
well: when possible, have it run two threads, and create the hash tables
in parallel.  Adding a third thread here to update the
demangled_names_hash might help too?  Maybe this approach would
eliminate the need for your "Compute msymbol hash codes in parallel"
patch ... the issue there being that it makes the minsyms larger.
(Another way to handle that would be to keep the hashes in a local array
of some kind that is discarded once the hash tables are updated.)

Tom

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

* Re: [PATCH] Don't use the mutex for each symbol_set_names call
  2019-10-02 17:18               ` Tom Tromey
@ 2019-10-02 18:20                 ` Christian Biesinger via gdb-patches
  2019-10-02 22:02                   ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-10-02 18:20 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Christian Biesinger via gdb-patches

On Wed, Oct 2, 2019 at 12:18 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
>
> Christian> It speeds up "attach to Chrome's content_shell binary" from 44 sec -> 30
> Christian> sec (32%) (compared to GDB trunk), or from 37 sec compared to this
> Christian> patchset before this patch.
>
> Nice.

I do need to redo these measurements with the latest version of the
branch and patch...

> Christian> +      [&] (minimal_symbol *first, minimal_symbol* last) {
> Christian> +        std::lock_guard<std::mutex> guard (demangled_mutex);
> Christian> +        for (; first < last; ++first) {
> Christian> +          symbol_set_names (first, first->name,
> Christian> +              strlen (first->name), 0,
> Christian> +              m_objfile->per_bfd);
> Christian> +        }
> Christian> +      });
>
> IIUC the idea is to separate the demangling from updating the
> demangled_names_hash.

That's correct.

> A couple of thoughts on that...
>
> Christian> +          *slot
> Christian> +            = ((struct demangled_name_entry *)
> Christian> +             obstack_alloc (&per_bfd->storage_obstack,
> Christian> +                            offsetof (struct demangled_name_entry, demangled)
> Christian> +                            + len + demangled_len + 2));
> Christian> +          mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
> Christian> +          strcpy (mangled_ptr, linkage_name_copy);
> Christian> +          (*slot)->mangled = mangled_ptr;
>
> There's no deep reason that these things have to be stored on the
> per-BFD obstack -- and requiring this means an extra copy.  Instead we
> could change the hash table to use ordinary heap allocation, and I think
> this would be more efficient when demangling in worker threads, because
> we could just reuse the existing allocation.

Yes indeed. I was actually thinking of that last night -- we could
change to a hash_set<demangled_name_entry> + reuse the alloc from
gdb_demangle and avoid any allocations here.

> Also, it seems fine to serialize the calls to symbol_set_names.  There's
> no need for a lock at all, then.

True, though this way, if some threads finish faster than others it's
possible to parallelize the work a bit more.

> One idea I had was to parallelize build_minimal_symbol_hash_tables as
> well: when possible, have it run two threads, and create the hash tables
> in parallel.

Hmm, yeah, that's a good idea. I hadn't thought of doing it quite that way.

>  Adding a third thread here to update the
> demangled_names_hash might help too?  Maybe this approach would
> eliminate the need for your "Compute msymbol hash codes in parallel"
> patch ... the issue there being that it makes the minsyms larger.
> (Another way to handle that would be to keep the hashes in a local array
> of some kind that is discarded once the hash tables are updated.)

The local array is a bit tricky... it needs an entry for each msymbol,
which is only known at runtime. So it can't be stack-allocated with a
fixed size, and I'm hesitant to use alloca for this unbounded size. So
it would require a heap allocation for that vector. Maybe it's still
worth it...

OK, let me play with some ideas.


Christian

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

* Re: [PATCH] Don't use the mutex for each symbol_set_names call
  2019-10-02 18:20                 ` Christian Biesinger via gdb-patches
@ 2019-10-02 22:02                   ` Christian Biesinger via gdb-patches
  2019-10-03 18:15                     ` [PATCH v2 1/2] " Christian Biesinger via gdb-patches
  2019-10-03 18:15                     ` [PATCH v2 2/2] Precompute hash value for symbol_set_names Christian Biesinger via gdb-patches
  0 siblings, 2 replies; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-10-02 22:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Christian Biesinger via gdb-patches

On Wed, Oct 2, 2019 at 1:20 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> On Wed, Oct 2, 2019 at 12:18 PM Tom Tromey <tom@tromey.com> wrote:
> >
> > >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
> >
> > Christian> It speeds up "attach to Chrome's content_shell binary" from 44 sec -> 30
> > Christian> sec (32%) (compared to GDB trunk), or from 37 sec compared to this
> > Christian> patchset before this patch.
> >
> > Nice.
>
> I do need to redo these measurements with the latest version of the
> branch and patch...

Here are some new measurements on trunk (this is also with a
recompiled Chrome); either trunk gdb is slower or trunk Chrome is
bigger. I'll call tromey/t/parallel-minsyms-mutex "tromey" below.

GDB Trunk: 49.8s
tromey: 53-54s (!)
tromey + my patch here: ~30.3s
tromey + my patch here +
https://sourceware.org/ml/gdb-patches/2019-09/msg00633.html: 24.8s
(-18% compared to previous)
tromey + my patch here +
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/cbiesinger/minsym-hash-one-thread:
28.8s (only -5%)

I repeatedly measured "tromey" because it is so slow and got
consistent results. It must be due to the lock contention.

> > Christian> +      [&] (minimal_symbol *first, minimal_symbol* last) {
> > Christian> +        std::lock_guard<std::mutex> guard (demangled_mutex);
> > Christian> +        for (; first < last; ++first) {
> > Christian> +          symbol_set_names (first, first->name,
> > Christian> +              strlen (first->name), 0,
> > Christian> +              m_objfile->per_bfd);
> > Christian> +        }
> > Christian> +      });
> >
> > IIUC the idea is to separate the demangling from updating the
> > demangled_names_hash.
>
> That's correct.
>
> > A couple of thoughts on that...
> >
> > Christian> +          *slot
> > Christian> +            = ((struct demangled_name_entry *)
> > Christian> +             obstack_alloc (&per_bfd->storage_obstack,
> > Christian> +                            offsetof (struct demangled_name_entry, demangled)
> > Christian> +                            + len + demangled_len + 2));
> > Christian> +          mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
> > Christian> +          strcpy (mangled_ptr, linkage_name_copy);
> > Christian> +          (*slot)->mangled = mangled_ptr;
> >
> > There's no deep reason that these things have to be stored on the
> > per-BFD obstack -- and requiring this means an extra copy.  Instead we
> > could change the hash table to use ordinary heap allocation, and I think
> > this would be more efficient when demangling in worker threads, because
> > we could just reuse the existing allocation.
>
> Yes indeed. I was actually thinking of that last night -- we could
> change to a hash_set<demangled_name_entry> + reuse the alloc from
> gdb_demangle and avoid any allocations here.

https://sourceware.org/ml/gdb-patches/2019-10/msg00085.html

Although now that I re-read this, I'm not sure if I understood you
correctly, did you want to allocate more things with regular
malloc/new?

> > Also, it seems fine to serialize the calls to symbol_set_names.  There's
> > no need for a lock at all, then.
>
> True, though this way, if some threads finish faster than others it's
> possible to parallelize the work a bit more.

Trying this out, it seems to be about 1-1.5s slower (3-5%). However,
this did make me realize that there's no real reason why the mutex
should be global, so I'm going to move it inside this function.

> > One idea I had was to parallelize build_minimal_symbol_hash_tables as
> > well: when possible, have it run two threads, and create the hash tables
> > in parallel.
>
> Hmm, yeah, that's a good idea. I hadn't thought of doing it quite that way.

See measurements above for users/cbiesinger/minsym-hash-one-thread;
it's not nearly as fast as my "Compute msymbol hash codes in parallel"
patch. However I couldn't do it quite as you suggested (as mentioned
in IRC, but writing it down here as well for anyone watching):
- Building the demangled minsym hash table requires having the
demangled names available, so it needs to happen at the very least
after find_demangled_name
- But it can't happen in parallel with symbol_set_names either,
because that may change the pointer (to one from the hashtable)
- So in practice, I can only build the msymbol_hash table on a thread,
and that's the faster one (search_name_hash is slowish)

> >  Adding a third thread here to update the
> > demangled_names_hash might help too?  Maybe this approach would
> > eliminate the need for your "Compute msymbol hash codes in parallel"
> > patch ... the issue there being that it makes the minsyms larger.
> > (Another way to handle that would be to keep the hashes in a local array
> > of some kind that is discarded once the hash tables are updated.)
>
> The local array is a bit tricky... it needs an entry for each msymbol,
> which is only known at runtime. So it can't be stack-allocated with a
> fixed size, and I'm hesitant to use alloca for this unbounded size. So
> it would require a heap allocation for that vector. Maybe it's still
> worth it...

Putting this in a vector works out! It might possibly be a couple of
tenths of a second faster even. Pushed to:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=85f7818c32fdf5b9fbd24f08320c54e9f9d50b4c

Actually makes me wonder if I could precompute the hash code for
demangled_names_hash in a similar way...

Will send an updated version of this patch in a bit.

Christian

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

* [PATCH v2 2/2] Precompute hash value for symbol_set_names
  2019-10-02 22:02                   ` Christian Biesinger via gdb-patches
  2019-10-03 18:15                     ` [PATCH v2 1/2] " Christian Biesinger via gdb-patches
@ 2019-10-03 18:15                     ` Christian Biesinger via gdb-patches
  1 sibling, 0 replies; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-10-03 18:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

We can also compute the hash for the mangled name on a background
thread so make this function even faster (about a 7% speedup).

gdb/ChangeLog:

2019-10-03  Christian Biesinger  <cbiesinger@google.com>

	* minsyms.c (minimal_symbol_reader::install): Also compute the hash
	of the mangled name on the background thread.
	* symtab.c (symbol_set_names): Allow passing in the hash of the
	linkage_name.
	* symtab.h (symbol_set_names): Likewise.
---
 gdb/minsyms.c | 12 +++++++++++-
 gdb/symtab.c  |  8 +++++---
 gdb/symtab.h  |  7 +++++--
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 95ca9f6c93..b60381a0c9 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1342,6 +1342,11 @@ minimal_symbol_reader::install ()
       std::mutex demangled_mutex;
 #endif
 
+      struct computed_hash_values {
+	hashval_t mangled_name_hash;
+      };
+      std::vector<computed_hash_values> hash_values (mcount);
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
       gdb::parallel_for_each
 	(&msymbols[0], &msymbols[mcount],
@@ -1361,6 +1366,9 @@ minimal_symbol_reader::install ()
 		   symbol_set_demangled_name (msym, demangled_name,
 					      &m_objfile->per_bfd->storage_obstack);
 		   msym->name_set = 1;
+
+		   size_t idx = msym - msymbols;
+		   hash_values[idx].mangled_name_hash = htab_hash_string (msym->name);
 		 }
 	     }
 	   {
@@ -1371,9 +1379,11 @@ minimal_symbol_reader::install ()
 #endif
 	     for (minimal_symbol *msym = start; msym < end; ++msym)
 	       {
+		 size_t idx = msym - msymbols;
 		 symbol_set_names (msym, msym->name,
 				   strlen (msym->name), 0,
-				   m_objfile->per_bfd);
+				   m_objfile->per_bfd,
+				   hash_values[idx].mangled_name_hash);
 	       }
 	   }
 	 });
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 47da5cf4e8..40cc34b205 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -808,7 +808,7 @@ symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 void
 symbol_set_names (struct general_symbol_info *gsymbol,
 		  const char *linkage_name, int len, bool copy_name,
-		  struct objfile_per_bfd_storage *per_bfd)
+		  struct objfile_per_bfd_storage *per_bfd, hashval_t hash)
 {
   struct demangled_name_entry **slot;
   /* A 0-terminated copy of the linkage name.  */
@@ -854,9 +854,11 @@ symbol_set_names (struct general_symbol_info *gsymbol,
     create_demangled_names_hash (per_bfd);
 
   entry.mangled = linkage_name_copy;
+  if (hash == 0)
+    hash = hash_demangled_name_entry (&entry);
   slot = ((struct demangled_name_entry **)
-          htab_find_slot (per_bfd->demangled_names_hash.get (),
-      		    &entry, INSERT));
+          htab_find_slot_with_hash (per_bfd->demangled_names_hash.get (),
+				    &entry, hash, INSERT));
 
   /* If this name is not in the hash table, add it.  */
   if (*slot == NULL
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 17903df92d..2814f401fe 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -502,13 +502,16 @@ extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
   (symbol)->ginfo.name = (linkage_name)
 
 /* Set the linkage and natural names of a symbol, by demangling
-   the linkage name.  */
+   the linkage name.  Optionally, HASH can be set to the value
+   of htab_hash_string (linkage_name) (if nullterminated), to
+   speed up this function.  */
 #define SYMBOL_SET_NAMES(symbol,linkage_name,len,copy_name,objfile)	\
   symbol_set_names (&(symbol)->ginfo, linkage_name, len, copy_name, \
 		    (objfile)->per_bfd)
 extern void symbol_set_names (struct general_symbol_info *symbol,
 			      const char *linkage_name, int len, bool copy_name,
-			      struct objfile_per_bfd_storage *per_bfd);
+			      struct objfile_per_bfd_storage *per_bfd,
+			      hashval_t hash = 0);
 
 /* Now come lots of name accessor macros.  Short version as to when to
    use which: Use SYMBOL_NATURAL_NAME to refer to the name of the
-- 
2.23.0.444.g18eeb5a265-goog

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

* [PATCH v2 1/2] Don't use the mutex for each symbol_set_names call
  2019-10-02 22:02                   ` Christian Biesinger via gdb-patches
@ 2019-10-03 18:15                     ` Christian Biesinger via gdb-patches
  2019-10-03 18:15                     ` [PATCH v2 2/2] Precompute hash value for symbol_set_names Christian Biesinger via gdb-patches
  1 sibling, 0 replies; 30+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-10-03 18:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

This avoids the lock contention that was seen with tromey's patch (it
moves it to a once- per-thread lock call from a once-per-symbol call).

It speeds up "attach to Chrome's content_shell binary" from 44 sec -> 30
sec (32%) (compared to GDB trunk), or from 37 sec compared to this
patchset before this patch.

gdb/ChangeLog:

2019-09-28  Christian Biesinger  <cbiesinger@google.com>

	* minsyms.c (minimal_symbol_reader::install): Only do
	demangling on the background thread; still call
	symbol_set_names on the main thread.
	* symtab.c (symbol_find_demangled_name): Make public,
	so that minsyms.c can call it.
	(symbol_set_names): Remove mutex. Avoid demangle call
	if the demangled name is already set.
	* symtab.h (symbol_find_demangled_name): Make public.
---
 gdb/minsyms.c |  30 +++++++++-
 gdb/symtab.c  | 162 +++++++++++++++++++++++---------------------------
 gdb/symtab.h  |  10 ++++
 3 files changed, 110 insertions(+), 92 deletions(-)

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 84bf2bb61e..95ca9f6c93 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -56,6 +56,10 @@
 #include "gdbsupport/alt-stack.h"
 #include "gdbsupport/parallel-for.h"
 
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
+
 /* See minsyms.h.  */
 
 bool
@@ -1333,6 +1337,11 @@ minimal_symbol_reader::install ()
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex we hold for calling symbol_set_names.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
       gdb::parallel_for_each
 	(&msymbols[0], &msymbols[mcount],
@@ -1346,12 +1355,27 @@ minimal_symbol_reader::install ()
 	     {
 	       if (!msym->name_set)
 		 {
-		   symbol_set_names (msym, msym->name,
-				     strlen (msym->name), 0,
-				     m_objfile->per_bfd);
+		   /* This will be freed later, by symbol_set_names.  */
+		   char* demangled_name = symbol_find_demangled_name (msym,
+								      msym->name);
+		   symbol_set_demangled_name (msym, demangled_name,
+					      &m_objfile->per_bfd->storage_obstack);
 		   msym->name_set = 1;
 		 }
 	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name,
+				   strlen (msym->name), 0,
+				   m_objfile->per_bfd);
+	       }
+	   }
 	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8adcff7cf2..47da5cf4e8 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,9 +69,6 @@
 #include "arch-utils.h"
 #include <algorithm>
 #include "gdbsupport/pathstuff.h"
-#if CXX_STD_THREAD
-#include <mutex>
-#endif
 
 /* Forward declarations for local functions.  */
 
@@ -713,16 +710,6 @@ symbol_set_language (struct general_symbol_info *gsymbol,
 
 /* Functions to initialize a symbol's mangled name.  */
 
-#if CXX_STD_THREAD
-/* Mutex that is used when modifying or accessing the demangled hash
-   table.  This is a global mutex simply because the only current
-   multi-threaded user of the hash table does not process multiple
-   objfiles in parallel.  The mutex could easily live on the per-BFD
-   object, but this approach avoids using extra memory when it is not
-   needed.  */
-static std::mutex demangled_mutex;
-#endif
-
 /* Objects of this type are stored in the demangled name hash table.  */
 struct demangled_name_entry
 {
@@ -772,13 +759,9 @@ create_demangled_names_hash (struct objfile_per_bfd_storage *per_bfd)
      NULL, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -867,78 +850,79 @@ symbol_set_names (struct general_symbol_info *gsymbol,
 
   struct demangled_name_entry *found_entry;
 
-  {
-#if CXX_STD_THREAD
-    std::lock_guard<std::mutex> guard (demangled_mutex);
-#endif
-
-    if (per_bfd->demangled_names_hash == NULL)
-      create_demangled_names_hash (per_bfd);
-
-    entry.mangled = linkage_name_copy;
-    slot = ((struct demangled_name_entry **)
-	    htab_find_slot (per_bfd->demangled_names_hash.get (),
-			    &entry, INSERT));
-
-    /* If this name is not in the hash table, add it.  */
-    if (*slot == NULL
-	/* A C version of the symbol may have already snuck into the table.
-	   This happens to, e.g., main.init (__go_init_main).  Cope.  */
-	|| (gsymbol->language == language_go
-	    && (*slot)->demangled[0] == '\0'))
-      {
-	char *demangled_name_ptr
-	  = symbol_find_demangled_name (gsymbol, linkage_name_copy);
-	gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
-	int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
-
-	/* Suppose we have demangled_name==NULL, copy_name==0, and
-	   linkage_name_copy==linkage_name.  In this case, we already have the
-	   mangled name saved, and we don't have a demangled name.  So,
-	   you might think we could save a little space by not recording
-	   this in the hash table at all.
-	 
-	   It turns out that it is actually important to still save such
-	   an entry in the hash table, because storing this name gives
-	   us better bcache hit rates for partial symbols.  */
-	if (!copy_name && linkage_name_copy == linkage_name)
-	  {
-	    *slot
-	      = ((struct demangled_name_entry *)
-		 obstack_alloc (&per_bfd->storage_obstack,
-				offsetof (struct demangled_name_entry, demangled)
-				+ demangled_len + 1));
-	    (*slot)->mangled = linkage_name;
-	  }
-	else
-	  {
-	    char *mangled_ptr;
-
-	    /* If we must copy the mangled name, put it directly after
-	       the demangled name so we can have a single
-	       allocation.  */
-	    *slot
-	      = ((struct demangled_name_entry *)
-		 obstack_alloc (&per_bfd->storage_obstack,
-				offsetof (struct demangled_name_entry, demangled)
-				+ len + demangled_len + 2));
-	    mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
-	    strcpy (mangled_ptr, linkage_name_copy);
-	    (*slot)->mangled = mangled_ptr;
-	  }
-	(*slot)->language = gsymbol->language;
+  if (per_bfd->demangled_names_hash == NULL)
+    create_demangled_names_hash (per_bfd);
+
+  entry.mangled = linkage_name_copy;
+  slot = ((struct demangled_name_entry **)
+          htab_find_slot (per_bfd->demangled_names_hash.get (),
+      		    &entry, INSERT));
+
+  /* If this name is not in the hash table, add it.  */
+  if (*slot == NULL
+      /* A C version of the symbol may have already snuck into the table.
+         This happens to, e.g., main.init (__go_init_main).  Cope.  */
+      || (gsymbol->language == language_go
+          && (*slot)->demangled[0] == '\0'))
+    {
+      /* The const_cast is safe because the only reason it is already initialized
+         is if we purposefully set it from a background thread to avoid doing the
+         work here. However, it is still allocated from the heap and needs to
+         be freed by us, just like if we called symbol_find_demangled_name
+         here.  */
+      char *demangled_name_ptr
+        = gsymbol->language_specific.demangled_name
+        ? const_cast<char*>(gsymbol->language_specific.demangled_name)
+        : symbol_find_demangled_name (gsymbol, linkage_name_copy);
+      gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
+      int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
+
+      /* Suppose we have demangled_name==NULL, copy_name==0, and
+         linkage_name_copy==linkage_name.  In this case, we already have the
+         mangled name saved, and we don't have a demangled name.  So,
+         you might think we could save a little space by not recording
+         this in the hash table at all.
+         
+         It turns out that it is actually important to still save such
+         an entry in the hash table, because storing this name gives
+         us better bcache hit rates for partial symbols.  */
+      if (!copy_name && linkage_name_copy == linkage_name)
+        {
+          *slot
+            = ((struct demangled_name_entry *)
+	       obstack_alloc (&per_bfd->storage_obstack,
+			      offsetof (struct demangled_name_entry, demangled)
+			      + demangled_len + 1));
+          (*slot)->mangled = linkage_name;
+        }
+      else
+        {
+          char *mangled_ptr;
+
+          /* If we must copy the mangled name, put it directly after
+             the demangled name so we can have a single
+             allocation.  */
+          *slot
+            = ((struct demangled_name_entry *)
+	       obstack_alloc (&per_bfd->storage_obstack,
+			      offsetof (struct demangled_name_entry, demangled)
+			      + len + demangled_len + 2));
+          mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
+          strcpy (mangled_ptr, linkage_name_copy);
+          (*slot)->mangled = mangled_ptr;
+        }
+      (*slot)->language = gsymbol->language;
 
-	if (demangled_name != NULL)
-	  strcpy ((*slot)->demangled, demangled_name.get ());
-	else
-	  (*slot)->demangled[0] = '\0';
-      }
-    else if (gsymbol->language == language_unknown
-	     || gsymbol->language == language_auto)
-      gsymbol->language = (*slot)->language;
+      if (demangled_name != NULL)
+        strcpy ((*slot)->demangled, demangled_name.get ());
+      else
+        (*slot)->demangled[0] = '\0';
+    }
+  else if (gsymbol->language == language_unknown
+           || gsymbol->language == language_auto)
+    gsymbol->language = (*slot)->language;
 
-    found_entry = *slot;
-  }
+  found_entry = *slot;
 
   gsymbol->name = found_entry->mangled;
   if (found_entry->demangled[0] != '\0')
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c3918a85af..17903df92d 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -483,6 +483,16 @@ extern void symbol_set_language (struct general_symbol_info *symbol,
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must
-- 
2.23.0.444.g18eeb5a265-goog

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

end of thread, other threads:[~2019-10-03 18:15 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
2019-05-18 21:00 ` [PATCH v2 1/8] Defer minimal symbol name-setting Tom Tromey
2019-05-18 21:00 ` [PATCH v2 4/8] Lock the demangled hash table Tom Tromey
2019-05-18 21:00 ` [PATCH v2 5/8] Introduce run_on_main_thread Tom Tromey
2019-05-18 21:00 ` [PATCH v2 2/8] Remove static buffer from ada_decode Tom Tromey
2019-05-18 21:00 ` [PATCH v2 3/8] Add configure check for std::thread Tom Tromey
2019-05-18 21:00 ` [PATCH v2 6/8] Introduce thread-safe way to handle SIGSEGV Tom Tromey
2019-05-18 21:00 ` [PATCH v2 7/8] Demangle minsyms in parallel Tom Tromey
2019-05-18 21:00 ` [PATCH v2 8/8] Add maint set/show enable-threads Tom Tromey
2019-05-22  5:01   ` Eli Zaretskii
2019-05-26 20:46     ` Tom Tromey
2019-05-27  2:32       ` Eli Zaretskii
2019-05-19 13:59 ` [PATCH v2 0/8] Demangle minimal symbol names in worker threads Philippe Waroquiers
2019-05-19 18:55   ` Tom Tromey
2019-05-21  0:35     ` Philippe Waroquiers
2019-05-21  7:35     ` Andrew Burgess
2019-05-21 15:45       ` Tom Tromey
2019-05-21 16:21         ` Andrew Burgess
2019-05-31  2:48     ` Tom Tromey
2019-05-31 17:13       ` Philippe Waroquiers
2019-09-29  0:35         ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
2019-09-30 14:18           ` Tom Tromey
2019-09-30 16:55             ` Christian Biesinger via gdb-patches
2019-10-02 17:18               ` Tom Tromey
2019-10-02 18:20                 ` Christian Biesinger via gdb-patches
2019-10-02 22:02                   ` Christian Biesinger via gdb-patches
2019-10-03 18:15                     ` [PATCH v2 1/2] " Christian Biesinger via gdb-patches
2019-10-03 18:15                     ` [PATCH v2 2/2] Precompute hash value for symbol_set_names Christian Biesinger via gdb-patches
2019-09-30 21:45             ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
2019-10-01 17:02               ` Tom Tromey

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).