This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[4/7] RFC: basic thread safety in gdb


This patch adds some basic thread-safety to gdb.

gdb has a few modules, used by the DWARF reader, that reference global
state.

Cleanups use a global, fixed by making it __thread.

Exceptions use a global and also have a surprisingly ugly hack in
throw_exception to call bpstat_clear_actions and
disable_current_display.  Fixed by using __thread and also by making the
special code conditional on being in the main thread.  We also needed a
new function to let us clean up some thread state when a thread exits.

cp-name-parser.y is not thread-safe.  I added some __thread markers.

I also turned it into a "pure" parser.  This introduces a hard
dependency on bison.  This patch needs (but does not yet have) a
configure check for that.  If this dependency is not ok we could also
run sed on the .y file at build time, or something like that.

There are still plenty of thread-unsafe modules in gdb.  E.g., all the
I/O stuff, because things like wrap_here are not per-stream (this seems
like a plain old bug).  It would be tough to read full symbols in the
background because the block creation code all uses global state.  Etc.

I didn't try to fix any of that since most of it does not matter for
psymtab reading.

Tom

>From daf91bc3b40dd465b38c4cccfc83c2f39e94f095 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Fri, 11 Jun 2010 15:04:27 -0600
Subject: [PATCH 4/7] basic thread-safety

2010-06-14  Tom Tromey  <tromey@redhat.com>

	* utils.c (cleanup_chain): Use __thread.
	* main.c (captured_main): Call this_is_the_main_thread.
	* exceptions.c (current_catcher): Use __thread.
	(main_thread): New global.
	(throw_exception): Only call bpstat_clear_actions and
	disable_current_display in main thread.
	(last_message): Use __thread.
	(this_is_the_main_thread, clear_exception_cache): New functions.
	* exceptions.h (this_is_the_main_thread, clear_exception_cache):
	Declare.
	* cp-name-parser.y: Use %define api.pure.
	(lexptr, prev_lexptr, error_lexptr, global_errmsg): Use __thread.
	(errbuf): New global.
	(demangle_info, global_result): Use __thread.
	(parse_number): Add 'lvalp' argument.
	(HANDLE_SPECIAL, HANDLE_TOKEN2, HANDLE_TOKEN3): Use lvalp.
	(yylex): Add 'lvalp' argument.
	(cp_demangled_name_to_comp): Use global errbuf.
---
 gdb/cp-name-parser.y |   52 ++++++++++++++++++++++++++++++-------------------
 gdb/exceptions.c     |   43 ++++++++++++++++++++++++++++++----------
 gdb/exceptions.h     |    9 ++++++++
 gdb/main.c           |    2 +
 gdb/utils.c          |    2 +-
 5 files changed, 76 insertions(+), 32 deletions(-)

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 6d7b600..962214b 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -28,6 +28,8 @@
    too messy, particularly when such includes can be inserted at random
    times by the parser generator.  */
 
