Patchwork [RFC,3/6] Lock the demangled hash table

login
register
mail settings
Submitter Tom Tromey
Date March 9, 2019, 5:22 p.m.
Message ID <20190309172300.2764-4-tom@tromey.com>
Download mbox | patch
Permalink /patch/31801/
State New
Headers show

Comments

Tom Tromey - March 9, 2019, 5:22 p.m.
This introduces a lock that is used when modifying the demangled hash
table.

2019-03-03  Tom Tromey  <tom@tromey.com>

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

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 449bc4cd2b3..0dcdc1e8b01 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,6 +69,7 @@ 
 #include "arch-utils.h"
 #include <algorithm>
 #include "common/pathstuff.h"
+#include <mutex>
 
 /* Forward declarations for local functions.  */
 
@@ -697,6 +698,9 @@  symbol_set_language (struct general_symbol_info *gsymbol,
 
 /* Functions to initialize a symbol's mangled name.  */
 
+/* Mutex that is used when modifying the demangled hash table.  */
+static std::mutex demangled_mutex;
+
 /* Objects of this type are stored in the demangled name hash table.  */
 struct demangled_name_entry
 {
@@ -825,9 +829,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;
@@ -846,64 +847,75 @@  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;
+
+  {
+    std::lock_guard<std::mutex> guard (demangled_mutex);
+
+    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);