Patchwork [trans-mem] random segfault

login
register
mail settings
Submitter Aldy Hernandez
Date June 16, 2010, 8:17 p.m.
Message ID <20100616201705.GA2844@redhat.com>
Download mbox | patch
Permalink /patch/55934/
State New
Headers show

Comments

Aldy Hernandez - June 16, 2010, 8:17 p.m.
> This should fix both bugs since (1) we detect recursion on longer
> chains and (2) we don't touch SLOT after recursion; we only touch
> ELT_P, which does not change with hash resizing.

Poop, I thought about fixing it this way, but I figured my way was
easier.  I obviously didn't take into account longer chains.

OK?

	* trans-mem.c (thread_private_new_memory): Avoid infinite recursion.
Richard Henderson - June 16, 2010, 9:30 p.m.
On 06/16/2010 01:17 PM, Aldy Hernandez wrote:
> 	* trans-mem.c (thread_private_new_memory): Avoid infinite recursion.

Ok.


r~

Patch

Index: testsuite/gcc.dg/tm/20100615-2.c
===================================================================
--- testsuite/gcc.dg/tm/20100615-2.c	(revision 0)
+++ testsuite/gcc.dg/tm/20100615-2.c	(revision 0)
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O" } */
+
+__attribute__((transaction_safe))
+void Info_RemoveKey (char *s)
+{
+        char    *o = 0;
+        while (1)
+        {
+                s++;
+                while (*s)
+                {
+                        if (!*s)
+                                return;
+                        *o++ = *s++;
+                }
+                *o = 0;
+        }
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 160765)
+++ trans-mem.c	(working copy)
@@ -1268,12 +1268,16 @@  thread_private_new_memory (basic_block e
 
   /* Look in cache first.  */
   elt.val = x;
-  slot = htab_find_slot (tm_new_mem_hash, &elt, NO_INSERT);
-  if (slot)
-    {
-      elt_p = (tm_new_mem_map_t *) *slot;
-      return elt_p->local_new_memory;
-    }
+  slot = htab_find_slot (tm_new_mem_hash, &elt, INSERT);
+  elt_p = (tm_new_mem_map_t *) *slot;
+  if (elt_p)
+    return elt_p->local_new_memory;
+
+  /* Optimistically assume the memory is transaction local during
+     processing.  This catches recursion into this variable.  */
+  *slot = elt_p = XNEW (tm_new_mem_map_t);
+  elt_p->val = val;
+  elt_p->local_new_memory = mem_transaction_local;
 
   /* Search DEF chain to find the original definition of this address.  */
   do
@@ -1353,15 +1357,7 @@  thread_private_new_memory (basic_block e
     retval = mem_non_local;
 
  new_memory_ret:
-  /* A recursive call to thread_private_new_memory() may change the
-     slot from under us if we resize the hash table, so get a fresh
-     slot after all is said and done.  */
-  slot = htab_find_slot (tm_new_mem_hash, &elt, INSERT);
-
-  elt_p = XNEW (tm_new_mem_map_t);
-  elt_p->val = val;
   elt_p->local_new_memory = retval;
-  *slot = elt_p;
   return retval;
 }