From patchwork Tue Jun 8 23:06:29 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 55043 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 87CE6B7D47 for ; Wed, 9 Jun 2010 09:07:49 +1000 (EST) Received: (qmail 11136 invoked by alias); 8 Jun 2010 23:07:48 -0000 Received: (qmail 11114 invoked by uid 22791); 8 Jun 2010 23:07:46 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from c2beaomr08.btconnect.com (HELO c2beaomr08.btconnect.com) (213.123.26.186) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Jun 2010 23:07:40 +0000 Received: from thor.office (host81-138-1-83.in-addr.btopenworld.com [81.138.1.83]) by c2beaomr08.btconnect.com with ESMTP id EVN79891; Wed, 9 Jun 2010 00:06:30 +0100 (BST) X-Mirapoint-IP-Reputation: reputation=Fair-1, source=Queried, refid=0001.0A0B0302.4C0ECCF6.00A9, actions=tag Cc: Jakub Jelinek , GCC Patches , Ralf Wildenhues , Kai Tietz Message-Id: <4E347EB0-900D-4A65-BC7A-D04E8B9AB241@sandoe-acoustics.co.uk> From: IainS To: Paolo Bonzini In-Reply-To: <20100608195806.GE9471@gmx.de> Mime-Version: 1.0 (Apple Message framework v936) Subject: Re: [Patch, toplevel/config] fix tls.m4 configure race condition (bootstrap/PR43170) Date: Wed, 9 Jun 2010 00:06:29 +0100 References: <4C0E7078.4060006@gnu.org> <20100608170157.GH9740@ins.uni-bonn.de> <4C0E79A2.4030508@gnu.org> <20100608172711.GA10293@tyan-ft48-01.lab.bos.redhat.com> <20100608182753.GB4626@gmx.de> <20100608185058.GB10293@tyan-ft48-01.lab.bos.redhat.com> <20100608190436.GB9471@gmx.de> <20100608191557.GC10293@tyan-ft48-01.lab.bos.redhat.com> <20100608192115.GC9471@gmx.de> <6D50CE6B-1FF0-4E11-9C70-0475110A1612@sandoe-acoustics.co.uk> <20100608195806.GE9471@gmx.de> X-IsSubscribed: yes 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 On 8 Jun 2010, at 20:58, Ralf Wildenhues wrote: > * IainS wrote on Tue, Jun 08, 2010 at 09:41:45PM CEST: >> On 8 Jun 2010, at 20:21, Ralf Wildenhues wrote: >>> * Jakub Jelinek wrote on Tue, Jun 08, 2010 at 09:15:57PM CEST: >>>> The test is testing for a bug in old glibcs which was violating the >>>> __thread property, and I believe you really need some optimization >>>> barriers for the test, otherwise some smart optimization in the >>>> future will optimize the test away and let it succeed on buggy old >>>> glibcs. >>> >>> Thank you, finally we are getting somewhere! I think this paragraph >>> should be put in a comment before the test in tls.m4. >> >> as a suggestion: >> >> # The 'volatile' qualifier on the references coupled with the >> positioning of the >> # reference to 'a' is necessary to achieve an optimization barrier >> for compilers >> # using whole program or other advanced optimizations. > > Well, if it doesn't mention that this is really about an old glibc bug > which caused the __thread property to be violated, then I would never > understand why the test is written the way it is. The glibc bug is > the > crucial bit of information; the need to work around compiler > optimization is only to ensure the test is effective in two > directions. > I'd suggest something like: > > dnl Test for an old glibc bug that violated the __thread property. > dnl Use volatile to ensure the compiler won't optimize away pointer > dnl accesses it might otherwise know to be different, or reorder them > dnl and reuse storage, which may let them point to the same location. updated patch attached, please check the final wording of the comment is OK. bootstrapped on i686-darwin9, x86_64-darwin10. Confirmed that libgomp links emutls, that the tls tests give the same results across stage1-3, that the tls-test fragment runs consistently for 10,000 iterations and that the testsuites for tls.exp and libgomp run as expected. Obviously it would be nice to collect a result from another emutls target - and also it seems that this fix would clear http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42829, Kai do you agree? OK now for trunk, and 4.5 ? Iain. config/ChangeLog: PR bootstrap/43170 * tls.m4 (GCC_CHECK_TLS): Add volatile qualifier to test references. Move main () reference ahead of thread_create(). Add comment to explain the requirements of the test. libgomp/ChangeLog: PR bootstrap/43170 * configure: Regenerated. libstdc++-v3/ChangeLog: PR bootstrap/43170 * configure: Regenerated. libmudflap/ChangeLog: PR bootstrap/43170 * configure: Regenerated. libjava/ChangeLog: PR bootstrap/43170 * configure: Regenerated. Index: libgomp/configure =================================================================== --- libgomp/configure (revision 160446) +++ libgomp/configure (working copy) @@ -15380,7 +15380,7 @@ else /* end confdefs.h. */ #include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -15392,11 +15392,11 @@ main () { pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread); Index: libjava/configure =================================================================== --- libjava/configure (revision 160446) +++ libjava/configure (working copy) @@ -24390,7 +24390,7 @@ else /* end confdefs.h. */ #include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -24402,11 +24402,11 @@ main () { pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread); Index: libmudflap/configure =================================================================== --- libmudflap/configure (revision 160446) +++ libmudflap/configure (working copy) @@ -11479,7 +11479,7 @@ else /* end confdefs.h. */ #include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -11491,11 +11491,11 @@ main () { pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread); Index: libstdc++-v3/configure =================================================================== --- libstdc++-v3/configure (revision 160446) +++ libstdc++-v3/configure (working copy) @@ -25548,7 +25548,7 @@ else /* end confdefs.h. */ #include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -25560,11 +25560,11 @@ main () { pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread); @@ -44486,7 +44486,7 @@ else /* end confdefs.h. */ #include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -44498,11 +44498,11 @@ main () { pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread); @@ -50571,7 +50571,7 @@ else /* end confdefs.h. */ #include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -50583,11 +50583,11 @@ main () { pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread); Index: config/tls.m4 =================================================================== --- config/tls.m4 (revision 160446) +++ config/tls.m4 (working copy) @@ -38,11 +38,16 @@ AC_DEFUN([GCC_CHECK_TLS], [ CFLAGS="$chktls_save_CFLAGS" if test "X$thread_CFLAGS" != Xfailed; then CFLAGS="$thread_CFLAGS $chktls_save_CFLAGS" + dnl Test for an old glibc bug that violated the __thread property. + dnl Use volatile to ensure the compiler won't optimize away pointer + dnl accesses it might otherwise assume to be redundant, or reorder + dnl them and reuse storage, which might lead to them pointing to + dnl the same location. AC_RUN_IFELSE( [AC_LANG_PROGRAM( [#include __thread int a; - static int *a_in_other_thread; + static int *volatile a_in_other_thread; static void * thread_func (void *arg) { @@ -51,11 +56,11 @@ AC_DEFUN([GCC_CHECK_TLS], [ }], [pthread_t thread; void *thread_retval; - int *a_in_main_thread; + int *volatile a_in_main_thread; + a_in_main_thread = &a; if (pthread_create (&thread, (pthread_attr_t *)0, thread_func, (void *)0)) return 0; - a_in_main_thread = &a; if (pthread_join (thread, &thread_retval)) return 0; return (a_in_other_thread == a_in_main_thread);])],