diff mbox

libbacktrace patch committed: Fix threaded race

Message ID mcrhalxutqf.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Jan. 31, 2013, 5:50 p.m. UTC 
Using libbacktrace more extensively from Go has revealed a race
condition in libbacktrace.  I cleverly save some memory by reusing a
vector in the DWARF data structure.  However, that is fairly hard to do
if multiple threads are simultaneously getting a backtrace.  This patch
changes the code to always use a new vector when threaded.  This will
introduce additional memory fragmentation, but that seems impossible to
avoid.  Bootstrapped and ran libbacktrace and Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


2013-01-31  Ian Lance Taylor  <iant@google.com>

	* dwarf.c (read_function_info): Permit fvec parameter to be NULL.
	(dwarf_lookup_pc): Don't use ddata->fvec if threaded.
diff mbox

Patch

Index: dwarf.c
===================================================================
--- dwarf.c	(revision 195478)
+++ dwarf.c	(working copy)
@@ -2473,10 +2473,21 @@  read_function_info (struct backtrace_sta
 		    struct function_addrs **ret_addrs,
 		    size_t *ret_addrs_count)
 {
+  struct function_vector lvec;
+  struct function_vector *pfvec;
   struct dwarf_buf unit_buf;
   struct function_addrs *addrs;
   size_t addrs_count;
 
+  /* Use FVEC if it is not NULL.  Otherwise use our own vector.  */
+  if (fvec != NULL)
+    pfvec = fvec;
+  else
+    {
+      memset (&lvec, 0, sizeof lvec);
+      pfvec = &lvec;
+    }
+
   unit_buf.name = ".debug_info";
   unit_buf.start = ddata->dwarf_info;
   unit_buf.buf = u->unit_data;
@@ -2489,20 +2500,28 @@  read_function_info (struct backtrace_sta
   while (unit_buf.left > 0)
     {
       if (!read_function_entry (state, ddata, u, 0, &unit_buf, lhdr,
-				error_callback, data, fvec))
+				error_callback, data, pfvec))
 	return;
     }
 
-  if (fvec->count == 0)
+  if (pfvec->count == 0)
     return;
 
-  addrs = (struct function_addrs *) fvec->vec.base;
-  addrs_count = fvec->count;
+  addrs = (struct function_addrs *) pfvec->vec.base;
+  addrs_count = pfvec->count;
 
-  /* Finish this list of addresses, but leave the remaining space in
-     the vector available for the next function unit.  */
-  backtrace_vector_finish (state, &fvec->vec);
-  fvec->count = 0;
+  if (fvec == NULL)
+    {
+      if (!backtrace_vector_release (state, &lvec.vec, error_callback, data))
+	return;
+    }
+  else
+    {
+      /* Finish this list of addresses, but leave the remaining space in
+	 the vector available for the next function unit.  */
+      backtrace_vector_finish (state, &fvec->vec);
+      fvec->count = 0;
+    }
 
   qsort (addrs, addrs_count, sizeof (struct function_addrs),
 	 function_addrs_compare);
@@ -2663,8 +2682,16 @@  dwarf_lookup_pc (struct backtrace_state
       if (read_line_info (state, ddata, error_callback, data, entry->u, &lhdr,
 			  &lines, &count))
 	{
+	  struct function_vector *pfvec;
+
+	  /* If not threaded, reuse DDATA->FVEC for better memory
+	     consumption.  */
+	  if (state->threaded)
+	    pfvec = NULL;
+	  else
+	    pfvec = &ddata->fvec;
 	  read_function_info (state, ddata, &lhdr, error_callback, data,
-			      entry->u, &ddata->fvec, &function_addrs,
+			      entry->u, pfvec, &function_addrs,
 			      &function_addrs_count);
 	  free_line_header (state, &lhdr, error_callback, data);
 	  new_data = 1;