Message ID | alpine.LNX.2.00.1201181517430.4999@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
On Wed, Jan 18, 2012 at 3:21 PM, Richard Guenther <rguenther@suse.de> wrote: > > This fixes PR49484 by protecting __gcov_flush against concurrent > execution. To be able to use the gthread facility I have to > introduce the requirement that __GTHREAD_MUTEX_INIT_FUNCTION > is always available, even if __GTHREAD_MUTEX_INIT is available as > otherwise no dynamic initialization of a mutex is possible. > I have adjusted gthr-posix.h and gthr-single.h only - target > maintainers, please update your ports accordingly in advance. > > Boostrap and regtest on x86_64-unknown-linux-gnu is ongoing, > but this looks like something for stage1 anyway. Bootstrap and regtest finished ok. I am going to commit this without "fixing" eventually broken targets that I know nothing of. See above. Unless I hear objections until tomorrow. Thanks, Richard. > Richard. > > 2012-01-18 Richard Guenther <rguenther@suse.de> > > * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. > * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. > (__gthread_mutex_init_function): New function. > * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. > > PR gcov/49484 > * libgcov.c: Include gthr.h. > (__gcov_flush_mx): New global variable. > (init_mx, init_mx_once): New functions. > (__gcov_flush): Protect self with a mutex. > (__gcov_fork): Re-initialize mutex after forking. > * unwind-dw2-fde.c: Change condition under which to use > __GTHREAD_MUTEX_INIT_FUNCTION. > > Index: libgcc/libgcov.c > =================================================================== > *** libgcc/libgcov.c (revision 183270) > --- libgcc/libgcov.c (working copy) > *************** see the files COPYING3 and COPYING.RUNTI > *** 30,35 **** > --- 30,36 ---- > #include "coretypes.h" > #include "tm.h" > #include "libgcc_tm.h" > + #include "gthr.h" > > #if defined(inhibit_libc) > #define IN_LIBGCOV (-1) > *************** __gcov_init (struct gcov_info *info) > *** 705,710 **** > --- 706,730 ---- > info->version = 0; > } > > + #ifdef __GTHREAD_MUTEX_INIT > + ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT; > + #define init_mx_once() > + #else > + __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN; > + > + static void > + init_mx (void) > + { > + __GTHREAD_MUTEX_INIT_FUNCTION (&mx); > + } > + static void > + init_mx_once (void) > + { > + static __gthread_once_t once = __GTHREAD_ONCE_INIT; > + __gthread_once (&once, init_mx); > + } > + #endif > + > /* Called before fork or exec - write out profile information gathered so > far and reset it to zero. This avoids duplication or loss of the > profile information gathered so far. */ > *************** __gcov_flush (void) > *** 714,719 **** > --- 734,742 ---- > { > const struct gcov_info *gi_ptr; > > + init_mx_once (); > + __gthread_mutex_lock (&__gcov_flush_mx); > + > gcov_exit (); > for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next) > { > *************** __gcov_flush (void) > *** 737,742 **** > --- 760,767 ---- > } > } > } > + > + __gthread_mutex_unlock (&__gcov_flush_mx); > } > > #endif /* L_gcov */ > *************** __gcov_ior_profiler (gcov_type *counters > *** 975,982 **** > pid_t > __gcov_fork (void) > { > __gcov_flush (); > ! return fork (); > } > #endif > > --- 1000,1012 ---- > pid_t > __gcov_fork (void) > { > + pid_t pid; > + extern __gthread_mutex_t __gcov_flush_mx; > __gcov_flush (); > ! pid = fork (); > ! if (pid == 0) > ! __GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx); > ! return pid; > } > #endif > > Index: libgcc/unwind-dw2-fde.c > =================================================================== > *** libgcc/unwind-dw2-fde.c (revision 183270) > --- libgcc/unwind-dw2-fde.c (working copy) > *************** static struct object *seen_objects; > *** 47,57 **** > > #ifdef __GTHREAD_MUTEX_INIT > static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT; > #else > static __gthread_mutex_t object_mutex; > - #endif > > - #ifdef __GTHREAD_MUTEX_INIT_FUNCTION > static void > init_object_mutex (void) > { > --- 47,56 ---- > > #ifdef __GTHREAD_MUTEX_INIT > static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT; > + #define init_object_mutex_once() > #else > static __gthread_mutex_t object_mutex; > > static void > init_object_mutex (void) > { > *************** init_object_mutex_once (void) > *** 64,71 **** > static __gthread_once_t once = __GTHREAD_ONCE_INIT; > __gthread_once (&once, init_object_mutex); > } > - #else > - #define init_object_mutex_once() > #endif > > /* Called from crtbegin.o to register the unwind info for an object. */ > --- 63,68 ---- > Index: libgcc/gthr-posix.h > =================================================================== > *** libgcc/gthr-posix.h (revision 183270) > --- libgcc/gthr-posix.h (working copy) > *************** typedef struct timespec __gthread_time_t > *** 63,68 **** > --- 63,69 ---- > #define __GTHREAD_HAS_COND 1 > > #define __GTHREAD_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER > + #define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function > #define __GTHREAD_ONCE_INIT PTHREAD_ONCE_INIT > #if defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER) > #define __GTHREAD_RECURSIVE_MUTEX_INIT PTHREAD_RECURSIVE_MUTEX_INITIALIZER > *************** __gthread_setspecific (__gthread_key_t _ > *** 731,736 **** > --- 732,745 ---- > } > > static inline int > + __gthread_mutex_init_function (__gthread_mutex_t *__mutex) > + { > + if (__gthread_active_p ()) > + return __gthrw_(pthread_mutex_init) (__mutex, NULL); > + return 0; > + } > + > + static inline int > __gthread_mutex_destroy (__gthread_mutex_t *__mutex) > { > if (__gthread_active_p ()) > Index: libgcc/gthr-single.h > =================================================================== > *** libgcc/gthr-single.h (revision 183270) > --- libgcc/gthr-single.h (working copy) > *************** typedef int __gthread_recursive_mutex_t; > *** 36,41 **** > --- 36,42 ---- > > #define __GTHREAD_ONCE_INIT 0 > #define __GTHREAD_MUTEX_INIT 0 > + #define __GTHREAD_MUTEX_INIT_FUNCTION (mx) > #define __GTHREAD_RECURSIVE_MUTEX_INIT 0 > > #define UNUSED __attribute__((unused)) > Index: libgcc/gthr.h > =================================================================== > --- libgcc/gthr.h (revision 183270) > +++ libgcc/gthr.h (working copy) > @@ -52,11 +52,12 @@ see the files COPYING3 and COPYING.RUNTI > to initialize __gthread_mutex_t to get a fast > non-recursive mutex. > __GTHREAD_MUTEX_INIT_FUNCTION > - some systems can't initialize a mutex without a > - function call. On such systems, define this to a > - function which looks like this: > + to initialize __gthread_mutex_t to get a fast > + non-recursive mutex. > + Define this to a function which looks like this: > void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *) > - Don't define __GTHREAD_MUTEX_INIT in this case > + Some systems can't initialize a mutex without a > + function call. Don't define __GTHREAD_MUTEX_INIT in this case. > __GTHREAD_RECURSIVE_MUTEX_INIT > __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION > as above, but for a recursive mutex.
On Mon, Mar 5, 2012 at 1:17 PM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Wed, Jan 18, 2012 at 3:21 PM, Richard Guenther <rguenther@suse.de> wrote: >> >> This fixes PR49484 by protecting __gcov_flush against concurrent >> execution. To be able to use the gthread facility I have to >> introduce the requirement that __GTHREAD_MUTEX_INIT_FUNCTION >> is always available, even if __GTHREAD_MUTEX_INIT is available as >> otherwise no dynamic initialization of a mutex is possible. >> I have adjusted gthr-posix.h and gthr-single.h only - target >> maintainers, please update your ports accordingly in advance. >> >> Boostrap and regtest on x86_64-unknown-linux-gnu is ongoing, >> but this looks like something for stage1 anyway. > > Bootstrap and regtest finished ok. I am going to commit this without "fixing" > eventually broken targets that I know nothing of. See above. > > Unless I hear objections until tomorrow. Done now, after waiting another week. Thanks, Richard. >> 2012-01-18 Richard Guenther <rguenther@suse.de> >> >> * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. >> * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >> (__gthread_mutex_init_function): New function. >> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >> >> PR gcov/49484 >> * libgcov.c: Include gthr.h. >> (__gcov_flush_mx): New global variable. >> (init_mx, init_mx_once): New functions. >> (__gcov_flush): Protect self with a mutex. >> (__gcov_fork): Re-initialize mutex after forking. >> * unwind-dw2-fde.c: Change condition under which to use >> __GTHREAD_MUTEX_INIT_FUNCTION.
On Mon, 12 Mar 2012, Richard Guenther wrote: >>> 2012-01-18 Richard Guenther <rguenther@suse.de> >>> >>> * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. >>> * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>> (__gthread_mutex_init_function): New function. >>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>> >>> PR gcov/49484 >>> * libgcov.c: Include gthr.h. >>> (__gcov_flush_mx): New global variable. >>> (init_mx, init_mx_once): New functions. >>> (__gcov_flush): Protect self with a mutex. >>> (__gcov_fork): Re-initialize mutex after forking. >>> * unwind-dw2-fde.c: Change condition under which to use >>> __GTHREAD_MUTEX_INIT_FUNCTION. Richi, I'm afraid this caused the following on i386-unknown-freebsd10? /scratch2/tmp/gerald/gcc-HEAD/libgcc/libgcov.c:710:54: error: 'NULL' undeclared here (not in a function) gmake[3]: *** [_gcov.o] Error 1 gmake[3]: *** Waiting for unfinished jobs.... gmake[3]: Leaving directory `/scratch2/tmp/gerald/OBJ-0312-1454/i386-unknown-freebsd10.0/libgcc' gmake[2]: *** [all-stage1-target-libgcc] Error 2 Gerald
On Mon, Mar 12, 2012 at 6:44 PM, Gerald Pfeifer <gerald@pfeifer.com> wrote: > On Mon, 12 Mar 2012, Richard Guenther wrote: >>>> 2012-01-18 Richard Guenther <rguenther@suse.de> >>>> >>>> * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. >>>> * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>> (__gthread_mutex_init_function): New function. >>>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>> >>>> PR gcov/49484 >>>> * libgcov.c: Include gthr.h. >>>> (__gcov_flush_mx): New global variable. >>>> (init_mx, init_mx_once): New functions. >>>> (__gcov_flush): Protect self with a mutex. >>>> (__gcov_fork): Re-initialize mutex after forking. >>>> * unwind-dw2-fde.c: Change condition under which to use >>>> __GTHREAD_MUTEX_INIT_FUNCTION. > > Richi, I'm afraid this caused the following on i386-unknown-freebsd10? > > /scratch2/tmp/gerald/gcc-HEAD/libgcc/libgcov.c:710:54: error: 'NULL' undeclared here (not in a function) > gmake[3]: *** [_gcov.o] Error 1 > gmake[3]: *** Waiting for unfinished jobs.... > gmake[3]: Leaving directory `/scratch2/tmp/gerald/OBJ-0312-1454/i386-unknown-freebsd10.0/libgcc' > gmake[2]: *** [all-stage1-target-libgcc] Error 2 This is #ifdef __GTHREAD_MUTEX_INIT ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT; thus if __GTHREAD_MUTEX_INIT uses NULL and gthr.h does not include everything to make that initializer valid it is a freebsd header bug, unless it is #if defined(inhibit_libc) #define IN_LIBGCOV (-1) #else #undef NULL /* Avoid errors if stdio.h and our stddef.h mismatch. */ #include <stdio.h> wtf? This is a target library, why is stdio.h not properly included by tsystem.h? It is. Anyone remembers? Can you verify that theory, thus remove that #undef and the #include? Richard. > Gerald
On Tue, Mar 13, 2012 at 10:44 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Mon, Mar 12, 2012 at 6:44 PM, Gerald Pfeifer <gerald@pfeifer.com> wrote: >> On Mon, 12 Mar 2012, Richard Guenther wrote: >>>>> 2012-01-18 Richard Guenther <rguenther@suse.de> >>>>> >>>>> * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. >>>>> * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>>> (__gthread_mutex_init_function): New function. >>>>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>>> >>>>> PR gcov/49484 >>>>> * libgcov.c: Include gthr.h. >>>>> (__gcov_flush_mx): New global variable. >>>>> (init_mx, init_mx_once): New functions. >>>>> (__gcov_flush): Protect self with a mutex. >>>>> (__gcov_fork): Re-initialize mutex after forking. >>>>> * unwind-dw2-fde.c: Change condition under which to use >>>>> __GTHREAD_MUTEX_INIT_FUNCTION. >> >> Richi, I'm afraid this caused the following on i386-unknown-freebsd10? >> >> /scratch2/tmp/gerald/gcc-HEAD/libgcc/libgcov.c:710:54: error: 'NULL' undeclared here (not in a function) >> gmake[3]: *** [_gcov.o] Error 1 >> gmake[3]: *** Waiting for unfinished jobs.... >> gmake[3]: Leaving directory `/scratch2/tmp/gerald/OBJ-0312-1454/i386-unknown-freebsd10.0/libgcc' >> gmake[2]: *** [all-stage1-target-libgcc] Error 2 > > This is > > #ifdef __GTHREAD_MUTEX_INIT > ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT; > > thus if __GTHREAD_MUTEX_INIT uses NULL and gthr.h does not include > everything to make that initializer valid it is a freebsd header bug, unless > it is > > #if defined(inhibit_libc) > #define IN_LIBGCOV (-1) > #else > #undef NULL /* Avoid errors if stdio.h and our stddef.h mismatch. */ > #include <stdio.h> > > wtf? This is a target library, why is stdio.h not properly included > by tsystem.h? > It is. Anyone remembers? Goes back to rev. 5880 by rms, at which time tsystem.h did not exist. I'm going to remove those two lines, bootstrap & test it and commit as obvious. Richard. > Can you verify that theory, thus remove that #undef and the #include? > > Richard. > >> Gerald
On 13/03/12 10:05, Richard Guenther wrote: > On Tue, Mar 13, 2012 at 10:44 AM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> On Mon, Mar 12, 2012 at 6:44 PM, Gerald Pfeifer <gerald@pfeifer.com> wrote: >>> On Mon, 12 Mar 2012, Richard Guenther wrote: >>>>>> 2012-01-18 Richard Guenther <rguenther@suse.de> >>>>>> >>>>>> * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. >>>>>> * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>>>> (__gthread_mutex_init_function): New function. >>>>>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>>>> >>>>>> PR gcov/49484 >>>>>> * libgcov.c: Include gthr.h. >>>>>> (__gcov_flush_mx): New global variable. >>>>>> (init_mx, init_mx_once): New functions. >>>>>> (__gcov_flush): Protect self with a mutex. >>>>>> (__gcov_fork): Re-initialize mutex after forking. >>>>>> * unwind-dw2-fde.c: Change condition under which to use >>>>>> __GTHREAD_MUTEX_INIT_FUNCTION. >>> >>> Richi, I'm afraid this caused the following on i386-unknown-freebsd10? >>> >>> /scratch2/tmp/gerald/gcc-HEAD/libgcc/libgcov.c:710:54: error: 'NULL' undeclared here (not in a function) >>> gmake[3]: *** [_gcov.o] Error 1 >>> gmake[3]: *** Waiting for unfinished jobs.... >>> gmake[3]: Leaving directory `/scratch2/tmp/gerald/OBJ-0312-1454/i386-unknown-freebsd10.0/libgcc' >>> gmake[2]: *** [all-stage1-target-libgcc] Error 2 >> >> This is >> >> #ifdef __GTHREAD_MUTEX_INIT >> ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT; >> >> thus if __GTHREAD_MUTEX_INIT uses NULL and gthr.h does not include >> everything to make that initializer valid it is a freebsd header bug, unless >> it is >> >> #if defined(inhibit_libc) >> #define IN_LIBGCOV (-1) >> #else >> #undef NULL /* Avoid errors if stdio.h and our stddef.h mismatch. */ >> #include <stdio.h> >> >> wtf? This is a target library, why is stdio.h not properly included >> by tsystem.h? >> It is. Anyone remembers? > > Goes back to rev. 5880 by rms, at which time tsystem.h did not exist. > > I'm going to remove those two lines, bootstrap & test it and commit as > obvious. > Please also check this on a bare-metal build. R. > Richard. > >> Can you verify that theory, thus remove that #undef and the #include? >> >> Richard. >> >>> Gerald >
On Tue, Mar 13, 2012 at 11:22 AM, Richard Earnshaw <rearnsha@arm.com> wrote: > On 13/03/12 10:05, Richard Guenther wrote: >> On Tue, Mar 13, 2012 at 10:44 AM, Richard Guenther >> <richard.guenther@gmail.com> wrote: >>> On Mon, Mar 12, 2012 at 6:44 PM, Gerald Pfeifer <gerald@pfeifer.com> wrote: >>>> On Mon, 12 Mar 2012, Richard Guenther wrote: >>>>>>> 2012-01-18 Richard Guenther <rguenther@suse.de> >>>>>>> >>>>>>> * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification. >>>>>>> * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>>>>> (__gthread_mutex_init_function): New function. >>>>>>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define. >>>>>>> >>>>>>> PR gcov/49484 >>>>>>> * libgcov.c: Include gthr.h. >>>>>>> (__gcov_flush_mx): New global variable. >>>>>>> (init_mx, init_mx_once): New functions. >>>>>>> (__gcov_flush): Protect self with a mutex. >>>>>>> (__gcov_fork): Re-initialize mutex after forking. >>>>>>> * unwind-dw2-fde.c: Change condition under which to use >>>>>>> __GTHREAD_MUTEX_INIT_FUNCTION. >>>> >>>> Richi, I'm afraid this caused the following on i386-unknown-freebsd10? >>>> >>>> /scratch2/tmp/gerald/gcc-HEAD/libgcc/libgcov.c:710:54: error: 'NULL' undeclared here (not in a function) >>>> gmake[3]: *** [_gcov.o] Error 1 >>>> gmake[3]: *** Waiting for unfinished jobs.... >>>> gmake[3]: Leaving directory `/scratch2/tmp/gerald/OBJ-0312-1454/i386-unknown-freebsd10.0/libgcc' >>>> gmake[2]: *** [all-stage1-target-libgcc] Error 2 >>> >>> This is >>> >>> #ifdef __GTHREAD_MUTEX_INIT >>> ATTRIBUTE_HIDDEN __gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT; >>> >>> thus if __GTHREAD_MUTEX_INIT uses NULL and gthr.h does not include >>> everything to make that initializer valid it is a freebsd header bug, unless >>> it is >>> >>> #if defined(inhibit_libc) >>> #define IN_LIBGCOV (-1) >>> #else >>> #undef NULL /* Avoid errors if stdio.h and our stddef.h mismatch. */ >>> #include <stdio.h> >>> >>> wtf? This is a target library, why is stdio.h not properly included >>> by tsystem.h? >>> It is. Anyone remembers? >> >> Goes back to rev. 5880 by rms, at which time tsystem.h did not exist. >> >> I'm going to remove those two lines, bootstrap & test it and commit as >> obvious. >> > > Please also check this on a bare-metal build. Bare-metal should not be affected as that defines inhibit_libc(?). What is an example for a bare-metal target triplet? Richard. > R. > >> Richard. >> >>> Can you verify that theory, thus remove that #undef and the #include? >>> >>> Richard. >>> >>>> Gerald >> > >
On Tue, 13 Mar 2012, Richard Guenther wrote: > Goes back to rev. 5880 by rms, at which time tsystem.h did not exist. > > I'm going to remove those two lines, bootstrap & test it and commit as > obvious. Thanks, Richard. I could not do the tests you suggested yesterday before seeing this other mail. But I can confirm that GCC again bootstraps on the FreeBSD tester(s) in question. Gerald
Index: libgcc/gthr.h =================================================================== --- libgcc/gthr.h (revision 183270) +++ libgcc/gthr.h (working copy) @@ -52,11 +52,12 @@ see the files COPYING3 and COPYING.RUNTI to initialize __gthread_mutex_t to get a fast non-recursive mutex. __GTHREAD_MUTEX_INIT_FUNCTION - some systems can't initialize a mutex without a - function call. On such systems, define this to a - function which looks like this: + to initialize __gthread_mutex_t to get a fast + non-recursive mutex. + Define this to a function which looks like this: void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *) - Don't define __GTHREAD_MUTEX_INIT in this case + Some systems can't initialize a mutex without a + function call. Don't define __GTHREAD_MUTEX_INIT in this case. __GTHREAD_RECURSIVE_MUTEX_INIT __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION as above, but for a recursive mutex.