From patchwork Wed Jun 16 20:17:05 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [trans-mem] random segfault
Date: Wed, 16 Jun 2010 10:17:05 -0000
From: Aldy Hernandez <aldyh@redhat.com>
X-Patchwork-Id: 55934
Message-Id: <20100616201705.GA2844@redhat.com>
To: Richard Henderson <rth@redhat.com>
Cc: Patrick Marlier <patrick.marlier@unine.ch>,
 FELBER Pascal <pascal.felber@unine.ch>, gcc-patches@gcc.gnu.org

> 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.


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;
 }