+%define api.pure
+
 %{
 
 #include "defs.h"
@@ -51,7 +53,12 @@
    ERROR_LEXPTR is the first place an error occurred.  GLOBAL_ERRMSG
    is the first error message encountered.  */
 
-static const char *lexptr, *prev_lexptr, *error_lexptr, *global_errmsg;
+static __thread const char *lexptr;
+static __thread const char *prev_lexptr;
+static __thread const char *error_lexptr;
+static __thread const char *global_errmsg;
+
+static __thread char errbuf[60];
 
 /* The components built by the parser are allocated ahead of time,
    and cached in this structure.  */
@@ -64,7 +71,7 @@ struct demangle_info {
   struct demangle_component comps[ALLOC_CHUNK];
 };
 
-static struct demangle_info *demangle_info;
+static __thread struct demangle_info *demangle_info;
 
 static struct demangle_component *
 d_grab (void)
@@ -92,7 +99,7 @@ d_grab (void)
 /* The parse tree created by the parser is stored here after a successful
    parse.  */
 
-static struct demangle_component *global_result;
+static __thread struct demangle_component *global_result;
 
 /* Prototypes for helper functions used when constructing the parse
    tree.  */
@@ -173,7 +180,7 @@ static struct demangle_component *d_binary (const char *,
 #define yycheck	 cpname_yycheck
 
 int yyparse (void);
-static int yylex (void);
+/* static int yylex (YYSTYPE *); */
 static void yyerror (char *);
 
 /* Enable yydebug for the stand-alone parser.  */
@@ -350,6 +357,11 @@ make_name (const char *name, int len)
 %right ARROW '.' '[' /* '(' */
 %left COLONCOLON
 
+%{
+static int yylex (YYSTYPE *);
+%}
+
+
 
 %%
 
@@ -1310,7 +1322,7 @@ symbol_end (const char *lexptr)
    YYLVAL.  */
 
 static int
-parse_number (const char *p, int len, int parsed_float)
+parse_number (const char *p, int len, int parsed_float, YYSTYPE *lvalp)
 {
   int unsigned_p = 0;
 
@@ -1360,7 +1372,7 @@ parse_number (const char *p, int len, int parsed_float)
 	return ERROR;
 
       name = make_name (p, len);
-      yylval.comp = fill_comp (literal_type, type, name);
+      lvalp->comp = fill_comp (literal_type, type, name);
 
       return FLOAT;
     }
@@ -1409,7 +1421,7 @@ parse_number (const char *p, int len, int parsed_float)
      type = signed_type;
 
    name = make_name (p, len);
-   yylval.comp = fill_comp (literal_type, type, name);
+   lvalp->comp = fill_comp (literal_type, type, name);
 
    return INT;
 }
@@ -1514,7 +1526,7 @@ cp_parse_escape (const char **string_ptr)
   if (strncmp (tokstart, string, sizeof (string) - 1) == 0)	\
     {								\
       lexptr = tokstart + sizeof (string) - 1;			\
-      yylval.lval = comp;					\
+      lvalp->lval = comp;					\
       return DEMANGLER_SPECIAL;					\
     }
 
@@ -1522,7 +1534,7 @@ cp_parse_escape (const char **string_ptr)
   if (lexptr[1] == string[1])				\
     {							\
       lexptr += 2;					\
-      yylval.opname = string;				\
+      lvalp->opname = string;				\
       return token;					\
     }      
 
@@ -1530,14 +1542,14 @@ cp_parse_escape (const char **string_ptr)
   if (lexptr[1] == string[1] && lexptr[2] == string[2])	\
     {							\
       lexptr += 3;					\
-      yylval.opname = string;				\
+      lvalp->opname = string;				\
       return token;					\
     }      
 
 /* Read one token, getting characters through LEXPTR.  */
 
 static int
-yylex (void)
+yylex (YYSTYPE *lvalp)
 {
   int c;
   int namelen;
@@ -1583,7 +1595,7 @@ yylex (void)
 	 presumably the same one that appears in manglings - the decimal
 	 representation.  But if that isn't in our input then we have to
 	 allocate memory for it somewhere.  */
-      yylval.comp = fill_comp (DEMANGLE_COMPONENT_LITERAL,
+      lvalp->comp = fill_comp (DEMANGLE_COMPONENT_LITERAL,
 				 make_builtin_type ("char"),
 				 make_name (tokstart, lexptr - tokstart));
 
@@ -1593,7 +1605,7 @@ yylex (void)
       if (strncmp (tokstart, "(anonymous namespace)", 21) == 0)
 	{
 	  lexptr += 21;
-	  yylval.comp = make_name ("(anonymous namespace)",
+	  lvalp->comp = make_name ("(anonymous namespace)",
 				     sizeof "(anonymous namespace)" - 1);
 	  return NAME;
 	}
@@ -1692,7 +1704,8 @@ yylex (void)
 	    else if (! ISALNUM (*p))
 	      break;
 	  }
-	toktype = parse_number (tokstart, p - tokstart, got_dot|got_e);
+	toktype = parse_number (tokstart, p - tokstart, got_dot|got_e,
+				lvalp);
         if (toktype == ERROR)
 	  {
 	    char *err_copy = (char *) alloca (p - tokstart + 1);
@@ -1845,10 +1858,10 @@ yylex (void)
 	{
 	  const char *p;
 	  lexptr = tokstart + 29;
-	  yylval.lval = DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS;
+	  lvalp->lval = DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS;
 	  /* Find the end of the symbol.  */
 	  p = symbol_end (lexptr);
-	  yylval.comp = make_name (lexptr, p - lexptr);
+	  lvalp->comp = make_name (lexptr, p - lexptr);
 	  lexptr = p;
 	  return DEMANGLER_SPECIAL;
 	}
@@ -1856,10 +1869,10 @@ yylex (void)
 	{
 	  const char *p;
 	  lexptr = tokstart + 28;
-	  yylval.lval = DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS;
+	  lvalp->lval = DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS;
 	  /* Find the end of the symbol.  */
 	  p = symbol_end (lexptr);
-	  yylval.comp = make_name (lexptr, p - lexptr);
+	  lvalp->comp = make_name (lexptr, p - lexptr);
 	  lexptr = p;
 	  return DEMANGLER_SPECIAL;
 	}
@@ -1917,7 +1930,7 @@ yylex (void)
       break;
     }
 
-  yylval.comp = make_name (tokstart, namelen);
+  lvalp->comp = make_name (tokstart, namelen);
   return NAME;
 }
 
@@ -1975,7 +1988,6 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len)
 struct demangle_component *
 cp_demangled_name_to_comp (const char *demangled_name, const char **errmsg)
 {
-  static char errbuf[60];
   struct demangle_component *result;
 
   prev_lexptr = lexptr = demangled_name;
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index ec9b9be..4627fa3 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -67,7 +67,10 @@ struct catcher
 };
 
 /* Where to go for throw_exception().  */
-static struct catcher *current_catcher;
+static __thread struct catcher *current_catcher;
+
+/* True if this is the main thread.  */
+static __thread int main_thread;
 
 EXCEPTIONS_SIGJMP_BUF *
 exceptions_state_mc_init (struct ui_out *func_uiout,
@@ -215,20 +218,24 @@ exceptions_state_mc_action_iter_1 (void)
 void
 throw_exception (struct gdb_exception exception)
 {
-  struct thread_info *tp = NULL;
-
   quit_flag = 0;
   immediate_quit = 0;
 
-  if (!ptid_equal (inferior_ptid, null_ptid))
-    tp = find_thread_ptid (inferior_ptid);
+  if (main_thread)
+    {
+      struct thread_info *tp = NULL;
+
+      if (!ptid_equal (inferior_ptid, null_ptid))
+	tp = find_thread_ptid (inferior_ptid);
 
-  /* Perhaps it would be cleaner to do this via the cleanup chain (not sure
-     I can think of a reason why that is vital, though).  */
-  if (tp != NULL)
-    bpstat_clear_actions (tp->stop_bpstat);	/* Clear queued breakpoint commands */
+      /* Perhaps it would be cleaner to do this via the cleanup chain
+	 (not sure I can think of a reason why that is vital,
+	 though).  */
+      if (tp != NULL)
+	bpstat_clear_actions (tp->stop_bpstat);	/* Clear queued breakpoint commands */
 
-  disable_current_display ();
+      disable_current_display ();
+    }
   do_cleanups (ALL_CLEANUPS);
 
   /* Jump to the containing catch_errors() call, communicating REASON
@@ -239,7 +246,7 @@ throw_exception (struct gdb_exception exception)
   EXCEPTIONS_SIGLONGJMP (current_catcher->buf, exception.reason);
 }
 
-static char *last_message;
+static __thread char *last_message;
 
 void
 deprecated_throw_reason (enum return_reason reason)
@@ -538,3 +545,17 @@ catch_command_errors (catch_command_errors_ftype * command,
     return 0;
   return 1;
 }
+
+void
+this_is_the_main_thread (void)
+{
+  main_thread = 1;
+}
+
+void
+clear_exception_cache (void)
+{
+  gdb_assert (!main_thread);
+  gdb_assert (current_catcher == NULL);
+  xfree (last_message);
+}
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 7d68a36..09f7ad1 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -252,4 +252,13 @@ extern int catch_errors (catch_errors_ftype *, void *, char *, return_mask);
 typedef void (catch_command_errors_ftype) (char *, int);
 extern int catch_command_errors (catch_command_errors_ftype *func, char *command, int from_tty, return_mask);
 
+
+/* This is used when initializing the exception system to mark gdb's
+   main thread.  */
+extern void this_is_the_main_thread (void);
+
+/* This is called by a thread just before shutdown to free any
+   per-thread exception information.  */
+extern void clear_exception_cache (void);
+
 #endif
diff --git a/gdb/main.c b/gdb/main.c
index c5c712a..e0bb252 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -314,6 +314,8 @@ captured_main (void *data)
   lim_at_start = (char *) sbrk (0);
 #endif
 
+  this_is_the_main_thread ();
+
   cmdsize = 1;
   cmdarg = (struct cmdarg *) xmalloc (cmdsize * sizeof (*cmdarg));
   ncmd = 0;
diff --git a/gdb/utils.c b/gdb/utils.c
index 472cf47..c45b340 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -110,7 +110,7 @@ static int debug_timestamp = 0;
 /* Chain of cleanup actions established with make_cleanup,
    to be executed if an error happens.  */
 
-static struct cleanup *cleanup_chain;	/* cleaned up after a failed command */
+static __thread struct cleanup *cleanup_chain;	/* cleaned up after a failed command */
 static struct cleanup *final_cleanup_chain;	/* cleaned up when gdb exits */
 
 /* Nonzero if we have job control. */
-- 
1.6.2.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]