Message ID | 20100615142525.GC27062@redhat.com |
---|---|
State | New |
Headers | show |
On 06/15/2010 07:25 AM, Aldy Hernandez wrote: > Fixed by making sure we're not recursing on the same variable. > > OK for branch? > > * trans-mem.c (thread_private_new_memory): Avoid infinite recursion. Ah, a side-effect of the last patch that changed the insert handling. I'm not sure this is right, Aldy, since this only detects a chain of length 1, and not longer chains. I think you have to go back and change 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... */ ... new_memory_ret: elt_p->local_new_memory = retval; return retval; 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. r~
On 06/15/2010 06:32 PM, Richard Henderson wrote: > On 06/15/2010 07:25 AM, Aldy Hernandez wrote: >> Fixed by making sure we're not recursing on the same variable. >> >> OK for branch? >> >> * trans-mem.c (thread_private_new_memory): Avoid infinite recursion. > > Ah, a side-effect of the last patch that changed the insert handling. > I'm not sure this is right, Aldy, since this only detects a chain of > length 1, and not longer chains. > > 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. Hello Aldy, I think I fall into this problem with this testcase: #include <stdlib.h> __attribute__((transaction_safe)) static void cfltcvt(char *buffer) { int decpt, pos; char *digits = NULL; pos = decpt = 0; while (*digits) { if (pos++ == decpt) *buffer++ = '.'; *buffer++ = *digits++; } } __attribute__((transaction_pure)) double varg(); __attribute__((transaction_safe)) int Q_vsprintf() { char *str; varg(); cfltcvt(str); return 0; } Here the backtrace: (gdb) bt #0 0x0000000001096c74 in htab_mod (hash=4285609909, htab=Cannot access memory at address 0x7fffff5feff8 ) at ../../transactional-memory/libiberty/hashtab.c:271 #1 0x00000000010977b0 in htab_find_slot_with_hash (htab=0x1750110, element=0x7fffff5ff0e0, hash=4285609909, insert=NO_INSERT) at ../../transactional-memory/libiberty/hashtab.c:627 #2 0x000000000109799e in htab_find_slot (htab=0x1750110, element=0x7fffff5ff0e0, insert=NO_INSERT) at ../../transactional-memory/libiberty/hashtab.c:681 #3 0x00000000009ae186 in thread_private_new_memory (entry_block=0x7ffff7f5e750, x=0x7ffff7137b58) at ../../transactional-memory/gcc/trans-mem.c:1271 #4 0x00000000009ae34e in thread_private_new_memory (entry_block=0x7ffff7f5e750, x=0x7ffff7137630) at ../../transactional-memory/gcc/trans-mem.c:1338 ... #72785 0x00000000009ae34e in thread_private_new_memory (entry_block=0x7ffff7f5e750, x=0x7ffff7137b58) at ../../transactional-memory/gcc/trans-mem.c:1338 #72786 0x00000000009ae34e in thread_private_new_memory (entry_block=0x7ffff7f5e750, x=0x7ffff7137630) at ../../transactional-memory/gcc/trans-mem.c:1338 #72787 0x00000000009ae34e in thread_private_new_memory (entry_block=0x7ffff7f5e750, x=0x7ffff7137b58) at ../../transactional-memory/gcc/trans-mem.c:1338 #72788 0x00000000009ae60b in requires_barrier (entry_block=0x7ffff7f5e750, x=0x7ffff70d7e10, stmt=0x0) at ../../transactional-memory/gcc/trans-mem.c:1395 #72789 0x00000000009b00f0 in expand_assign_tm (region=0x1797480, gsi=0x7fffffffe010) at ../../transactional-memory/gcc/trans-mem.c:2047 #72790 0x00000000009b05ea in expand_block_tm (region=0x1797480, bb=0x7ffff71384e0) at ../../transactional-memory/gcc/trans-mem.c:2191 #72791 0x00000000009b0998 in execute_tm_mark () at ../../transactional-memory/gcc/trans-mem.c:2301 #72792 0x00000000008af445 in execute_one_pass (pass=0x165c260) at ../../transactional-memory/gcc/passes.c:1579 #72793 0x00000000008af63f in execute_pass_list (pass=0x165c260) at ../../transactional-memory/gcc/passes.c:1634 #72794 0x00000000008af660 in execute_pass_list (pass=0x165c200) at ../../transactional-memory/gcc/passes.c:1635 #72795 0x0000000000a3602c in tree_rest_of_compilation (fndecl=0x7ffff7127d00) at ../../transactional-memory/gcc/tree-optimize.c:413 #72796 0x0000000000c915ca in cgraph_expand_function (node=0x7ffff7e834e0) at ../../transactional-memory/gcc/cgraphunit.c:1548 #72797 0x0000000000c9185f in cgraph_expand_all_functions () at ../../transactional-memory/gcc/cgraphunit.c:1627 #72798 0x0000000000c91ec0 in cgraph_optimize () at ../../transactional-memory/gcc/cgraphunit.c:1875 #72799 0x0000000000c8faae in cgraph_finalize_compilation_unit () at ../../transactional-memory/gcc/cgraphunit.c:1096 #72800 0x00000000004afdd2 in c_write_global_declarations () at ../../transactional-memory/gcc/c-decl.c:9552 #72801 0x00000000009a5b52 in compile_file () at ../../transactional-memory/gcc/toplev.c:1065 #72802 0x00000000009a7e19 in do_compile () at ../../transactional-memory/gcc/toplev.c:2424 #72803 0x00000000009a7ee9 in toplev_main (argc=17, argv=0x7fffffffe438) at ../../transactional-memory/gcc/toplev.c:2466 #72804 0x000000000056c0f0 in main (argc=17, argv=0x7fffffffe438) at ../../transactional-memory/gcc/main.c:35 Have a nice day, Patrick.
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) @@ -1331,6 +1331,10 @@ thread_private_new_memory (basic_block e if (phi_result == op) continue; + /* Ignore self-recursion in calculation. */ + if (val == op) + continue; + mem = thread_private_new_memory (entry_block, op); if (mem == mem_non_local) {