From patchwork Wed Jun 16 20:17:05 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 55934 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 1CC781007D2 for ; Thu, 17 Jun 2010 06:17:24 +1000 (EST) Received: (qmail 24240 invoked by alias); 16 Jun 2010 20:17:15 -0000 Received: (qmail 24225 invoked by uid 22791); 16 Jun 2010 20:17:09 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 16 Jun 2010 20:17:02 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5GKGxAU008866 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 16 Jun 2010 16:17:00 -0400 Received: from toil.yyz.redhat.com (toil.yyz.redhat.com [10.15.16.222]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5GKGvd5028960 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 16 Jun 2010 16:16:58 -0400 Received: from toil.yyz.redhat.com (localhost.localdomain [127.0.0.1]) by toil.yyz.redhat.com (8.14.2/8.14.2) with ESMTP id o5GKH6wL003145; Wed, 16 Jun 2010 16:17:06 -0400 Received: (from aldyh@localhost) by toil.yyz.redhat.com (8.14.2/8.14.2/Submit) id o5GKH5Pp003144; Wed, 16 Jun 2010 16:17:05 -0400 Date: Wed, 16 Jun 2010 16:17:05 -0400 From: Aldy Hernandez To: Richard Henderson Cc: Patrick Marlier , FELBER Pascal , gcc-patches@gcc.gnu.org Subject: Re: [trans-mem] random segfault Message-ID: <20100616201705.GA2844@redhat.com> References: <4BF51CF2.4060201@unine.ch> <4C167D42.6030600@unine.ch> <20100615142525.GC27062@redhat.com> <4C17AB16.8080804@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4C17AB16.8080804@redhat.com> User-Agent: Mutt/1.5.18 (2008-05-17) Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list 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; }