Lock the demangled hash table

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

gdb/ChangeLog
2019-09-30  Tom Tromey  <tom@tromey.com>

	* symtab.c (demangled_mutex): New global.
	(symbol_set_names): Use a lock_guard.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 395a44fd4298771ad344310b20b7e0f423705f96..d46b44df2aa26255f51f3785d297133307cb8e4f 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-09-30  Tom Tromey  <tom@tromey.com>
+
+       * symtab.c (demangled_mutex): New global.
+       (symbol_set_names): Use a lock_guard.
+
 2019-09-30  Tom Tromey  <tom@tromey.com>
 
        * main.c (setup_alternate_signal_stack): Remove.

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 6ea9fc6971e0f0228f0934321aae81aa75a44e0e..8adcff7cf2b693933d35ff36bbc29c6a6de6bc64 100644 (file)
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,6 +69,9 @@
 #include "arch-utils.h"
 #include <algorithm>
 #include "gdbsupport/pathstuff.h"
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* Forward declarations for local functions.  */
 
@@ -710,6 +713,16 @@ 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
 {
@@ -839,9 +852,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;
@@ -855,71 +865,84 @@ symbol_set_names (struct general_symbol_info *gsymbol,
   else
     linkage_name_copy = linkage_name;
 
-  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.
+  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;
+          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;
+  }
 
-  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);