[PATCH v4 06/11] Lock the demangled hash table

Tom Tromey
2019-10-01T20:16:11Z

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

gdb/ChangeLog
2019-10-01 Tom Tromey <tom@tromey.com>

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

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 6ea9fc6971e..8adcff7cf2b 100644
--- 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);
--
2.17.2